From c99811f06179a35ec930f16d94e92685ba8578cd Mon Sep 17 00:00:00 2001
From: Artem Dergachev <artem.dergachev@gmail.com>
Date: Wed, 4 Oct 2017 15:59:40 +0000
Subject: [PATCH] [analyzer] Fix autodetection of binding types.

In ProgramState::getSVal(Location, Type) API which dereferences a pointer value,
when the optional Type parameter is not supplied and the Location is not typed,
type should have been guessed on a best-effort basis by inspecting the Location
more deeply. However, this never worked; the auto-detected type was instead
a pointer type to the correct type.

Fixed the issue and added various test cases to demonstrate which parts of the
analyzer were affected (uninitialized pointer argument checker, C++ trivial copy
modeling, Google test API modeling checker).

Additionally, autodetected void types are automatically replaced with char,
in order to simplify checker APIs. Which means that if the location is a void
pointer, getSVal() would read the first byte through this pointer
and return its symbolic value.

Fixes pr34305.

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


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@314910 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/StaticAnalyzer/Core/RegionStore.cpp | 19 +++++++++++--------
 test/Analysis/ctor.mm                   | 11 ++++++++++-
 test/Analysis/exercise-ps.c             |  8 ++++++++
 test/Analysis/gtest.cpp                 | 14 ++++++++++++++
 4 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index b034ec4b47..fae7634405 100644
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1393,16 +1393,19 @@ SVal RegionStoreManager::getBinding(RegionBindingsConstRef B, Loc L, QualType T)
     return UnknownVal();
   }
 
-  if (isa<AllocaRegion>(MR) ||
-      isa<SymbolicRegion>(MR) ||
-      isa<CodeTextRegion>(MR)) {
+  if (!isa<TypedValueRegion>(MR)) {
     if (T.isNull()) {
       if (const TypedRegion *TR = dyn_cast<TypedRegion>(MR))
-        T = TR->getLocationType();
-      else {
-        const SymbolicRegion *SR = cast<SymbolicRegion>(MR);
-        T = SR->getSymbol()->getType();
-      }
+        T = TR->getLocationType()->getPointeeType();
+      else if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(MR))
+        T = SR->getSymbol()->getType()->getPointeeType();
+      else if (isa<AllocaRegion>(MR))
+        T = Ctx.VoidTy;
+    }
+    assert(!T.isNull() && "Unable to auto-detect binding type!");
+    if (T->isVoidType()) {
+      // When trying to dereference a void pointer, read the first byte.
+      T = Ctx.CharTy;
     }
     MR = GetElementZeroRegion(cast<SubRegion>(MR), T);
   }
diff --git a/test/Analysis/ctor.mm b/test/Analysis/ctor.mm
index 619e2cb0f0..e903263431 100644
--- a/test/Analysis/ctor.mm
+++ b/test/Analysis/ctor.mm
@@ -199,7 +199,7 @@ namespace PODUninitialized {
     Inner p;
   };
 
-  void testPOD() {
+  void testPOD(const POD &pp) {
     POD p;
     p.x = 1;
     POD p2 = p; // no-warning
@@ -210,6 +210,15 @@ namespace PODUninitialized {
     // Use rvalues as well.
     clang_analyzer_eval(POD(p3).x == 1); // expected-warning{{TRUE}}
 
+    // Copy from symbolic references correctly.
+    POD p4 = pp;
+    // Make sure that p4.x contains a symbol after copy.
+    if (p4.x > 0)
+      clang_analyzer_eval(p4.x > 0); // expected-warning{{TRUE}}
+    // FIXME: Element region gets in the way, so these aren't the same symbols
+    // as they should be.
+    clang_analyzer_eval(pp.x == p4.x); // expected-warning{{UNKNOWN}}
+
     PODWrapper w;
     w.p.y = 1;
     PODWrapper w2 = w; // no-warning
diff --git a/test/Analysis/exercise-ps.c b/test/Analysis/exercise-ps.c
index 577b88bf83..c459260c2a 100644
--- a/test/Analysis/exercise-ps.c
+++ b/test/Analysis/exercise-ps.c
@@ -21,3 +21,11 @@ static void f2(void *buf) {
   memcpy((&x[1]), (buf), 1); // expected-warning{{implicitly declaring library function 'memcpy' with type 'void *(void *, const void *}} \
   // expected-note{{include the header <string.h> or explicitly provide a declaration for 'memcpy'}}
 }
+
+// AllocaRegion is untyped. Void pointer isn't of much help either. Before
+// realizing that the value is undefined, we need to somehow figure out
+// what type of value do we expect.
+void f3(void *dest) {
+  void *src = __builtin_alloca(5);
+  memcpy(dest, src, 1); // expected-warning{{2nd function call argument is a pointer to uninitialized value}}
+}
diff --git a/test/Analysis/gtest.cpp b/test/Analysis/gtest.cpp
index 5797a773b4..98f415eea4 100644
--- a/test/Analysis/gtest.cpp
+++ b/test/Analysis/gtest.cpp
@@ -151,3 +151,17 @@ void testConstrainState(int p) {
   ASSERT_TRUE(false);
   clang_analyzer_warnIfReached(); // no-warning
 }
+
+void testAssertSymbolicPtr(const bool *b) {
+  ASSERT_TRUE(*b); // no-crash
+
+  // FIXME: Our solver doesn't handle this well yet.
+  clang_analyzer_eval(*b); // expected-warning{{UNKNOWN}}
+}
+
+void testAssertSymbolicRef(const bool &b) {
+  ASSERT_TRUE(b); // no-crash
+
+  // FIXME: Our solver doesn't handle this well yet.
+  clang_analyzer_eval(b); // expected-warning{{UNKNOWN}}
+}
-- 
2.49.0