[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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@328247 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index 29e44b377c67280a5968230aa8e02b8ce564bb7e..0f507fde23a91ce9063420d50de724790ecdd573 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -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

diff --git a/test/Analysis/symbol-reaper.c b/test/Analysis/symbol-reaper.c
index 72fcc7e2b9dfad09e9d747fd7d468f4b08ba2084..a47161bea2e3aca0b24bc52402aed10b07c34938 100644 (file)
--- a/test/Analysis/symbol-reaper.c
+++ b/test/Analysis/symbol-reaper.c
@@ -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_complicated_hierarchy_of_regions() {
   } 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(int *x) {
   (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}}