[clang] 145f353 - [clang][dataflow] fix failing assert in copyRecord

[Paul Semel via cfe-commits]

Author: Paul Semel
Date: 2023-07-26T08:52:06Z
New Revision: 145f353fd67909e03c39b968b464ac625edde6cb

URL: https://github.com/llvm/llvm-project/commit/145f353fd67909e03c39b968b464ac625edde6cb
DIFF: https://github.com/llvm/llvm-project/commit/145f353fd67909e03c39b968b464ac625edde6cb.diff

LOG: [clang][dataflow] fix failing assert in copyRecord

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

Added: 
    

Modified: 
    clang/lib/Analysis/FlowSensitive/RecordOps.cpp
    clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index 60144531c25186..95693f2e933a49 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -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));

diff  --git a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
index 2b4b64c74481fb..b4eb0f26bf76be 100644
--- a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -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