[PATCH] D40939: [analyzer] Avoid element regions of void type.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
Herald added subscribers: cfe-commits, rnkovacs.

Add an assertion that `ElementRegion`'s element type is not void.

Fix two violations of this rule that shown up on four of our existing test files.

I accidentally noticed that problem when i was looking at how `c++-allocator-inlining` mode handles `int *x = new int(5)` - apparently, it binds `5` to such broken region if a void pointer is returned by `operator new()`, which is a regression compared to the default mode. But this patch doesn't fix it (there were no tests for that).


Repository:
  rC Clang

https://reviews.llvm.org/D40939

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp


Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -988,6 +988,12 @@
         elementType = resultTy->getPointeeType();
     }
 
+    // Represent arithmetic on void pointers as arithmetic on char pointers.
+    // It is fine when a TypedValueRegion of char value type represents
+    // a void pointer.
+    if (elementType->isVoidType())
+      elementType = getContext().CharTy;
+
     if (Optional<NonLoc> indexV = index.getAs<NonLoc>()) {
       return loc::MemRegionVal(MemMgr.getElementRegion(elementType, *indexV,
                                                        superR, getContext()));
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2169,7 +2169,14 @@
   for (auto *Node : CheckerPreStmt) {
     const LocationContext *LCtx = Node->getLocationContext();
     ProgramStateRef state = Node->getState();
-    SVal V = state->getLValue(A->getType(),
+    QualType T = A->getType();
+
+    // One of the forbidden LValue types! We still need to have sensible
+    // symbolic lvalues to represent this stuff.
+    if (T->isVoidType())
+      T = getContext().CharTy;
+
+    SVal V = state->getLValue(T,
                               state->getSVal(Idx, LCtx),
                               state->getSVal(Base, LCtx));
     Bldr.generateNode(A, Node, state->BindExpr(A, LCtx, V), nullptr,
Index: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -516,6 +516,10 @@
     assert(classof(this));
   }
 
+  static bool isValidTypeForRegion(QualType T) {
+    return !T.isNull() && !T->isVoidType();
+  }
+
 public:
   virtual QualType getValueType() const = 0;
 
@@ -961,7 +965,10 @@
   CXXThisRegion(const PointerType *thisPointerTy,
                 const StackArgumentsSpaceRegion *sReg)
       : TypedValueRegion(sReg, CXXThisRegionKind),
-        ThisPointerTy(thisPointerTy) {}
+        ThisPointerTy(thisPointerTy) {
+    assert(isValidTypeForRegion(thisPointerTy->getPointeeType()) &&
+           "Invalid region type!");
+  }
 
   static void ProfileRegion(llvm::FoldingSetNodeID &ID,
                             const PointerType *PT,
@@ -1075,6 +1082,7 @@
     assert((!Idx.getAs<nonloc::ConcreteInt>() ||
             Idx.castAs<nonloc::ConcreteInt>().getValue().isSigned()) &&
            "The index must be signed");
+    assert(isValidTypeForRegion(elementType) && "Invalid region type!");
   }
 
   static void ProfileRegion(llvm::FoldingSetNodeID& ID, QualType elementType,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40939.125871.patch
Type: text/x-patch
Size: 2939 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171207/8bcf1605/attachment.bin>