From 6ca703b27f038bb5acb89e5c47be055469ec6a61 Mon Sep 17 00:00:00 2001
From: Artem Dergachev <artem.dergachev@gmail.com>
Date: Mon, 25 Apr 2016 14:44:25 +0000
Subject: [PATCH] [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


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@267413 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../Checkers/CStringChecker.cpp               |  2 +-
 lib/StaticAnalyzer/Core/CallEvent.cpp         |  2 +-
 test/Analysis/call-invalidation.cpp           | 47 +++++++++++++++++++
 test/Analysis/malloc.c                        | 19 ++++++++
 4 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 5130dd610a..ff4922dac9 100644
--- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/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/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index 626775846b..5261318667 100644
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/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/test/Analysis/call-invalidation.cpp b/test/Analysis/call-invalidation.cpp
index 7297d1ebec..80323ffcf1 100644
--- a/test/Analysis/call-invalidation.cpp
+++ b/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/test/Analysis/malloc.c b/test/Analysis/malloc.c
index 30d7269195..51e2cd6043 100644
--- a/test/Analysis/malloc.c
+++ b/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
-- 
2.50.1