Author: martinboehme Date: 2023-11-07T09:48:40+01:00 New Revision: 6b573f4611df470c23c7048fe820e0f216e9a8e1
URL: https://github.com/llvm/llvm-project/commit/6b573f4611df470c23c7048fe820e0f216e9a8e1 DIFF: https://github.com/llvm/llvm-project/commit/6b573f4611df470c23c7048fe820e0f216e9a8e1.diff LOG: [clang][dataflow] Fix assert-fail when calling assignment operator with by-value parameter. (#71384) The code assumed that the source parameter of an assignment operator is always passed by reference, but it is legal for it to be passed by value. This patch includes a test that assert-fails without the fix. Added: Modified: clang/lib/Analysis/FlowSensitive/Transfer.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index bb26e9f8dc8db85..8b2f8ecc5027e8a 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -514,8 +514,14 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { !Method->isMoveAssignmentOperator()) return; - auto *LocSrc = - cast_or_null<RecordStorageLocation>(Env.getStorageLocation(*Arg1)); + RecordStorageLocation *LocSrc = nullptr; + if (Arg1->isPRValue()) { + if (auto *Val = cast_or_null<RecordValue>(Env.getValue(*Arg1))) + LocSrc = &Val->getLoc(); + } else { + LocSrc = + cast_or_null<RecordStorageLocation>(Env.getStorageLocation(*Arg1)); + } auto *LocDst = cast_or_null<RecordStorageLocation>(Env.getStorageLocation(*Arg0)); diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 0f9f13df817075e..bd9b98178b5d4e3 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -2213,6 +2213,43 @@ TEST(TransferTest, AssignmentOperator) { }); } +// It's legal for the assignment operator to take its source parameter by value. +// Check that we handle this correctly. (This is a repro -- we used to +// assert-fail on this.) +TEST(TransferTest, AssignmentOperator_ArgByValue) { + std::string Code = R"( + struct A { + int Baz; + A &operator=(A); + }; + + void target() { + A Foo = { 1 }; + A Bar = { 2 }; + Foo = Bar; + // [[p]] + } + )"; + runDataflow( + Code, + [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results, + ASTContext &ASTCtx) { + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); + + const auto &FooLoc = + getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Foo"); + const auto &BarLoc = + getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Bar"); + + const auto *FooBazVal = + cast<IntegerValue>(getFieldValue(&FooLoc, *BazDecl, Env)); + const auto *BarBazVal = + cast<IntegerValue>(getFieldValue(&BarLoc, *BazDecl, Env)); + EXPECT_EQ(FooBazVal, BarBazVal); + }); +} + TEST(TransferTest, AssignmentOperatorFromBase) { // This is a crash repro. We don't model the copy this case, so no // expectations on the copied field of the base class are checked. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits