[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

[Denys Petrov via Phabricator via cfe-commits]

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

During pointer dereferencing CastRetrievedVal uses wrong type from the Store after type punning. Namely, the pointer casts to another type and then assigns with a value of one more another type. It produces NonLoc value when Loc is expected.

Example for visibility:

  void foo(char ***c, int *i) {
    *(unsigned**)c = (unsigned*)i; // type punning
    ***c; // uses 'unsigned**' from the Store instead of 'char***'
  }

Fixes: https://bugs.llvm.org/show_bug.cgi?id=37503


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89055

Files:
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/nonloc-as-loc-crash.c
  clang/test/Analysis/string.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/string.c
===================================================================
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -363,6 +363,16 @@
     strcpy(x, y); // no-warning
 }
 
+// PR37503
+void *func_strcpy_no_assertion();
+char ***ptr_strcpy_no_assertion;
+void strcpy_no_assertion() {
+  *(unsigned char **)ptr_strcpy_no_assertion = (unsigned char *)(func_strcpy_no_assertion());
+  char c;
+  strcpy(**ptr_strcpy_no_assertion, &c); // no-assertion
+}
+
+
 //===----------------------------------------------------------------------===
 // stpcpy()
 //===----------------------------------------------------------------------===
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/Store.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -426,12 +426,17 @@
   // We might need to do that for non-void pointers as well.
   // FIXME: We really need a single good function to perform casts for us
   // correctly every time we need it.
-  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+  if (castTy->isPointerType() && !castTy->isVoidPointerType()) {
     if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(V.getAsRegion())) {
-      QualType sr = SR->getSymbol()->getType();
-      if (!hasSameUnqualifiedPointeeType(sr, castTy))
-          return loc::MemRegionVal(castRegion(SR, castTy));
+        QualType sr = SR->getSymbol()->getType();
+        if (!hasSameUnqualifiedPointeeType(sr, castTy))
+            return loc::MemRegionVal(castRegion(SR, castTy));
     }
+    // Next fixes pointer dereference using type different from its initial one
+    // See PR37503 for details
+    if (const auto *SR = dyn_cast_or_null<ElementRegion>(V.getAsRegion()))
+      return loc::MemRegionVal(castRegion(SR, castTy));
+  }
 
   return svalBuilder.dispatchCast(V, castTy);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89055.296977.patch
Type: text/x-patch
Size: 3208 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201008/a68015fe/attachment-0001.bin>