[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: NoQ, vsavchenko, baloghadamsoftware, xazax.hun.
Herald added subscribers: cfe-commits, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
steakhal requested review of this revision.

When the analyzer loads a value through a given type which was previously stored with a different type, it will implicitly cast the associated value to the type in which we are trying to access it.
However, sometimes this //implicit// cast done by the `CastRetrievedVal` in `RegionStoreManager::getBinding` casted the value to a wrong type.
In this example, it cast to the `unsigned char` (which is the type of the stored value of `**b`, stored at `#1`) instead of the static type of the access (the type of `**b` is  `char`at `#2`).

If the analyzer wouldn't overwrite the already given non-null type, it would cast it to the correct one instead.
This patch simply modifies the code to overwrite the cast type to the underlying type of the pointee's region only if the type were `null`.

This also resolves a FIXME in the test suite.

---

This test case crashes the analyzer:

  // RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
  // expected-no-diagnostics
  
  void test(int *a, char ***b, float *c) {
    *(unsigned char **)b = (unsigned char *)a; // #1
    if (**b == 0) // no-crash, #2
      ;
  
    *(unsigned char **)b = (unsigned char *)c;
    if (**b == 0) // no-crash
      ;
  }

Thank you @ASDenysPetrov for rising this issue at D77062 <https://reviews.llvm.org/D77062>.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88477

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/nonloc-as-loc-crash.c
  clang/test/Analysis/svalbuilder-float-cast.c


Index: clang/test/Analysis/svalbuilder-float-cast.c
===================================================================
--- clang/test/Analysis/svalbuilder-float-cast.c
+++ clang/test/Analysis/svalbuilder-float-cast.c
@@ -4,13 +4,9 @@
 
 void SymbolCast_of_float_type_aux(int *p) {
   *p += 0;
-  // FIXME: Ideally, all unknown values should be symbolicated.
-  clang_analyzer_denote(*p, "$x"); // expected-warning{{Not a symbol}}
-
+  clang_analyzer_denote(*p, "$x");
   *p += 1;
-  // This should NOT be (float)$x + 1. Symbol $x was never casted to float.
-  // FIXME: Ideally, this should be $x + 1.
-  clang_analyzer_express(*p); // expected-warning{{Not a symbol}}
+  clang_analyzer_express(*p); // expected-warning{{$x + 1}}
 }
 
 void SymbolCast_of_float_type() {
Index: clang/test/Analysis/nonloc-as-loc-crash.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/nonloc-as-loc-crash.c
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+void test(int *a, char ***b, float *c) {
+  *(unsigned char **)b = (unsigned char *)a;
+  if (**b == 0) // no-crash
+    ;
+
+  *(unsigned char **)b = (unsigned char *)c;
+  if (**b == 0) // no-crash
+    ;
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1439,7 +1439,8 @@
     assert(!T->isVoidType() && "Attempting to dereference a void pointer!");
     MR = GetElementZeroRegion(cast<SubRegion>(MR), T);
   } else {
-    T = cast<TypedValueRegion>(MR)->getValueType();
+    if (T.isNull())
+      T = cast<TypedValueRegion>(MR)->getValueType();
   }
 
   // FIXME: Perhaps this method should just take a 'const MemRegion*' argument


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88477.294928.patch
Type: text/x-patch
Size: 1873 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200929/c32c68f1/attachment.bin>