NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.

Default global `operator new()`, like `malloc()`, should return heap pointers, 
which in the analyzer are represented by `SymbolicRegion`s with 
`HeapSpaceRegion` as their parent.

In the `-analyzer-config c++-allocator-inlining` mode, this was broken, and 
regular `SymbolicRegion`s were returned instead,  which have 
`UnknownSpaceRegion` as their parent.

This patch fixes this straightforwardly on `ExprEngine` side. We may want to 
delegate this job to the checkers though `evalCall`, but for now this mode 
doesn't support `evalCall` for `operator new()`, and i'm not sure if it'd be 
used much.

With this patch going on top of previous patches, enabling 
`c++-allocator-inlining` by default causes no regressions on tests (causes some 
improvements though). It doesn't mean it works (we still have callbacks broken, 
path diagnostic pieces unsupported, and i've just noticed one more void element 
region crash), just a psychological checkpoint.


Repository:
  rC Clang

https://reviews.llvm.org/D41266

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  test/Analysis/NewDelete-checker-test.cpp

Index: test/Analysis/NewDelete-checker-test.cpp
===================================================================
--- test/Analysis/NewDelete-checker-test.cpp
+++ 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;
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -561,7 +561,19 @@
   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 (isStandardGlobalOperatorNewFunction(CNE)) {
+      // 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);
 }
 
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -458,8 +458,10 @@
 
   ExplodedNodeSet DstPostCall;
   StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
-  for (auto I: DstPreCall)
+  for (auto I: DstPreCall) {
+    // FIXME: Provide evalCall for checkers?
     defaultEvalCall(CallBldr, I, *Call);
+  }
 
   // Store return value of operator new() for future use, until the actual
   // CXXNewExpr gets processed.
@@ -475,6 +477,22 @@
                                              *Call, *this);
 }
 
+bool ExprEngine::isStandardGlobalOperatorNewFunction(const CXXNewExpr *CNE) {
+  const FunctionDecl *FD = CNE->getOperatorNew();
+  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.
+        return II->getName().equals("nothrow_t");
+      }
+    } else {
+      // Placement forms are considered non-standard.
+      return (FD->getNumParams() == 1);
+    }
+  }
+  return false;
+}
 
 void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
                                    ExplodedNodeSet &Dst) {
@@ -488,18 +506,7 @@
   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 = isStandardGlobalOperatorNewFunction(CNE);
 
   ProgramStateRef State = Pred->getState();
 
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -605,6 +605,8 @@
   bool inlineCall(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr,
                   ExplodedNode *Pred, ProgramStateRef State);
 
+  static bool isStandardGlobalOperatorNewFunction(const CXXNewExpr *CNE);
+
   /// \brief Conservatively evaluate call by invalidating regions and binding
   /// a conjured return value.
   void conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to