[clang][dataflow] Fix Record initialization with InitListExpr and inheritances

Usually RecordValues for record objects (e.g. struct) are initialized with
`Environment::createValue()` which internally calls `getObjectFields()` to
collects all fields from the current and base classes, and then filter them
with `ModeledValues` via `DACtx::getModeledFields()` so that the fields that
are actually referenced are modeled.

The consistent set of fields should be initialized when a record is initialized
with an initializer list (InitListExpr), however the existing code's behavior
was different.

Before this patch:
* When a struct is initialized with InitListExpr, its fields are
  initialized based on what is returned by `getFieldsForInitListExpr()`, which
  only collects the direct fields in the current class, but not from the base
  classes. Moreover, if the base classes have their own InitListExpr, values
  that are initialized by their InitListExpr's weren't merged into the
  child objects.

After this patch:
* When a struct is initialized with InitListExpr, it collects and merges the
  fields in the base classes that were initialized by their InitListExpr's.
  The code also asserts that the consistent set of fields are initialized
  with the ModeledFields.

Reviewed By: mboehme

Differential Revision: https://reviews.llvm.org/D159284

Author: kinu

clang/lib/Analysis/FlowSensitive/Transfer.cpp

@@ -27,12 +27,12 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/OperatorKinds.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
-#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Debug.h"
+#include <assert.h>
 #include <cassert>
-#include <memory>
-#include <tuple>
+
+#define DEBUG_TYPE "dataflow"
 
 namespace clang {
 namespace dataflow {
@@ -629,17 +629,66 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       return;
     }
 
-    std::vector<FieldDecl *> Fields =
-        getFieldsForInitListExpr(Type->getAsRecordDecl());
     llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
 
-    for (auto [Field, Init] : llvm::zip(Fields, S->inits())) {
-      assert(Field != nullptr);
-      assert(Init != nullptr);
+    // This only contains the direct fields for the given type.
+    std::vector<FieldDecl *> FieldsForInit =
+        getFieldsForInitListExpr(Type->getAsRecordDecl());
 
-      FieldLocs.insert({Field, &Env.createObject(Field->getType(), Init)});
+    // `S->inits()` contains all the initializer epressions, including the
+    // ones for direct base classes.
+    auto Inits = S->inits();
+    size_t InitIdx = 0;
+
+    // Initialize base classes.
+    if (auto* R = S->getType()->getAsCXXRecordDecl()) {
+      assert(FieldsForInit.size() + R->getNumBases() == Inits.size());
+      for ([[maybe_unused]] const CXXBaseSpecifier &Base : R->bases()) {
+        assert(InitIdx < Inits.size());
+        auto Init = Inits[InitIdx++];
+        assert(Base.getType().getCanonicalType() ==
+               Init->getType().getCanonicalType());
+        auto* BaseVal = cast_or_null<RecordValue>(Env.getValue(*Init));
+        if (!BaseVal)
+          BaseVal = cast<RecordValue>(Env.createValue(Init->getType()));
+        // Take ownership of the fields of the `RecordValue` for the base class
+        // and incorporate them into the "flattened" set of fields for the
+        // derived class.
+        auto Children = BaseVal->getLoc().children();
+        FieldLocs.insert(Children.begin(), Children.end());
+      }
     }
 
+    assert(FieldsForInit.size() == Inits.size() - InitIdx);
+    for (auto Field : FieldsForInit) {
+      assert(InitIdx < Inits.size());
+      auto Init = Inits[InitIdx++];
+      assert(
+          // The types are same, or
+          Field->getType().getCanonicalType().getUnqualifiedType() ==
+              Init->getType().getCanonicalType() ||
+          // The field's type is T&, and initializer is T
+          (Field->getType()->isReferenceType() &&
+              Field->getType().getCanonicalType()->getPointeeType() ==
+              Init->getType().getCanonicalType()));
+      auto& Loc = Env.createObject(Field->getType(), Init);
+      FieldLocs.insert({Field, &Loc});
+    }
+
+    LLVM_DEBUG({
+      // Check that we satisfy the invariant that a `RecordStorageLoation`
+      // contains exactly the set of modeled fields for that type.
+      // `ModeledFields` includes fields from all the bases, but only the
+      // modeled ones. However, if a class type is initialized with an
+      // `InitListExpr`, all fields in the class, including those from base
+      // classes, are included in the set of modeled fields. The code above
+      // should therefore populate exactly the modeled fields.
+      auto ModeledFields = Env.getDataflowAnalysisContext().getModeledFields(Type);
+      assert(ModeledFields.size() == FieldLocs.size());
+      for ([[maybe_unused]] auto [Field, Loc] : FieldLocs)
+        assert(ModeledFields.contains(cast_or_null<FieldDecl>(Field)));
+    });
+
     auto &Loc =
         Env.getDataflowAnalysisContext().arena().create<RecordStorageLocation>(
             Type, std::move(FieldLocs));

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

@@ -1446,6 +1446,99 @@ TEST(TransferTest, BaseClassInitializer) {
       llvm::Succeeded());
 }
 
+TEST(TransferTest, StructModeledFieldsWithComplicatedInheritance) {
+  std::string Code = R"(
+    struct Base1 {
+      int base1_1;
+      int base1_2;
+    };
+    struct Intermediate : Base1 {
+      int intermediate_1;
+      int intermediate_2;
+    };
+    struct Base2 {
+      int base2_1;
+      int base2_2;
+    };
+    struct MostDerived : public Intermediate, Base2 {
+      int most_derived_1;
+      int most_derived_2;
+    };
+
+    void target() {
+      MostDerived MD;
+      MD.base1_2 = 1;
+      MD.intermediate_2 = 1;
+      MD.base2_2 = 1;
+      MD.most_derived_2 = 1;
+      // [[p]]
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        const Environment &Env =
+              getEnvironmentAtAnnotation(Results, "p");
+
+        // Only the accessed fields should exist in the model.
+        auto &MDLoc = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "MD");
+        std::vector<const ValueDecl*> Fields;
+        for (auto [Field, _] : MDLoc.children())
+          Fields.push_back(Field);
+        ASSERT_THAT(Fields, UnorderedElementsAre(
+            findValueDecl(ASTCtx, "base1_2"),
+            findValueDecl(ASTCtx, "intermediate_2"),
+            findValueDecl(ASTCtx, "base2_2"),
+            findValueDecl(ASTCtx, "most_derived_2")));
+      });
+}
+
+TEST(TransferTest, StructInitializerListWithComplicatedInheritance) {
+  std::string Code = R"(
+    struct Base1 {
+      int base1;
+    };
+    struct Intermediate : Base1 {
+      int intermediate;
+    };
+    struct Base2 {
+      int base2;
+    };
+    struct MostDerived : public Intermediate, Base2 {
+      int most_derived;
+    };
+
+    void target() {
+      MostDerived MD = {};
+      // [[p]]
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        const Environment &Env =
+              getEnvironmentAtAnnotation(Results, "p");
+
+        // When a struct is initialized with a initializer list, all the
+        // fields are considered "accessed", and therefore do exist.
+        auto &MD = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "MD");
+        ASSERT_THAT(cast<IntegerValue>(
+            getFieldValue(&MD, *findValueDecl(ASTCtx, "base1"), Env)),
+            NotNull());
+        ASSERT_THAT(cast<IntegerValue>(
+            getFieldValue(&MD, *findValueDecl(ASTCtx, "intermediate"), Env)),
+            NotNull());
+        ASSERT_THAT(cast<IntegerValue>(
+            getFieldValue(&MD, *findValueDecl(ASTCtx, "base2"), Env)),
+            NotNull());
+        ASSERT_THAT(cast<IntegerValue>(
+            getFieldValue(&MD, *findValueDecl(ASTCtx, "most_derived"), Env)),
+            NotNull());
+      });
+}
+
 TEST(TransferTest, ReferenceMember) {
   std::string Code = R"(
     struct A {
@@ -2051,6 +2144,26 @@ TEST(TransferTest, AssignmentOperatorFromCallResult) {
       });
 }
 
+TEST(TransferTest, AssignmentOperatorWithInitAndInheritance) {
+  // This is a crash repro.
+  std::string Code = R"(
+    struct B { int Foo; };
+    struct S : public B {};
+    void target() {
+      S S1 = { 1 };
+      S S2;
+      S S3;
+      S1 = S2;  // Only Dst has InitListExpr.
+      S3 = S1;  // Only Src has InitListExpr.
+      // [[p]]
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {});
+}
+
 TEST(TransferTest, CopyConstructor) {
   std::string Code = R"(
     struct A {