[clang][dataflow] Initialize fields of anonymous records correctly.

[mboehme]

Summary

Previously, the newly added test would crash.

Depends On D153851

[mboehme]

clang/lib/Analysis/FlowSensitive/Transfer.cpp

575

It turns out this was doing strictly too much work.

CXXDefaultInitExpr should only be producing the default value. The actual initialization should be left to CXXCtorInitializer.

[mboehme]

Ensure we model a field of an anonymous record even if the only place it's used
is in an initializer.

[mboehme]

Pre-merge failure looks unrelated

[gribozavr2]

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

390

Is there a chance that there will be no such child if we hit one of our limits for modeling recursive data structures?

[mboehme]

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

390

Happily, this isn't a concern. AggregateStorageLocations always contain the same set of fields (the ones that are "modeled", i.e. accessed in some way). And this is a nice invariant to have.

Where we do the cutoff for modeling recursive data structures is that the storage location returned by getChild() may not have a Value associated with it.

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

void Environment::initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl) {
   llvm::DenseSet<const FieldDecl *> Fields;
   llvm::DenseSet<const VarDecl *> Vars;
   llvm::DenseSet<const FunctionDecl *> Funcs;
 
   // Look for global variable and field references in the
   // constructor-initializers.
   if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(FuncDecl)) {
     for (const auto *Init : CtorDecl->inits()) {
-      if (const auto *M = Init->getAnyMember())
-          Fields.insert(M);
+      if (Init->isMemberInitializer()) {
+        Fields.insert(Init->getMember());
+      } else if (Init->isIndirectMemberInitializer()) {
+        for (const auto *I : Init->getIndirectMember()->chain())
+          Fields.insert(cast<FieldDecl>(I));
+      }
       const Expr *E = Init->getInit();
       assert(E != nullptr);
       getFieldsGlobalsAndFuncs(*E, Fields, Vars, Funcs);
     }
     // Add all fields mentioned in default member initializers.
     for (const FieldDecl *F : CtorDecl->getParent()->fields())
       if (const auto *I = F->getInClassInitializer())
           getFieldsGlobalsAndFuncs(*I, Fields, Vars, Funcs);

clang/lib/Analysis/FlowSensitive/Transfer.cpp

 }
 
 static void propagateStorageLocation(const Expr &From, const Expr &To,
                                      Environment &Env) {
   if (auto *Loc = Env.getStorageLocationStrict(From))
     Env.setStorageLocationStrict(To, *Loc);
 }
 
-// Forwards the value or storage location of `From` to `To` in cases where
+// Propagates the value or storage location of `From` to `To` in cases where
 // `From` may be either a glvalue or a prvalue. `To` must be a glvalue iff
 // `From` is a glvalue.
 static void propagateValueOrStorageLocation(const Expr &From, const Expr &To,
                                             Environment &Env) {
   assert(From.isGLValue() == To.isGLValue());
   if (From.isGLValue())
     propagateStorageLocation(From, To, Env);
   else
if (MemberLoc.getType()->isReferenceType()) {
       Env.setStorageLocation(*S, Loc);
       Env.setValue(Loc, Env.create<ReferenceValue>(MemberLoc));
     }
   }
 
   void VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *S) {
     const Expr *InitExpr = S->getExpr();
     assert(InitExpr != nullptr);
-
-    Value *InitExprVal = Env.getValue(*InitExpr, SkipPast::None);
-    if (InitExprVal == nullptr)
-      return;
-
-    const FieldDecl *Field = S->getField();
-    assert(Field != nullptr);
-
-    auto &ThisLoc =
-        *cast<AggregateStorageLocation>(Env.getThisPointeeStorageLocation());
-    auto &FieldLoc = ThisLoc.getChild(*Field);
-    Env.setValue(FieldLoc, *InitExprVal);
+    propagateValueOrStorageLocation(*InitExpr, *S, Env);
   }
 
   void VisitCXXConstructExpr(const CXXConstructExpr *S) {
     const CXXConstructorDecl *ConstructorDecl = S->getConstructor();
     assert(ConstructorDecl != nullptr);
 
     if (ConstructorDecl->isCopyOrMoveConstructor()) {
       // It is permissible for a copy/move constructor to have additional

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

 /// Built-in transfer function for `CFGInitializer`.
 static void
 builtinTransferInitializer(const CFGInitializer &Elt,
                            TypeErasedDataflowAnalysisState &InputState) {
   const CXXCtorInitializer *Init = Elt.getInitializer();
   assert(Init != nullptr);
 
   auto &Env = InputState.Env;
-  const auto &ThisLoc =
+  auto &ThisLoc =
       *cast<AggregateStorageLocation>(Env.getThisPointeeStorageLocation());
 
-  const FieldDecl *Member = Init->getMember();
-  if (Member == nullptr)
-    // Not a field initializer.
+  if (!Init->isAnyMemberInitializer())
+    // FIXME: Handle base initialization
     return;
 
   auto *InitStmt = Init->getInit();
   assert(InitStmt != nullptr);
 
+  const FieldDecl *Member = nullptr;
+  StorageLocation *MemberLoc = nullptr;
+  if (Init->isMemberInitializer()) {
+    Member = Init->getMember();
+    MemberLoc = &ThisLoc.getChild(*Member);
+  } else {
+    IndirectFieldDecl *IndirectField = Init->getIndirectMember();
+    assert(IndirectField != nullptr);
+    MemberLoc = &ThisLoc;
+    for (const auto *I : IndirectField->chain()) {
+      Member = cast<FieldDecl>(I);
+      MemberLoc = &cast<AggregateStorageLocation>(MemberLoc)->getChild(*Member);
+    }
+  }
+  assert(Member != nullptr);
+  assert(MemberLoc != nullptr);
+
   if (Member->getType()->isReferenceType()) {
     auto *InitStmtLoc = Env.getStorageLocationStrict(*InitStmt);
     if (InitStmtLoc == nullptr)
       return;
 
-    auto &MemberLoc = ThisLoc.getChild(*Member);
-    Env.setValue(MemberLoc, Env.create<ReferenceValue>(*InitStmtLoc));
+    Env.setValue(*MemberLoc, Env.create<ReferenceValue>(*InitStmtLoc));
   } else if (auto *InitStmtVal = Env.getValueStrict(*InitStmt)) {
-    auto &MemberLoc = ThisLoc.getChild(*Member);
-    Env.setValue(MemberLoc, *InitStmtVal);
+    Env.setValue(*MemberLoc, *InitStmtVal);
   }
 }
 
 static void builtinTransfer(const CFGElement &Elt,
                             TypeErasedDataflowAnalysisState &State,
                             AnalysisContext &AC) {
   switch (Elt.getKind()) {
   case CFGElement::Statement:

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

runDataflow(
         auto &AnonStruct = cast<AggregateStorageLocation>(
             S->getChild(*cast<ValueDecl>(IndirectField->chain().front())));
 
         auto *B = cast<BoolValue>(Env.getValue(AnonStruct.getChild(*BDecl)));
         ASSERT_TRUE(Env.flowConditionImplies(*B));
       });
 }
 
+TEST(TransferTest, AnonymousStructWithInitializer) {
+  std::string Code = R"(
+    struct target {
+      target() {
+        (void)0;
+        // [[p]]
+      }
+      struct {
+        bool b = true;
+      };
+    };
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+        const ValueDecl *BDecl = findValueDecl(ASTCtx, "b");
+        const IndirectFieldDecl *IndirectField =
+            findIndirectFieldDecl(ASTCtx, "b");
+
+        auto *ThisLoc =
+            cast<AggregateStorageLocation>(Env.getThisPointeeStorageLocation());
+        auto &AnonStruct = cast<AggregateStorageLocation>(ThisLoc->getChild(
+            *cast<ValueDecl>(IndirectField->chain().front())));
+
+        auto *B = cast<BoolValue>(Env.getValue(AnonStruct.getChild(*BDecl)));
+        ASSERT_TRUE(Env.flowConditionImplies(*B));
+      });
+}
+
 } // namespace