Skip to content

Commit

Permalink
[clang][dataflow] fix failing assert in copyRecord
Browse files Browse the repository at this point in the history
When dealing with copy constructor, the compiler can emit an
UncheckedDerivedToBase implicit cast for the CXXConstructorExpr of the
base class. In such case, when trying to copy the src storage location
to its destination, we will fail on the assert checking that location
types are the same.

When copying from derived to base class, it is acceptable to break that
assumption to only copy common fields from the base class.

Note: the provided test crashes the process without the changes made to
copyRecord.

Differential Revision: https://reviews.llvm.org/D155844
  • Loading branch information
paulsemel committed Jul 26, 2023
1 parent bc37893 commit 145f353
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 6 deletions.
21 changes: 15 additions & 6 deletions clang/lib/Analysis/FlowSensitive/RecordOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,27 @@
void clang::dataflow::copyRecord(AggregateStorageLocation &Src,
AggregateStorageLocation &Dst,
Environment &Env) {
auto SrcType = Src.getType().getCanonicalType().getUnqualifiedType();
auto DstType = Dst.getType().getCanonicalType().getUnqualifiedType();

auto SrcDecl = SrcType->getAsCXXRecordDecl();
auto DstDecl = DstType->getAsCXXRecordDecl();

bool compatibleTypes =
SrcType == DstType ||
(SrcDecl && DstDecl && SrcDecl->isDerivedFrom(DstDecl));
(void)compatibleTypes;

LLVM_DEBUG({
if (Dst.getType().getCanonicalType().getUnqualifiedType() !=
Src.getType().getCanonicalType().getUnqualifiedType()) {
if (!compatibleTypes) {
llvm::dbgs() << "Source type " << Src.getType() << "\n";
llvm::dbgs() << "Destination type " << Dst.getType() << "\n";
}
});
assert(Dst.getType().getCanonicalType().getUnqualifiedType() ==
Src.getType().getCanonicalType().getUnqualifiedType());
assert(compatibleTypes);

for (auto [Field, SrcFieldLoc] : Src.children()) {
StorageLocation *DstFieldLoc = Dst.getChild(*Field);
for (auto [Field, DstFieldLoc] : Dst.children()) {
StorageLocation *SrcFieldLoc = Src.getChild(*Field);

assert(Field->getType()->isReferenceType() ||
(SrcFieldLoc != nullptr && DstFieldLoc != nullptr));
Expand Down
34 changes: 34 additions & 0 deletions clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,40 @@ TEST(RecordOpsTest, RecordsEqual) {
});
}

TEST(TransferTest, CopyRecordFromDerivedToBase) {
std::string Code = R"(
struct A {
int i;
};
struct B : public A {
};
void target(A a, B b) {
(void)a.i;
// [[p]]
}
)";
runDataflow(
Code,
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
ASTContext &ASTCtx) {
Environment Env = getEnvironmentAtAnnotation(Results, "p").fork();

const ValueDecl *IDecl = findValueDecl(ASTCtx, "i");
auto &A = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "a");
auto &B = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "b");

EXPECT_NE(Env.getValue(*A.getChild(*IDecl)),
Env.getValue(*B.getChild(*IDecl)));

copyRecord(B, A, Env);

EXPECT_EQ(Env.getValue(*A.getChild(*IDecl)),
Env.getValue(*B.getChild(*IDecl)));
});
}

} // namespace
} // namespace test
} // namespace dataflow
Expand Down

0 comments on commit 145f353

Please sign in to comment.