r177572 - [analyzer] Don't invalidate globals when there's no call involved.

Jordan Rose jordan_rose at apple.com
Wed Mar 20 13:36:01 PDT 2013


Author: jrose
Date: Wed Mar 20 15:36:01 2013
New Revision: 177572

URL: http://llvm.org/viewvc/llvm-project?rev=177572&view=rev
Log:
[analyzer] Don't invalidate globals when there's no call involved.

This fixes some mistaken condition logic in RegionStore that caused
global variables to be invalidated when /any/ region was invalidated,
rather than only as part of opaque function calls. This was only
being used by CStringChecker, and so users will now see that strcpy()
and friends do not invalidate global variables.

Also, add a test case we don't handle properly: explicitly-assigned
global variables aren't being invalidated by opaque calls. This is
being tracked by <rdar://problem/13464044>.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/test/Analysis/global_region_invalidation.mm
    cfe/trunk/test/Analysis/string.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=177572&r1=177571&r2=177572&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Mar 20 15:36:01 2013
@@ -1073,20 +1073,18 @@ RegionStoreManager::invalidateRegions(St
   // Return the new bindings.
   B = W.getRegionBindings();
 
-  // For all globals which are not static nor immutable: determine which global
-  // regions should be invalidated and invalidate them.
+  // For calls, determine which global regions should be invalidated and
+  // invalidate them. (Note that function-static and immutable globals are never
+  // invalidated by this.)
   // TODO: This could possibly be more precise with modules.
-  //
-  // System calls invalidate only system globals.
-  if (Call && Call->isInSystemHeader()) {
+  if (Call) {
     B = invalidateGlobalRegion(MemRegion::GlobalSystemSpaceRegionKind,
                                Ex, Count, LCtx, B, Invalidated);
-  // Internal calls might invalidate both system and internal globals.
-  } else {
-    B = invalidateGlobalRegion(MemRegion::GlobalSystemSpaceRegionKind,
-                               Ex, Count, LCtx, B, Invalidated);
-    B = invalidateGlobalRegion(MemRegion::GlobalInternalSpaceRegionKind,
-                               Ex, Count, LCtx, B, Invalidated);
+
+    if (!Call->isInSystemHeader()) {
+      B = invalidateGlobalRegion(MemRegion::GlobalInternalSpaceRegionKind,
+                                 Ex, Count, LCtx, B, Invalidated);
+    }
   }
 
   return StoreRef(B.asStore(), *this);

Modified: cfe/trunk/test/Analysis/global_region_invalidation.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/global_region_invalidation.mm?rev=177572&r1=177571&r2=177572&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/global_region_invalidation.mm (original)
+++ cfe/trunk/test/Analysis/global_region_invalidation.mm Wed Mar 20 15:36:01 2013
@@ -11,9 +11,35 @@ id foo(int x) {
   return p;
 }
 
-const int &globalInt = 42;
+const int &globalIntRef = 42;
 
-void testGlobal() {
+void testGlobalRef() {
   // FIXME: Should be TRUE, but should at least not crash.
+  clang_analyzer_eval(globalIntRef == 42); // expected-warning{{UNKNOWN}}
+}
+
+extern int globalInt;
+extern void invalidateGlobals();
+
+void testGlobalInvalidation() {
+  if (globalInt != 42)
+    return;
+  clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}}
+
+  invalidateGlobals();
   clang_analyzer_eval(globalInt == 42); // expected-warning{{UNKNOWN}}
 }
+
+
+//---------------------------------
+// False negatives
+//---------------------------------
+
+void testGlobalInvalidationWithDirectBinding() {
+  globalInt = 42;
+  clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}}
+
+  invalidateGlobals();
+  // FIXME: Should be UNKNOWN.
+  clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}}
+}

Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=177572&r1=177571&r2=177572&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Wed Mar 20 15:36:01 2013
@@ -279,12 +279,16 @@ void strcpy_fn_const(char *x) {
   strcpy(x, (const char*)&strcpy_fn); // expected-warning{{Argument to string copy function is the address of the function 'strcpy_fn', which is not a null-terminated string}}
 }
 
+extern int globalInt;
 void strcpy_effects(char *x, char *y) {
   char a = x[0];
+  if (globalInt != 42)
+    return;
 
   clang_analyzer_eval(strcpy(x, y) == x); // expected-warning{{TRUE}}
   clang_analyzer_eval(strlen(x) == strlen(y)); // expected-warning{{TRUE}}
   clang_analyzer_eval(a == x[0]); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}}
 }
 
 void strcpy_overflow(char *y) {





More information about the cfe-commits mailing list