[clang] [clang][dataflow] Fix bug in transfer function of CK_BaseToDerived (PR #208726)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: Yitzhak Mandelbaum (ymand)

<details>
<summary>Changes</summary>

Bug fix. In the transfer function (one of the cast cases), the loop adding
synthetic fields to the derived record storage location was incorrectly nested
inside the loop that iterates over modeled fields (`getModeledFields`).

If the derived class has 0 modeled fields, `getModeledFields(Derived)` is
empty. Consequently, synthetic fields were never added to the storage location,
causing an assertion failure in `StorageLocation::getSyntheticField` when
initializing field values.

This patch moves the synthetic fields loop outside of the modeled fields loop
and adds a regression unit test in `TransferTest.cpp`.


---
Full diff: https://github.com/llvm/llvm-project/pull/208726.diff


2 Files Affected:

- (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+5-5) 
- (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+27) 


``````````diff
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index d40843ff4ec43..bf4b245a3cf6f 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -371,12 +371,12 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
         } else {
           Loc->addChild(*Field, &Env.createStorageLocation(FieldType));
         }
+      }
 
-        for (const auto &Entry :
-             Env.getDataflowAnalysisContext().getSyntheticFields(Derived)) {
-          Loc->addSyntheticField(Entry.getKey(),
-                                 Env.createStorageLocation(Entry.getValue()));
-        }
+      for (const auto &Entry :
+           Env.getDataflowAnalysisContext().getSyntheticFields(Derived)) {
+        Loc->addSyntheticField(Entry.getKey(),
+                               Env.createStorageLocation(Entry.getValue()));
       }
       Env.initializeFieldsWithValues(*Loc, Derived);
 
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index e147f3dc7a195..501fa549afd43 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3859,6 +3859,33 @@ TEST(TransferTest, StaticCastBaseToDerivedUnknown) {
       });
 }
 
+TEST(TransferTest, StaticCastBaseToDerivedWithSyntheticFieldsNoModeledFields) {
+  std::string Code = R"cc(
+    struct Base {};
+    struct Derived : public Base {};
+    void target(Base* BPtr) {
+      Derived* DPtr = static_cast<Derived*>(BPtr);
+      (void)DPtr;
+      // [[p]]
+    }
+  )cc";
+  ASSERT_THAT_ERROR(
+      checkDataflowWithNoopAnalysis(
+          Code, ast_matchers::hasName("target"),
+          [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+             ASTContext &ASTCtx) {
+            ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+          },
+          {BuiltinOptions()}, LangStandard::lang_cxx17,
+          [](QualType Ty) -> llvm::StringMap<QualType> {
+            RecordDecl *RD = Ty->getAsRecordDecl();
+            if (RD != nullptr && RD->getName() == "Derived")
+              return {{"synth", RD->getASTContext().IntTy}};
+            return {};
+          }),
+      llvm::Succeeded());
+}
+
 TEST(TransferTest, MultipleConstructionsFromStaticCastsBaseToDerived) {
   std::string Code = R"cc(
  struct Base {};

``````````

</details>


https://github.com/llvm/llvm-project/pull/208726