[analyzer] Let TK_PreserveContents span across the whole base region.

If an address of a field is passed through a const pointer,
the whole structure's base region should receive the
TK_PreserveContents trait and avoid invalidation.

Additionally, include a few FIXME tests shown up during testing.

Differential Revision: http://reviews.llvm.org/D19057

llvm-svn: 267413

diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 5130dd6..ff4922d 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -920,7 +920,7 @@ ProgramStateRef CStringChecker::InvalidateBuffer(CheckerContext &C,
     // Invalidate and escape only indirect regions accessible through the source
     // buffer.
     if (IsSourceBuffer) {
-      ITraits.setTrait(R,
+      ITraits.setTrait(R->getBaseRegion(),
                        RegionAndSymbolInvalidationTraits::TK_PreserveContents);
       ITraits.setTrait(R, RegionAndSymbolInvalidationTraits::TK_SuppressEscape);
       CausesPointerEscape = true;

diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index 6267758..5261318 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -177,7 +177,7 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount,
     // below for efficiency.
     if (PreserveArgs.count(Idx))
       if (const MemRegion *MR = getArgSVal(Idx).getAsRegion())
-        ETraits.setTrait(MR->StripCasts(),
+        ETraits.setTrait(MR->getBaseRegion(),
                         RegionAndSymbolInvalidationTraits::TK_PreserveContents);
         // TODO: Factor this out + handle the lower level const pointers.
 

diff --git a/clang/test/Analysis/call-invalidation.cpp b/clang/test/Analysis/call-invalidation.cpp
index 7297d1e..80323ff 100644 (file)
--- a/clang/test/Analysis/call-invalidation.cpp
+++ b/clang/test/Analysis/call-invalidation.cpp
@@ -118,3 +118,50 @@ void testPureConst() {
 }
 
 
+struct PlainStruct {
+  int x, y;
+  mutable int z;
+};
+
+PlainStruct glob;
+
+void useAnything(void *);
+void useAnythingConst(const void *);
+
+void testInvalidationThroughBaseRegionPointer() {
+  PlainStruct s1;
+  s1.x = 1;
+  s1.z = 1;
+  clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(s1.z == 1); // expected-warning{{TRUE}}
+  // Not only passing a structure pointer through const pointer parameter,
+  // but also passing a field pointer through const pointer parameter
+  // should preserve the contents of the structure.
+  useAnythingConst(&(s1.y));
+  clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
+  // FIXME: Should say "UNKNOWN", because it is not uncommon to
+  // modify a mutable member variable through const pointer.
+  clang_analyzer_eval(s1.z == 1); // expected-warning{{TRUE}}
+  useAnything(&(s1.y));
+  clang_analyzer_eval(s1.x == 1); // expected-warning{{UNKNOWN}}
+}
+
+
+void useFirstConstSecondNonConst(const void *x, void *y);
+void useFirstNonConstSecondConst(void *x, const void *y);
+
+void testMixedConstNonConstCalls() {
+  PlainStruct s2;
+  s2.x = 1;
+  useFirstConstSecondNonConst(&(s2.x), &(s2.y));
+  clang_analyzer_eval(s2.x == 1); // expected-warning{{UNKNOWN}}
+  s2.x = 1;
+  useFirstNonConstSecondConst(&(s2.x), &(s2.y));
+  clang_analyzer_eval(s2.x == 1); // expected-warning{{UNKNOWN}}
+  s2.y = 1;
+  useFirstConstSecondNonConst(&(s2.x), &(s2.y));
+  clang_analyzer_eval(s2.y == 1); // expected-warning{{UNKNOWN}}
+  s2.y = 1;
+  useFirstNonConstSecondConst(&(s2.x), &(s2.y));
+  clang_analyzer_eval(s2.y == 1); // expected-warning{{UNKNOWN}}
+}

diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 30d7269..51e2cd6 100644 (file)
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1750,6 +1750,19 @@ void testEscapeThroughSystemCallTakingVoidPointer3(fake_rb_tree_t *rbt) {
   fake_rb_tree_insert_node(rbt, data); // no warning
 }
 
+struct IntAndPtr {
+  int x;
+  int *p;
+};
+
+void constEscape(const void *ptr);
+
+void testConstEscapeThroughAnotherField() {
+  struct IntAndPtr s;
+  s.p = malloc(sizeof(int));
+  constEscape(&(s.x)); // could free s->p!
+} // no-warning
+
 // ----------------------------------------------------------------------------
 // False negatives.
 
@@ -1769,3 +1782,9 @@ void testPassToSystemHeaderFunctionIndirectly() {
   // FIXME: This is a leak: if we think a system function won't free p, it
   // won't free (p-1) either.
 }
+
+void testMallocIntoMalloc() {
+  StructWithPtr *s = malloc(sizeof(StructWithPtr));
+  s->memP = malloc(sizeof(int));
+  free(s);
+} // FIXME: should warn here