r328247 - [analyzer] Make symbol_iterator iterate over SVal's symbolic base.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Thu Mar 22 14:30:58 2018
New Revision: 328247

URL: http://llvm.org/viewvc/llvm-project?rev=328247&view=rev
Log:
[analyzer] Make symbol_iterator iterate over SVal's symbolic base.

If a memory region (or an SVal that represents a pointer to that memory region)
is a (direct or indirect, not necessarily proper) sub-region of a SymbolicRegion
then it is said to have a symbolic base.

For now SVal::symbol_iterator explores the symbol within a symbolic region
only when the SVal represents a pointer to the symbolic region itself,
not to any of its sub-regions.

This behavior is not indended by any user of symbol_iterator; all users who
cared about such behavior were expecting the iterator to descend into the
symbolic base of an arbitrary region, find the parent symbol of the symbolic
base region, and iterate over that symbol. Lack of such behavior resulted in
bugs demonstarted by the test cases.

Hence the decision to change the API to behave more intuitively.

Differential Revision: https://reviews.llvm.org/D44347

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
    cfe/trunk/test/Analysis/symbol-reaper.c

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h?rev=328247&r1=328246&r2=328247&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h Thu Mar 22 14:30:58 2018
@@ -195,7 +195,7 @@ public:
   void dump() const;
 
   SymExpr::symbol_iterator symbol_begin() const {
-    const SymExpr *SE = getAsSymbolicExpression();
+    const SymExpr *SE = getAsSymbol(/*IncludeBaseRegions=*/true);
     if (SE)
       return SE->symbol_begin();
     else

Modified: cfe/trunk/test/Analysis/symbol-reaper.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/symbol-reaper.c?rev=328247&r1=328246&r2=328247&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/symbol-reaper.c (original)
+++ cfe/trunk/test/Analysis/symbol-reaper.c Thu Mar 22 14:30:58 2018
@@ -3,6 +3,9 @@
 void clang_analyzer_eval(int);
 void clang_analyzer_warnOnDeadSymbol(int);
 void clang_analyzer_numTimesReached();
+void clang_analyzer_warnIfReached();
+
+void exit(int);
 
 int conjure_index();
 
@@ -58,6 +61,12 @@ struct S1 {
 struct S2 {
   struct S1 array[5];
 } s2;
+struct S3 {
+  void *field;
+};
+
+struct S1 *conjure_S1();
+struct S3 *conjure_S3();
 
 void test_element_index_lifetime_with_complicated_hierarchy_of_regions() {
   do {
@@ -68,6 +77,18 @@ void test_element_index_lifetime_with_co
   } while (0); // no-warning
 }
 
+void test_loc_as_integer_element_index_lifetime() {
+  do {
+    int x;
+    struct S3 *s = conjure_S3();
+    clang_analyzer_warnOnDeadSymbol((int)s);
+    x = (int)&(s->field);
+    ptr = &arr[x];
+    if (s) {}
+  // FIXME: Should not warn. The symbol is still alive within the ptr's index.
+  } while (0); // expected-warning{{SYMBOL DEAD}}
+}
+
 // Test below checks lifetime of SymbolRegionValue in certain conditions.
 
 int **ptrptr;
@@ -78,3 +99,38 @@ void test_region_lifetime_as_store_value
   (void)0; // No-op; make sure the environment forgets things and the GC runs.
   clang_analyzer_eval(**ptrptr); // expected-warning{{TRUE}}
 } // no-warning
+
+int *produce_region_referenced_only_through_field_in_environment_value() {
+  struct S1 *s = conjure_S1();
+  clang_analyzer_warnOnDeadSymbol((int) s);
+  int *x = &s->field;
+  return x;
+}
+
+void test_region_referenced_only_through_field_in_environment_value() {
+  produce_region_referenced_only_through_field_in_environment_value();
+} // expected-warning{{SYMBOL DEAD}}
+
+void test_region_referenced_only_through_field_in_store_value() {
+  struct S1 *s = conjure_S1();
+  clang_analyzer_warnOnDeadSymbol((int) s);
+  ptr = &s->field; // Write the symbol into a global. It should live forever.
+  if (!s) {
+    exit(0); // no-warning (symbol should not die here)
+    // exit() is noreturn.
+    clang_analyzer_warnIfReached(); // no-warning
+  }
+  if (!ptr) { // no-warning (symbol should not die here)
+    // We exit()ed under these constraints earlier.
+    clang_analyzer_warnIfReached(); // no-warning
+  }
+  // The exit() call invalidates globals. The symbol will die here because
+  // the exit() statement itself is already over and there's no better statement
+  // to put the diagnostic on.
+} // expected-warning{{SYMBOL DEAD}}
+
+void test_zombie_referenced_only_through_field_in_store_value() {
+  struct S1 *s = conjure_S1();
+  clang_analyzer_warnOnDeadSymbol((int) s);
+  int *x = &s->field;
+} // expected-warning{{SYMBOL DEAD}}