Skip to content

Commit

Permalink
[clang][dataflow] Fix assert-fail when calling assignment operator wi…
Browse files Browse the repository at this point in the history
…th 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.
  • Loading branch information
martinboehme committed Nov 7, 2023
1 parent e09184f commit 6b573f4
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
10 changes: 8 additions & 2 deletions clang/lib/Analysis/FlowSensitive/Transfer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
37 changes: 37 additions & 0 deletions clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 6b573f4

Please sign in to comment.