NoQ updated this revision to Diff 149337. NoQ added a comment. Hmm, actually composition looks very pretty if you use the magic word "impl".
https://reviews.llvm.org/D47350 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp test/Analysis/cxx17-mandatory-elision.cpp
Index: test/Analysis/cxx17-mandatory-elision.cpp =================================================================== --- test/Analysis/cxx17-mandatory-elision.cpp +++ test/Analysis/cxx17-mandatory-elision.cpp @@ -21,6 +21,37 @@ } // namespace variable_functional_cast_crash +namespace ctor_initializer { + +struct S { + int x, y, z; +}; + +struct T { + S s; + int w; + T(int w): s(), w(w) {} +}; + +class C { + T t; +public: + C() : t(T(4)) { + S s = {1, 2, 3}; + t.s = s; + // FIXME: Should be TRUE in C++11 as well. + clang_analyzer_eval(t.w == 4); +#if __cplusplus >= 201703L + // expected-warning@-2{{TRUE}} +#else + // expected-warning@-4{{UNKNOWN}} +#endif + } +}; + +} // namespace ctor_initializer + + namespace address_vector_tests { template <typename T> struct AddressVector { Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -153,6 +153,7 @@ QualType Ty = Field->getType(); FieldVal = makeZeroElementRegion(State, FieldVal, Ty, CallOpts.IsArrayCtorOrDtor); + State = addObjectUnderConstruction(State, Init, LCtx, FieldVal); return std::make_pair(State, FieldVal); } case ConstructionContext::NewAllocatedObjectKind: { @@ -272,35 +273,6 @@ State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx))); } -const CXXConstructExpr * -ExprEngine::findDirectConstructorForCurrentCFGElement() { - // Go backward in the CFG to see if the previous element (ignoring - // destructors) was a CXXConstructExpr. If so, that constructor - // was constructed directly into an existing region. - // This process is essentially the inverse of that performed in - // findElementDirectlyInitializedByCurrentConstructor(). - if (currStmtIdx == 0) - return nullptr; - - const CFGBlock *B = getBuilderContext().getBlock(); - - unsigned int PreviousStmtIdx = currStmtIdx - 1; - CFGElement Previous = (*B)[PreviousStmtIdx]; - - while (Previous.getAs<CFGImplicitDtor>() && PreviousStmtIdx > 0) { - --PreviousStmtIdx; - Previous = (*B)[PreviousStmtIdx]; - } - - if (Optional<CFGStmt> PrevStmtElem = Previous.getAs<CFGStmt>()) { - if (auto *CtorExpr = dyn_cast<CXXConstructExpr>(PrevStmtElem->getStmt())) { - return CtorExpr; - } - } - - return nullptr; -} - void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE, ExplodedNode *Pred, ExplodedNodeSet &destNodes) { Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -109,7 +109,75 @@ // to the object's location, so that on every such statement the location // could have been retrieved. -typedef std::pair<const Stmt *, const LocationContext *> ConstructedObjectKey; +/// ConstructedObjectKey is used for being able to find the path-sensitive +/// memory region of a freshly constructed object while modeling the AST node +/// that syntactically represents the object that is being constructed. +/// Semantics of such nodes may sometimes require access to the region that's +/// not otherwise present in the program state, or to the very fact that +/// the construction context was present and contained references to these +/// AST nodes. +class ConstructedObjectKey { + typedef std::pair< + llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *>, + const LocationContext *> ConstructedObjectKeyImpl; + + ConstructedObjectKeyImpl Impl; + + const void *getAnyASTNodePtr() const { + if (const Stmt *S = getStmt()) + return S; + else + return getCXXCtorInitializer(); + } + +public: + ConstructedObjectKey( + llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P, + const LocationContext *LC) + : Impl(P, LC) { + // This is the full list of statements that require additional actions when + // encountered. This list may be expanded when new actions are implemented. + assert(getCXXCtorInitializer() || isa<DeclStmt>(getStmt()) || + isa<CXXNewExpr>(getStmt()) || isa<CXXBindTemporaryExpr>(getStmt()) || + isa<MaterializeTemporaryExpr>(getStmt())); + } + + const Stmt *getStmt() const { + return Impl.first.dyn_cast<const Stmt *>(); + } + + const CXXCtorInitializer *getCXXCtorInitializer() const { + return Impl.first.dyn_cast<const CXXCtorInitializer *>(); + } + + const LocationContext *getLocationContext() const { + return Impl.second; + } + + void print(llvm::raw_ostream &OS, PrinterHelper *Helper, PrintingPolicy &PP) { + OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ") "; + if (const Stmt *S = getStmt()) { + S->printPretty(OS, Helper, PP); + } else { + const CXXCtorInitializer *I = getCXXCtorInitializer(); + OS << I->getAnyMember()->getNameAsString(); + } + } + + void Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddPointer(Impl.first.getOpaqueValue()); + ID.AddPointer(Impl.second); + } + + bool operator==(const ConstructedObjectKey &RHS) const { + return Impl == RHS.Impl; + } + + bool operator<(const ConstructedObjectKey &RHS) const { + return Impl < RHS.Impl; + } +}; + typedef llvm::ImmutableMap<ConstructedObjectKey, SVal> ObjectsUnderConstructionMap; REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction, @@ -378,26 +446,31 @@ return State; } -ProgramStateRef -ExprEngine::addObjectUnderConstruction(ProgramStateRef State, const Stmt *S, - const LocationContext *LC, SVal V) { - ConstructedObjectKey Key(S, LC->getCurrentStackFrame()); +ProgramStateRef ExprEngine::addObjectUnderConstruction( + ProgramStateRef State, + llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P, + const LocationContext *LC, SVal V) { + ConstructedObjectKey Key(P, LC->getCurrentStackFrame()); // FIXME: Currently the state might already contain the marker due to // incorrect handling of temporaries bound to default parameters. assert(!State->get<ObjectsUnderConstruction>(Key) || - isa<CXXBindTemporaryExpr>(S)); + isa<CXXBindTemporaryExpr>(Key.getStmt())); return State->set<ObjectsUnderConstruction>(Key, V); } -Optional<SVal> ExprEngine::getObjectUnderConstruction(ProgramStateRef State, - const Stmt *S, const LocationContext *LC) { - ConstructedObjectKey Key(S, LC->getCurrentStackFrame()); +Optional<SVal> ExprEngine::getObjectUnderConstruction( + ProgramStateRef State, + llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P, + const LocationContext *LC) { + ConstructedObjectKey Key(P, LC->getCurrentStackFrame()); return Optional<SVal>::create(State->get<ObjectsUnderConstruction>(Key)); } -ProgramStateRef ExprEngine::finishObjectConstruction(ProgramStateRef State, - const Stmt *S, const LocationContext *LC) { - ConstructedObjectKey Key(S, LC->getCurrentStackFrame()); +ProgramStateRef ExprEngine::finishObjectConstruction( + ProgramStateRef State, + llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P, + const LocationContext *LC) { + ConstructedObjectKey Key(P, LC->getCurrentStackFrame()); assert(State->contains<ObjectsUnderConstruction>(Key)); return State->remove<ObjectsUnderConstruction>(Key); } @@ -409,7 +482,7 @@ while (LC != ToLC) { assert(LC && "ToLC must be a parent of FromLC!"); for (auto I : State->get<ObjectsUnderConstruction>()) - if (I.first.second == LC) + if (I.first.getLocationContext() == LC) return false; LC = LC->getParent(); @@ -451,10 +524,9 @@ for (auto I : State->get<ObjectsUnderConstruction>()) { ConstructedObjectKey Key = I.first; SVal Value = I.second; - if (Key.second != LC) + if (Key.getLocationContext() != LC) continue; - Out << '(' << Key.second << ',' << Key.first << ") "; - Key.first->printPretty(Out, nullptr, PP); + Key.print(Out, nullptr, PP); Out << " : " << Value << NL; } } @@ -677,9 +749,11 @@ Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx); } -void ExprEngine::ProcessInitializer(const CFGInitializer Init, +void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit, ExplodedNode *Pred) { - const CXXCtorInitializer *BMI = Init.getInitializer(); + const CXXCtorInitializer *BMI = CFGInit.getInitializer(); + const Expr *Init = BMI->getInit()->IgnoreImplicit(); + const LocationContext *LC = Pred->getLocationContext(); PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(), BMI->getSourceLocation(), @@ -692,19 +766,21 @@ ProgramStateRef State = Pred->getState(); SVal thisVal = State->getSVal(svalBuilder.getCXXThis(decl, stackFrame)); - ExplodedNodeSet Tmp(Pred); + ExplodedNodeSet Tmp; SVal FieldLoc; // Evaluate the initializer, if necessary if (BMI->isAnyMemberInitializer()) { // Constructors build the object directly in the field, // but non-objects must be copied in from the initializer. - if (const auto *CtorExpr = findDirectConstructorForCurrentCFGElement()) { - assert(BMI->getInit()->IgnoreImplicit() == CtorExpr); - (void)CtorExpr; + if (getObjectUnderConstruction(State, BMI, LC)) { // The field was directly constructed, so there is no need to bind. + // But we still need to stop tracking the object under construction. + State = finishObjectConstruction(State, BMI, LC); + NodeBuilder Bldr(Pred, Tmp, *currBldrCtx); + PostStore PS(Init, LC, /*Loc*/ nullptr, /*tag*/ nullptr); + Bldr.generateNode(PS, State, Pred); } else { - const Expr *Init = BMI->getInit()->IgnoreImplicit(); const ValueDecl *Field; if (BMI->isIndirectMemberInitializer()) { Field = BMI->getIndirectMember(); @@ -738,25 +814,24 @@ InitVal = State->getSVal(BMI->getInit(), stackFrame); } - assert(Tmp.size() == 1 && "have not generated any new nodes yet"); - assert(*Tmp.begin() == Pred && "have not generated any new nodes yet"); - Tmp.clear(); - PostInitializer PP(BMI, FieldLoc.getAsRegion(), stackFrame); evalBind(Tmp, Init, Pred, FieldLoc, InitVal, /*isInit=*/true, &PP); } } else { assert(BMI->isBaseInitializer() || BMI->isDelegatingInitializer()); + Tmp.insert(Pred); // We already did all the work when visiting the CXXConstructExpr. } // Construct PostInitializer nodes whether the state changed or not, // so that the diagnostics don't get confused. PostInitializer PP(BMI, FieldLoc.getAsRegion(), stackFrame); ExplodedNodeSet Dst; NodeBuilder Bldr(Tmp, Dst, *currBldrCtx); - for (const auto I : Tmp) - Bldr.generateNode(PP, I->getState(), I); + for (const auto I : Tmp) { + ProgramStateRef State = I->getState(); + Bldr.generateNode(PP, State, I); + } // Enqueue the new nodes onto the work list. Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx); @@ -2119,11 +2194,11 @@ while (LC != ToLC) { assert(LC && "ToLC must be a parent of FromLC!"); for (auto I : State->get<ObjectsUnderConstruction>()) - if (I.first.second == LC) { + if (I.first.getLocationContext() == LC) { // The comment above only pardons us for not cleaning up a // CXXBindTemporaryExpr. If any other statements are found here, // it must be a separate problem. - assert(isa<CXXBindTemporaryExpr>(I.first.first)); + assert(isa<CXXBindTemporaryExpr>(I.first.getStmt())); State = State->remove<ObjectsUnderConstruction>(I.first); } Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -762,20 +762,23 @@ /// made within the constructor alive until its declaration actually /// goes into scope. static ProgramStateRef addObjectUnderConstruction( - ProgramStateRef State, const Stmt *S, + ProgramStateRef State, + llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P, const LocationContext *LC, SVal V); /// Mark the object sa fully constructed, cleaning up the state trait /// that tracks objects under construction. - static ProgramStateRef finishObjectConstruction(ProgramStateRef State, - const Stmt *S, - const LocationContext *LC); + static ProgramStateRef finishObjectConstruction( + ProgramStateRef State, + llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P, + const LocationContext *LC); /// If the given statement corresponds to an object under construction, /// being part of its construciton context, retrieve that object's location. - static Optional<SVal> getObjectUnderConstruction(ProgramStateRef State, - const Stmt *S, - const LocationContext *LC); + static Optional<SVal> getObjectUnderConstruction( + ProgramStateRef State, + llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P, + const LocationContext *LC); /// Check if all objects under construction have been fully constructed /// for the given context range (including FromLC, not including ToLC).
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits