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 @@ -613,6 +613,11 @@ return svalBuilder.evalBinOp(ST, Op, LHS, RHS, T); } + /// Retreives which element is being constructed in a non POD type array. + static Optional + getIndexOfElementToConstruct(ProgramStateRef State, const Expr *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. @@ -705,9 +710,11 @@ /// out-parameter CallOpts; in such cases a fake temporary region is /// returned, which is better than nothing but does not represent /// the actual behavior of the program. - SVal computeObjectUnderConstruction( - const Expr *E, ProgramStateRef State, const LocationContext *LCtx, - const ConstructionContext *CC, EvalCallOptions &CallOpts); + SVal computeObjectUnderConstruction(const Expr *E, ProgramStateRef State, + const LocationContext *LCtx, + const ConstructionContext *CC, + EvalCallOptions &CallOpts, + unsigned Idx = 0); /// Update the program state with all the path-sensitive information /// that's necessary to perform construction of an object with a given @@ -723,9 +730,23 @@ std::pair handleConstructionContext( const Expr *E, ProgramStateRef State, const LocationContext *LCtx, const ConstructionContext *CC, EvalCallOptions &CallOpts) { - SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts); - return std::make_pair( - updateObjectsUnderConstruction(V, E, State, LCtx, CC, CallOpts), V); + + unsigned Idx = 0; + if (const ConstantArrayType *CAT = + dyn_cast(E->getType())) { + if (auto OptionalIdx = getIndexOfElementToConstruct(State, E, LCtx)) { + if (shouldRepeatCtorCall(State, E, LCtx)) { + Idx = *OptionalIdx; + } + } + } + + SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts, Idx); + State = updateObjectsUnderConstruction(V, E, State, LCtx, CC, CallOpts); + + State = addIndexOfElementToConstruct(State, E, LCtx, Idx + 1); + + return std::make_pair(State, V); } private: @@ -792,6 +813,11 @@ const ExplodedNode *Pred, const EvalCallOptions &CallOpts = {}); + /// Checks whether we construct an array of non POD typed, and decides if the + /// constructor should be inkoved once again. + bool shouldRepeatCtorCall(ProgramStateRef State, const Expr *E, + const LocationContext *LCtx); + void inlineCall(WorkList *WList, const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, ExplodedNode *Pred, ProgramStateRef State); @@ -843,7 +869,8 @@ /// If the type is not an array type at all, the original value is returned. /// Otherwise the "IsArray" flag is set. static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue, - QualType &Ty, bool &IsArray); + QualType &Ty, bool &IsArray, + unsigned Idx = 0); /// For a DeclStmt or CXXInitCtorInitializer, walk backward in the current CFG /// block to find the constructor expression that directly constructed into @@ -874,6 +901,16 @@ const ObjCForCollectionStmt *O, const LocationContext *LC); private: + /// Assuming we construct an array of non POD types, this method allows us + /// to store which element is to be constructed next. + static ProgramStateRef + addIndexOfElementToConstruct(ProgramStateRef State, const Expr *E, + const LocationContext *LCtx, unsigned Idx); + + static ProgramStateRef + removeIndexOfElementToConstruct(ProgramStateRef State, const Expr *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/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -185,6 +185,11 @@ REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction, ObjectsUnderConstructionMap) +typedef llvm::ImmutableMap, + unsigned> + IndexOfElementToConstructMap; +REGISTER_TRAIT_WITH_PROGRAMSTATE(IndexOfElementToConstruct, + IndexOfElementToConstructMap) //===----------------------------------------------------------------------===// // Engine construction and deletion. //===----------------------------------------------------------------------===// @@ -441,6 +446,31 @@ return State; } +ProgramStateRef +ExprEngine::addIndexOfElementToConstruct(ProgramStateRef State, const Expr *E, + const LocationContext *LCtx, + unsigned Idx) { + + return State->set({E, LCtx->getStackFrame()}, Idx); +} + +Optional +ExprEngine::getIndexOfElementToConstruct(ProgramStateRef State, const Expr *E, + const LocationContext *LCtx) { + + return Optional::create( + State->get({E, LCtx->getStackFrame()})); +} + +ProgramStateRef ExprEngine::removeIndexOfElementToConstruct( + ProgramStateRef State, const Expr *E, const LocationContext *LCtx) { + auto Key = std::make_pair(E, LCtx->getStackFrame()); + + if (!E || !State->contains(Key)) + return State; + return State->remove(Key); +} + ProgramStateRef ExprEngine::addObjectUnderConstruction(ProgramStateRef State, const ConstructionContextItem &Item, @@ -448,6 +478,9 @@ ConstructedObjectKey Key(Item, LC->getStackFrame()); // FIXME: Currently the state might already contain the marker due to // incorrect handling of temporaries bound to default parameters. + if (State->get(Key)) + return State; + assert(!State->get(Key) || Key.getItem().getKind() == ConstructionContextItem::TemporaryDestructorKind); Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -94,15 +94,18 @@ } } - SVal ExprEngine::makeZeroElementRegion(ProgramStateRef State, SVal LValue, - QualType &Ty, bool &IsArray) { + QualType &Ty, bool &IsArray, + unsigned Idx) { SValBuilder &SVB = State->getStateManager().getSValBuilder(); ASTContext &Ctx = SVB.getContext(); while (const ArrayType *AT = Ctx.getAsArrayType(Ty)) { - Ty = AT->getElementType(); - LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue); + while (AT) { + Ty = AT->getElementType(); + AT = dyn_cast(AT->getElementType()); + } + LValue = State->getLValue(Ty, SVB.makeArrayIndex(Idx), LValue); IsArray = true; } @@ -111,7 +114,7 @@ SVal ExprEngine::computeObjectUnderConstruction( const Expr *E, ProgramStateRef State, const LocationContext *LCtx, - const ConstructionContext *CC, EvalCallOptions &CallOpts) { + const ConstructionContext *CC, EvalCallOptions &CallOpts, unsigned Idx) { SValBuilder &SVB = getSValBuilder(); MemRegionManager &MRMgr = SVB.getRegionManager(); ASTContext &ACtx = SVB.getContext(); @@ -126,7 +129,7 @@ const auto *Var = cast(DS->getSingleDecl()); QualType Ty = Var->getType(); return makeZeroElementRegion(State, State->getLValue(Var, LCtx), Ty, - CallOpts.IsArrayCtorOrDtor); + CallOpts.IsArrayCtorOrDtor, Idx); } case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind: case ConstructionContext::SimpleConstructorInitializerKind: { @@ -159,7 +162,7 @@ QualType Ty = Field->getType(); return makeZeroElementRegion(State, FieldVal, Ty, - CallOpts.IsArrayCtorOrDtor); + CallOpts.IsArrayCtorOrDtor, Idx); } case ConstructionContext::NewAllocatedObjectKind: { if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) { @@ -172,8 +175,12 @@ // TODO: In fact, we need to call the constructor for every // allocated element, not just the first one! CallOpts.IsArrayCtorOrDtor = true; - return loc::MemRegionVal(getStoreManager().GetElementZeroRegion( - MR, NE->getType()->getPointeeType())); + + auto R = MRMgr.getElementRegion(NE->getType()->getPointeeType(), + svalBuilder.makeArrayIndex(Idx), MR, + SVB.getContext()); + + return loc::MemRegionVal(R); } return V; } @@ -484,10 +491,6 @@ } } - // FIXME: Handle arrays, which run the same constructor for every element. - // For now, we just run the first constructor (which should still invalidate - // the entire array). - EvalCallOptions CallOpts; auto C = getCurrentCFGElement().getAs(); assert(C || getCurrentCFGElement().getAs()); Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -359,8 +359,18 @@ // Enqueue the next element in the block. for (ExplodedNodeSet::iterator PSI = Dst.begin(), PSE = Dst.end(); PSI != PSE; ++PSI) { - Engine.getWorkList()->enqueue(*PSI, calleeCtx->getCallSiteBlock(), - calleeCtx->getIndex()+1); + + unsigned Idx = calleeCtx->getIndex(); + ProgramStateRef PSIState = (*PSI)->getState(); + if (!shouldRepeatCtorCall(PSIState, dyn_cast_or_null(CE), + callerCtx)) { + ++Idx; + } + + PSIState = removeIndexOfElementToConstruct( + PSIState, dyn_cast_or_null(CE), callerCtx); + + Engine.getWorkList()->enqueue(*PSI, calleeCtx->getCallSiteBlock(), Idx); } } } @@ -795,14 +805,6 @@ !Opts.MayInlineCXXAllocator) 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) - return CIP_DisallowedOnce; - // Inlining constructors requires including initializers in the CFG. const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); assert(ADC->getCFGBuildOptions().AddInitializers && "No CFG initializers"); @@ -852,10 +854,6 @@ assert(ADC->getCFGBuildOptions().AddImplicitDtors && "No CFG destructors"); (void)ADC; - // FIXME: We don't handle constructors or destructors for arrays properly. - if (CallOpts.IsArrayCtorOrDtor) - return CIP_DisallowedOnce; - // Allow disabling temporary destructor inlining with a separate option. if (CallOpts.IsTemporaryCtorOrDtor && !Opts.MayInlineCXXTemporaryDtors) @@ -1065,6 +1063,37 @@ return true; } +bool ExprEngine::shouldRepeatCtorCall(ProgramStateRef State, const Expr *E, + const LocationContext *LCtx) { + + if (!E) + return false; + + if (!dyn_cast(E)) + return false; + + // FIXME: Handle non constant array types + auto Ty = E->getType(); + if (const auto *CAT = dyn_cast(Ty)) { + // In case of multi dimensional arrays the type is + // ConstantArrayType + // `ConstantArrayType + // `... + // + // To get the count of the elements we traverse this chain + // and multiply the sizes of the sub arrays. + + unsigned Size = 1; + while (CAT) { + Size *= CAT->getSize().getLimitedValue(); + CAT = dyn_cast(CAT->getElementType()); + } + return Size > getIndexOfElementToConstruct(State, E, LCtx); + } + + return false; +} + static bool isTrivialObjectAssignment(const CallEvent &Call) { const CXXInstanceCall *ICall = dyn_cast(&Call); if (!ICall) Index: clang/test/Analysis/ctor-array.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/ctor-array.cpp @@ -0,0 +1,110 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=constructors -verify %s + +void clang_analyzer_eval(bool); + +struct s { + int x; + int y; +}; + +void a1(void) { + s arr[3]; + int x = arr[0].x; + // expected-warning@-1{{Assigned value is garbage or undefined}} +} + +void a2(void) { + s arr[3]; + int x = arr[1].x; + // expected-warning@-1{{Assigned value is garbage or undefined}} +} + +void a3(void) { + s arr[3]; + int x = arr[2].x; + // expected-warning@-1{{Assigned value is garbage or undefined}} +} + +struct s2 { + int x; + int y = 2; +}; + +void b1(void) { + s2 arr[3]; + + clang_analyzer_eval(arr[0].y == 2); // expected-warning{{TRUE}} + int x = arr[0].x; + // expected-warning@-1{{Assigned value is garbage or undefined}} +} + +void b2(void) { + s2 arr[3]; + + clang_analyzer_eval(arr[1].y == 2); // expected-warning{{TRUE}} + int x = arr[1].x; + // expected-warning@-1{{Assigned value is garbage or undefined}} +} + +void b3(void) { + s2 arr[3]; + + clang_analyzer_eval(arr[2].y == 2); // expected-warning{{TRUE}} + int x = arr[2].x; + // expected-warning@-1{{Assigned value is garbage or undefined}} +} + +void c1(void) { + { + s2 arr[2]; + arr[1].x = 3; + + clang_analyzer_eval(arr[1].y == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(arr[1].x == 3); // expected-warning{{TRUE}} + } + + { + s2 arr[2]; + + clang_analyzer_eval(arr[1].y == 2); // expected-warning{{TRUE}} + int x = arr[1].x; + // expected-warning@-1{{Assigned value is garbage or undefined}} + } +} + +struct s3 { + int x = 1; + int y = 2; +}; + +struct s4 { + s3 arr[2]; + s sarr[2]; +}; + +void e1(void) { + s4 arr[2]; + + clang_analyzer_eval(arr[0].arr[0].x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(arr[0].arr[0].y == 2); // expected-warning{{TRUE}} + + clang_analyzer_eval(arr[0].arr[1].x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(arr[0].arr[1].y == 2); // expected-warning{{TRUE}} + + clang_analyzer_eval(arr[1].arr[0].x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(arr[1].arr[0].y == 2); // expected-warning{{TRUE}} + + clang_analyzer_eval(arr[1].arr[1].x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(arr[1].arr[1].y == 2); // expected-warning{{TRUE}} + + int x = arr[1].sarr[1].x; + // expected-warning@-1{{Assigned value is garbage or undefined}} +} + +void f1(void) { + s2 arr[2][2]; + + clang_analyzer_eval(arr[1][1].y == 2); // expected-warning{{TRUE}} + int x = arr[1][1].x; + // expected-warning@-1{{Assigned value is garbage or undefined}} +} \ No newline at end of file Index: clang/test/Analysis/ctor.mm =================================================================== --- clang/test/Analysis/ctor.mm +++ clang/test/Analysis/ctor.mm @@ -581,12 +581,11 @@ } void testArrayNew() { - // FIXME: Pending proper implementation of constructors for 'new[]'. raw_pair *p = new raw_pair[2](); - clang_analyzer_eval(p[0].p1 == 0); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(p[0].p2 == 0); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(p[1].p1 == 0); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(p[1].p2 == 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(p[0].p1 == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(p[0].p2 == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(p[1].p1 == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(p[1].p2 == 0); // expected-warning{{TRUE}} } struct initializing_pair { Index: clang/test/Analysis/dtor.cpp =================================================================== --- clang/test/Analysis/dtor.cpp +++ clang/test/Analysis/dtor.cpp @@ -140,10 +140,8 @@ IntWrapper arr[2]; // There should be no undefined value warnings here. - // Eventually these should be TRUE as well, but right now - // we can't handle array constructors. - clang_analyzer_eval(arr[0].x == 0); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(arr[1].x == 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(arr[0].x == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(arr[1].x == 0); // expected-warning{{TRUE}} arr[0].x = &i; arr[1].x = &j; @@ -312,10 +310,8 @@ IntWrapper arr[2][2]; // There should be no undefined value warnings here. - // Eventually these should be TRUE as well, but right now - // we can't handle array constructors. - clang_analyzer_eval(arr[0][0].x == 0); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(arr[1][1].x == 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(arr[0][0].x == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(arr[1][1].x == 0); // expected-warning{{TRUE}} arr[0][0].x = &i; arr[1][1].x = &j; Index: clang/test/Analysis/handle_constructors_with_new_array.cpp =================================================================== --- clang/test/Analysis/handle_constructors_with_new_array.cpp +++ clang/test/Analysis/handle_constructors_with_new_array.cpp @@ -59,12 +59,9 @@ init_in_body a2[1]; init_default_member a3[1]; - // FIXME: Should be TRUE, not FALSE. - clang_analyzer_eval(a1[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}} - // FIXME: Should be TRUE, not FALSE. - clang_analyzer_eval(a2[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}} - // FIXME: Should be TRUE, not FALSE. - clang_analyzer_eval(a3[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}} + clang_analyzer_eval(a1[0].a == 1); // expected-warning {{TRUE}} + clang_analyzer_eval(a2[0].a == 1); // expected-warning {{TRUE}} + clang_analyzer_eval(a3[0].a == 1); // expected-warning {{TRUE}} } void test_dynamic_aggregate() { @@ -73,12 +70,9 @@ auto *a2 = new init_in_body[1]; auto *a3 = new init_default_member[1]; - // FIXME: Should be TRUE, not FALSE. - clang_analyzer_eval(a1[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}} - // FIXME: Should be TRUE, not FALSE. - clang_analyzer_eval(a2[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}} - // FIXME: Should be TRUE, not FALSE. - clang_analyzer_eval(a3[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}} + clang_analyzer_eval(a1[0].a == 1); // expected-warning {{TRUE}} + clang_analyzer_eval(a2[0].a == 1); // expected-warning {{TRUE}} + clang_analyzer_eval(a3[0].a == 1); // expected-warning {{TRUE}} delete[] a1; delete[] a2; Index: clang/test/Analysis/new-ctor-conservative.cpp =================================================================== --- clang/test/Analysis/new-ctor-conservative.cpp +++ clang/test/Analysis/new-ctor-conservative.cpp @@ -25,9 +25,8 @@ void checkNewArray() { S *s = new S[10]; - // FIXME: Should be true once we inline array constructors. - clang_analyzer_eval(s[0].x == 1); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(s[1].x == 1); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(s[0].x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(s[1].x == 1); // expected-warning{{TRUE}} } struct NullS {