NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs. Herald added subscribers: cfe-commits, baloghadamsoftware.
As noticed in http://lists.llvm.org/pipermail/cfe-dev/2018-May/058055.html we need a path-sensitive program state trait that for, essentially, every constructed object maintains the region of that object until the statement that consumes the object is encountered. We already have such trait for new-expressions and bind/materialize temporary expressions, which are three separate traits. Because we plan to add more, it doesn't make sense to maintain that many traits that do the same thing in different circumstances, so i guess it's better to merge them into a single multi-purpose trait. "Multi-purpose" is definitely not the top buzzword in programming, but here i believe it's worth it because the underlying reasoning for needing these traits and needing them to work in a particular manner is the same. We need them because our constructor expressions are turned inside out, and we need a better connection between "inside" and "out" in both directions. One of these directions is handled by the ConstructionContext; the other is path-sensitive. No functional change intended here; this is a refactoring. Repository: rC Clang https://reviews.llvm.org/D47303 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -288,8 +288,8 @@ AllocV, CNE->getType(), getContext().getPointerType(getContext().VoidTy)); - state = - setCXXNewAllocatorValue(state, CNE, calleeCtx->getParent(), AllocV); + state = addObjectUnderConstruction(state, CNE, calleeCtx->getParent(), + AllocV); } } @@ -354,8 +354,9 @@ /*WasInlined=*/true); for (auto I : DstPostPostCallCallback) { getCheckerManager().runCheckersForNewAllocator( - CNE, getCXXNewAllocatorValue(I->getState(), CNE, - calleeCtx->getParent()), + CNE, + *getObjectUnderConstruction(I->getState(), CNE, + calleeCtx->getParent()), DstPostCall, I, *this, /*WasInlined=*/true); } @@ -588,8 +589,8 @@ // Conjure a temporary if the function returns an object by value. MemRegionManager &MRMgr = svalBuilder.getRegionManager(); const CXXTempObjectRegion *TR = MRMgr.getCXXTempObjectRegion(E, LCtx); - State = addAllNecessaryTemporaryInfo(State, RTC->getConstructionContext(), - LCtx, TR); + State = markStatementsCorrespondingToConstructedObject( + State, RTC->getConstructionContext(), LCtx, loc::MemRegionVal(TR)); // Invalidate the region so that it didn't look uninitialized. Don't notify // the checkers. Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -111,11 +111,10 @@ } -const MemRegion * -ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE, - ExplodedNode *Pred, - const ConstructionContext *CC, - EvalCallOptions &CallOpts) { +SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE, + ExplodedNode *Pred, + const ConstructionContext *CC, + EvalCallOptions &CallOpts) { const LocationContext *LCtx = Pred->getLocationContext(); ProgramStateRef State = Pred->getState(); MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); @@ -130,9 +129,8 @@ const auto *Var = cast<VarDecl>(DS->getSingleDecl()); SVal LValue = State->getLValue(Var, LCtx); QualType Ty = Var->getType(); - LValue = - makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor); - return LValue.getAsRegion(); + return makeZeroElementRegion(State, LValue, Ty, + CallOpts.IsArrayCtorOrDtor); } case ConstructionContext::SimpleConstructorInitializerKind: { const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC); @@ -156,25 +154,26 @@ QualType Ty = Field->getType(); FieldVal = makeZeroElementRegion(State, FieldVal, Ty, CallOpts.IsArrayCtorOrDtor); - return FieldVal.getAsRegion(); + return FieldVal; } case ConstructionContext::NewAllocatedObjectKind: { if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) { const auto *NECC = cast<NewAllocatedObjectConstructionContext>(CC); const auto *NE = NECC->getCXXNewExpr(); - // TODO: Detect when the allocator returns a null pointer. - // Constructor shall not be called in this case. - if (const SubRegion *MR = dyn_cast_or_null<SubRegion>( - getCXXNewAllocatorValue(State, NE, LCtx).getAsRegion())) { + SVal V = *getObjectUnderConstruction(State, NE, LCtx); + if (const SubRegion *MR = + dyn_cast_or_null<SubRegion>(V.getAsRegion())) { if (NE->isArray()) { // TODO: In fact, we need to call the constructor for every // allocated element, not just the first one! CallOpts.IsArrayCtorOrDtor = true; - return getStoreManager().GetElementZeroRegion( - MR, NE->getType()->getPointeeType()); + return loc::MemRegionVal(getStoreManager().GetElementZeroRegion( + MR, NE->getType()->getPointeeType())); } - return MR; + return V; } + // TODO: Detect when the allocator returns a null pointer. + // Constructor shall not be called in this case. } break; } @@ -196,7 +195,7 @@ // TODO: Support temporaries lifetime-extended via static references. // They'd need a getCXXStaticTempObjectRegion(). CallOpts.IsTemporaryCtorOrDtor = true; - return MRMgr.getCXXTempObjectRegion(CE, LCtx); + return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx)); } case ConstructionContext::SimpleReturnedValueKind: { // The temporary is to be managed by the parent stack frame. @@ -217,16 +216,16 @@ CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; } else if (!isa<TemporaryObjectConstructionContext>( RTC->getConstructionContext())) { - // FXIME: The return value is constructed directly into a + // FIXME: The return value is constructed directly into a // non-temporary due to C++17 mandatory copy elision. This is not // implemented yet. assert(getContext().getLangOpts().CPlusPlus17); CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; } TempLCtx = CallerLCtx; } CallOpts.IsTemporaryCtorOrDtor = true; - return MRMgr.getCXXTempObjectRegion(CE, TempLCtx); + return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, TempLCtx)); } case ConstructionContext::CXX17ElidedCopyVariableKind: case ConstructionContext::CXX17ElidedCopyReturnedValueKind: @@ -238,7 +237,7 @@ // If we couldn't find an existing region to construct into, assume we're // constructing a temporary. Notify the caller of our failure. CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; - return MRMgr.getCXXTempObjectRegion(CE, LCtx); + return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx)); } const CXXConstructExpr * @@ -276,7 +275,7 @@ const LocationContext *LCtx = Pred->getLocationContext(); ProgramStateRef State = Pred->getState(); - const MemRegion *Target = nullptr; + SVal Target = UnknownVal(); // FIXME: Handle arrays, which run the same constructor for every element. // For now, we just run the first constructor (which should still invalidate @@ -289,7 +288,7 @@ switch (CE->getConstructionKind()) { case CXXConstructExpr::CK_Complete: { - Target = getRegionForConstructedObject(CE, Pred, CC, CallOpts); + Target = getLocationForConstructedObject(CE, Pred, CC, CallOpts); break; } case CXXConstructExpr::CK_VirtualBase: @@ -325,7 +324,7 @@ // otherwise always available during construction. if (dyn_cast_or_null<InitListExpr>(LCtx->getParentMap().getParent(CE))) { MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); - Target = MRMgr.getCXXTempObjectRegion(CE, LCtx); + Target = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx)); CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; break; } @@ -337,22 +336,22 @@ SVal ThisVal = State->getSVal(ThisPtr); if (CE->getConstructionKind() == CXXConstructExpr::CK_Delegating) { - Target = ThisVal.getAsRegion(); + Target = ThisVal; } else { // Cast to the base type. bool IsVirtual = (CE->getConstructionKind() == CXXConstructExpr::CK_VirtualBase); SVal BaseVal = getStoreManager().evalDerivedToBase(ThisVal, CE->getType(), IsVirtual); - Target = BaseVal.getAsRegion(); + Target = BaseVal; } break; } } CallEventManager &CEMgr = getStateManager().getCallEventManager(); CallEventRef<CXXConstructorCall> Call = - CEMgr.getCXXConstructorCall(CE, Target, State, LCtx); + CEMgr.getCXXConstructorCall(CE, Target.getAsRegion(), State, LCtx); ExplodedNodeSet DstPreVisit; getCheckerManager().runCheckersForPreStmt(DstPreVisit, Pred, CE, *this); @@ -378,10 +377,11 @@ // actually make things worse. Placement new makes this tricky as well, // since it's then possible to be initializing one part of a multi- // dimensional array. - State = State->bindDefaultZero(loc::MemRegionVal(Target), LCtx); + State = State->bindDefaultZero(Target, LCtx); } - State = addAllNecessaryTemporaryInfo(State, CC, LCtx, Target); + State = markStatementsCorrespondingToConstructedObject(State, CC, LCtx, + Target); Bldr.generateNode(CE, *I, State, /*tag=*/nullptr, ProgramPoint::PreStmtKind); @@ -541,16 +541,17 @@ State = State->assume(RetVal.castAs<DefinedOrUnknownSVal>(), true); } - ValueBldr.generateNode(CNE, I, - setCXXNewAllocatorValue(State, CNE, LCtx, RetVal)); + ValueBldr.generateNode( + CNE, I, addObjectUnderConstruction(State, CNE, LCtx, RetVal)); } ExplodedNodeSet DstPostPostCallCallback; getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback, DstPostValue, *Call, *this); for (auto I : DstPostPostCallCallback) { getCheckerManager().runCheckersForNewAllocator( - CNE, getCXXNewAllocatorValue(I->getState(), CNE, LCtx), Dst, I, *this); + CNE, *getObjectUnderConstruction(I->getState(), CNE, LCtx), Dst, I, + *this); } } @@ -573,8 +574,8 @@ // Retrieve the stored operator new() return value. if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) { - symVal = getCXXNewAllocatorValue(State, CNE, LCtx); - State = clearCXXNewAllocatorValue(State, CNE, LCtx); + symVal = *getObjectUnderConstruction(State, CNE, LCtx); + State = finishObjectConstruction(State, CNE, LCtx); } // We assume all standard global 'operator new' functions allocate memory in Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -98,42 +98,14 @@ STATISTIC(NumTimesRetriedWithoutInlining, "The # of times we re-evaluated a call without inlining"); -using InitializedTemporariesMap = - llvm::ImmutableMap<std::pair<const CXXBindTemporaryExpr *, - const StackFrameContext *>, - const CXXTempObjectRegion *>; - -// Keeps track of whether CXXBindTemporaryExpr nodes have been evaluated. -// The StackFrameContext assures that nested calls due to inlined recursive -// functions do not interfere. -REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporaries, - InitializedTemporariesMap) - -using TemporaryMaterializationMap = - llvm::ImmutableMap<std::pair<const MaterializeTemporaryExpr *, - const StackFrameContext *>, - const CXXTempObjectRegion *>; - -// Keeps track of temporaries that will need to be materialized later. -// The StackFrameContext assures that nested calls due to inlined recursive -// functions do not interfere. -REGISTER_TRAIT_WITH_PROGRAMSTATE(TemporaryMaterializations, - TemporaryMaterializationMap) - -using CXXNewAllocatorValuesMap = - llvm::ImmutableMap<std::pair<const CXXNewExpr *, const LocationContext *>, - SVal>; - -// Keeps track of return values of various operator new() calls between -// evaluation of the inlined operator new(), through the constructor call, -// to the actual evaluation of the CXXNewExpr. -// TODO: Refactor the key for this trait into a LocationContext sub-class, -// which would be put on the stack of location contexts before operator new() -// is evaluated, and removed from the stack when the whole CXXNewExpr -// is fully evaluated. -// Probably do something similar to the previous trait as well. -REGISTER_TRAIT_WITH_PROGRAMSTATE(CXXNewAllocatorValues, - CXXNewAllocatorValuesMap) +// Keeps track of whether objects corresponding to the statement have been +// fully constructed. +typedef std::pair<const Stmt *, const LocationContext *> ConstructedObjectKey; +typedef llvm::ImmutableMap<ConstructedObjectKey, SVal> + ObjectsUnderConstructionMap; +REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction, + ObjectsUnderConstructionMap) + //===----------------------------------------------------------------------===// // Engine construction and deletion. @@ -312,13 +284,11 @@ bool FoundOriginalMaterializationRegion = false; const TypedValueRegion *TR = nullptr; if (const auto *MT = dyn_cast<MaterializeTemporaryExpr>(Result)) { - auto Key = std::make_pair(MT, LC->getCurrentStackFrame()); - if (const CXXTempObjectRegion *const *TRPtr = - State->get<TemporaryMaterializations>(Key)) { + if (Optional<SVal> V = getObjectUnderConstruction(State, MT, LC)) { FoundOriginalMaterializationRegion = true; - TR = *TRPtr; + TR = cast<CXXTempObjectRegion>(V->getAsRegion()); assert(TR); - State = State->remove<TemporaryMaterializations>(Key); + State = finishObjectConstruction(State, MT, LC); } else { StorageDuration SD = MT->getStorageDuration(); // If this object is bound to a reference with static storage duration, we @@ -399,12 +369,12 @@ return State; } -ProgramStateRef ExprEngine::addInitializedTemporary( - ProgramStateRef State, const CXXBindTemporaryExpr *BTE, - const LocationContext *LC, const CXXTempObjectRegion *R) { - const auto &Key = std::make_pair(BTE, LC->getCurrentStackFrame()); - if (!State->contains<InitializedTemporaries>(Key)) { - return State->set<InitializedTemporaries>(Key, R); +ProgramStateRef +ExprEngine::addObjectUnderConstruction(ProgramStateRef State, const Stmt *S, + const LocationContext *LC, SVal V) { + ConstructedObjectKey Key(S, LC->getCurrentStackFrame()); + if (!State->contains<ObjectsUnderConstruction>(Key)) { + return State->set<ObjectsUnderConstruction>(Key, V); } // FIXME: Currently the state might already contain the marker due to @@ -414,50 +384,36 @@ return State; } -bool ExprEngine::areInitializedTemporariesClear(ProgramStateRef State, - const LocationContext *FromLC, - const LocationContext *ToLC) { - const LocationContext *LC = FromLC; - while (LC != ToLC) { - assert(LC && "ToLC must be a parent of FromLC!"); - for (auto I : State->get<InitializedTemporaries>()) - if (I.first.second == LC) - return false; - - LC = LC->getParent(); - } - return true; +Optional<SVal> ExprEngine::getObjectUnderConstruction(ProgramStateRef State, + const Stmt *S, const LocationContext *LC) { + ConstructedObjectKey Key(S, LC->getCurrentStackFrame()); + return Optional<SVal>::create(State->get<ObjectsUnderConstruction>(Key)); } -ProgramStateRef ExprEngine::addTemporaryMaterialization( - ProgramStateRef State, const MaterializeTemporaryExpr *MTE, - const LocationContext *LC, const CXXTempObjectRegion *R) { - const auto &Key = std::make_pair(MTE, LC->getCurrentStackFrame()); - assert(!State->contains<TemporaryMaterializations>(Key)); - return State->set<TemporaryMaterializations>(Key, R); +ProgramStateRef ExprEngine::finishObjectConstruction(ProgramStateRef State, + const Stmt *S, const LocationContext *LC) { + ConstructedObjectKey Key(S, LC->getCurrentStackFrame()); + return State->remove<ObjectsUnderConstruction>(Key); } -bool ExprEngine::areTemporaryMaterializationsClear( - ProgramStateRef State, const LocationContext *FromLC, - const LocationContext *ToLC) { +bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State, + const LocationContext *FromLC, + const LocationContext *ToLC) { const LocationContext *LC = FromLC; while (LC != ToLC) { assert(LC && "ToLC must be a parent of FromLC!"); - for (auto I : State->get<TemporaryMaterializations>()) + for (auto I : State->get<ObjectsUnderConstruction>()) if (I.first.second == LC) return false; LC = LC->getParent(); } return true; } -ProgramStateRef ExprEngine::addAllNecessaryTemporaryInfo( +ProgramStateRef ExprEngine::markStatementsCorrespondingToConstructedObject( ProgramStateRef State, const ConstructionContext *CC, - const LocationContext *LC, const MemRegion *R) { - const CXXBindTemporaryExpr *BTE = nullptr; - const MaterializeTemporaryExpr *MTE = nullptr; - + const LocationContext *LC, SVal V) { if (CC) { // If the temporary is being returned from the function, it will be // destroyed or lifetime-extended in the caller stack frame. @@ -498,13 +454,14 @@ // Proceed to deal with the temporary we've found on the parent // stack frame. } - // In case of temporary object construction, extract data necessary for // destruction and lifetime extension. if (const auto *TCC = dyn_cast<TemporaryObjectConstructionContext>(CC)) { if (AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG()) { - BTE = TCC->getCXXBindTemporaryExpr(); - MTE = TCC->getMaterializedTemporaryExpr(); + const CXXBindTemporaryExpr *BTE = + TCC->getCXXBindTemporaryExpr(); + const MaterializeTemporaryExpr *MTE = + TCC->getMaterializedTemporaryExpr(); if (!BTE) { // FIXME: Lifetime extension for temporaries without destructors // is not implemented yet. @@ -516,59 +473,19 @@ // destructor. BTE = nullptr; } - } - } - if (BTE) { - State = addInitializedTemporary(State, BTE, LC, - cast<CXXTempObjectRegion>(R)); - } + if (BTE) + State = addObjectUnderConstruction(State, BTE, LC, V); - if (MTE) { - State = addTemporaryMaterialization(State, MTE, LC, - cast<CXXTempObjectRegion>(R)); + if (MTE) + State = addObjectUnderConstruction(State, MTE, LC, V); + } } } return State; } -ProgramStateRef -ExprEngine::setCXXNewAllocatorValue(ProgramStateRef State, - const CXXNewExpr *CNE, - const LocationContext *CallerLC, SVal V) { - assert(!State->get<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC)) && - "Allocator value already set!"); - return State->set<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC), V); -} - -SVal ExprEngine::getCXXNewAllocatorValue(ProgramStateRef State, - const CXXNewExpr *CNE, - const LocationContext *CallerLC) { - return *State->get<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC)); -} - -ProgramStateRef -ExprEngine::clearCXXNewAllocatorValue(ProgramStateRef State, - const CXXNewExpr *CNE, - const LocationContext *CallerLC) { - return State->remove<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC)); -} - -bool ExprEngine::areCXXNewAllocatorValuesClear(ProgramStateRef State, - const LocationContext *FromLC, - const LocationContext *ToLC) { - const LocationContext *LC = FromLC; - while (LC != ToLC) { - assert(LC && "ToLC must be a parent of FromLC!"); - for (auto I : State->get<CXXNewAllocatorValues>()) - if (I.first.second == LC) - return false; - - LC = LC->getParent(); - } - return true; -} //===----------------------------------------------------------------------===// // Top-level transfer function logic (Dispatcher). @@ -593,55 +510,15 @@ LCtx, Call); } -static void printInitializedTemporariesForContext(raw_ostream &Out, - ProgramStateRef State, - const char *NL, - const char *Sep, - const LocationContext *LC) { - PrintingPolicy PP = - LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy(); - for (auto I : State->get<InitializedTemporaries>()) { - std::pair<const CXXBindTemporaryExpr *, const LocationContext *> Key = - I.first; - const MemRegion *Value = I.second; - if (Key.second != LC) - continue; - Out << '(' << Key.second << ',' << Key.first << ") "; - Key.first->printPretty(Out, nullptr, PP); - if (Value) - Out << " : " << Value; - Out << NL; - } -} - -static void printTemporaryMaterializationsForContext( - raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep, - const LocationContext *LC) { - PrintingPolicy PP = - LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy(); - for (auto I : State->get<TemporaryMaterializations>()) { - std::pair<const MaterializeTemporaryExpr *, const LocationContext *> Key = - I.first; - const MemRegion *Value = I.second; - if (Key.second != LC) - continue; - Out << '(' << Key.second << ',' << Key.first << ") "; - Key.first->printPretty(Out, nullptr, PP); - assert(Value); - Out << " : " << Value << NL; - } -} - -static void printCXXNewAllocatorValuesForContext(raw_ostream &Out, - ProgramStateRef State, - const char *NL, - const char *Sep, - const LocationContext *LC) { +static void printObjectsUnderConstructionForContext(raw_ostream &Out, + ProgramStateRef State, + const char *NL, + const char *Sep, + const LocationContext *LC) { PrintingPolicy PP = LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy(); - - for (auto I : State->get<CXXNewAllocatorValues>()) { - std::pair<const CXXNewExpr *, const LocationContext *> Key = I.first; + for (auto I : State->get<ObjectsUnderConstruction>()) { + ConstructedObjectKey Key = I.first; SVal Value = I.second; if (Key.second != LC) continue; @@ -655,27 +532,11 @@ const char *NL, const char *Sep, const LocationContext *LCtx) { if (LCtx) { - if (!State->get<InitializedTemporaries>().isEmpty()) { - Out << Sep << "Initialized temporaries:" << NL; - - LCtx->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) { - printInitializedTemporariesForContext(Out, State, NL, Sep, LC); - }); - } - - if (!State->get<TemporaryMaterializations>().isEmpty()) { - Out << Sep << "Temporaries to be materialized:" << NL; - - LCtx->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) { - printTemporaryMaterializationsForContext(Out, State, NL, Sep, LC); - }); - } - - if (!State->get<CXXNewAllocatorValues>().isEmpty()) { - Out << Sep << "operator new() allocator return values:" << NL; + if (!State->get<ObjectsUnderConstruction>().isEmpty()) { + Out << Sep << "Objects under construction:" << NL; LCtx->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) { - printCXXNewAllocatorValuesForContext(Out, State, NL, Sep, LC); + printObjectsUnderConstructionForContext(Out, State, NL, Sep, LC); }); } } @@ -779,19 +640,11 @@ const StackFrameContext *SFC = LC ? LC->getCurrentStackFrame() : nullptr; SymbolReaper SymReaper(SFC, ReferenceStmt, SymMgr, getStoreManager()); - for (auto I : CleanedState->get<InitializedTemporaries>()) - if (I.second) - SymReaper.markLive(I.second); - - for (auto I : CleanedState->get<TemporaryMaterializations>()) - if (I.second) - SymReaper.markLive(I.second); - - for (auto I : CleanedState->get<CXXNewAllocatorValues>()) { + for (auto I : CleanedState->get<ObjectsUnderConstruction>()) { if (SymbolRef Sym = I.second.getAsSymbol()) SymReaper.markLive(Sym); if (const MemRegion *MR = I.second.getAsRegion()) - SymReaper.markElementIndicesLive(MR); + SymReaper.markLive(MR); } getCheckerManager().runCheckersForLiveSymbols(CleanedState, SymReaper); @@ -1148,17 +1001,15 @@ StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx); ProgramStateRef State = Pred->getState(); const MemRegion *MR = nullptr; - if (const CXXTempObjectRegion *const *MRPtr = - State->get<InitializedTemporaries>(std::make_pair( - D.getBindTemporaryExpr(), Pred->getStackFrame()))) { + if (Optional<SVal> V = + getObjectUnderConstruction(State, D.getBindTemporaryExpr(), + Pred->getLocationContext())) { // FIXME: Currently we insert temporary destructors for default parameters, // but we don't insert the constructors, so the entry in - // InitializedTemporaries may be missing. - State = State->remove<InitializedTemporaries>( - std::make_pair(D.getBindTemporaryExpr(), Pred->getStackFrame())); - // *MRPtr may still be null when the construction context for the temporary - // was not implemented. - MR = *MRPtr; + // ObjectsUnderConstruction may be missing. + State = finishObjectConstruction(State, D.getBindTemporaryExpr(), + Pred->getLocationContext()); + MR = V->getAsRegion(); } StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State); @@ -1197,13 +1048,14 @@ const CFGBlock *DstT, const CFGBlock *DstF) { BranchNodeBuilder TempDtorBuilder(Pred, Dst, BldCtx, DstT, DstF); - if (Pred->getState()->contains<InitializedTemporaries>( - std::make_pair(BTE, Pred->getStackFrame()))) { + ProgramStateRef State = Pred->getState(); + const LocationContext *LC = Pred->getLocationContext(); + if (getObjectUnderConstruction(State, BTE, LC)) { TempDtorBuilder.markInfeasible(false); - TempDtorBuilder.generateNode(Pred->getState(), true, Pred); + TempDtorBuilder.generateNode(State, true, Pred); } else { TempDtorBuilder.markInfeasible(true); - TempDtorBuilder.generateNode(Pred->getState(), false, Pred); + TempDtorBuilder.generateNode(State, false, Pred); } } @@ -1222,14 +1074,13 @@ StmtNodeBuilder StmtBldr(PreVisit, Dst, *currBldrCtx); for (ExplodedNode *Node : PreVisit) { ProgramStateRef State = Node->getState(); - const auto &Key = std::make_pair(BTE, Node->getStackFrame()); - - if (!State->contains<InitializedTemporaries>(Key)) { + const LocationContext *LC = Node->getLocationContext(); + if (!getObjectUnderConstruction(State, BTE, LC)) { // FIXME: Currently the state might also already contain the marker due to // incorrect handling of temporaries bound to default parameters; for // those, we currently skip the CXXBindTemporaryExpr but rely on adding // temporary destructor nodes. - State = State->set<InitializedTemporaries>(Key, nullptr); + State = addObjectUnderConstruction(State, BTE, LC, UnknownVal()); } StmtBldr.generateNode(BTE, Node, State); } @@ -2320,30 +2171,30 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext& BC, ExplodedNode *Pred, const ReturnStmt *RS) { - // See if we have any stale C++ allocator values. - assert(areCXXNewAllocatorValuesClear(Pred->getState(), - Pred->getLocationContext(), - Pred->getStackFrame()->getParent())); - // FIXME: We currently cannot assert that temporaries are clear, because // lifetime extended temporaries are not always modelled correctly. In some // cases when we materialize the temporary, we do // createTemporaryRegionIfNeeded(), and the region changes, and also the // respective destructor becomes automatic from temporary. So for now clean up // the state manually before asserting. Ideally, the code above the assertion // should go away, but the assertion should remain. { - ExplodedNodeSet CleanUpTemporaries; - NodeBuilder Bldr(Pred, CleanUpTemporaries, BC); + ExplodedNodeSet CleanUpObjects; + NodeBuilder Bldr(Pred, CleanUpObjects, BC); ProgramStateRef State = Pred->getState(); const LocationContext *FromLC = Pred->getLocationContext(); const LocationContext *ToLC = FromLC->getCurrentStackFrame()->getParent(); const LocationContext *LC = FromLC; while (LC != ToLC) { assert(LC && "ToLC must be a parent of FromLC!"); - for (auto I : State->get<InitializedTemporaries>()) - if (I.first.second == LC) - State = State->remove<InitializedTemporaries>(I.first); + for (auto I : State->get<ObjectsUnderConstruction>()) + if (I.first.second == 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)); + State = State->remove<ObjectsUnderConstruction>(I.first); + } LC = LC->getParent(); } @@ -2356,12 +2207,9 @@ } } } - assert(areInitializedTemporariesClear(Pred->getState(), - Pred->getLocationContext(), - Pred->getStackFrame()->getParent())); - assert(areTemporaryMaterializationsClear(Pred->getState(), - Pred->getLocationContext(), - Pred->getStackFrame()->getParent())); + assert(areAllObjectsFullyConstructed(Pred->getState(), + Pred->getLocationContext(), + Pred->getStackFrame()->getParent())); PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext()); StateMgr.EndPath(Pred->getState()); Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -748,68 +748,48 @@ /// determine the region into which an object will be constructed by \p CE. /// When the lookahead fails, a temporary region is returned, and the /// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts. - const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE, - ExplodedNode *Pred, - const ConstructionContext *CC, - EvalCallOptions &CallOpts); - - /// Store the region of a C++ temporary object corresponding to a - /// CXXBindTemporaryExpr for later destruction. - static ProgramStateRef addInitializedTemporary( - ProgramStateRef State, const CXXBindTemporaryExpr *BTE, - const LocationContext *LC, const CXXTempObjectRegion *R); - - /// Check if all initialized temporary regions are clear for the given - /// context range (including FromLC, not including ToLC). - /// This is useful for assertions. - static bool areInitializedTemporariesClear(ProgramStateRef State, - const LocationContext *FromLC, - const LocationContext *ToLC); - - /// Store the region of a C++ temporary object corresponding to a - /// CXXBindTemporaryExpr for later destruction. - static ProgramStateRef addTemporaryMaterialization( - ProgramStateRef State, const MaterializeTemporaryExpr *MTE, - const LocationContext *LC, const CXXTempObjectRegion *R); - - /// Check if all temporary materialization regions are clear for the given - /// context range (including FromLC, not including ToLC). + SVal getLocationForConstructedObject(const CXXConstructExpr *CE, + ExplodedNode *Pred, + const ConstructionContext *CC, + EvalCallOptions &CallOpts); + + /// 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 addObjectUnderConstruction( + ProgramStateRef State, const Stmt *S, + 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); + + /// 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); + + /// Check if all objects under construction have been fully constructed + /// for the given context range (including FromLC, not including ToLC). /// This is useful for assertions. - static bool areTemporaryMaterializationsClear(ProgramStateRef State, - const LocationContext *FromLC, - const LocationContext *ToLC); + static bool areAllObjectsFullyConstructed(ProgramStateRef State, + const LocationContext *FromLC, + const LocationContext *ToLC); /// Adds an initialized temporary and/or a materialization, whichever is /// necessary, by looking at the whole construction context. Handles /// function return values, which need the construction context of the parent /// stack frame, automagically. - ProgramStateRef addAllNecessaryTemporaryInfo( + ProgramStateRef markStatementsCorrespondingToConstructedObject( ProgramStateRef State, const ConstructionContext *CC, - const LocationContext *LC, const MemRegion *R); - - /// Store the region returned by operator new() so that the constructor - /// that follows it knew what location to initialize. The value should be - /// cleared once the respective CXXNewExpr CFGStmt element is processed. - static ProgramStateRef - setCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE, - const LocationContext *CallerLC, SVal V); - - /// Retrieve the location returned by the current operator new(). - static SVal - getCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE, - const LocationContext *CallerLC); - - /// Clear the location returned by the respective operator new(). This needs - /// to be done as soon as CXXNewExpr CFG block is evaluated. - static ProgramStateRef - clearCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE, - const LocationContext *CallerLC); - - /// Check if all allocator values are clear for the given context range - /// (including FromLC, not including ToLC). This is useful for assertions. - static bool areCXXNewAllocatorValuesClear(ProgramStateRef State, - const LocationContext *FromLC, - const LocationContext *ToLC); + const LocationContext *LC, SVal V); }; /// Traits for storing the call processing policy inside GDM.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits