Skip to content

Commit 6b573f4

Browse files
authored
[clang][dataflow] Fix assert-fail when calling assignment operator with by-value parameter. (llvm#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.
1 parent e09184f commit 6b573f4

File tree

2 files changed

+45
-2
lines changed

2 files changed

+45
-2
lines changed

clang/lib/Analysis/FlowSensitive/Transfer.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,8 +514,14 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
514514
!Method->isMoveAssignmentOperator())
515515
return;
516516

517-
auto *LocSrc =
518-
cast_or_null<RecordStorageLocation>(Env.getStorageLocation(*Arg1));
517+
RecordStorageLocation *LocSrc = nullptr;
518+
if (Arg1->isPRValue()) {
519+
if (auto *Val = cast_or_null<RecordValue>(Env.getValue(*Arg1)))
520+
LocSrc = &Val->getLoc();
521+
} else {
522+
LocSrc =
523+
cast_or_null<RecordStorageLocation>(Env.getStorageLocation(*Arg1));
524+
}
519525
auto *LocDst =
520526
cast_or_null<RecordStorageLocation>(Env.getStorageLocation(*Arg0));
521527

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2213,6 +2213,43 @@ TEST(TransferTest, AssignmentOperator) {
22132213
});
22142214
}
22152215

2216+
// It's legal for the assignment operator to take its source parameter by value.
2217+
// Check that we handle this correctly. (This is a repro -- we used to
2218+
// assert-fail on this.)
2219+
TEST(TransferTest, AssignmentOperator_ArgByValue) {
2220+
std::string Code = R"(
2221+
struct A {
2222+
int Baz;
2223+
A &operator=(A);
2224+
};
2225+
2226+
void target() {
2227+
A Foo = { 1 };
2228+
A Bar = { 2 };
2229+
Foo = Bar;
2230+
// [[p]]
2231+
}
2232+
)";
2233+
runDataflow(
2234+
Code,
2235+
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
2236+
ASTContext &ASTCtx) {
2237+
const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
2238+
const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
2239+
2240+
const auto &FooLoc =
2241+
getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Foo");
2242+
const auto &BarLoc =
2243+
getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Bar");
2244+
2245+
const auto *FooBazVal =
2246+
cast<IntegerValue>(getFieldValue(&FooLoc, *BazDecl, Env));
2247+
const auto *BarBazVal =
2248+
cast<IntegerValue>(getFieldValue(&BarLoc, *BazDecl, Env));
2249+
EXPECT_EQ(FooBazVal, BarBazVal);
2250+
});
2251+
}
2252+
22162253
TEST(TransferTest, AssignmentOperatorFromBase) {
22172254
// This is a crash repro. We don't model the copy this case, so no
22182255
// expectations on the copied field of the base class are checked.

0 commit comments

Comments
 (0)