From e0208ff84598f48e0aafecf5b543afeff8574045 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Mon, 15 Apr 2013 20:39:41 +0000 Subject: [PATCH] [analyzer] Properly invalidate global regions on opaque function calls. This fixes a regression where a call to a function we can't reason about would not actually invalidate global regions that had explicit bindings. void test_that_now_works() { globalInt = 42; clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}} invalidateGlobals(); clang_analyzer_eval(globalInt == 42); // expected-warning{{UNKNOWN}} } This has probably been around since the initial "cluster" refactoring of RegionStore, if not longer. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179553 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/StaticAnalyzer/Core/RegionStore.cpp | 81 +++++++++++++++------ test/Analysis/global_region_invalidation.mm | 24 ++++-- 2 files changed, 77 insertions(+), 28 deletions(-) diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp index 51fe56ea86..09ee549ebb 100644 --- a/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -604,6 +604,17 @@ ento::CreateFieldsOnlyRegionStoreManager(ProgramStateManager &StMgr) { //===----------------------------------------------------------------------===// namespace { +/// Used to determine which global regions are automatically included in the +/// initial worklist of a ClusterAnalysis. +enum GlobalsFilterKind { + /// Don't include any global regions. + GFK_None, + /// Only include system globals. + GFK_SystemOnly, + /// Include all global regions. + GFK_All +}; + template class ClusterAnalysis { protected: @@ -620,19 +631,36 @@ protected: SValBuilder &svalBuilder; RegionBindingsRef B; - - const bool includeGlobals; +private: + GlobalsFilterKind GlobalsFilter; + +protected: const ClusterBindings *getCluster(const MemRegion *R) { return B.lookup(R); } + /// Returns true if the memory space of the given region is one of the global + /// regions specially included at the start of analysis. + bool isInitiallyIncludedGlobalRegion(const MemRegion *R) { + switch (GlobalsFilter) { + case GFK_None: + return false; + case GFK_SystemOnly: + return isa(R->getMemorySpace()); + case GFK_All: + return isa(R->getMemorySpace()); + } + + llvm_unreachable("unknown globals filter"); + } + public: ClusterAnalysis(RegionStoreManager &rm, ProgramStateManager &StateMgr, - RegionBindingsRef b, const bool includeGlobals) + RegionBindingsRef b, GlobalsFilterKind GFK) : RM(rm), Ctx(StateMgr.getContext()), svalBuilder(StateMgr.getSValBuilder()), - B(b), includeGlobals(includeGlobals) {} + B(b), GlobalsFilter(GFK) {} RegionBindingsRef getRegionBindings() const { return B; } @@ -650,9 +678,9 @@ public: assert(!Cluster.isEmpty() && "Empty clusters should be removed"); static_cast(this)->VisitAddedToCluster(Base, Cluster); - if (includeGlobals) - if (isa(Base->getMemorySpace())) - AddToWorkList(Base, &Cluster); + // If this is an interesting global region, add it the work list up front. + if (isInitiallyIncludedGlobalRegion(Base)) + AddToWorkList(WorkListElement(Base), &Cluster); } } @@ -905,8 +933,8 @@ public: InvalidatedSymbols &is, InvalidatedSymbols &inConstIS, StoreManager::InvalidatedRegions *r, - bool includeGlobals) - : ClusterAnalysis(rm, stateMgr, b, includeGlobals), + GlobalsFilterKind GFK) + : ClusterAnalysis(rm, stateMgr, b, GFK), Ex(ex), Count(count), LCtx(lctx), IS(is), ConstIS(inConstIS), Regions(r){} /// \param IsConst Specifies if the region we are invalidating is constant. @@ -1032,8 +1060,7 @@ void invalidateRegionsWorker::VisitCluster(const MemRegion *baseR, return; } - if (includeGlobals && - isa(baseR->getMemorySpace())) { + if (isInitiallyIncludedGlobalRegion(baseR)) { // If the region is a global and we are invalidating all globals, // just erase the entry. This causes all globals to be lazily // symbolicated from the same base symbol. @@ -1116,9 +1143,19 @@ RegionStoreManager::invalidateRegions(Store store, InvalidatedRegions *TopLevelRegions, InvalidatedRegions *TopLevelConstRegions, InvalidatedRegions *Invalidated) { - RegionBindingsRef B = RegionStoreManager::getRegionBindings(store); + GlobalsFilterKind GlobalsFilter; + if (Call) { + if (Call->isInSystemHeader()) + GlobalsFilter = GFK_SystemOnly; + else + GlobalsFilter = GFK_All; + } else { + GlobalsFilter = GFK_None; + } + + RegionBindingsRef B = getRegionBindings(store); invalidateRegionsWorker W(*this, StateMgr, B, Ex, Count, LCtx, IS, ConstIS, - Invalidated, false); + Invalidated, GlobalsFilter); // Scan the bindings and generate the clusters. W.GenerateClusters(); @@ -1138,14 +1175,17 @@ RegionStoreManager::invalidateRegions(Store store, // invalidate them. (Note that function-static and immutable globals are never // invalidated by this.) // TODO: This could possibly be more precise with modules. - if (Call) { + switch (GlobalsFilter) { + case GFK_All: + B = invalidateGlobalRegion(MemRegion::GlobalInternalSpaceRegionKind, + Ex, Count, LCtx, B, Invalidated); + // FALLTHROUGH + case GFK_SystemOnly: B = invalidateGlobalRegion(MemRegion::GlobalSystemSpaceRegionKind, Ex, Count, LCtx, B, Invalidated); - - if (!Call->isInSystemHeader()) { - B = invalidateGlobalRegion(MemRegion::GlobalInternalSpaceRegionKind, - Ex, Count, LCtx, B, Invalidated); - } + // FALLTHROUGH + case GFK_None: + break; } return StoreRef(B.asStore(), *this); @@ -2115,8 +2155,7 @@ public: ProgramStateManager &stateMgr, RegionBindingsRef b, SymbolReaper &symReaper, const StackFrameContext *LCtx) - : ClusterAnalysis(rm, stateMgr, b, - /* includeGlobals = */ false), + : ClusterAnalysis(rm, stateMgr, b, GFK_None), SymReaper(symReaper), CurrentLCtx(LCtx) {} // Called by ClusterAnalysis. diff --git a/test/Analysis/global_region_invalidation.mm b/test/Analysis/global_region_invalidation.mm index f853470a5f..5e9f1bcb83 100644 --- a/test/Analysis/global_region_invalidation.mm +++ b/test/Analysis/global_region_invalidation.mm @@ -19,27 +19,37 @@ void testGlobalRef() { } extern int globalInt; +extern struct { + int value; +} globalStruct; extern void invalidateGlobals(); void testGlobalInvalidation() { + clang_analyzer_eval(globalInt == 42); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(globalStruct.value == 43); // expected-warning{{UNKNOWN}} + if (globalInt != 42) return; + if (globalStruct.value != 43) + return; clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}} + clang_analyzer_eval(globalStruct.value == 43); // expected-warning{{TRUE}} invalidateGlobals(); clang_analyzer_eval(globalInt == 42); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(globalStruct.value == 43); // expected-warning{{UNKNOWN}} } - -//--------------------------------- -// False negatives -//--------------------------------- - void testGlobalInvalidationWithDirectBinding() { + clang_analyzer_eval(globalInt == 42); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(globalStruct.value == 43); // expected-warning{{UNKNOWN}} + globalInt = 42; + globalStruct.value = 43; clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}} + clang_analyzer_eval(globalStruct.value == 43); // expected-warning{{TRUE}} invalidateGlobals(); - // FIXME: Should be UNKNOWN. - clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}} + clang_analyzer_eval(globalInt == 42); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(globalStruct.value == 43); // expected-warning{{UNKNOWN}} } -- 2.40.0