[analyzer] ArrayInitLoopExpr with array of non-POD type.

[isuckatcs]

Summary

At the moment we are unable to handle ArrayInitLoopExpr if the array it initializes is an array of non-POD types.
The expression is supposed to create a new array, and invoke the copy constructor for each element. The CFG only
contains the copy constructor, which is supposed to be invoked for each corresponding element, however it doesn't
contain the index of each element or a construction context.

The idea is to add a construction context to the copy ctor, so that after we evaluate it, the elements are placed in the
proper region of the store. Besides that in handleConstructor, we manually select which element of the array is going
to be passed to the ctor as parameter. Everything else is the same as it happens with a simple non-POD array construction,
in another patch, which this one depends on.

Depends on: https://reviews.llvm.org/D127973

[NoQ]

Once again very clean patch, thanks!! I have a few ideas on how to make it even neater.

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

466

Maybe turn this function into a member function of VariableConstructionContext? That'll make the caller code a bit more verbose but both the problem and the solution seems to be generic enough to be put there.

I wouldn't even mind subclassing ConstructionContext to handle this special case with a dedicated subclass, then the ArrayInitLoopExpr could become an entire layer. But that's probably an overkill since everything fits together nicely without it.

485

OpaqueValueExpr is often an indication that its sub-expression is duplicated in the AST (i.e., pointed-to from multiple different parents). I'm curious if the DeclRefExpr on the next line is the same DeclRefExpr that you're looking for. In this case we can simplify this function dramatically by consulting the ArrayInitLoopExpr instead (which is available in this function's caller).

[isuckatcs]

• ArrayInitLoopExpr with non-POD type array in implicit copy/move ctor is now handled properly.

In the third case, when a lambda captures a non-POD type array by value, there is no construction context,
so the objects would be constructed into the same temporary region. At the moment this case is ignored, and
I added a FIXME about it near the appropriate test case.

[isuckatcs]

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

466

Sadly we can't create 1 subclass that handles ArrayInitLoopExpr, as in all 3 different use cases we have a different ConstructionContext.

• in the implicit copy/move constructor for a class with an array member - ConstructorInitializerConstructionContext
• when a lambda-expression captures an array by value - <<NULL>>
• when a decomposition declaration decomposes an array - VariableConstructionContext

[isuckatcs]

• Added a virtual method to ConstructionContext that returns ArrayInitLoopExpr.

[NoQ]

Aha this looks great! Nitpicking time.

clang/include/clang/Analysis/ConstructionContext.h

325 ↗

(On Diff #444197)

I think or_null can be dropped because we can be certain that this variable's initializer contains, at the very least, the constructor to which the context is provided.

396 ↗

(On Diff #444197)

Same here, or_null can be dropped.

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

505–508

We should probably assert() that there's no else here. If this assert is ever hit, we'll know that we're missing something and change it to a FIXME.

[martong]

Nice work, thanks!

clang/test/Analysis/array-init-loop.cpp

164

I think, the matching should be more precise; it is a.i that is garbage, isn't it?
Perhaps this would also make sense (unless the previous warning is a sink):

clang_analyzer_eval(a.i); // expected-warning{{UNDEFINED}}

202–204

I think, It would be useful to emphase more explicitly what we want from the S5 type. Same above with S2.

Note, is_pod is basically is_trivial && is_standard_layout from C++20 (the PODType type requirement is deprecated since then).

[isuckatcs]

• Added a new ConstructionContext for lambas, which will be polished later.
• ArrayInitLoopExpr is evaluated properly in case of lambda captures.

[NoQ]

This looks awesome!

I think the lambda code should go into a separate patch, this one's already quite big and they're logically quite separate.

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

301–302

This tiny piece of code currently mimics/duplicates ExprEngine::VisitLambdaExpr.

I'm not sure if that tiny piece of code represents the correct solution in general, I suspect that eg. if lambda is put into a variable then it needs to be constructed directly in the variable, through copy elision (?) So eventually we'll probably need to assign a construction context to LambdaExpr itself, and then use both of them here, maybe recursively. But this is what we have for now.

So for this patch, I think the right thing to do is to have ExprEngine::VisitLambdaExpr() lookup the region created here instead, from objects-under-construction. This way future improvements could be done in one place, and the infrastructure will already be there.

1135

Should we clean up the state on the else-branch as well?

[isuckatcs]

Rebased onto latest relevant changes.

[xazax.hun]

clang/include/clang/Analysis/CFG.h

205–206 ↗

(On Diff #444822)

Nit: is this how clang format formatted this expression? Don't we miss a bit of indentation?

clang/include/clang/Analysis/ConstructionContext.h

705–710 ↗

(On Diff #444822)

We have many types of captures and iterators for them. Captures, capture inits, explicit captures, implicit captures. Are you sure that fields and capture inits can be paired up this way?
Also, I think capture iterators might be random access. you might be able to get to the right element with a single arithmetic. Unfortunately, that might not be the case for fields. But you could use std::advance for that.

[isuckatcs]

clang/include/clang/Analysis/CFG.h

205–206 ↗

(On Diff #444822)

Yes, clang format really formatted it this way. We miss the indentation, but if I add it and run clang format, it outputs the same result you see here.

[isuckatcs]

clang/include/clang/Analysis/ConstructionContext.h

705–710 ↗

(On Diff #444822)

Are you sure that fields and capture inits can be paired up this way?

Technically this solution is copied from VisitLamdaExpr, so I assumed it to be correct.

Since you created the original solution, I assume you already know the answer too, but I did
some digging regarding the question. The answer comes from Sema::BuildLambdaExpr.
They way the function works is that it iterates over the captures in the LambdaScopeInfo
and creates a FieldDecl and an initializer for each capture.

ExprResult Sema::BuildLambdaExpr(..., LambdaScopeInfo *LSI) {
  ...
  for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I) {
    const Capture &From = LSI->Captures[I];     
    ...
    // Form the initializer for the capture field.
    ExprResult Init = BuildCaptureInit(From, ImplicitCaptureLoc);

    // FIXME: Skip this capture if the capture is not used, the initializer
    // has no side-effects, the type of the capture is trivial, and the
    // lambda is not externally visible.

    // Add a FieldDecl for the capture and form its initializer.
    BuildCaptureField(Class, From);
    Captures.push_back(Capture);
    CaptureInits.push_back(Init.get());
    ...
  }
  ...
  Class->setCaptures(Context, Captures);
  ...
  LambdaExpr *Lambda = LambdaExpr::Create(..., CaptureInits, ...);
  ...
}

The captures and the initializers are put into 2 separate lists in the same order, so they
have the same indices, while the fields are directly constructed into the class.

FieldDecl *Sema::BuildCaptureField(RecordDecl *RD, const sema::Capture &Capture) {
  ...
  FieldDecl *Field = FieldDecl::Create(..., RD, ...);
  ...
  RD->addDecl(Field);
  ...
}

void DeclContext::addDecl(Decl *D) {
  addHiddenDecl(D);
  ...
}

void DeclContext::addHiddenDecl(Decl *D) {
  ...
  if (FirstDecl) {
    LastDecl->NextInContextAndBits.setPointer(D);
    LastDecl = D;
  } else {
    FirstDecl = LastDecl = D;
  }
  ...
}

The FieldDecls are stored in a linked list, where the new Decl is inserted to the end.
That means, the captures, the initializers and the FieldDecls have the same order and
the same index at this point.

Later the capture list is passed to the class, and the initializer list is passed to
LambdaExpr::Create(). In both cases the same simple linear copy is performed, so the
indices keep their original order. The fields are already in their final place and order.

void CXXRecordDecl::setCaptures(ASTContext &Context,
                                ArrayRef<LambdaCapture> Captures) {
  CXXRecordDecl::LambdaDefinitionData &Data = getLambdaData();

  // Copy captures.
  Data.NumCaptures = Captures.size();
  Data.NumExplicitCaptures = 0;
  Data.Captures = (LambdaCapture *)Context.Allocate(sizeof(LambdaCapture) *
                                                    Captures.size());
  LambdaCapture *ToCapture = Data.Captures;
  for (unsigned I = 0, N = Captures.size(); I != N; ++I) {
    if (Captures[I].isExplicit())
      ++Data.NumExplicitCaptures;

    *ToCapture++ = Captures[I];
  }

  if (!lambdaIsDefaultConstructibleAndAssignable())
    Data.DefaultedCopyAssignmentIsDeleted = true;
}

LambdaExpr::LambdaExpr(..., ArrayRef<Expr *> CaptureInits, ...){
  LambdaExprBits.NumCaptures = CaptureInits.size();
  LambdaExprBits.CaptureDefault = CaptureDefault; 
  ...
  CXXRecordDecl *Class = getLambdaClass();
  (void)Class;
  assert(capture_size() == Class->capture_size() && "Wrong number of captures");
  assert(getCaptureDefault() == Class->getLambdaCaptureDefault());
  ...
  // Copy initialization expressions for the non-static data members.
  Stmt **Stored = getStoredStmts();
  for (unsigned I = 0, N = CaptureInits.size(); I != N; ++I)
    *Stored++ = CaptureInits[I];  
  ...
}

Hence the capture and initializer indices kept their original orders, and the fields were also
created in the same order, we know that they still have the same indices. As a result the
iterators can be paired.

[isuckatcs]

• Renamed LambdaConstructionContext to LambdaCaptureConstructionContext.
• Addressed comments.
• Polished existing solutions.

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

1135

Capturing a VLA by value is a compile time error. If the VLA is captured by reference, there is no CXXConstructExpr we need to evaluate, so that case is not affected by this patch. I think there's nothing to clean up.

clang/test/Analysis/array-init-loop.cpp

164

Yes a.i is the Undefined variable, but the warning we get is a sink, so there's no way to confirm it (unless we look at the egraph).

[isuckatcs]

Rebased.

[NoQ]

clang/include/clang/Analysis/CFG.h

205–206 ↗

(On Diff #444822)

Hmm looks like there's extra paretheses after isa<ArgumentConstructionContext>(C).

[NoQ]

Aside from these nits and the gut feeling that the patch needs to be split, I don't have any other comments. The patch looks great, let's test it a bit and land it.

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

1135

Ok maybe assert() then, to make sure there's nothing to clean up(?)

[isuckatcs]

Addressed comments.

[isuckatcs]

Rebased

[isuckatcs]

Extracted the part about lambdas to a separate patch, D129967.

[isuckatcs]

Rebased

[NoQ]

Awesome, thanks, I'm happy with the code! Let's see how it performs in real life and then commit.

[isuckatcs]

Added testcases for structured binding syntax 3. Syntax 2 is not applicable for arrays.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

SVal evalBinOp(ProgramStateRef ST, BinaryOperator::Opcode Op,
     return svalBuilder.evalBinOp(ST, Op, LHS, RHS, T);
   }
 
   /// Retreives which element is being constructed in a non POD type array.
   static Optional<unsigned>
   getIndexOfElementToConstruct(ProgramStateRef State, const CXXConstructExpr *E,
                                const LocationContext *LCtx);
 
+  /// Retreives the size of the array in the pending ArrayInitLoopExpr.
+  static Optional<unsigned> getPendingInitLoop(ProgramStateRef State,
+                                               const CXXConstructExpr *E,
+                                               const LocationContext *LCtx);
+
   /// By looking at a certain item that may be potentially part of an object's
   /// ConstructionContext, retrieve such object's location. A particular
   /// statement can be transparently passed as \p Item in most cases.
   static Optional<SVal>
   getObjectUnderConstruction(ProgramStateRef State,
                              const ConstructionContextItem &Item,
                              const LocationContext *LC);
 
private:
 
   /// Checks our policies and decides weither the given call should be inlined.
   bool shouldInlineCall(const CallEvent &Call, const Decl *D,
                         const ExplodedNode *Pred,
                         const EvalCallOptions &CallOpts = {});
 
   /// Checks whether our policies allow us to inline a non-POD type array
   /// construction.
-  bool shouldInlineArrayConstruction(const ArrayType *Type);
+  bool shouldInlineArrayConstruction(const ProgramStateRef State,
+                                     const CXXConstructExpr *CE,
+                                     const LocationContext *LCtx);
 
   /// Checks whether we construct an array of non-POD type, and decides if the
   /// constructor should be inkoved once again.
   bool shouldRepeatCtorCall(ProgramStateRef State, const CXXConstructExpr *E,
                             const LocationContext *LCtx);
 
   void inlineCall(WorkList *WList, const CallEvent &Call, const Decl *D,
                   NodeBuilder &Bldr, ExplodedNode *Pred, ProgramStateRef State);
private:
   setIndexOfElementToConstruct(ProgramStateRef State, const CXXConstructExpr *E,
                                const LocationContext *LCtx, unsigned Idx);
 
   static ProgramStateRef
   removeIndexOfElementToConstruct(ProgramStateRef State,
                                   const CXXConstructExpr *E,
                                   const LocationContext *LCtx);
 
+  /// Sets the size of the array in a pending ArrayInitLoopExpr.
+  static ProgramStateRef setPendingInitLoop(ProgramStateRef State,
+                                            const CXXConstructExpr *E,
+                                            const LocationContext *LCtx,
+                                            unsigned Idx);
+
+  static ProgramStateRef removePendingInitLoop(ProgramStateRef State,
+                                               const CXXConstructExpr *E,
+                                               const LocationContext *LCtx);
+
   /// Store the location of a C++ object corresponding to a statement
   /// until the statement is actually encountered. For example, if a DeclStmt
   /// has CXXConstructExpr as its initializer, the object would be considered
   /// to be "under construction" between CXXConstructExpr and DeclStmt.
   /// This allows, among other things, to keep bindings to variable's fields
   /// made within the constructor alive until its declaration actually
   /// goes into scope.
   static ProgramStateRef

clang/lib/Analysis/CFG.cpp

if (BuildOpts.AddTemporaryDtors && HasTemporaries) {
                              /*ExternallyDestructed=*/false, Context);
     }
   }
 
   autoCreateBlock();
   appendInitializer(Block, I);
 
   if (Init) {
+    // If the initializer is an ArrayInitLoopExpr, we want to extract the
+    // initializer, that's used for each element.
+    const auto *AILE = dyn_cast_or_null<ArrayInitLoopExpr>(Init);
+
     findConstructionContexts(
         ConstructionContextLayer::create(cfg->getBumpVectorContext(), I),
-        Init);
+        AILE ? AILE->getSubExpr() : Init);
 
     if (HasTemporaries) {
       // For expression with temporaries go directly to subexpression to omit
       // generating destructors for the second time.
       return Visit(cast<ExprWithCleanups>(Init)->getSubExpr());
     }
     if (BuildOpts.AddCXXDefaultInitExprInCtors) {
       if (CXXDefaultInitExpr *Default = dyn_cast<CXXDefaultInitExpr>(Init)) {
if (BuildOpts.AddTemporaryDtors && HasTemporaries) {
       VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr(),
                              /*ExternallyDestructed=*/true, Context);
     }
   }
 
   autoCreateBlock();
   appendStmt(Block, DS);
 
+  // If the initializer is an ArrayInitLoopExpr, we want to extract the
+  // initializer, that's used for each element.
+  const auto *AILE = dyn_cast_or_null<ArrayInitLoopExpr>(Init);
+
   findConstructionContexts(
       ConstructionContextLayer::create(cfg->getBumpVectorContext(), DS),
-      Init);
+      AILE ? AILE->getSubExpr() : Init);
 
   // Keep track of the last non-null block, as 'Block' can be nulled out
   // if the initializer expression is something like a 'while' in a
   // statement-expression.
   CFGBlock *LastBlock = Block;
 
   if (Init) {
     if (HasTemporaries) {

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

 // memory region, which is important for multi-dimensional arrays. E.g:: int
 // arr[2][2]; assume arr[1][1] will be the next element under construction, so
 // the index is 3.
 typedef llvm::ImmutableMap<
     std::pair<const CXXConstructExpr *, const LocationContext *>, unsigned>
     IndexOfElementToConstructMap;
 REGISTER_TRAIT_WITH_PROGRAMSTATE(IndexOfElementToConstruct,
                                  IndexOfElementToConstructMap)
+
+// This trait is responsible for holding our pending ArrayInitLoopExprs.
+// It pairs the LocationContext and the initializer CXXConstructExpr with
+// the size of the array that's being copy initialized.
+typedef llvm::ImmutableMap<
+    std::pair<const CXXConstructExpr *, const LocationContext *>, unsigned>
+    PendingInitLoopMap;
+REGISTER_TRAIT_WITH_PROGRAMSTATE(PendingInitLoop, PendingInitLoopMap)
 //===----------------------------------------------------------------------===//
 // Engine construction and deletion.
 //===----------------------------------------------------------------------===//
 
 static const char* TagProviderName = "ExprEngine";
 
 ExprEngine::ExprEngine(cross_tu::CrossTranslationUnitContext &CTU,
                        AnalysisManager &mgr, SetOfConstDecls *VisitedCalleesIn,
ProgramStateRef ExprEngine::setIndexOfElementToConstruct(
     const LocationContext *LCtx, unsigned Idx) {
   auto Key = std::make_pair(E, LCtx->getStackFrame());
 
   assert(!State->contains<IndexOfElementToConstruct>(Key) || Idx > 0);
 
   return State->set<IndexOfElementToConstruct>(Key, Idx);
 }
 
+Optional<unsigned> ExprEngine::getPendingInitLoop(ProgramStateRef State,
+                                                  const CXXConstructExpr *E,
+                                                  const LocationContext *LCtx) {
+
+  return Optional<unsigned>::create(
+      State->get<PendingInitLoop>({E, LCtx->getStackFrame()}));
+}
+
+ProgramStateRef ExprEngine::removePendingInitLoop(ProgramStateRef State,
+                                                  const CXXConstructExpr *E,
+                                                  const LocationContext *LCtx) {
+  auto Key = std::make_pair(E, LCtx->getStackFrame());
+
+  assert(E && State->contains<PendingInitLoop>(Key));
+  return State->remove<PendingInitLoop>(Key);
+}
+
+ProgramStateRef ExprEngine::setPendingInitLoop(ProgramStateRef State,
+                                               const CXXConstructExpr *E,
+                                               const LocationContext *LCtx,
+                                               unsigned Size) {
+  auto Key = std::make_pair(E, LCtx->getStackFrame());
+
+  assert(!State->contains<PendingInitLoop>(Key) && Size > 0);
+
+  return State->set<PendingInitLoop>(Key, Size);
+}
+
 Optional<unsigned>
 ExprEngine::getIndexOfElementToConstruct(ProgramStateRef State,
                                          const CXXConstructExpr *E,
                                          const LocationContext *LCtx) {
 
   return Optional<unsigned>::create(
       State->get<IndexOfElementToConstruct>({E, LCtx->getStackFrame()}));
 }

 }
 
 ProgramStateRef
 ExprEngine::addObjectUnderConstruction(ProgramStateRef State,
                                        const ConstructionContextItem &Item,
                                        const LocationContext *LC, SVal V) {
   ConstructedObjectKey Key(Item, LC->getStackFrame());
 
-  const CXXConstructExpr *E = nullptr;
+  const Expr *Init = nullptr;
 
   if (auto DS = dyn_cast_or_null<DeclStmt>(Item.getStmtOrNull())) {
     if (auto VD = dyn_cast_or_null<VarDecl>(DS->getSingleDecl()))
-      E = dyn_cast<CXXConstructExpr>(VD->getInit());
+      Init = VD->getInit();
   }
 
-  if (!E && !Item.getStmtOrNull()) {
-    auto CtorInit = Item.getCXXCtorInitializer();
-    E = dyn_cast<CXXConstructExpr>(CtorInit->getInit());
-  }
+  if (!Init && !Item.getStmtOrNull())
+    Init = Item.getCXXCtorInitializer()->getInit();
+
+  // In an ArrayInitLoopExpr the real initializer is returned by
+  // getSubExpr().
+  if (const auto *AILE = dyn_cast_or_null<ArrayInitLoopExpr>(Init))
+    Init = AILE->getSubExpr();
+
+  const auto *E = dyn_cast_or_null<CXXConstructExpr>(Init);
 
   // FIXME: Currently the state might already contain the marker due to
   // incorrect handling of temporaries bound to default parameters.
   // The state will already contain the marker if we construct elements
   // in an array, as we visit the same statement multiple times before
   // the array declaration. The marker is removed when we exit the
   // constructor call.
   assert((!State->get<ObjectsUnderConstruction>(Key) ||
void ExprEngine::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *Ex,
   getCheckerManager().runCheckersForPreStmt(CheckerPreStmt, Pred, Ex, *this);
 
   ExplodedNodeSet EvalSet;
   StmtNodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx);
 
   const Expr *Arr = Ex->getCommonExpr()->getSourceExpr();
 
   for (auto *Node : CheckerPreStmt) {
+
+    // The constructor visitior has already taken care of everything.
+    if (auto *CE = dyn_cast<CXXConstructExpr>(Ex->getSubExpr()))
+      break;
+
     const LocationContext *LCtx = Node->getLocationContext();
     ProgramStateRef state = Node->getState();
 
     SVal Base = UnknownVal();
 
     // As in case of this expression the sub-expressions are not visited by any
     // other transfer functions, they are handled by matching their AST.
 

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

if (CC) {
     case ConstructionContext::ArgumentKind: {
       // Arguments are technically temporaries.
       CallOpts.IsTemporaryCtorOrDtor = true;
 
       const auto *ACC = cast<ArgumentConstructionContext>(CC);
       const Expr *E = ACC->getCallLikeExpr();
       unsigned Idx = ACC->getIndex();
 
       CallEventManager &CEMgr = getStateManager().getCallEventManager();
       auto getArgLoc = [&](CallEventRef<> Caller) -> Optional<SVal> {
         const LocationContext *FutureSFC =
             Caller->getCalleeStackFrame(currBldrCtx->blockCount());
         // Return early if we are unable to reliably foresee
         // the future stack frame.
         if (!FutureSFC)
           return None;
 
         // This should be equivalent to Caller->getDecl() for now, but
case ConstructionContext::ArgumentKind: {
 
       return addObjectUnderConstruction(
           State, {ACC->getCallLikeExpr(), ACC->getIndex()}, LCtx, V);
     }
   }
   llvm_unreachable("Unhandled construction context!");
 }
 
+static const ArrayInitLoopExpr *
+tryGetArrayInitLoop(const ConstructionContext *CC) {
+  if (const auto *DSCC = dyn_cast_or_null<VariableConstructionContext>(CC)) {
+    const auto *DS = DSCC->getDeclStmt();
+    const auto *Var = cast<VarDecl>(DS->getSingleDecl());
+
+    return dyn_cast_or_null<ArrayInitLoopExpr>(Var->getInit());
+  }
+
+  if (const auto *CICC =
+          dyn_cast_or_null<ConstructorInitializerConstructionContext>(CC)) {
+    const auto *CtorInit = CICC->getCXXCtorInitializer();
+    return dyn_cast_or_null<ArrayInitLoopExpr>(CtorInit->getInit());
+  }
+
+  return nullptr;
+}
+
+static ProgramStateRef
+bindRequiredArrayElementToEnvironment(ProgramStateRef State,
+                                      const ArrayInitLoopExpr *AILE,
+                                      const LocationContext *LCtx, SVal Idx) {
+  // The ctor in this case is guaranteed to be a copy ctor, otherwise we hit a
+  // compile time error.
+  //
+  //  -ArrayInitLoopExpr                <-- we're here
+  //   |-OpaqueValueExpr
+  //   | `-DeclRefExpr                  <-- match this
+  //   `-CXXConstructExpr
+  //     `-ImplicitCastExpr
+  //       `-ArraySubscriptExpr
+  //         |-ImplicitCastExpr
+  //         | `-OpaqueValueExpr
+  //         |   `-DeclRefExpr
+  //         `-ArrayInitIndexExpr
+  //
+  // The resulting expression might look like the one below in an implicit
+  // copy/move ctor.
+  //
+  //   ArrayInitLoopExpr                <-- we're here
+  //   |-OpaqueValueExpr
+  //   | `-MemberExpr                   <-- match this
+  //   |   `-DeclRefExpr
+  //   `-CXXConstructExpr
+  //     `-ArraySubscriptExpr
+  //       |-ImplicitCastExpr
+  //       | `-OpaqueValueExpr
+  //       |   `-MemberExpr
+  //       |     `-DeclRefExpr
+  //       `-ArrayInitIndexExpr
+  //
+  // HACK: There is no way we can put the index of the array element into the
+  // CFG unless we unroll the loop, so we manually select and bind the required
+  // parameter to the environment.
+  const auto *CE = cast<CXXConstructExpr>(AILE->getSubExpr());
+  const auto *OVESrc = AILE->getCommonExpr()->getSourceExpr();
+
+  SVal Base = UnknownVal();
+  if (const auto *ME = dyn_cast<MemberExpr>(OVESrc))
+    Base = State->getSVal(ME, LCtx);
+  else if (const auto *DRE = cast<DeclRefExpr>(OVESrc))
+    Base = State->getLValue(cast<VarDecl>(DRE->getDecl()), LCtx);
+
+  SVal NthElem = State->getLValue(CE->getType(), Idx, Base);
+
+  return State->BindExpr(CE->getArg(0), LCtx, NthElem);
+}
+
 void ExprEngine::handleConstructor(const Expr *E,
                                    ExplodedNode *Pred,
                                    ExplodedNodeSet &destNodes) {
   const auto *CE = dyn_cast<CXXConstructExpr>(E);
   const auto *CIE = dyn_cast<CXXInheritedCtorInitExpr>(E);
   assert(CE || CIE);
 
   const LocationContext *LCtx = Pred->getLocationContext();
void ExprEngine::handleConstructor(const Expr *E,
 
   const CXXConstructExpr::ConstructionKind CK =
       CE ? CE->getConstructionKind() : CIE->getConstructionKind();
   switch (CK) {
   case CXXConstructExpr::CK_Complete: {
     // Inherited constructors are always base class constructors.
     assert(CE && !CIE && "A complete constructor is inherited?!");
 
+    // If the ctor is part of an ArrayInitLoopExpr, we want to handle it
+    // differently.
+    auto *AILE = tryGetArrayInitLoop(CC);
+
     unsigned Idx = 0;
-    if (CE->getType()->isArrayType()) {
+    if (CE->getType()->isArrayType() || AILE) {
       Idx = getIndexOfElementToConstruct(State, CE, LCtx).getValueOr(0u);
       State = setIndexOfElementToConstruct(State, CE, LCtx, Idx + 1);
     }
 
+    if (AILE) {
+      // Only set this once even though we loop through it multiple times.
+      if (!getPendingInitLoop(State, CE, LCtx))
+        State = setPendingInitLoop(State, CE, LCtx,
+                                   AILE->getArraySize().getLimitedValue());
+
+      State = bindRequiredArrayElementToEnvironment(
+          State, AILE, LCtx, svalBuilder.makeArrayIndex(Idx));
+    }
+
     // The target region is found from construction context.
     std::tie(State, Target) =
         handleConstructionContext(CE, State, LCtx, CC, CallOpts, Idx);
     break;
   }
   case CXXConstructExpr::CK_VirtualBase: {
     // Make sure we are not calling virtual base class initializers twice.
     // Only the most-derived object should initialize virtual base classes.
for (LambdaExpr::const_capture_init_iterator i = LE->capture_init_begin(),
     FieldDecl *FieldForCapture = *CurField;
     SVal FieldLoc = State->getLValue(FieldForCapture, V);
 
     SVal InitVal;
     if (!FieldForCapture->hasCapturedVLAType()) {
       Expr *InitExpr = *i;
       assert(InitExpr && "Capture missing initialization expression");
       InitVal = State->getSVal(InitExpr, LocCtxt);
     } else {
       // The field stores the length of a captured variable-length array.
       // These captures don't have initialization expressions; instead we
       // get the length from the VLAType size expression.
       Expr *SizeExpr = FieldForCapture->getCapturedVLAType()->getSizeExpr();
       InitVal = State->getSVal(SizeExpr, LocCtxt);
     }
 
     State = State->bindLoc(FieldLoc, InitVal, LocCtxt);

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

if (const CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(CE)) {
       loc::MemRegionVal This =
         svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), calleeCtx);
       SVal ThisV = state->getSVal(This);
       ThisV = state->getSVal(ThisV.castAs<Loc>());
       state = state->BindExpr(CCE, callerCtx, ThisV);
 
       ShouldLoopCall = shouldRepeatCtorCall(state, CCE, callerCtx);
 
-      if (!ShouldLoopCall &&
-          getIndexOfElementToConstruct(state, CCE, callerCtx))
+      if (!ShouldLoopCall) {
+        if (getIndexOfElementToConstruct(state, CCE, callerCtx))
           state = removeIndexOfElementToConstruct(state, CCE, callerCtx);
+
+        if (getPendingInitLoop(state, CCE, callerCtx))
+          state = removePendingInitLoop(state, CCE, callerCtx);
+      }
     }
 
     if (const auto *CNE = dyn_cast<CXXNewExpr>(CE)) {
       // We are currently evaluating a CXXNewAllocator CFGElement. It takes a
       // while to reach the actual CXXNewExpr element from here, so keep the
       // region for later use.
       // Additionally cast the return value of the inlined operator new
       // (which is of type 'void *') to the correct object type.
if (llvm::isa_and_nonnull<NewAllocatedObjectConstructionContext>(CC) &&
       return CIP_DisallowedOnce;
 
     // FIXME: We don't handle constructors or destructors for arrays properly.
     // Even once we do, we still need to be careful about implicitly-generated
     // initializers for array fields in default move/copy constructors.
     // We still allow construction into ElementRegion targets when they don't
     // represent array elements.
     if (CallOpts.IsArrayCtorOrDtor) {
-      if (!shouldInlineArrayConstruction(
-              dyn_cast<ArrayType>(CtorExpr->getType())))
+      if (!shouldInlineArrayConstruction(Pred->getState(), CtorExpr, CurLC))
         return CIP_DisallowedOnce;
     }
 
     // Inlining constructors requires including initializers in the CFG.
     const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext();
     assert(ADC->getCFGBuildOptions().AddInitializers && "No CFG initializers");
     (void)ADC;
 
bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
   }
 
   if (HowToInline == Inline_Minimal && (!isSmall(CalleeADC) || IsRecursive))
     return false;
 
   return true;
 }
 
-bool ExprEngine::shouldInlineArrayConstruction(const ArrayType *Type) {
-  if (!Type)
+bool ExprEngine::shouldInlineArrayConstruction(const ProgramStateRef State,
+                                               const CXXConstructExpr *CE,
+                                               const LocationContext *LCtx) {
+  if (!CE)
     return false;
 
+  auto Type = CE->getType();
+
   // FIXME: Handle other arrays types.
   if (const auto *CAT = dyn_cast<ConstantArrayType>(Type)) {
     unsigned Size = getContext().getConstantArrayElementCount(CAT);
 
     return Size <= AMgr.options.maxBlockVisitOnPath;
   }
 
+  // Check if we're inside an ArrayInitLoopExpr, and it's sufficiently small.
+  if (auto Size = getPendingInitLoop(State, CE, LCtx))
+    return *Size <= AMgr.options.maxBlockVisitOnPath;
+
   return false;
 }
 
 bool ExprEngine::shouldRepeatCtorCall(ProgramStateRef State,
                                       const CXXConstructExpr *E,
                                       const LocationContext *LCtx) {
 
   if (!E)
     return false;
 
   auto Ty = E->getType();
 
   // FIXME: Handle non constant array types
   if (const auto *CAT = dyn_cast<ConstantArrayType>(Ty)) {
     unsigned Size = getContext().getConstantArrayElementCount(CAT);
     return Size > getIndexOfElementToConstruct(State, E, LCtx);
   }
 
+  if (auto Size = getPendingInitLoop(State, E, LCtx))
+    return Size > getIndexOfElementToConstruct(State, E, LCtx);
+
   return false;
 }
 
 static bool isTrivialObjectAssignment(const CallEvent &Call) {
   const CXXInstanceCall *ICall = dyn_cast<CXXInstanceCall>(&Call);
   if (!ICall)
     return false;
 

clang/test/Analysis/array-init-loop.cpp

void move_ctor_uninit() {
 
   // FIXME: These should be Undefined, but we fail to read Undefined from a lazyCompoundVal.
   clang_analyzer_eval(moved.arr[0]); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(moved.arr[1]); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(moved.arr[2]); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(moved.arr[3]); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(moved.arr[4]); // expected-warning{{UNKNOWN}}
 }
+
+struct S2 {
+  inline static int c = 0;
+  int i;
+
+  S2() : i(++c) {}
+
+  S2(const S2 &copy) {
+    i = copy.i + 1;
+  }
+
+  S2(S2 &&move) {
+    i = move.i + 2;
+  }
+};
+
+void array_init_non_pod() {
+  S2::c = 0;
+  S2 arr[4];
+
+  auto [a, b, c, d] = arr;
+
+  clang_analyzer_eval(a.i == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(b.i == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(c.i == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d.i == 5); // expected-warning{{TRUE}}
+}
+
+struct S3 {
+  int i;
+};
+
+void array_uninit_non_pod() {
+  S3 arr[1];
+
+  auto [a] = arr;
+  // expected-warning@159{{ in implicit constructor is garbage or undefined }}
+}
+
+void lambda_init_non_pod() {
+  S2::c = 0;
+  S2 arr[4];
+
+  // FIXME: These should be True, but we fail to construct the array in a lambda capture.
+  auto l = [arr] { return arr[0].i; }();
+  clang_analyzer_eval(l == 2); // expected-warning{{TRUE}} expected-warning{{FALSE}}
+
+  l = [arr] { return arr[1].i; }();
+  clang_analyzer_eval(l == 3); // expected-warning{{TRUE}} expected-warning{{FALSE}}
+
+  l = [arr] { return arr[2].i; }();
+  clang_analyzer_eval(l == 4); // expected-warning{{TRUE}} expected-warning{{FALSE}}
+
+  l = [arr] { return arr[3].i; }();
+  clang_analyzer_eval(l == 5); // expected-warning{{TRUE}} expected-warning{{FALSE}}
+}
+
+void lambda_uninit_non_pod() {
+  S3 arr[4];
+
+  // FIXME: These should be Undefined, but we fail to read Undefined from a lazyCompoundVal.
+  int l = [arr] { return arr[0].i; }();
+  clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
+
+  l = [arr] { return arr[1].i; }();
+  clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
+
+  l = [arr] { return arr[2].i; }();
+  clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
+
+  l = [arr] { return arr[3].i; }();
+  clang_analyzer_eval(l); // expected-warning{{UNKNOWN}}
+}
+
+struct S5 {
+  S2 arr[4];
+};
+
+void copy_ctor_init_non_pod() {
+  S2::c = 0;
+  S5 orig;
+
+  S5 copy = orig;
+  clang_analyzer_eval(copy.arr[0].i == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(copy.arr[1].i == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(copy.arr[2].i == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(copy.arr[3].i == 5); // expected-warning{{TRUE}}
+}
+
+void move_ctor_init_non_pod() {
+  S2::c = 0;
+  S5 orig;
+
+  S5 moved = (S5 &&) orig;
+
+  clang_analyzer_eval(moved.arr[0].i == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(moved.arr[1].i == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(moved.arr[2].i == 5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(moved.arr[3].i == 6); // expected-warning{{TRUE}}
+}

clang/test/Analysis/ctor.mm

void testNonPOD() {
     a.values[2].x = 3;
 
     clang_analyzer_eval(a.values[0].x == 1); // expected-warning{{TRUE}}
     clang_analyzer_eval(a.values[1].x == 2); // expected-warning{{TRUE}}
     clang_analyzer_eval(a.values[2].x == 3); // expected-warning{{TRUE}}
 
     NonPOD b = a;
 
-    clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{UNKNOWN}}
-    clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{UNKNOWN}}
-    clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{TRUE}}
+    clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{TRUE}}
 
     NonPOD c;
     c = b;
 
-    clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{UNKNOWN}}
-    clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{UNKNOWN}}
-    clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{TRUE}}
+    clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{TRUE}}
   }
 
   struct NestedNonPOD {
     NonPODIntWrapper values[2][3];
   };
 
   void testNestedNonPOD() {
     NestedNonPOD a;
void testNonPODDefaulted() {
     a.values[2].x = 3;
 
     clang_analyzer_eval(a.values[0].x == 1); // expected-warning{{TRUE}}
     clang_analyzer_eval(a.values[1].x == 2); // expected-warning{{TRUE}}
     clang_analyzer_eval(a.values[2].x == 3); // expected-warning{{TRUE}}
 
     NonPODDefaulted b = a;
 
-    clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{UNKNOWN}}
-    clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{UNKNOWN}}
-    clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{TRUE}}
+    clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{TRUE}}
 
     NonPODDefaulted c;
     c = b;
 
-    clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{UNKNOWN}}
-    clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{UNKNOWN}}
-    clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{TRUE}}
+    clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{TRUE}}
   }
 };
 
 namespace VirtualInheritance {
   int counter;
 
   struct base {
     base() {

clang/test/Analysis/uninit-structured-binding-array.cpp

void array_big_a(void) {
   // FIXME: These will be Undefined when we handle reading Undefined values from lazyCompoundVal.
   clang_analyzer_eval(a == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(b == 2); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(c == 3); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(d == 4); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(e == 5); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(f == 6); // expected-warning{{UNKNOWN}}
 }
+
+struct S {
+  int a = 1;
+  int b = 2;
+};
+
+void non_pod_val(void) {
+  S arr[2];
+
+  auto [x, y] = arr;
+
+  clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
+}
+
+void non_pod_lref(void) {
+  S arr[2];
+
+  auto &[x, y] = arr;
+
+  clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
+}
+
+void non_pod_rref(void) {
+  S arr[2];
+
+  auto &&[x, y] = arr;
+
+  clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
+}
+
+struct SUD {
+  int a = 1;
+  int b = 2;
+
+  SUD() = default;
+
+  SUD(const SUD &copy) {
+    a = copy.a + 1;
+    b = copy.b + 1;
+  }
+};
+
+void non_pod_user_defined_val(void) {
+  SUD arr[2];
+
+  auto [x, y] = arr;
+
+  clang_analyzer_eval(x.a == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 3); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 3); // expected-warning{{TRUE}}
+}