[analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocato…

…rCall

Party based on this thread:
http://lists.llvm.org/pipermail/cfe-dev/2020-February/064754.html.

This patch merges two of CXXAllocatorCall's parameters, so that we are able to
supply a CallEvent object to check::NewAllocatorCall (see the description of
D75430 to see why this would be great).

One of the things mentioned by @noQ was the following:

  I think at this point we might actually do a good job sorting out this
  check::NewAllocator issue because we have a "separate" "Environment" to hold
  the other SVal, which is "objects under construction"! - so we should probably
  simply teach CXXAllocatorCall to extract the value from the
  objects-under-construction trait of the program state and we're good.

I had MallocChecker in my crosshair for now, so I admittedly threw together
something as a proof of concept. Now that I know that this effort is worth
pursuing though, I'll happily look for a solution better then demonstrated in
this patch.

Differential Revision: https://reviews.llvm.org/D75431

clang/include/clang/StaticAnalyzer/Core/Checker.h

@@ -285,9 +285,9 @@ class BranchCondition {
 
 class NewAllocator {
   template <typename CHECKER>
-  static void _checkNewAllocator(void *checker, const CXXNewExpr *NE,
-                                 SVal Target, CheckerContext &C) {
-    ((const CHECKER *)checker)->checkNewAllocator(NE, Target, C);
+  static void _checkNewAllocator(void *checker, const CXXAllocatorCall &Call,
+                                 CheckerContext &C) {
+    ((const CHECKER *)checker)->checkNewAllocator(Call, C);
   }
 
 public:

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h

@@ -37,6 +37,7 @@ class TranslationUnitDecl;
 namespace ento {
 
 class AnalysisManager;
+class CXXAllocatorCall;
 class BugReporter;
 class CallEvent;
 class CheckerBase;
@@ -361,11 +362,9 @@ class CheckerManager {
                                      ExprEngine &Eng);
 
   /// Run checkers between C++ operator new and constructor calls.
-  void runCheckersForNewAllocator(const CXXNewExpr *NE, SVal Target,
-                                  ExplodedNodeSet &Dst,
-                                  ExplodedNode *Pred,
-                                  ExprEngine &Eng,
-                                  bool wasInlined = false);
+  void runCheckersForNewAllocator(const CXXAllocatorCall &Call,
+                                  ExplodedNodeSet &Dst, ExplodedNode *Pred,
+                                  ExprEngine &Eng, bool wasInlined = false);
 
   /// Run checkers for live symbols.
   ///
@@ -506,7 +505,7 @@ class CheckerManager {
       CheckerFn<void (const Stmt *, CheckerContext &)>;
 
   using CheckNewAllocatorFunc =
-      CheckerFn<void (const CXXNewExpr *, SVal, CheckerContext &)>;
+      CheckerFn<void(const CXXAllocatorCall &Call, CheckerContext &)>;
 
   using CheckDeadSymbolsFunc =
       CheckerFn<void (SymbolReaper &, CheckerContext &)>;

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

@@ -39,6 +39,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -1010,6 +1011,12 @@ class CXXAllocatorCall : public AnyFunctionCall {
     return getOriginExpr()->getOperatorNew();
   }
 
+  SVal getObjectUnderConstruction() const {
+    return ExprEngine::getObjectUnderConstruction(getState(), getOriginExpr(),
+                                                  getLocationContext())
+        .getValue();
+  }
+
   /// Number of non-placement arguments to the call. It is equal to 2 for
   /// C++17 aligned operator new() calls that have alignment implicitly
   /// passed as the second argument, and to 1 for other operator new() calls.

clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp

@@ -165,7 +165,7 @@ class AnalysisOrderChecker
       llvm::errs() << "EndAnalysis\n";
   }
 
-  void checkNewAllocator(const CXXNewExpr *CNE, SVal Target,
+  void checkNewAllocator(const CXXAllocatorCall &Call,
                          CheckerContext &C) const {
     if (isCallbackEnabled(C, "NewAllocator"))
       llvm::errs() << "NewAllocator\n";

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -315,8 +315,8 @@ class MallocChecker
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const;
-  void checkNewAllocator(const CXXNewExpr *NE, SVal Target,
-                         CheckerContext &C) const;
+  void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
+  void checkNewAllocator(const CXXAllocatorCall &Call, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
   void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
@@ -1357,11 +1357,12 @@ void MallocChecker::checkPostStmt(const CXXNewExpr *NE,
   }
 }
 
-void MallocChecker::checkNewAllocator(const CXXNewExpr *NE, SVal Target,
+void MallocChecker::checkNewAllocator(const CXXAllocatorCall &Call,
                                       CheckerContext &C) const {
   if (!C.wasInlined) {
-    processNewAllocation(NE, C, Target,
-                         (NE->isArray() ? AF_CXXNewArray : AF_CXXNew));
+    processNewAllocation(
+        Call.getOriginExpr(), C, Call.getObjectUnderConstruction(),
+        (Call.getOriginExpr()->isArray() ? AF_CXXNewArray : AF_CXXNew));
   }
 }
 

clang/lib/StaticAnalyzer/Core/CheckerManager.cpp

@@ -243,7 +243,7 @@ void CheckerManager::runCheckersForObjCMessage(ObjCMessageVisitKind visitKind,
                                                const ObjCMethodCall &msg,
                                                ExprEngine &Eng,
                                                bool WasInlined) {
-  auto &checkers = getObjCMessageCheckers(visitKind);
+  const auto &checkers = getObjCMessageCheckers(visitKind);
   CheckObjCMessageContext C(visitKind, checkers, msg, Eng, WasInlined);
   expandGraphWithCheckers(C, Dst, Src);
 }
@@ -507,35 +507,38 @@ namespace {
     using CheckersTy = std::vector<CheckerManager::CheckNewAllocatorFunc>;
 
     const CheckersTy &Checkers;
-    const CXXNewExpr *NE;
-    SVal Target;
+    const CXXAllocatorCall &Call;
     bool WasInlined;
     ExprEngine &Eng;
 
-    CheckNewAllocatorContext(const CheckersTy &Checkers, const CXXNewExpr *NE,
-                             SVal Target, bool WasInlined, ExprEngine &Eng)
-        : Checkers(Checkers), NE(NE), Target(Target), WasInlined(WasInlined),
-          Eng(Eng) {}
+    CheckNewAllocatorContext(const CheckersTy &Checkers,
+                             const CXXAllocatorCall &Call, bool WasInlined,
+                             ExprEngine &Eng)
+        : Checkers(Checkers), Call(Call), WasInlined(WasInlined), Eng(Eng) {}
 
     CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); }
     CheckersTy::const_iterator checkers_end() { return Checkers.end(); }
 
     void runChecker(CheckerManager::CheckNewAllocatorFunc checkFn,
                     NodeBuilder &Bldr, ExplodedNode *Pred) {
-      ProgramPoint L = PostAllocatorCall(NE, Pred->getLocationContext());
+      ProgramPoint L =
+          PostAllocatorCall(Call.getOriginExpr(), Pred->getLocationContext());
       CheckerContext C(Bldr, Eng, Pred, L, WasInlined);
-      checkFn(NE, Target, C);
+      checkFn(cast<CXXAllocatorCall>(*Call.cloneWithState(Pred->getState())),
+              C);
     }
   };
 
 } // namespace
 
-void CheckerManager::runCheckersForNewAllocator(
-    const CXXNewExpr *NE, SVal Target, ExplodedNodeSet &Dst, ExplodedNode *Pred,
-    ExprEngine &Eng, bool WasInlined) {
+void CheckerManager::runCheckersForNewAllocator(const CXXAllocatorCall &Call,
+                                                ExplodedNodeSet &Dst,
+                                                ExplodedNode *Pred,
+                                                ExprEngine &Eng,
+                                                bool WasInlined) {
   ExplodedNodeSet Src;
   Src.insert(Pred);
-  CheckNewAllocatorContext C(NewAllocatorCheckers, NE, Target, WasInlined, Eng);
+  CheckNewAllocatorContext C(NewAllocatorCheckers, Call, WasInlined, Eng);
   expandGraphWithCheckers(C, Dst, Src);
 }
 
@@ -651,7 +654,7 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst,
                                             const ExplodedNodeSet &Src,
                                             const CallEvent &Call,
                                             ExprEngine &Eng) {
-  for (const auto Pred : Src) {
+  for (auto *const Pred : Src) {
     bool anyEvaluated = false;
 
     ExplodedNodeSet checkDst;

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

@@ -577,9 +577,10 @@ void ExprEngine::handleConstructor(const Expr *E,
   // paths when no-return temporary destructors are used for assertions.
   const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
   if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
-    if (TargetRegion && isa<CXXTempObjectRegion>(TargetRegion) &&
+    if (llvm::isa_and_nonnull<CXXTempObjectRegion>(TargetRegion) &&
         cast<CXXConstructorDecl>(Call->getDecl())
-            ->getParent()->isAnyDestructorNoReturn()) {
+            ->getParent()
+            ->isAnyDestructorNoReturn()) {
 
       // If we've inlined the constructor, then DstEvaluated would be empty.
       // In this case we still want a sink, which could be implemented
@@ -603,7 +604,7 @@ void ExprEngine::handleConstructor(const Expr *E,
   }
 
   ExplodedNodeSet DstPostArgumentCleanup;
-  for (auto I : DstEvaluated)
+  for (ExplodedNode *I : DstEvaluated)
     finishArgumentConstruction(DstPostArgumentCleanup, I, *Call);
 
   // If there were other constructors called for object-type arguments
@@ -712,7 +713,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
 
   ExplodedNodeSet DstPostCall;
   StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
-  for (auto I : DstPreCall) {
+  for (ExplodedNode *I : DstPreCall) {
     // FIXME: Provide evalCall for checkers?
     defaultEvalCall(CallBldr, I, *Call);
   }
@@ -722,7 +723,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
   // CXXNewExpr gets processed.
   ExplodedNodeSet DstPostValue;
   StmtNodeBuilder ValueBldr(DstPostCall, DstPostValue, *currBldrCtx);
-  for (auto I : DstPostCall) {
+  for (ExplodedNode *I : DstPostCall) {
     // FIXME: Because CNE serves as the "call site" for the allocator (due to
     // lack of a better expression in the AST), the conjured return value symbol
     // is going to be of the same type (C++ object pointer type). Technically
@@ -756,10 +757,8 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
   ExplodedNodeSet DstPostPostCallCallback;
   getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback,
                                              DstPostValue, *Call, *this);
-  for (auto I : DstPostPostCallCallback) {
-    getCheckerManager().runCheckersForNewAllocator(
-        CNE, *getObjectUnderConstruction(I->getState(), CNE, LCtx), Dst, I,
-        *this);
+  for (ExplodedNode *I : DstPostPostCallCallback) {
+    getCheckerManager().runCheckersForNewAllocator(*Call, Dst, I, *this);
   }
 }
 

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

@@ -21,6 +21,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/SaveAndRestore.h"
 
@@ -325,17 +326,14 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
     CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState);
 
     ExplodedNodeSet DstPostCall;
-    if (const CXXNewExpr *CNE = dyn_cast_or_null<CXXNewExpr>(CE)) {
+    if (llvm::isa_and_nonnull<CXXNewExpr>(CE)) {
       ExplodedNodeSet DstPostPostCallCallback;
       getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback,
                                                  CEENode, *UpdatedCall, *this,
                                                  /*wasInlined=*/true);
-      for (auto I : DstPostPostCallCallback) {
+      for (ExplodedNode *I : DstPostPostCallCallback) {
         getCheckerManager().runCheckersForNewAllocator(
-            CNE,
-            *getObjectUnderConstruction(I->getState(), CNE,
-                                        calleeCtx->getParent()),
-            DstPostCall, I, *this,
+            cast<CXXAllocatorCall>(*UpdatedCall), DstPostCall, I, *this,
             /*wasInlined=*/true);
       }
     } else {
@@ -591,7 +589,7 @@ void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
   // If there were other constructors called for object-type arguments
   // of this call, clean them up.
   ExplodedNodeSet dstArgumentCleanup;
-  for (auto I : dstCallEvaluated)
+  for (ExplodedNode *I : dstCallEvaluated)
     finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
   ExplodedNodeSet dstPostCall;
@@ -605,7 +603,7 @@ void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
 
   // Run pointerEscape callback with the newly conjured symbols.
   SmallVector<std::pair<SVal, SVal>, 8> Escaped;
-  for (auto I : dstPostCall) {
+  for (ExplodedNode *I : dstPostCall) {
     NodeBuilder B(I, Dst, *currBldrCtx);
     ProgramStateRef State = I->getState();
     Escaped.clear();
@@ -743,7 +741,7 @@ ExprEngine::mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred,
     const ConstructionContext *CC = CCE ? CCE->getConstructionContext()
                                         : nullptr;
 
-    if (CC && isa<NewAllocatedObjectConstructionContext>(CC) &&
+    if (llvm::isa_and_nonnull<NewAllocatedObjectConstructionContext>(CC) &&
         !Opts.MayInlineCXXAllocator)
       return CIP_DisallowedOnce;