[analyzer] operator new: Fix callback order for CXXNewExpr.

PreStmt<CXXNewExpr> was never called.

Additionally, under c++-allocator-inlining=true, PostStmt<CXXNewExpr> was
called twice when the allocator was inlined: once after evaluating the
new-expression itself, once after evaluating the allocator call which, for the
lack of better options, uses the new-expression as the call site.

This patch fixes both problems.

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


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@322797 91177308-0d34-0410-b5e6-96231b3b80d8

lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp

@@ -15,8 +15,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "ClangSACheckers.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
 using namespace clang;
@@ -29,6 +31,11 @@ class AnalysisOrderChecker
                      check::PostStmt<CastExpr>,
                      check::PreStmt<ArraySubscriptExpr>,
                      check::PostStmt<ArraySubscriptExpr>,
+                     check::PreStmt<CXXNewExpr>,
+                     check::PostStmt<CXXNewExpr>,
+                     check::PreCall,
+                     check::PostCall,
+                     check::NewAllocator,
                      check::Bind,
                      check::RegionChanges> {
   bool isCallbackEnabled(AnalyzerOptions &Opts, StringRef CallbackName) const {
@@ -72,6 +79,40 @@ public:
       llvm::errs() << "PostStmt<ArraySubscriptExpr>\n";
   }
 
+  void checkPreStmt(const CXXNewExpr *NE, CheckerContext &C) const {
+    if (isCallbackEnabled(C, "PreStmtCXXNewExpr"))
+      llvm::errs() << "PreStmt<CXXNewExpr>\n";
+  }
+
+  void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const {
+    if (isCallbackEnabled(C, "PostStmtCXXNewExpr"))
+      llvm::errs() << "PostStmt<CXXNewExpr>\n";
+  }
+
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const {
+    if (isCallbackEnabled(C, "PreCall")) {
+      llvm::errs() << "PreCall";
+      if (const NamedDecl *ND = dyn_cast_or_null<NamedDecl>(Call.getDecl()))
+        llvm::errs() << " (" << ND->getQualifiedNameAsString() << ')';
+      llvm::errs() << '\n';
+    }
+  }
+
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
+    if (isCallbackEnabled(C, "PostCall")) {
+      llvm::errs() << "PostCall";
+      if (const NamedDecl *ND = dyn_cast_or_null<NamedDecl>(Call.getDecl()))
+        llvm::errs() << " (" << ND->getQualifiedNameAsString() << ')';
+      llvm::errs() << '\n';
+    }
+  }
+
+  void checkNewAllocator(const CXXNewExpr *CNE, SVal Target,
+                         CheckerContext &C) const {
+    if (isCallbackEnabled(C, "NewAllocator"))
+      llvm::errs() << "NewAllocator\n";
+  }
+
   void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const {
     if (isCallbackEnabled(C, "Bind"))
       llvm::errs() << "Bind\n";

lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -1324,8 +1324,16 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
 
     case Stmt::CXXNewExprClass: {
       Bldr.takeNodes(Pred);
+
+      ExplodedNodeSet PreVisit;
+      getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+
       ExplodedNodeSet PostVisit;
-      VisitCXXNewExpr(cast<CXXNewExpr>(S), Pred, PostVisit);
+      for (ExplodedNodeSet::iterator i = PreVisit.begin(),
+                                     e = PreVisit.end(); i != e ; ++i) {
+        VisitCXXNewExpr(cast<CXXNewExpr>(S), *i, PostVisit);
+      }
+
       getCheckerManager().runCheckersForPostStmt(Dst, PostVisit, S, *this);
       Bldr.addNodes(Dst);
       break;

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

@@ -372,7 +372,9 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
       getCheckerManager().runCheckersForPostObjCMessage(Dst, DstPostCall, *Msg,
                                                         *this,
                                                         /*WasInlined=*/true);
-    } else if (CE) {
+    } else if (CE &&
+               !(isa<CXXNewExpr>(CE) && // Called when visiting CXXNewExpr.
+                 AMgr.getAnalyzerOptions().mayInlineCXXAllocator())) {
       getCheckerManager().runCheckersForPostStmt(Dst, DstPostCall, CE,
                                                  *this, /*WasInlined=*/true);
     } else {

test/Analysis/cxxnewexpr-callback-inline.cpp

@@ -0,0 +1,32 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config c++-allocator-inlining=true,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true %s 2>&1 | FileCheck %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+namespace std {
+  void *malloc(size_t);
+}
+
+void *operator new(size_t size) { return std::malloc(size); }
+
+struct S {
+  S() {}
+};
+
+void foo();
+
+void test() {
+  S *s = new S();
+  foo();
+}
+
+// CHECK:      PreCall (operator new)
+// CHECK-NEXT: PreCall (std::malloc)
+// CHECK-NEXT: PostCall (std::malloc)
+// CHECK-NEXT: PostCall (operator new)
+// CHECK-NEXT: NewAllocator
+// CHECK-NEXT: PreCall (S::S)
+// CHECK-NEXT: PostCall (S::S)
+// CHECK-NEXT: PreStmt<CXXNewExpr>
+// CHECK-NEXT: PostStmt<CXXNewExpr>
+// CHECK-NEXT: PreCall (foo)
+// CHECK-NEXT: PostCall (foo)

test/Analysis/cxxnewexpr-callback-noinline.cpp

@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config c++-allocator-inlining=false,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true %s 2>&1 | FileCheck %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+namespace std {
+  void *malloc(size_t);
+}
+
+void *operator new(size_t size) { return std::malloc(size); }
+
+struct S {
+  S() {}
+};
+
+void foo();
+
+void test() {
+  S *s = new S();
+  foo();
+}
+
+// CHECK:      PreCall (S::S)
+// CHECK-NEXT: PostCall (S::S)
+// CHECK-NEXT: PreStmt<CXXNewExpr>
+// CHECK-NEXT: PostStmt<CXXNewExpr>
+// CHECK-NEXT: PreCall (foo)
+// CHECK-NEXT: PostCall (foo)
+// CHECK-NEXT: PreCall (std::malloc)
+// CHECK-NEXT: PostCall (std::malloc)