[PATCH] D38358: [analyzer] Fix autodetection of getSVal()'s type argument.

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 28 07:21:09 PDT 2017


NoQ created this revision.

In `ProgramState::getSVal(Loc, QualType)`, the `QualType` parameter, which represents the type of the expected value, is optional. If it is not supplied, it is auto-detected by looking at the memory region in `Loc`. For typed-value regions such autodetection is trivial. For other kinds of regions (most importantly, for symbolic regions) it apparently never worked correctly: it detected the location type (pointer type), not the value type in this location (pointee type).

Our tests contain numerous cases when such autodetection was working incorrectly, but for existing tests it never mattered. There are three notable places where the type was regularly auto-detected incorrectly:

1. `ExprEngine::performTrivialCopy()`. Trivial copy from symbolic references never worked - test case attached.
2. `CallAndMessageChecker::uninitRefOrPointer()`. Here the code only cares if the value is `Undefined` or not, so autodetected type didn't matter.
3. `GTestChecker::modelAssertionResultBoolConstructor()`. This is how the issue was found in https://bugs.llvm.org/show_bug.cgi?id=34305 - test case attached.

I added a few more sanity checks - we'd now also fail with an assertion if we are unable to autodetect the pointer type, warning the author of the checker to pass the type explicitly.

It is sometimes indeed handy to put a void-pointer-typed `Loc` into `getSVal(Loc)`, as demonstrated in the `exercise-ps.c`'s `f2()` test through `CallAndMessageChecker` (case '2.' above). I handled this on the API side in order to simplify checker API, implicitly turning `void` into `char`, though i don't mind modifying `CallAndMessageChecker` to pass `CharTy` explicitly instead.


https://reviews.llvm.org/D38358

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/ctor.mm
  test/Analysis/gtest.cpp


Index: test/Analysis/gtest.cpp
===================================================================
--- test/Analysis/gtest.cpp
+++ test/Analysis/gtest.cpp
@@ -151,3 +151,9 @@
   ASSERT_TRUE(false);
   clang_analyzer_warnIfReached(); // no-warning
 }
+
+void testAssertSymbolic(const bool &b) {
+  ASSERT_TRUE(b);
+
+  clang_analyzer_eval(b); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -199,7 +199,7 @@
     Inner p;
   };
 
-  void testPOD() {
+  void testPOD(const POD &pp) {
     POD p;
     p.x = 1;
     POD p2 = p; // no-warning
@@ -210,6 +210,15 @@
     // 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
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1393,17 +1393,20 @@
     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();
+      if (const TypedRegion *TR = dyn_cast<TypedRegion>(MR)) {
+        T = TR->getLocationType()->getPointeeType();
+      } else if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(MR)) {
+        T = SR->getSymbol()->getType()->getPointeeType();
+        if (T->isVoidType()) {
+          // When trying to dereference a void pointer, read the first byte.
+          T = Ctx.CharTy;
+        }
       }
     }
+    assert(!T.isNull() && "Unable to auto-detect binding type!");
+    assert(!T->isVoidType() && "Attempted to retrieve a void value!");
     MR = GetElementZeroRegion(cast<SubRegion>(MR), T);
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38358.116980.patch
Type: text/x-patch
Size: 2517 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170928/bb992046/attachment.bin>


More information about the cfe-commits mailing list