From: Ted Kremenek Date: Thu, 1 Jul 2010 20:16:50 +0000 (+0000) Subject: Fix PR 7475 by enhancing the static analyzer to also invalidate bindings for non... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dcee3ce97fc76f20ce8f5a7451071e3dec537073;p=clang Fix PR 7475 by enhancing the static analyzer to also invalidate bindings for non-static global variables when calling a function/method whose impact on global variables we cannot accurately estimate. This change introduces two new MemSpaceRegions that divide up the memory space of globals, and causes RegionStore and BasicStore to consult a binding to the NonStaticGlobalsMemSpaceRegion when lazily determining the value of a global. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@107423 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Checker/PathSensitive/MemRegion.h b/include/clang/Checker/PathSensitive/MemRegion.h index 2ab3b420c3..8425c3bd62 100644 --- a/include/clang/Checker/PathSensitive/MemRegion.h +++ b/include/clang/Checker/PathSensitive/MemRegion.h @@ -35,6 +35,7 @@ class MemSpaceRegion; class LocationContext; class StackFrameContext; class VarRegion; +class CodeTextRegion; //===----------------------------------------------------------------------===// // Base region classes. @@ -46,14 +47,17 @@ class MemRegion : public llvm::FoldingSetNode { public: enum Kind { // Memory spaces. - BEG_MEMSPACES, - GenericMemSpaceRegionKind = BEG_MEMSPACES, + GenericMemSpaceRegionKind, StackLocalsSpaceRegionKind, StackArgumentsSpaceRegionKind, HeapSpaceRegionKind, UnknownSpaceRegionKind, - GlobalsSpaceRegionKind, - END_MEMSPACES = GlobalsSpaceRegionKind, + NonStaticGlobalSpaceRegionKind, + StaticGlobalSpaceRegionKind, + BEG_GLOBAL_MEMSPACES = NonStaticGlobalSpaceRegionKind, + END_GLOBAL_MEMSPACES = StaticGlobalSpaceRegionKind, + BEG_MEMSPACES = GenericMemSpaceRegionKind, + END_MEMSPACES = StaticGlobalSpaceRegionKind, // Untyped regions. SymbolicRegionKind, AllocaRegionKind, @@ -146,13 +150,43 @@ public: }; class GlobalsSpaceRegion : public MemSpaceRegion { +protected: + GlobalsSpaceRegion(MemRegionManager *mgr, Kind k) + : MemSpaceRegion(mgr, k) {} +public: + static bool classof(const MemRegion *R) { + Kind k = R->getKind(); + return k >= BEG_GLOBAL_MEMSPACES && k <= END_GLOBAL_MEMSPACES; + } +}; + +class StaticGlobalSpaceRegion : public GlobalsSpaceRegion { friend class MemRegionManager; - GlobalsSpaceRegion(MemRegionManager *mgr) - : MemSpaceRegion(mgr, GlobalsSpaceRegionKind) {} + const CodeTextRegion *CR; + + StaticGlobalSpaceRegion(MemRegionManager *mgr, const CodeTextRegion *cr) + : GlobalsSpaceRegion(mgr, StaticGlobalSpaceRegionKind), CR(cr) {} + +public: + void Profile(llvm::FoldingSetNodeID &ID) const; + + const CodeTextRegion *getCodeRegion() const { return CR; } + + static bool classof(const MemRegion *R) { + return R->getKind() == StaticGlobalSpaceRegionKind; + } +}; + +class NonStaticGlobalSpaceRegion : public GlobalsSpaceRegion { + friend class MemRegionManager; + + NonStaticGlobalSpaceRegion(MemRegionManager *mgr) + : GlobalsSpaceRegion(mgr, NonStaticGlobalSpaceRegionKind) {} + public: static bool classof(const MemRegion *R) { - return R->getKind() == GlobalsSpaceRegionKind; + return R->getKind() == NonStaticGlobalSpaceRegionKind; } }; @@ -793,12 +827,14 @@ class MemRegionManager { llvm::BumpPtrAllocator& A; llvm::FoldingSet Regions; - GlobalsSpaceRegion *globals; + NonStaticGlobalSpaceRegion *globals; llvm::DenseMap StackLocalsSpaceRegions; llvm::DenseMap StackArgumentsSpaceRegions; + llvm::DenseMap + StaticsGlobalSpaceRegions; HeapSpaceRegion *heap; UnknownSpaceRegion *unknown; @@ -825,8 +861,8 @@ public: getStackArgumentsRegion(const StackFrameContext *STC); /// getGlobalsRegion - Retrieve the memory region associated with - /// all global variables. - const GlobalsSpaceRegion *getGlobalsRegion(); + /// global variables. + const GlobalsSpaceRegion *getGlobalsRegion(const CodeTextRegion *R = 0); /// getHeapRegion - Retrieve the memory region associated with the /// generic "heap". diff --git a/include/clang/Checker/PathSensitive/Store.h b/include/clang/Checker/PathSensitive/Store.h index 7938856158..9becfe328e 100644 --- a/include/clang/Checker/PathSensitive/Store.h +++ b/include/clang/Checker/PathSensitive/Store.h @@ -168,7 +168,8 @@ public: const MemRegion * const *Begin, const MemRegion * const *End, const Expr *E, unsigned Count, - InvalidatedSymbols *IS); + InvalidatedSymbols *IS, + bool invalidateGlobals) = 0; // FIXME: Make out-of-line. virtual const GRState *setExtent(const GRState *state, diff --git a/lib/Checker/BasicStore.cpp b/lib/Checker/BasicStore.cpp index e1c488d0d2..62c8d9c248 100644 --- a/lib/Checker/BasicStore.cpp +++ b/lib/Checker/BasicStore.cpp @@ -46,9 +46,14 @@ public: SVal Retrieve(Store store, Loc loc, QualType T = QualType()); - Store InvalidateRegion(Store store, const MemRegion *R, const Expr *E, + Store InvalidateRegion(Store store, const MemRegion *R, const Expr *E, unsigned Count, InvalidatedSymbols *IS); + Store InvalidateRegions(Store store, const MemRegion * const *Begin, + const MemRegion * const *End, const Expr *E, + unsigned Count, InvalidatedSymbols *IS, + bool invalidateGlobals); + Store scanForIvars(Stmt *B, const Decl* SelfDecl, const MemRegion *SelfRegion, Store St); @@ -73,8 +78,8 @@ public: /// RemoveDeadBindings - Scans a BasicStore of 'state' for dead values. /// It updatees the GRState object in place with the values removed. const GRState *RemoveDeadBindings(GRState &state, - const StackFrameContext *LCtx, - SymbolReaper& SymReaper, + const StackFrameContext *LCtx, + SymbolReaper& SymReaper, llvm::SmallVectorImpl& RegionRoots); void iterBindings(Store store, BindingsHandler& f); @@ -144,9 +149,30 @@ SVal BasicStoreManager::LazyRetrieve(Store store, const TypedRegion *R) { // Globals and parameters start with symbolic values. // Local variables initially are undefined. + + // Non-static globals may have had their values reset by InvalidateRegions. + const MemSpaceRegion *MS = VR->getMemorySpace(); + if (isa(MS)) { + BindingsTy B = GetBindings(store); + // FIXME: Copy-and-pasted from RegionStore.cpp. + if (BindingsTy::data_type *Val = B.lookup(MS)) { + if (SymbolRef parentSym = Val->getAsSymbol()) + return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R); + + if (Val->isZeroConstant()) + return ValMgr.makeZeroVal(T); + + if (Val->isUnknownOrUndef()) + return *Val; + + assert(0 && "Unknown default value."); + } + } + if (VR->hasGlobalsOrParametersStorage() || isa(VR->getMemorySpace())) return ValMgr.getRegionValueSymbolVal(R); + return UndefinedVal(); } @@ -194,6 +220,14 @@ Store BasicStoreManager::Bind(Store store, Loc loc, SVal V) { return store; const MemRegion* R = cast(loc).getRegion(); + + // Special case: a default symbol assigned to the NonStaticGlobalsSpaceRegion + // that is used to derive other symbols. + if (isa(R)) { + BindingsTy B = GetBindings(store); + return VBFactory.Add(B, R, V).getRoot(); + } + ASTContext &C = StateMgr.getContext(); // Special case: handle store of pointer values (Loc) to pointers via @@ -268,9 +302,9 @@ const GRState *BasicStoreManager::RemoveDeadBindings(GRState &state, else continue; } - else if (isa(I.getKey())) { + else if (isa(I.getKey()) || + isa(I.getKey())) RegionRoots.push_back(I.getKey()); - } else continue; @@ -292,7 +326,8 @@ const GRState *BasicStoreManager::RemoveDeadBindings(GRState &state, SymReaper.markLive(SymR->getSymbol()); break; } - else if (isa(MR) || isa(MR)) { + else if (isa(MR) || isa(MR) || + isa(MR)) { if (Marked.count(MR)) break; @@ -486,6 +521,49 @@ StoreManager::BindingsHandler::~BindingsHandler() {} // Binding invalidation. //===----------------------------------------------------------------------===// + +Store BasicStoreManager::InvalidateRegions(Store store, + const MemRegion * const *I, + const MemRegion * const *End, + const Expr *E, unsigned Count, + InvalidatedSymbols *IS, + bool invalidateGlobals) { + if (invalidateGlobals) { + BindingsTy B = GetBindings(store); + for (BindingsTy::iterator I=B.begin(), End=B.end(); I != End; ++I) { + const MemRegion *R = I.getKey(); + if (isa(R->getMemorySpace())) + store = InvalidateRegion(store, R, E, Count, IS); + } + } + + for ( ; I != End ; ++I) { + const MemRegion *R = *I; + // Don't invalidate globals twice. + if (invalidateGlobals) { + if (isa(R->getMemorySpace())) + continue; + } + store = InvalidateRegion(store, *I, E, Count, IS); + } + + // FIXME: This is copy-and-paste from RegionStore.cpp. + if (invalidateGlobals) { + // Bind the non-static globals memory space to a new symbol that we will + // use to derive the bindings for all non-static globals. + const GlobalsSpaceRegion *GS = MRMgr.getGlobalsRegion(); + SVal V = + ValMgr.getConjuredSymbolVal(/* SymbolTag = */ (void*) GS, E, + /* symbol type, doesn't matter */ Ctx.IntTy, + Count); + + store = Bind(store, loc::MemRegionVal(GS), V); + } + + return store; +} + + Store BasicStoreManager::InvalidateRegion(Store store, const MemRegion *R, const Expr *E, diff --git a/lib/Checker/CFRefCount.cpp b/lib/Checker/CFRefCount.cpp index 8f65bf77bc..3c74cd8f9b 100644 --- a/lib/Checker/CFRefCount.cpp +++ b/lib/Checker/CFRefCount.cpp @@ -228,111 +228,111 @@ public: ErrorOverAutorelease, ErrorReturnedNotOwned }; - + private: Kind kind; RetEffect::ObjKind okind; unsigned Cnt; unsigned ACnt; QualType T; - + RefVal(Kind k, RetEffect::ObjKind o, unsigned cnt, unsigned acnt, QualType t) : kind(k), okind(o), Cnt(cnt), ACnt(acnt), T(t) {} - + RefVal(Kind k, unsigned cnt = 0) : kind(k), okind(RetEffect::AnyObj), Cnt(cnt), ACnt(0) {} - + public: Kind getKind() const { return kind; } - + RetEffect::ObjKind getObjKind() const { return okind; } - + unsigned getCount() const { return Cnt; } unsigned getAutoreleaseCount() const { return ACnt; } unsigned getCombinedCounts() const { return Cnt + ACnt; } void clearCounts() { Cnt = 0; ACnt = 0; } void setCount(unsigned i) { Cnt = i; } void setAutoreleaseCount(unsigned i) { ACnt = i; } - + QualType getType() const { return T; } - + // Useful predicates. - + static bool isError(Kind k) { return k >= ERROR_START; } - + static bool isLeak(Kind k) { return k >= ERROR_LEAK_START; } - + bool isOwned() const { return getKind() == Owned; } - + bool isNotOwned() const { return getKind() == NotOwned; } - + bool isReturnedOwned() const { return getKind() == ReturnedOwned; } - + bool isReturnedNotOwned() const { return getKind() == ReturnedNotOwned; } - + bool isNonLeakError() const { Kind k = getKind(); return isError(k) && !isLeak(k); } - + static RefVal makeOwned(RetEffect::ObjKind o, QualType t, unsigned Count = 1) { return RefVal(Owned, o, Count, 0, t); } - + static RefVal makeNotOwned(RetEffect::ObjKind o, QualType t, unsigned Count = 0) { return RefVal(NotOwned, o, Count, 0, t); } - + // Comparison, profiling, and pretty-printing. - + bool operator==(const RefVal& X) const { return kind == X.kind && Cnt == X.Cnt && T == X.T && ACnt == X.ACnt; } - + RefVal operator-(size_t i) const { return RefVal(getKind(), getObjKind(), getCount() - i, getAutoreleaseCount(), getType()); } - + RefVal operator+(size_t i) const { return RefVal(getKind(), getObjKind(), getCount() + i, getAutoreleaseCount(), getType()); } - + RefVal operator^(Kind k) const { return RefVal(k, getObjKind(), getCount(), getAutoreleaseCount(), getType()); } - + RefVal autorelease() const { return RefVal(getKind(), getObjKind(), getCount(), getAutoreleaseCount()+1, getType()); } - + void Profile(llvm::FoldingSetNodeID& ID) const { ID.AddInteger((unsigned) kind); ID.AddInteger(Cnt); ID.AddInteger(ACnt); ID.Add(T); } - + void print(llvm::raw_ostream& Out) const; }; void RefVal::print(llvm::raw_ostream& Out) const { if (!T.isNull()) Out << "Tracked Type:" << T.getAsString() << '\n'; - + switch (getKind()) { default: assert(false); case Owned: { @@ -341,69 +341,69 @@ void RefVal::print(llvm::raw_ostream& Out) const { if (cnt) Out << " (+ " << cnt << ")"; break; } - + case NotOwned: { Out << "NotOwned"; unsigned cnt = getCount(); if (cnt) Out << " (+ " << cnt << ")"; break; } - + case ReturnedOwned: { Out << "ReturnedOwned"; unsigned cnt = getCount(); if (cnt) Out << " (+ " << cnt << ")"; break; } - + case ReturnedNotOwned: { Out << "ReturnedNotOwned"; unsigned cnt = getCount(); if (cnt) Out << " (+ " << cnt << ")"; break; } - + case Released: Out << "Released"; break; - + case ErrorDeallocGC: Out << "-dealloc (GC)"; break; - + case ErrorDeallocNotOwned: Out << "-dealloc (not-owned)"; break; - + case ErrorLeak: Out << "Leaked"; break; - + case ErrorLeakReturned: Out << "Leaked (Bad naming)"; break; - + case ErrorGCLeakReturned: Out << "Leaked (GC-ed at return)"; break; - + case ErrorUseAfterRelease: Out << "Use-After-Release [ERROR]"; break; - + case ErrorReleaseNotOwned: Out << "Release of Not-Owned [ERROR]"; break; - + case RefVal::ErrorOverAutorelease: Out << "Over autoreleased"; break; - + case RefVal::ErrorReturnedNotOwned: Out << "Non-owned object returned instead of owned"; break; } - + if (ACnt) { Out << " [ARC +" << ACnt << ']'; } @@ -897,7 +897,7 @@ public: RetainSummary *getInstanceMethodSummary(const ObjCMessageExpr *ME, const GRState *state, const LocationContext *LC); - + RetainSummary* getInstanceMethodSummary(const ObjCMessageExpr* ME, const ObjCInterfaceDecl* ID) { return getInstanceMethodSummary(ME->getSelector(), 0, @@ -927,7 +927,7 @@ public: break; } - return getClassMethodSummary(ME->getSelector(), + return getClassMethodSummary(ME->getSelector(), Class? Class->getIdentifier() : 0, Class, ME->getMethodDecl(), ME->getType()); @@ -1419,16 +1419,16 @@ RetainSummaryManager::getInstanceMethodSummary(const ObjCMessageExpr *ME, if (Receiver) { receiverV = state->getSValAsScalarOrLoc(Receiver); - + // FIXME: Eventually replace the use of state->get with // a generic API for reasoning about the Objective-C types of symbolic // objects. if (SymbolRef Sym = receiverV.getAsLocSymbol()) if (const RefVal *T = state->get(Sym)) - if (const ObjCObjectPointerType* PT = + if (const ObjCObjectPointerType* PT = T->getType()->getAs()) ID = PT->getInterfaceDecl(); - + // FIXME: this is a hack. This may or may not be the actual method // that is called. if (!ID) { @@ -1444,7 +1444,7 @@ RetainSummaryManager::getInstanceMethodSummary(const ObjCMessageExpr *ME, // FIXME: The receiver could be a reference to a class, meaning that // we should use the class method. RetainSummary *Summ = getInstanceMethodSummary(ME, ID); - + // Special-case: are we sending a mesage to "self"? // This is a hack. When we have full-IP this should be removed. if (isa(LC->getDecl()) && Receiver) { @@ -1461,7 +1461,7 @@ RetainSummaryManager::getInstanceMethodSummary(const ObjCMessageExpr *ME, } } } - + return Summ ? Summ : getDefaultSummary(); } @@ -2619,7 +2619,7 @@ void CFRefCount::EvalSummary(ExplodedNodeSet& Dst, SymbolRef ErrorSym = 0; llvm::SmallVector RegionsToInvalidate; - + for (ExprIterator I = arg_beg; I != arg_end; ++I, ++idx) { SVal V = state->getSValAsScalarOrLoc(*I); SymbolRef Sym = V.getAsLocSymbol(); @@ -2667,7 +2667,7 @@ void CFRefCount::EvalSummary(ExplodedNodeSet& Dst, } // FIXME: What about layers of ElementRegions? } - + // Mark this region for invalidation. We batch invalidate regions // below for efficiency. RegionsToInvalidate.push_back(R); @@ -2687,37 +2687,39 @@ void CFRefCount::EvalSummary(ExplodedNodeSet& Dst, goto tryAgain; } } - + // Block calls result in all captured values passed-via-reference to be // invalidated. if (const BlockDataRegion *BR = dyn_cast_or_null(Callee)) { RegionsToInvalidate.push_back(BR); } - + // Invalidate regions we designed for invalidation use the batch invalidation // API. - if (!RegionsToInvalidate.empty()) { - // FIXME: We can have collisions on the conjured symbol if the - // expression *I also creates conjured symbols. We probably want - // to identify conjured symbols by an expression pair: the enclosing - // expression (the context) and the expression itself. This should - // disambiguate conjured symbols. - unsigned Count = Builder.getCurrentBlockCount(); - StoreManager& StoreMgr = Eng.getStateManager().getStoreManager(); - - - StoreManager::InvalidatedSymbols IS; - Store store = state->getStore(); - store = StoreMgr.InvalidateRegions(store, RegionsToInvalidate.data(), - RegionsToInvalidate.data() + - RegionsToInvalidate.size(), - Ex, Count, &IS); - state = state->makeWithStore(store); - for (StoreManager::InvalidatedSymbols::iterator I = IS.begin(), - E = IS.end(); I!=E; ++I) { - // Remove any existing reference-count binding. - state = state->remove(*I); - } + + // FIXME: We can have collisions on the conjured symbol if the + // expression *I also creates conjured symbols. We probably want + // to identify conjured symbols by an expression pair: the enclosing + // expression (the context) and the expression itself. This should + // disambiguate conjured symbols. + unsigned Count = Builder.getCurrentBlockCount(); + StoreManager& StoreMgr = Eng.getStateManager().getStoreManager(); + StoreManager::InvalidatedSymbols IS; + Store store = state->getStore(); + + // NOTE: Even if RegionsToInvalidate is empty, we must still invalidate + // global variables. + store = StoreMgr.InvalidateRegions(store, RegionsToInvalidate.data(), + RegionsToInvalidate.data() + + RegionsToInvalidate.size(), + Ex, Count, &IS, + /* invalidateGlobals = */ true); + + state = state->makeWithStore(store); + for (StoreManager::InvalidatedSymbols::iterator I = IS.begin(), + E = IS.end(); I!=E; ++I) { + // Remove any existing reference-count binding. + state = state->remove(*I); } // Evaluate the effect on the message receiver. @@ -2862,7 +2864,7 @@ void CFRefCount::EvalCall(ExplodedNodeSet& Dst, ExplodedNode* Pred) { RetainSummary *Summ = 0; - + // FIXME: Better support for blocks. For now we stop tracking anything // that is passed to blocks. // FIXME: Need to handle variables that are "captured" by the block. @@ -3500,7 +3502,7 @@ class RetainReleaseChecker public: RetainReleaseChecker(CFRefCount *tf) : TF(tf) {} static void* getTag() { static int x = 0; return &x; } - + void PostVisitBlockExpr(CheckerContext &C, const BlockExpr *BE); }; } // end anonymous namespace @@ -3508,29 +3510,29 @@ public: void RetainReleaseChecker::PostVisitBlockExpr(CheckerContext &C, const BlockExpr *BE) { - + // Scan the BlockDecRefExprs for any object the retain/release checker - // may be tracking. + // may be tracking. if (!BE->hasBlockDeclRefExprs()) return; - + const GRState *state = C.getState(); const BlockDataRegion *R = cast(state->getSVal(BE).getAsRegion()); - + BlockDataRegion::referenced_vars_iterator I = R->referenced_vars_begin(), E = R->referenced_vars_end(); - + if (I == E) return; - + // FIXME: For now we invalidate the tracking of all symbols passed to blocks // via captured variables, even though captured variables result in a copy // and in implicit increment/decrement of a retain count. llvm::SmallVector Regions; const LocationContext *LC = C.getPredecessor()->getLocationContext(); MemRegionManager &MemMgr = C.getValueManager().getRegionManager(); - + for ( ; I != E; ++I) { const VarRegion *VR = *I; if (VR->getSuperRegion() == R) { @@ -3538,7 +3540,7 @@ void RetainReleaseChecker::PostVisitBlockExpr(CheckerContext &C, } Regions.push_back(VR); } - + state = state->scanReachableSymbols(Regions.data(), Regions.data() + Regions.size()).getState(); @@ -3551,28 +3553,28 @@ void RetainReleaseChecker::PostVisitBlockExpr(CheckerContext &C, void CFRefCount::RegisterChecks(GRExprEngine& Eng) { BugReporter &BR = Eng.getBugReporter(); - + useAfterRelease = new UseAfterRelease(this); BR.Register(useAfterRelease); - + releaseNotOwned = new BadRelease(this); BR.Register(releaseNotOwned); - + deallocGC = new DeallocGC(this); BR.Register(deallocGC); - + deallocNotOwned = new DeallocNotOwned(this); BR.Register(deallocNotOwned); - + overAutorelease = new OverAutorelease(this); BR.Register(overAutorelease); - + returnNotOwnedForOwned = new ReturnedNotOwnedForOwned(this); BR.Register(returnNotOwnedForOwned); - + // First register "return" leaks. const char* name = 0; - + if (isGCEnabled()) name = "Leak of returned object when using garbage collection"; else if (getLangOptions().getGCMode() == LangOptions::HybridGC) @@ -3582,12 +3584,12 @@ void CFRefCount::RegisterChecks(GRExprEngine& Eng) { assert(getLangOptions().getGCMode() == LangOptions::NonGC); name = "Leak of returned object"; } - + // Leaks should not be reported if they are post-dominated by a sink. leakAtReturn = new LeakAtReturn(this, name); leakAtReturn->setSuppressOnSink(true); BR.Register(leakAtReturn); - + // Second, register leaks within a function/method. if (isGCEnabled()) name = "Leak of object when using garbage collection"; @@ -3598,15 +3600,15 @@ void CFRefCount::RegisterChecks(GRExprEngine& Eng) { assert(getLangOptions().getGCMode() == LangOptions::NonGC); name = "Leak"; } - + // Leaks should not be reported if they are post-dominated by sinks. leakWithinFunction = new LeakWithinFunction(this, name); leakWithinFunction->setSuppressOnSink(true); BR.Register(leakWithinFunction); - + // Save the reference to the BugReporter. this->BR = &BR; - + // Register the RetainReleaseChecker with the GRExprEngine object. // Functionality in CFRefCount will be migrated to RetainReleaseChecker // over time. diff --git a/lib/Checker/FlatStore.cpp b/lib/Checker/FlatStore.cpp index 2b6b1acad9..64575b3c97 100644 --- a/lib/Checker/FlatStore.cpp +++ b/lib/Checker/FlatStore.cpp @@ -59,6 +59,11 @@ public: Store InvalidateRegion(Store store, const MemRegion *R, const Expr *E, unsigned Count, InvalidatedSymbols *IS); + + Store InvalidateRegions(Store store, const MemRegion * const *I, + const MemRegion * const *E, const Expr *Ex, + unsigned Count, InvalidatedSymbols *IS, + bool invalidateGlobals); void print(Store store, llvm::raw_ostream& Out, const char* nl, const char *sep); @@ -141,9 +146,20 @@ Store FlatStoreManager::BindDeclWithNoInit(Store store, const VarRegion *VR) { return store; } +Store FlatStoreManager::InvalidateRegions(Store store, + const MemRegion * const *I, + const MemRegion * const *E, + const Expr *Ex, unsigned Count, + InvalidatedSymbols *IS, + bool invalidateGlobals) { + assert(false && "Not implemented"); + return store; +} + Store FlatStoreManager::InvalidateRegion(Store store, const MemRegion *R, const Expr *E, unsigned Count, InvalidatedSymbols *IS) { + assert(false && "Not implemented"); return store; } diff --git a/lib/Checker/MallocChecker.cpp b/lib/Checker/MallocChecker.cpp index 85ce35bd46..a5bba1d8ca 100644 --- a/lib/Checker/MallocChecker.cpp +++ b/lib/Checker/MallocChecker.cpp @@ -341,7 +341,8 @@ bool MallocChecker::SummarizeRegion(llvm::raw_ostream& os, os << "the address of a parameter"; return true; } - case MemRegion::GlobalsSpaceRegionKind: { + case MemRegion::NonStaticGlobalSpaceRegionKind: + case MemRegion::StaticGlobalSpaceRegionKind: { const VarRegion *VR = dyn_cast(MR); const VarDecl *VD; if (VR) diff --git a/lib/Checker/MemRegion.cpp b/lib/Checker/MemRegion.cpp index 575458c9dc..66d2a41914 100644 --- a/lib/Checker/MemRegion.cpp +++ b/lib/Checker/MemRegion.cpp @@ -29,22 +29,22 @@ template struct MemRegionManagerTrait; template RegionTy* MemRegionManager::getRegion(const A1 a1) { - + const typename MemRegionManagerTrait::SuperRegionTy *superRegion = MemRegionManagerTrait::getSuperRegion(*this, a1); - + llvm::FoldingSetNodeID ID; RegionTy::ProfileRegion(ID, a1, superRegion); void* InsertPos; RegionTy* R = cast_or_null(Regions.FindNodeOrInsertPos(ID, InsertPos)); - + if (!R) { R = (RegionTy*) A.Allocate(); new (R) RegionTy(a1, superRegion); Regions.InsertNode(R, InsertPos); } - + return R; } @@ -56,72 +56,72 @@ RegionTy* MemRegionManager::getSubRegion(const A1 a1, void* InsertPos; RegionTy* R = cast_or_null(Regions.FindNodeOrInsertPos(ID, InsertPos)); - + if (!R) { R = (RegionTy*) A.Allocate(); new (R) RegionTy(a1, superRegion); Regions.InsertNode(R, InsertPos); } - + return R; } template RegionTy* MemRegionManager::getRegion(const A1 a1, const A2 a2) { - + const typename MemRegionManagerTrait::SuperRegionTy *superRegion = MemRegionManagerTrait::getSuperRegion(*this, a1, a2); - + llvm::FoldingSetNodeID ID; RegionTy::ProfileRegion(ID, a1, a2, superRegion); void* InsertPos; RegionTy* R = cast_or_null(Regions.FindNodeOrInsertPos(ID, InsertPos)); - + if (!R) { R = (RegionTy*) A.Allocate(); new (R) RegionTy(a1, a2, superRegion); Regions.InsertNode(R, InsertPos); } - + return R; } template RegionTy* MemRegionManager::getSubRegion(const A1 a1, const A2 a2, const MemRegion *superRegion) { - + llvm::FoldingSetNodeID ID; RegionTy::ProfileRegion(ID, a1, a2, superRegion); void* InsertPos; RegionTy* R = cast_or_null(Regions.FindNodeOrInsertPos(ID, InsertPos)); - + if (!R) { R = (RegionTy*) A.Allocate(); new (R) RegionTy(a1, a2, superRegion); Regions.InsertNode(R, InsertPos); } - + return R; } template RegionTy* MemRegionManager::getSubRegion(const A1 a1, const A2 a2, const A3 a3, const MemRegion *superRegion) { - + llvm::FoldingSetNodeID ID; RegionTy::ProfileRegion(ID, a1, a2, a3, superRegion); void* InsertPos; RegionTy* R = cast_or_null(Regions.FindNodeOrInsertPos(ID, InsertPos)); - + if (!R) { R = (RegionTy*) A.Allocate(); new (R) RegionTy(a1, a2, a3, superRegion); Regions.InsertNode(R, InsertPos); } - + return R; } @@ -183,6 +183,11 @@ void StackSpaceRegion::Profile(llvm::FoldingSetNodeID &ID) const { ID.AddPointer(getStackFrame()); } +void StaticGlobalSpaceRegion::Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddInteger((unsigned)getKind()); + ID.AddPointer(getCodeRegion()); +} + void StringRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, const StringLiteral* Str, const MemRegion* superRegion) { @@ -226,7 +231,7 @@ void CXXThisRegion::ProfileRegion(llvm::FoldingSetNodeID &ID, void CXXThisRegion::Profile(llvm::FoldingSetNodeID &ID) const { CXXThisRegion::ProfileRegion(ID, ThisPointerTy, superRegion); } - + void DeclRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, const Decl* D, const MemRegion* superRegion, Kind k) { ID.AddInteger((unsigned) k); @@ -412,7 +417,7 @@ const REG *MemRegionManager::LazyAllocate(REG*& region, ARG a) { region = (REG*) A.Allocate(); new (region) REG(this, a); } - + return region; } @@ -442,8 +447,18 @@ MemRegionManager::getStackArgumentsRegion(const StackFrameContext *STC) { return R; } -const GlobalsSpaceRegion *MemRegionManager::getGlobalsRegion() { - return LazyAllocate(globals); +const GlobalsSpaceRegion +*MemRegionManager::getGlobalsRegion(const CodeTextRegion *CR) { + if (!CR) + return LazyAllocate(globals); + + StaticGlobalSpaceRegion *&R = StaticsGlobalSpaceRegions[CR]; + if (R) + return R; + + R = A.Allocate(); + new (R) StaticGlobalSpaceRegion(this, CR); + return R; } const HeapSpaceRegion *MemRegionManager::getHeapRegion() { @@ -462,7 +477,7 @@ const MemSpaceRegion *MemRegionManager::getCodeRegion() { // Constructing regions. //===----------------------------------------------------------------------===// -const StringRegion* MemRegionManager::getStringRegion(const StringLiteral* Str) { +const StringRegion* MemRegionManager::getStringRegion(const StringLiteral* Str){ return getSubRegion(Str, getGlobalsRegion()); } @@ -470,7 +485,9 @@ const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D, const LocationContext *LC) { const MemRegion *sReg = 0; - if (D->hasLocalStorage()) { + if (D->hasGlobalStorage() && !D->isStaticLocal()) + sReg = getGlobalsRegion(); + else { // FIXME: Once we implement scope handling, we will need to properly lookup // 'D' to the proper LocationContext. const DeclContext *DC = D->getDeclContext(); @@ -479,15 +496,32 @@ const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D, if (!STC) sReg = getUnknownRegion(); else { - sReg = isa(D) || isa(D) - ? static_cast(getStackArgumentsRegion(STC)) - : static_cast(getStackLocalsRegion(STC)); + if (D->hasLocalStorage()) { + sReg = isa(D) || isa(D) + ? static_cast(getStackArgumentsRegion(STC)) + : static_cast(getStackLocalsRegion(STC)); + } + else { + assert(D->isStaticLocal()); + const Decl *D = STC->getDecl(); + if (const FunctionDecl *FD = dyn_cast(D)) + sReg = getGlobalsRegion(getFunctionTextRegion(FD)); + else if (const BlockDecl *BD = dyn_cast(D)) { + const BlockTextRegion *BTR = + getBlockTextRegion(BD, + C.getCanonicalType(BD->getSignatureAsWritten()->getType()), + STC->getAnalysisContext()); + sReg = getGlobalsRegion(BTR); + } + else { + // FIXME: For ObjC-methods, we need a new CodeTextRegion. For now + // just use the main global memspace. + sReg = getGlobalsRegion(); + } + } } } - else { - sReg = getGlobalsRegion(); - } - + return getSubRegion(D, sReg); } @@ -500,10 +534,10 @@ const BlockDataRegion * MemRegionManager::getBlockDataRegion(const BlockTextRegion *BC, const LocationContext *LC) { const MemRegion *sReg = 0; - - if (LC) { + + if (LC) { // FIXME: Once we implement scope handling, we want the parent region - // to be the scope. + // to be the scope. const StackFrameContext *STC = LC->getCurrentStackFrame(); assert(STC); sReg = getStackLocalsRegion(STC); @@ -520,9 +554,9 @@ MemRegionManager::getBlockDataRegion(const BlockTextRegion *BC, const CompoundLiteralRegion* MemRegionManager::getCompoundLiteralRegion(const CompoundLiteralExpr* CL, const LocationContext *LC) { - + const MemRegion *sReg = 0; - + if (CL->isFileScope()) sReg = getGlobalsRegion(); else { @@ -530,7 +564,7 @@ MemRegionManager::getCompoundLiteralRegion(const CompoundLiteralExpr* CL, assert(STC); sReg = getStackLocalsRegion(STC); } - + return getSubRegion(CL, sReg); } @@ -749,24 +783,24 @@ void BlockDataRegion::LazyInitializeReferencedVars() { AnalysisContext *AC = getCodeRegion()->getAnalysisContext(); AnalysisContext::referenced_decls_iterator I, E; llvm::tie(I, E) = AC->getReferencedBlockVars(BC->getDecl()); - + if (I == E) { ReferencedVars = (void*) 0x1; return; } - + MemRegionManager &MemMgr = *getMemRegionManager(); llvm::BumpPtrAllocator &A = MemMgr.getAllocator(); BumpVectorContext BC(A); - + typedef BumpVector VarVec; VarVec *BV = (VarVec*) A.Allocate(); new (BV) VarVec(BC, E - I); - + for ( ; I != E; ++I) { const VarDecl *VD = *I; const VarRegion *VR = 0; - + if (!VD->getAttr() && VD->hasLocalStorage()) VR = MemMgr.getVarRegion(VD, this); else { @@ -776,11 +810,11 @@ void BlockDataRegion::LazyInitializeReferencedVars() { VR = MemMgr.getVarRegion(VD, MemMgr.getUnknownRegion()); } } - + assert(VR); BV->push_back(VR, BC); } - + ReferencedVars = BV; } @@ -790,7 +824,7 @@ BlockDataRegion::referenced_vars_begin() const { BumpVector *Vec = static_cast*>(ReferencedVars); - + return BlockDataRegion::referenced_vars_iterator(Vec == (void*) 0x1 ? NULL : Vec->begin()); } @@ -801,7 +835,7 @@ BlockDataRegion::referenced_vars_end() const { BumpVector *Vec = static_cast*>(ReferencedVars); - + return BlockDataRegion::referenced_vars_iterator(Vec == (void*) 0x1 ? NULL : Vec->end()); } diff --git a/lib/Checker/RegionStore.cpp b/lib/Checker/RegionStore.cpp index c239adbe3d..8a64ec8d24 100644 --- a/lib/Checker/RegionStore.cpp +++ b/lib/Checker/RegionStore.cpp @@ -244,14 +244,16 @@ public: Store InvalidateRegion(Store store, const MemRegion *R, const Expr *E, unsigned Count, InvalidatedSymbols *IS) { - return RegionStoreManager::InvalidateRegions(store, &R, &R+1, E, Count, IS); + return RegionStoreManager::InvalidateRegions(store, &R, &R+1, E, Count, IS, + false); } Store InvalidateRegions(Store store, const MemRegion * const *Begin, const MemRegion * const *End, const Expr *E, unsigned Count, - InvalidatedSymbols *IS); + InvalidatedSymbols *IS, + bool invalidateGlobals); public: // Made public for helper classes. @@ -347,6 +349,12 @@ public: // Part of public interface to class. SVal RetrieveArray(Store store, const TypedRegion* R); + /// Used to lazily generate derived symbols for bindings that are defined + /// implicitly by default bindings in a super region. + Optional RetrieveDerivedDefaultValue(RegionBindings B, + const MemRegion *superR, + const TypedRegion *R, QualType Ty); + /// Get the state and region whose binding this region R corresponds to. std::pair GetLazyBinding(RegionBindings B, const MemRegion *R); @@ -496,12 +504,13 @@ public: RegionBindings getRegionBindings() const { return B; } - void AddToCluster(BindingKey K) { + RegionCluster &AddToCluster(BindingKey K) { const MemRegion *R = K.getRegion(); const MemRegion *baseR = R->getBaseRegion(); RegionCluster &C = getCluster(baseR); C.push_back(K, BVC); static_cast(this)->VisitAddedToCluster(baseR, C); + return C; } bool isVisited(const MemRegion *R) { @@ -517,15 +526,20 @@ public: return *CRef; } - void GenerateClusters() { + void GenerateClusters(bool includeGlobals = false) { // Scan the entire set of bindings and make the region clusters. for (RegionBindings::iterator RI = B.begin(), RE = B.end(); RI != RE; ++RI){ - AddToCluster(RI.getKey()); + RegionCluster &C = AddToCluster(RI.getKey()); if (const MemRegion *R = RI.getData().getAsRegion()) { // Generate a cluster, but don't add the region to the cluster // if there aren't any bindings. getCluster(R->getBaseRegion()); } + if (includeGlobals) { + const MemRegion *R = RI.getKey().getRegion(); + if (isa(R->getMemorySpace())) + AddToWorkList(R, C); + } } } @@ -719,13 +733,14 @@ Store RegionStoreManager::InvalidateRegions(Store store, const MemRegion * const *I, const MemRegion * const *E, const Expr *Ex, unsigned Count, - InvalidatedSymbols *IS) { + InvalidatedSymbols *IS, + bool invalidateGlobals) { InvalidateRegionsWorker W(*this, StateMgr, RegionStoreManager::GetRegionBindings(store), Ex, Count, IS); // Scan the bindings and generate the clusters. - W.GenerateClusters(); + W.GenerateClusters(invalidateGlobals); // Add I .. E to the worklist. for ( ; I != E; ++I) @@ -734,7 +749,20 @@ Store RegionStoreManager::InvalidateRegions(Store store, W.RunWorkList(); // Return the new bindings. - return W.getRegionBindings().getRoot(); + RegionBindings B = W.getRegionBindings(); + + if (invalidateGlobals) { + // Bind the non-static globals memory space to a new symbol that we will + // use to derive the bindings for all non-static globals. + const GlobalsSpaceRegion *GS = MRMgr.getGlobalsRegion(); + SVal V = + ValMgr.getConjuredSymbolVal(/* SymbolTag = */ (void*) GS, Ex, + /* symbol type, doesn't matter */ Ctx.IntTy, + Count); + B = Add(B, BindingKey::Make(GS, BindingKey::Default), V); + } + + return B.getRoot(); } //===----------------------------------------------------------------------===// @@ -752,7 +780,8 @@ DefinedOrUnknownSVal RegionStoreManager::getSizeInElements(const GRState *state, case MemRegion::StackLocalsSpaceRegionKind: case MemRegion::StackArgumentsSpaceRegionKind: case MemRegion::HeapSpaceRegionKind: - case MemRegion::GlobalsSpaceRegionKind: + case MemRegion::NonStaticGlobalSpaceRegionKind: + case MemRegion::StaticGlobalSpaceRegionKind: case MemRegion::UnknownSpaceRegionKind: assert(0 && "Cannot index into a MemSpace"); return UnknownVal(); @@ -937,7 +966,8 @@ SVal RegionStoreManager::EvalBinOp(BinaryOperator::Opcode Op, Loc L, NonLoc R, case MemRegion::StackLocalsSpaceRegionKind: case MemRegion::StackArgumentsSpaceRegionKind: case MemRegion::HeapSpaceRegionKind: - case MemRegion::GlobalsSpaceRegionKind: + case MemRegion::NonStaticGlobalSpaceRegionKind: + case MemRegion::StaticGlobalSpaceRegionKind: case MemRegion::UnknownSpaceRegionKind: assert(0 && "Cannot perform pointer arithmetic on a MemSpace"); return UnknownVal(); @@ -1224,7 +1254,7 @@ SVal RegionStoreManager::RetrieveElement(Store store, if (const Optional &V = getDirectBinding(B, superR)) { if (SymbolRef parentSym = V->getAsSymbol()) return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R); - + if (V->isUnknownOrUndef()) return *V; // Other cases: give up. We are indexing into a larger object @@ -1250,6 +1280,28 @@ SVal RegionStoreManager::RetrieveField(Store store, return RetrieveFieldOrElementCommon(store, R, Ty, R->getSuperRegion()); } +Optional +RegionStoreManager::RetrieveDerivedDefaultValue(RegionBindings B, + const MemRegion *superR, + const TypedRegion *R, + QualType Ty) { + + if (const Optional &D = getDefaultBinding(B, superR)) { + if (SymbolRef parentSym = D->getAsSymbol()) + return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R); + + if (D->isZeroConstant()) + return ValMgr.makeZeroVal(Ty); + + if (D->isUnknownOrUndef()) + return *D; + + assert(0 && "Unknown default value"); + } + + return Optional(); +} + SVal RegionStoreManager::RetrieveFieldOrElementCommon(Store store, const TypedRegion *R, QualType Ty, @@ -1261,18 +1313,8 @@ SVal RegionStoreManager::RetrieveFieldOrElementCommon(Store store, RegionBindings B = GetRegionBindings(store); while (superR) { - if (const Optional &D = getDefaultBinding(B, superR)) { - if (SymbolRef parentSym = D->getAsSymbol()) - return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R); - - if (D->isZeroConstant()) - return ValMgr.makeZeroVal(Ty); - - if (D->isUnknownOrUndef()) - return *D; - - assert(0 && "Unknown default value"); - } + if (const Optional &D = RetrieveDerivedDefaultValue(B, superR, R, Ty)) + return *D; // If our super region is a field or element itself, walk up the region // hierarchy to see if there is a default value installed in an ancestor. @@ -1353,7 +1395,7 @@ SVal RegionStoreManager::RetrieveVar(Store store, const VarRegion *R) { return ValMgr.getRegionValueSymbolVal(R); if (isa(MS)) { - if (VD->isFileVarDecl()) { + if (isa(MS)) { // Is 'VD' declared constant? If so, retrieve the constant value. QualType CT = Ctx.getCanonicalType(T); if (CT.isConstQualified()) { @@ -1368,6 +1410,9 @@ SVal RegionStoreManager::RetrieveVar(Store store, const VarRegion *R) { } } + if (const Optional &V = RetrieveDerivedDefaultValue(B, MS, R, CT)) + return V.getValue(); + return ValMgr.getRegionValueSymbolVal(R); } @@ -1735,7 +1780,7 @@ class RemoveDeadBindingsWorker : llvm::SmallVector Postponed; SymbolReaper &SymReaper; const StackFrameContext *CurrentLCtx; - + public: RemoveDeadBindingsWorker(RegionStoreManager &rm, GRStateManager &stateMgr, RegionBindings b, SymbolReaper &symReaper, @@ -1772,9 +1817,14 @@ void RemoveDeadBindingsWorker::VisitAddedToCluster(const MemRegion *baseR, return; } + if (isa(baseR)) { + AddToWorkList(baseR, C); + return; + } + // CXXThisRegion in the current or parent location context is live. if (const CXXThisRegion *TR = dyn_cast(baseR)) { - const StackArgumentsSpaceRegion *StackReg = + const StackArgumentsSpaceRegion *StackReg = cast(TR->getSuperRegion()); const StackFrameContext *RegCtx = StackReg->getStackFrame(); if (RegCtx == CurrentLCtx || RegCtx->isParentOf(CurrentLCtx)) @@ -1929,9 +1979,9 @@ GRState const *RegionStoreManager::EnterStackFrame(GRState const *state, SVal ArgVal = state->getSVal(*AI); store = Bind(store, ValMgr.makeLoc(MRMgr.getVarRegion(*PI,frame)),ArgVal); } - } else if (const CXXConstructExpr *CE = + } else if (const CXXConstructExpr *CE = dyn_cast(frame->getCallSite())) { - CXXConstructExpr::const_arg_iterator AI = CE->arg_begin(), + CXXConstructExpr::const_arg_iterator AI = CE->arg_begin(), AE = CE->arg_end(); // Copy the arg expression value to the arg variables. diff --git a/lib/Checker/Store.cpp b/lib/Checker/Store.cpp index c12065b89a..b128331705 100644 --- a/lib/Checker/Store.cpp +++ b/lib/Checker/Store.cpp @@ -91,7 +91,8 @@ const MemRegion *StoreManager::CastRegion(const MemRegion *R, QualType CastToTy) case MemRegion::StackArgumentsSpaceRegionKind: case MemRegion::HeapSpaceRegionKind: case MemRegion::UnknownSpaceRegionKind: - case MemRegion::GlobalsSpaceRegionKind: { + case MemRegion::NonStaticGlobalSpaceRegionKind: + case MemRegion::StaticGlobalSpaceRegionKind: { assert(0 && "Invalid region cast"); break; } @@ -232,17 +233,6 @@ SVal StoreManager::CastRetrievedVal(SVal V, const TypedRegion *R, return V; } -Store StoreManager::InvalidateRegions(Store store, - const MemRegion * const *I, - const MemRegion * const *End, - const Expr *E, unsigned Count, - InvalidatedSymbols *IS) { - for ( ; I != End ; ++I) - store = InvalidateRegion(store, *I, E, Count, IS); - - return store; -} - SVal StoreManager::getLValueFieldOrIvar(const Decl* D, SVal Base) { if (Base.isUnknownOrUndef()) return Base; diff --git a/test/Analysis/misc-ps.m b/test/Analysis/misc-ps.m index 0de5113072..253450cc42 100644 --- a/test/Analysis/misc-ps.m +++ b/test/Analysis/misc-ps.m @@ -999,3 +999,24 @@ void pr7491 () { } } +//===----------------------------------------------------------------------=== +// PR 7475 - Test that assumptions about global variables are reset after +// calling a global function. +//===----------------------------------------------------------------------=== + +int *pr7475_someGlobal; +void pr7475_setUpGlobal(); + +void pr7475() { + if (pr7475_someGlobal == 0) + pr7475_setUpGlobal(); + *pr7475_someGlobal = 0; // no-warning +} + +void pr7475_warn() { + static int *someStatic = 0; + if (someStatic == 0) + pr7475_setUpGlobal(); + *someStatic = 0; // expected-warning{{null pointer}} +} +