[analyzer] operator new: Fix memory space for the returned region.

Make sure that with c++-allocator-inlining=true we have the return value of
conservatively evaluated operator new() in the correct memory space (heap).
This is a regression/omission that worked well in c++-allocator-inlining=false.

Heap regions are superior to regular symbolic regions because they have
stricter aliasing constraints: heap regions do not alias each other or global
variables.

Differential Revision: https://reviews.llvm.org/D41266
rdar://problem/12180598

llvm-svn: 322780

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 2c15607..2d3f019 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -479,8 +479,10 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
 
   ExplodedNodeSet DstPostCall;
   StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
-  for (auto I : DstPreCall)
+  for (auto I : DstPreCall) {
+    // FIXME: Provide evalCall for checkers?
     defaultEvalCall(CallBldr, I, *Call);
+  }
   // If the call is inlined, DstPostCall will be empty and we bail out now.
 
   // Store return value of operator new() for future use, until the actual
@@ -507,7 +509,6 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
                                              *Call, *this);
 }
 
-
 void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
                                    ExplodedNodeSet &Dst) {
   // FIXME: Much of this should eventually migrate to CXXAllocatorCall.
@@ -520,18 +521,8 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
   SVal symVal = UnknownVal();
   FunctionDecl *FD = CNE->getOperatorNew();
 
-  bool IsStandardGlobalOpNewFunction = false;
-  if (FD && !isa<CXXMethodDecl>(FD) && !FD->isVariadic()) {
-    if (FD->getNumParams() == 2) {
-      QualType T = FD->getParamDecl(1)->getType();
-      if (const IdentifierInfo *II = T.getBaseTypeIdentifier())
-        // NoThrow placement new behaves as a standard new.
-        IsStandardGlobalOpNewFunction = II->getName().equals("nothrow_t");
-    }
-    else
-      // Placement forms are considered non-standard.
-      IsStandardGlobalOpNewFunction = (FD->getNumParams() == 1);
-  }
+  bool IsStandardGlobalOpNewFunction =
+      FD->isReplaceableGlobalAllocationFunction();
 
   ProgramStateRef State = Pred->getState();
 
@@ -587,9 +578,8 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
   if (CNE->isArray()) {
     // FIXME: allocating an array requires simulating the constructors.
     // For now, just return a symbolicated region.
-    if (!AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
-      const SubRegion *NewReg =
-          symVal.castAs<loc::MemRegionVal>().getRegionAs<SubRegion>();
+    if (const SubRegion *NewReg =
+            dyn_cast_or_null<SubRegion>(symVal.getAsRegion())) {
       QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
       const ElementRegion *EleReg =
           getStoreManager().GetElementZeroRegion(NewReg, ObjTy);

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index bb05acd..02326fe 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -573,7 +573,19 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
   QualType ResultTy = Call.getResultType();
   SValBuilder &SVB = getSValBuilder();
   unsigned Count = currBldrCtx->blockCount();
-  SVal R = SVB.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count);
+
+  // See if we need to conjure a heap pointer instead of
+  // a regular unknown pointer.
+  bool IsHeapPointer = false;
+  if (const auto *CNE = dyn_cast<CXXNewExpr>(E))
+    if (CNE->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+      // FIXME: Delegate this to evalCall in MallocChecker?
+      IsHeapPointer = true;
+    }
+
+  SVal R = IsHeapPointer
+               ? SVB.getConjuredHeapSymbolVal(E, LCtx, Count)
+               : SVB.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count);
   return State->BindExpr(E, LCtx, R);
 }
 

diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp
index 66e8375..2d15614 100644 (file)
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -verify %s
 #include "Inputs/system-header-simulator-cxx.h"
 
 typedef __typeof__(sizeof(int)) size_t;

diff --git a/clang/test/Analysis/new-ctor-null.cpp b/clang/test/Analysis/new-ctor-null.cpp
new file mode 100644 (file)
index 0000000..301c72a
--- /dev/null
+++ b/clang/test/Analysis/new-ctor-null.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+
+void clang_analyzer_eval(bool);
+
+typedef __typeof__(sizeof(int)) size_t;
+
+void *operator new(size_t size) throw() {
+  return nullptr;
+}
+void *operator new[](size_t size) throw() {
+  return nullptr;
+}
+
+struct S {
+  int x;
+  S() : x(1) {}
+  ~S() {}
+};
+
+void testArrays() {
+  S *s = new S[10]; // no-crash
+  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+}