Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -622,6 +622,11 @@ getIndexOfElementToConstruct(ProgramStateRef State, const CXXConstructExpr *E, const LocationContext *LCtx); + /// Retreives the size of the array in the pending ArrayInitLoopExpr. + static Optional 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. @@ -816,7 +821,9 @@ /// 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. @@ -916,6 +923,16 @@ 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 Index: clang/lib/Analysis/CFG.cpp =================================================================== --- clang/lib/Analysis/CFG.cpp +++ clang/lib/Analysis/CFG.cpp @@ -2927,9 +2927,13 @@ 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(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 Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -196,6 +196,14 @@ 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, unsigned> + PendingInitLoopMap; +REGISTER_TRAIT_WITH_PROGRAMSTATE(PendingInitLoop, PendingInitLoopMap) //===----------------------------------------------------------------------===// // Engine construction and deletion. //===----------------------------------------------------------------------===// @@ -462,6 +470,34 @@ return State->set(Key, Idx); } +Optional ExprEngine::getPendingInitLoop(ProgramStateRef State, + const CXXConstructExpr *E, + const LocationContext *LCtx) { + + return Optional::create( + State->get({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(Key)); + return State->remove(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(Key) && Size > 0); + + return State->set(Key, Size); +} + Optional ExprEngine::getIndexOfElementToConstruct(ProgramStateRef State, const CXXConstructExpr *E, @@ -490,8 +526,16 @@ const CXXConstructExpr *E = nullptr; if (auto DS = dyn_cast_or_null(Item.getStmtOrNull())) { - if (auto VD = dyn_cast_or_null(DS->getSingleDecl())) - E = dyn_cast(VD->getInit()); + if (auto VD = dyn_cast_or_null(DS->getSingleDecl())) { + const auto *Init = VD->getInit(); + + // In an ArrayInitLoopExpr the real initializer is returned by + // getSubExpr(). + if (const auto *AILE = dyn_cast(Init)) + Init = AILE->getSubExpr(); + + E = dyn_cast(Init); + } } if (!E && !Item.getStmtOrNull()) { @@ -2790,6 +2834,11 @@ const Expr *Arr = Ex->getCommonExpr()->getSourceExpr(); for (auto *Node : CheckerPreStmt) { + + // The constructor visitior has already taken care of everything. + if (auto *CE = dyn_cast(Ex->getSubExpr())) + break; + const LocationContext *LCtx = Node->getLocationContext(); ProgramStateRef state = Node->getState(); Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -462,6 +462,53 @@ llvm_unreachable("Unhandled construction context!"); } +static const ArrayInitLoopExpr * +tryGetArrayInitLoop(const ConstructionContext *CC) { + if (const auto *DSCC = dyn_cast_or_null(CC)) { + const auto *DS = DSCC->getDeclStmt(); + const auto *Var = cast(DS->getSingleDecl()); + + return dyn_cast_or_null(Var->getInit()); + } + + return nullptr; +} + +static ProgramStateRef +bindRequiredArrayElementToEnvironment(ProgramStateRef State, + const CXXConstructExpr *CE, + 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 + // |-OpaqueValueExpr + // | `-DeclRefExpr + // `-CXXConstructExpr <-- we're here + // `-ImplicitCastExpr + // `-ArraySubscriptExpr + // |-ImplicitCastExpr + // | `-OpaqueValueExpr + // | `-DeclRefExpr <-- match this + // `-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. + + auto *Arg0 = cast(CE->getArg(0)); + const auto ASE = cast(Arg0->getSubExpr()); + const auto ICE = cast(ASE->getLHS()); + const auto OVE = cast(ICE->getSubExpr()); + const auto DRE = cast(OVE->getSourceExpr()); + + SVal NthElem = + State->getLValue(CE->getType(), Idx, + State->getLValue(cast(DRE->getDecl()), LCtx)); + + return State->BindExpr(Arg0, LCtx, NthElem); +} + void ExprEngine::handleConstructor(const Expr *E, ExplodedNode *Pred, ExplodedNodeSet &destNodes) { @@ -502,12 +549,26 @@ // 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, CE, LCtx, svalBuilder.makeArrayIndex(Idx)); + } + // The target region is found from construction context. std::tie(State, Target) = handleConstructionContext(CE, State, LCtx, CC, CallOpts, Idx); Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -259,9 +259,13 @@ ShouldLoopCall = shouldRepeatCtorCall(state, CCE, callerCtx); - if (!ShouldLoopCall && - getIndexOfElementToConstruct(state, CCE, callerCtx)) - state = removeIndexOfElementToConstruct(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(CE)) { @@ -809,8 +813,7 @@ // We still allow construction into ElementRegion targets when they don't // represent array elements. if (CallOpts.IsArrayCtorOrDtor) { - if (!shouldInlineArrayConstruction( - dyn_cast(CtorExpr->getType()))) + if (!shouldInlineArrayConstruction(Pred->getState(), CtorExpr, CurLC)) return CIP_DisallowedOnce; } @@ -1076,10 +1079,14 @@ 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(Type)) { unsigned Size = getContext().getConstantArrayElementCount(CAT); @@ -1087,6 +1094,10 @@ 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; } @@ -1105,6 +1116,9 @@ return Size > getIndexOfElementToConstruct(State, E, LCtx); } + if (auto Size = getPendingInitLoop(State, E, LCtx)) + return Size > getIndexOfElementToConstruct(State, E, LCtx); + return false; } Index: clang/test/Analysis/uninit-structured-binding-array.cpp =================================================================== --- clang/test/Analysis/uninit-structured-binding-array.cpp +++ clang/test/Analysis/uninit-structured-binding-array.cpp @@ -292,3 +292,68 @@ 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 ©) { + 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}} +}