r177206 - [analyzer] Use isLiveRegion to determine when SymbolRegionValue is dead.

[Anna Zaks]

Author: zaks
Date: Fri Mar 15 18:34:29 2013
New Revision: 177206

URL: http://llvm.org/viewvc/llvm-project?rev=177206&view=rev
Log:
[analyzer] Use isLiveRegion to determine when SymbolRegionValue is dead.

Fixes a FIXME, improves dead symbol collection, suppresses a false positive,
which resulted from reusing the same symbol twice for simulation of 2 calls to the same function.

Fixing this lead to 2 possible false negatives in CString checker. Since the checker is still alpha and
the solution will not require revert of this commit, move the tests to a FIXME section.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
    cfe/trunk/test/Analysis/malloc.c
    cfe/trunk/test/Analysis/string.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp?rev=177206&r1=177205&r2=177206&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp Fri Mar 15 18:34:29 2013
@@ -449,9 +449,7 @@ bool SymbolReaper::isLive(SymbolRef sym)
   
   switch (sym->getKind()) {
   case SymExpr::RegionValueKind:
-    // FIXME: We should be able to use isLiveRegion here (this behavior
-    // predates isLiveRegion), but doing so causes test failures. Investigate.
-    KnownLive = true;
+    KnownLive = isLiveRegion(cast<SymbolRegionValue>(sym)->getRegion());
     break;
   case SymExpr::ConjuredKind:
     KnownLive = false;

Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=177206&r1=177205&r2=177206&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Fri Mar 15 18:34:29 2013
@@ -1191,3 +1191,14 @@ void testOffsetPassedAsConst() {
   free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}}
 }
 
+char **_vectorSegments;
+int _nVectorSegments;
+
+void poolFreeC(void* s) {
+  free(s); // no-warning
+}
+void freeMemory() {
+  while (_nVectorSegments) {
+    poolFreeC(_vectorSegments[_nVectorSegments++]);
+  }
+}

Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=177206&r1=177205&r2=177206&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Fri Mar 15 18:34:29 2013
@@ -410,12 +410,6 @@ void strcat_symbolic_dst_length(char *ds
   clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}}
 }
 
-void strcat_symbolic_src_length(char *src) {
-	char dst[8] = "1234";
-	strcat(dst, src);
-  clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}}
-}
-
 void strcat_symbolic_dst_length_taint(char *dst) {
   scanf("%s", dst); // Taint data.
   strcat(dst, "1234");
@@ -521,17 +515,6 @@ void strncpy_exactly_matching_buffer(cha
   clang_analyzer_eval(strlen(x) > 4); // expected-warning{{UNKNOWN}}
 }
 
-void strncpy_exactly_matching_buffer2(char *y) {
-	if (strlen(y) >= 4)
-		return;
-
-	char x[4];
-	strncpy(x, y, 4); // no-warning
-
-	// This time, we know that y fits in x anyway.
-  clang_analyzer_eval(strlen(x) <= 3); // expected-warning{{TRUE}}
-}
-
 void strncpy_zero(char *src) {
   char dst[] = "123";
   strncpy(dst, src, 0); // no-warning
@@ -1039,3 +1022,30 @@ void strncasecmp_diff_length_6() {
 void strncasecmp_embedded_null () {
 	clang_analyzer_eval(strncasecmp("ab\0zz", "ab\0yy", 4) == 0); // expected-warning{{TRUE}}
 }
+
+//===----------------------------------------------------------------------===
+// FIXMEs
+//===----------------------------------------------------------------------===
+
+// The analyzer_eval call below should evaluate to true. We are being too 
+// aggressive in marking the (length of) src symbol dead. The length of dst 
+// depends on src. This could be explicitely specified in the checker or the 
+// logic for handling MetadataSymbol in SymbolManager needs to change.
+void strcat_symbolic_src_length(char *src) {
+	char dst[8] = "1234";
+	strcat(dst, src);
+  clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{UNKNOWN}}
+}
+
+// The analyzer_eval call below should evaluate to true. Most likely the same
+// issue as the test above.
+void strncpy_exactly_matching_buffer2(char *y) {
+	if (strlen(y) >= 4)
+		return;
+
+	char x[4];
+	strncpy(x, y, 4); // no-warning
+
+	// This time, we know that y fits in x anyway.
+  clang_analyzer_eval(strlen(x) <= 3); // expected-warning{{UNKNOWN}}
+}