[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.
<rdar://problem/13464044>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179553 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index 51fe56e..09ee549 100644
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -604,6 +604,17 @@
//===----------------------------------------------------------------------===//
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 <typename DERIVED>
class ClusterAnalysis {
protected:
@@ -620,19 +631,36 @@
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<GlobalSystemSpaceRegion>(R->getMemorySpace());
+ case GFK_All:
+ return isa<NonStaticGlobalSpaceRegion>(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 @@
assert(!Cluster.isEmpty() && "Empty clusters should be removed");
static_cast<DERIVED*>(this)->VisitAddedToCluster(Base, Cluster);
- if (includeGlobals)
- if (isa<NonStaticGlobalSpaceRegion>(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 @@
InvalidatedSymbols &is,
InvalidatedSymbols &inConstIS,
StoreManager::InvalidatedRegions *r,
- bool includeGlobals)
- : ClusterAnalysis<invalidateRegionsWorker>(rm, stateMgr, b, includeGlobals),
+ GlobalsFilterKind GFK)
+ : ClusterAnalysis<invalidateRegionsWorker>(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 @@
return;
}
- if (includeGlobals &&
- isa<NonStaticGlobalSpaceRegion>(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 @@
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 @@
// 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 @@
ProgramStateManager &stateMgr,
RegionBindingsRef b, SymbolReaper &symReaper,
const StackFrameContext *LCtx)
- : ClusterAnalysis<removeDeadBindingsWorker>(rm, stateMgr, b,
- /* includeGlobals = */ false),
+ : ClusterAnalysis<removeDeadBindingsWorker>(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 f853470..5e9f1bc 100644
--- a/test/Analysis/global_region_invalidation.mm
+++ b/test/Analysis/global_region_invalidation.mm
@@ -19,27 +19,37 @@
}
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}}
}
