[analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

[Szelethus]

Summary

This is my first ever patch touching ExprEngine, sorry if the code causes lasting damage to the reader.

One of the pain points in simplifying MallocCheckers interface by gradually changing to CallEvent is that a variety of C++ allocation and deallocation functionalities are modeled through preStmt<...> where CallEvent is unavailable, and a single one of these callbacks can prevent a mass parameter change.

This patch introduces a new CallEvent, CXXDeallocatorCall, which happens after preStmt<CXXDeleteExpr>, and can completely replace that callback as demonstrated. Note how you can retrieve this call through preCall, yet there is no postCall to pair it with -- the reason behind this is that neither does preStmt<CXXDeleteExpr> have a postStmt<CXXDeleteExpr> pair. But I have no clue why that is :^)

[Charusso]

Cool! May it worth to mention the corresponding mail from the mailing list in the Summary: http://lists.llvm.org/pipermail/cfe-dev/2020-February/064754.html

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

1057

St -> State

1074

return getOriginExpr()->getNumArgs()

[xazax.hun]

This might be a silly question but is CXXDeleteExpr the only way to invoke a deallocator? What would happen if the user would call it by hand? Should we expect ExprEngine to trigger a callback in that case?

[NoQ]

I love where this is going.

the reason behind this is that neither does preStmt<CXXDeleteExpr> have a postStmt<CXXDeleteExpr> pair. But I have no clue why that is :^)

I'm pretty sure it's forgotten :/

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

70

After this you probably received compiler warnings saying "case isn't covered in switch". You'll need to clean them up.

Another thing to do would be to update CallEventManager's getCall() and getCaller() methods that'd allow the users to construct the new CallEvent easily without thinking about what specific kind of call event it is.

1074

This wouldn't compile. CXXDeleteExpr is not a CallExpr.

It sounds like there are currently [[ http://www.cplusplus.com/reference/new/operator%20delete/ | five different operator deletes ]]:

And, unlike operator new, there's no option to provide custom "placement" arguments.

So i think the logic in this patch is correct but we should do some custom logic for all 5 cases in the getArgExpr method, where argument expressions for the extra arguments will have to be conjured out of thin air (or we might as well return null).

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

882

Yes, we should absolutely have PostCall here as well, even if nothing happens in between.

And once it's there, the next major step would be to invoke ExprEngine::evalCall() instead if the operator was overloaded, so that we could inline the deallocator (or evaluate it conservatively, which might also be of value as it would invalidate the necessary globals).

[steakhal]

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

1074

It sounds like there are currently five different operator deletes:

I think it is even worse since C++17 and C++20 introduced a couple of others like:

• overloads with std::align_val_t parameter (C++17)
• overloads with std::destroying_delete_t parameter (C++20) which I haven't heard of yet :D

You can check it in the draft: http://eel.is/c++draft/new.delete
And of course at cppreference as well: https://en.cppreference.com/w/cpp/memory/new/operator_delete

[Szelethus]

• Add PostStmt<CXXDeleteExpr>
• Add PostCall for CXXDeallocatorCall
• Add a unittest, which is kind of pointless as-is, but I realized only later that most of the delete operators aren't recognized as CXXDeleteExpr
• Add a bunch of TODOs about incorrect CallEvent types.

[Szelethus]

Since this change had a few blocking issues, I'm placing it back to review for greater visibility.

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

70

No, I did not, infuriatingly. I did however get errors after trying to make a toString function for CallEventKind, apparently both CE_CXXDeallocator and CE_Block has the value of 7. When I moved the enum, everything was fine, and I did get the warnings you mentioned.

1074

Okay so here is the biggest issue: non-simple deletes don't appear in the AST as CXXDeleteExpr, but rather as a CXXOperatorCall. This is a big problem, what could be the reason for it?

[Szelethus]

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

59

I need to move this below CE_Function, as its not in the range of CE_BEG_FUNCTION_CALLS and CE_END_FUNCTION_CALLS.

[Szelethus]

Just a gentle ping :)

[Szelethus]

Ping^2 :)

[Szelethus]

Ping

[xazax.hun]

Overall looks good to me some questions and nits inline.

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

59

This is marked as done. If the conclusion is that you do not need to move it I think it would be worth to add a comment why :)

1052

How important and hard it is to fix this? If you did not find the reason why those CXXDeleteExprs are missing you can do two things:

1. Look at how such AST is consumed by CodeGen
2. Ask around on the mailing list with a minimal example.

1083

Is the index unused on purpose? If yes, wouldn't this trigger a warning on build bots? Adding a maybe unused annotation and/or a comment would be welcome.

clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp

95

I think this change might be a better fit for a separate commit. I think you don't even need to have such a small change reviewed. You could just commit it as is. Just do not forget to describe that these cases are really hard to have a test for but this is the idiomatic way of doing this in checkers.

[Szelethus]

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

59

Marked done automatically, I'll address this before commiting :)

1052

Fair point, but it surely is a thing to keep in mind because it could, or more probably will cause surprises. A TODO or a NOTE would be more appropriate. In any case, I'd prefer to address these in a followup patch.

1083

Unused argument warnings are disabled by the build system, but a comment wouldn't hurt. CXXDeleteExprs can only have a single argument, but as earlier inlines suggests, we will need to mind custom deletes later on.

[NoQ]

Sorry i'm not very responsive these days >.<

clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp

95

I'm also curious why did this suddenly become necessary. This is obviously the right thing to do but why the change? It might indicate that we accidentally started incorrectly agglutinating nodes that were previously considered different. Like, we might have messed up our program points somewhere in this patch.

clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp

31–33

That's fairly unobvious to me. Isn't the whole point of CXXAllocatorCall/CXXDeallocatorCall to give access to specific aspects of new/delete-expressions that aren't available in manual invocations with function syntax? On the other hand, i agree that it's nice to be able to cast the call event to CXXAllocatorCall/CXXDeallocatorCall and be done with allocators/deallocators.

[Szelethus]

In D75430#2028456, @NoQ wrote:

Sorry i'm not very responsive these days >.<

No worries, thanks! ^-^

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

1052

Actually, see my other inline. I do believe that this should be the one class to solve all deletes, or at the very least, standard-specified deletes.

clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp

95

The creduced program that causes this checker to crash (without the early return):

struct a {};
struct b : a {};
void c() {
  a *d = new b;
  delete d;
}

Turns out, as I said in an another inline, I accidentally run PreStmt on delete expressions twice. In any case, I decided to add this early return and a test case in this patch, because I think strongly related, and as seen here, tests the added functionality.

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

1653

Oh wow. I accidentally ran checkers on PreStmt here as well. Thats why I needed the new early return to not crash.

clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp

31–33

Yup, in my mind, this isn't CXXNewCall, but rather CXXAllocatorCall (same for delete), I would expect this to handle it all (maybe change the interfaces so that getOriginExpr returns with Expr, and add a getOriginAsCXXNewExpr method).

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

 class LocationContext;
 class ProgramPoint;
 class ProgramPointTag;
 class StackFrameContext;
 
 namespace ento {
 
 enum CallEventKind {
   CE_Function,
   CE_CXXMember,
   CE_CXXMemberOperator,
   CE_CXXDestructor,
   CE_BEG_CXX_INSTANCE_CALLS = CE_CXXMember,
   CE_END_CXX_INSTANCE_CALLS = CE_CXXDestructor,
   CE_CXXConstructor,
   CE_CXXInheritedConstructor,
   CE_BEG_CXX_CONSTRUCTOR_CALLS = CE_CXXConstructor,
   CE_END_CXX_CONSTRUCTOR_CALLS = CE_CXXInheritedConstructor,
   CE_CXXAllocator,
+  CE_CXXDeallocator,
   CE_BEG_FUNCTION_CALLS = CE_Function,
-  CE_END_FUNCTION_CALLS = CE_CXXAllocator,
+  CE_END_FUNCTION_CALLS = CE_CXXDeallocator,
   CE_Block,
   CE_ObjCMessage
 };
 
 class CallEvent;
 class CallDescription;
 
 template<typename T = CallEvent>
virtual StringRef getKindAsString() const override {
     return "CXXAllocatorCall";
   }
 
   static bool classof(const CallEvent *CE) {
     return CE->getKind() == CE_CXXAllocator;
   }
 };
 
+/// Represents the memory deallocation call in a C++ delete-expression.
+///
+/// This is a call to "operator delete".
+// FIXME: CXXDeleteExpr isn't present for custom delete operators, or even for
+// some those that are in the standard library, like the no-throw or align_val
+// versions.
+// Some pointers:
+// http://lists.llvm.org/pipermail/cfe-dev/2020-April/065080.html
+// clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp
+// clang/unittests/StaticAnalyzer/CallEventTest.cpp
+class CXXDeallocatorCall : public AnyFunctionCall {
+  friend class CallEventManager;
+
+protected:
+  CXXDeallocatorCall(const CXXDeleteExpr *E, ProgramStateRef St,
+                     const LocationContext *LCtx)
+      : AnyFunctionCall(E, St, LCtx) {}
+  CXXDeallocatorCall(const CXXDeallocatorCall &Other) = default;
+
+  void cloneTo(void *Dest) const override {
+    new (Dest) CXXDeallocatorCall(*this);
+  }
+
+public:
+  virtual const CXXDeleteExpr *getOriginExpr() const {
+    return cast<CXXDeleteExpr>(AnyFunctionCall::getOriginExpr());
+  }
+
+  const FunctionDecl *getDecl() const override {
+    return getOriginExpr()->getOperatorDelete();
+  }
+
+  unsigned getNumArgs() const override { return getDecl()->getNumParams(); }
+
+  const Expr *getArgExpr(unsigned Index) const override {
+    // CXXDeleteExpr's only have a single argument.
+    return getOriginExpr()->getArgument();
+  }
+
+  Kind getKind() const override { return CE_CXXDeallocator; }
+  virtual StringRef getKindAsString() const override {
+    return "CXXDeallocatorCall";
+  }
+
+  static bool classof(const CallEvent *CE) {
+    return CE->getKind() == CE_CXXDeallocator;
+  }
+};
+
 /// Represents the ways an Objective-C message send can occur.
 //
 // Note to maintainers: OCM_Message should always be last, since it does not
 // need to fit in the Data field's low bits.
 enum ObjCMessageKind {
   OCM_PropertyAccess,
   OCM_Subscript,
   OCM_Message
getCXXDestructorCall(const CXXDestructorDecl *DD, const Stmt *Trigger,
     return create<CXXDestructorCall>(DD, Trigger, Target, IsBase, State, LCtx);
   }
 
   CallEventRef<CXXAllocatorCall>
   getCXXAllocatorCall(const CXXNewExpr *E, ProgramStateRef State,
                       const LocationContext *LCtx) {
     return create<CXXAllocatorCall>(E, State, LCtx);
   }
+
+  CallEventRef<CXXDeallocatorCall>
+  getCXXDeallocatorCall(const CXXDeleteExpr *E, ProgramStateRef State,
+                        const LocationContext *LCtx) {
+    return create<CXXDeallocatorCall>(E, State, LCtx);
+  }
 };
 
 template <typename T>
 CallEventRef<T> CallEvent::cloneWithState(ProgramStateRef NewState) const {
   assert(isa<T>(*this) && "Cloning to unrelated type");
   static_assert(sizeof(T) == sizeof(CallEvent),
                 "Subclasses may not add fields");
 

clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp

void DeleteWithNonVirtualDtorChecker::checkPreStmt(const CXXDeleteExpr *DE,
 
   if (!BT)
     BT.reset(new BugType(this,
                          "Destruction of a polymorphic object with no "
                          "virtual destructor",
                          "Logic error"));
 
   ExplodedNode *N = C.generateNonFatalErrorNode();
+  if (!N)
+    return;
   auto R = std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N);
 
   // Mark region of problematic base class for later use in the BugVisitor.
   R->markInteresting(BaseClassRegion);
   R->addVisitor(std::make_unique<DeleteBugVisitor>());
   C.emitReport(std::move(R));
 }
 

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

 
 namespace {
 
 class MallocChecker
     : public Checker<check::DeadSymbols, check::PointerEscape,
                      check::ConstPointerEscape, check::PreStmt<ReturnStmt>,
                      check::EndFunction, check::PreCall, check::PostCall,
                      check::PostStmt<CXXNewExpr>, check::NewAllocator,
-                     check::PreStmt<CXXDeleteExpr>, check::PostStmt<BlockExpr>,
-                     check::PostObjCMessage, check::Location, eval::Assume> {
+                     check::PostStmt<BlockExpr>, check::PostObjCMessage,
+                     check::Location, eval::Assume> {
 public:
   /// In pessimistic mode, the checker assumes that it does not know which
   /// functions might free the memory.
   /// In optimistic mode, the checker assumes that all user-defined functions
   /// which might free a pointer are annotated.
   DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
 
   /// Many checkers are essentially built into this one, so enabling them will
public:
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
   CheckerNameRef CheckNames[CK_NumCheckKinds];
 
   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 checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
   void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
   void checkEndFunction(const ReturnStmt *S, CheckerContext &C) const;
   ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
                             bool Assumption) const;
   void checkLocation(SVal l, bool isLoad, const Stmt *S,
SVal SizeInBytes = svalBuilder.evalBinOpNN(
         svalBuilder.getArrayIndexType());
     DefinedOrUnknownSVal DynSizeMatchesSize = svalBuilder.evalEQ(
         State, DynSize, SizeInBytes.castAs<DefinedOrUnknownSVal>());
     State = State->assume(DynSizeMatchesSize, true);
   }
   return State;
 }
 
-void MallocChecker::checkPreStmt(const CXXDeleteExpr *DE,
-                                 CheckerContext &C) const {
-
-  if (!ChecksEnabled[CK_NewDeleteChecker])
-    if (SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol())
-      checkUseAfterFree(Sym, C, DE->getArgument());
-
-  if (!isStandardNewDelete(DE->getOperatorDelete()))
-    return;
-
-  ProgramStateRef State = C.getState();
-  bool IsKnownToBeAllocated;
-  State = FreeMemAux(C, DE->getArgument(), DE, State,
-                     /*Hold*/ false, IsKnownToBeAllocated,
-                     (DE->isArrayForm() ? AF_CXXNewArray : AF_CXXNew));
-
-  C.addTransition(State);
-}
-
 static bool isKnownDeallocObjCMethodName(const ObjCMethodCall &Call) {
   // If the first selector piece is one of the names below, assume that the
   // object takes ownership of the memory, promising to eventually deallocate it
   // with free().
   // Ex:  [NSData dataWithBytesNoCopy:bytes length:10];
   // (...unless a 'freeWhenDone' parameter is false, but that's checked later.)
   StringRef FirstSlot = Call.getSelector().getNameForSlot(0);
   return FirstSlot == "dataWithBytesNoCopy" ||
void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
   }
 
   C.addTransition(state->set<RegionState>(RS), N);
 }
 
 void MallocChecker::checkPreCall(const CallEvent &Call,
                                  CheckerContext &C) const {
 
-  if (const CXXDestructorCall *DC = dyn_cast<CXXDestructorCall>(&Call)) {
+  if (const auto *DC = dyn_cast<CXXDeallocatorCall>(&Call)) {
+    const CXXDeleteExpr *DE = DC->getOriginExpr();
+
+    if (!ChecksEnabled[CK_NewDeleteChecker])
+      if (SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol())
+        checkUseAfterFree(Sym, C, DE->getArgument());
+
+    if (!isStandardNewDelete(DC->getDecl()))
+      return;
+
+    ProgramStateRef State = C.getState();
+    bool IsKnownToBeAllocated;
+    State = FreeMemAux(C, DE->getArgument(), DE, State,
+                       /*Hold*/ false, IsKnownToBeAllocated,
+                       (DE->isArrayForm() ? AF_CXXNewArray : AF_CXXNew));
+
+    C.addTransition(State);
+    return;
+  }
+
+  if (const auto *DC = dyn_cast<CXXDestructorCall>(&Call)) {
     SymbolRef Sym = DC->getCXXThisVal().getAsSymbol();
     if (!Sym || checkDoubleDelete(Sym, C))
       return;
   }
 
   // We will check for double free in the post visit.
   if (const AnyFunctionCall *FC = dyn_cast<AnyFunctionCall>(&Call)) {
     const FunctionDecl *FD = FC->getDecl();

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

case Stmt::CXXNewExprClass: {
       break;
     }
 
     case Stmt::CXXDeleteExprClass: {
       Bldr.takeNodes(Pred);
       ExplodedNodeSet PreVisit;
       const auto *CDE = cast<CXXDeleteExpr>(S);
       getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
+      ExplodedNodeSet PostVisit;
+      getCheckerManager().runCheckersForPostStmt(PostVisit, PreVisit, S, *this);
 
-      for (const auto i : PreVisit)
+      for (const auto i : PostVisit)
         VisitCXXDeleteExpr(CDE, i, Dst);
 
       Bldr.addNodes(Dst);
       break;
     }
       // FIXME: ChooseExpr is really a constant.  We need to fix
       //        the CFG do not model them as explicit control-flow.
 

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

if (!isa<CXXConstructExpr>(Init)) {
       evalBind(Dst, CNE, NewN, Result, State->getSVal(Init, LCtx),
                /*FirstInit=*/IsStandardGlobalOpNewFunction);
     }
   }
 }
 
 void ExprEngine::VisitCXXDeleteExpr(const CXXDeleteExpr *CDE,
                                     ExplodedNode *Pred, ExplodedNodeSet &Dst) {
-  StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
-  ProgramStateRef state = Pred->getState();
-  Bldr.generateNode(CDE, Pred, state);
+
+  CallEventManager &CEMgr = getStateManager().getCallEventManager();
+  CallEventRef<CXXDeallocatorCall> Call = CEMgr.getCXXDeallocatorCall(
+      CDE, Pred->getState(), Pred->getLocationContext());
+
+  ExplodedNodeSet DstPreCall;
+  getCheckerManager().runCheckersForPreCall(DstPreCall, Pred, *Call, *this);
+
+  getCheckerManager().runCheckersForPostCall(Dst, DstPreCall, *Call, *this);
 }
 
-void ExprEngine::VisitCXXCatchStmt(const CXXCatchStmt *CS,
-                                   ExplodedNode *Pred,
+void ExprEngine::VisitCXXCatchStmt(const CXXCatchStmt *CS, ExplodedNode *Pred,
                                    ExplodedNodeSet &Dst) {
   const VarDecl *VD = CS->getExceptionDecl();
   if (!VD) {
     Dst.Add(Pred);
     return;
   }
 
   const LocationContext *LCtx = Pred->getLocationContext();

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

 //=-- ExprEngineCallAndReturn.cpp - Support for call/return -----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines ExprEngine's support for calls and returns.
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/AST/Decl.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "PrettyStackTraceLocationContext.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/ConstructionContext.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
 using namespace ento;
 
 #define DEBUG_TYPE "ExprEngine"
 
 STATISTIC(NumOfDynamicDispatchPathSplits,
case CE_CXXDestructor: {
 
     // If we did not find the correct this-region, it would be pointless
     // to inline the destructor. Instead we will simply invalidate
     // the fake temporary target.
     if (CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion)
       return CIP_DisallowedOnce;
     break;
   }
+  case CE_CXXDeallocator:
+    LLVM_FALLTHROUGH;
   case CE_CXXAllocator:
     if (Opts.MayInlineCXXAllocator)
       break;
     // Do not inline allocators until we model deallocators.
     // This is unfortunate, but basically necessary for smart pointers and such.
     return CIP_DisallowedAlways;
   case CE_ObjCMessage:
     if (!Opts.MayInlineObjCMethod)

clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp

void f() {
   //
   // C++20 standard draft 17.6.1.24, about nothrow operator delete:
   //   void operator delete(void* ptr, const std::nothrow_t&) noexcept;
   //   void operator delete(void* ptr, std::align_val_t alignment,
   //                        const std::nothrow_t&) noexcept;
   // Default behavior: Calls operator delete(ptr), or operator delete(ptr,
   // alignment), respectively.
 
-  // FIXME: All calls to operator new should be CXXAllocatorCall.
-  // FIXME: PostStmt<CXXDeleteExpr> should be present.
+  // FIXME: All calls to operator new should be CXXAllocatorCall, and calls to
+  // operator delete should be CXXDeallocatorCall.
   {
     int *p = new int;
     delete p;
     // CHECK:      PreCall (operator new) [CXXAllocatorCall]
     // CHECK-NEXT: PostCall (operator new) [CXXAllocatorCall]
     // CHECK-NEXT: PreStmt<CXXNewExpr>
     // CHECK-NEXT: PostStmt<CXXNewExpr>
     // CHECK-NEXT: PreStmt<CXXDeleteExpr>
+    // CHECK-NEXT: PostStmt<CXXDeleteExpr>
+    // CHECK-NEXT: PreCall (operator delete) [CXXDeallocatorCall]
+    // CHECK-NEXT: PostCall (operator delete) [CXXDeallocatorCall]
 
     p = new int;
     operator delete(p, 23542368);
     // CHECK-NEXT: PreCall (operator new) [CXXAllocatorCall]
     // CHECK-NEXT: PostCall (operator new) [CXXAllocatorCall]
     // CHECK-NEXT: PreStmt<CXXNewExpr>
     // CHECK-NEXT: PostStmt<CXXNewExpr>
     // CHECK-NEXT: PreCall (operator delete) [SimpleFunctionCall]
void f() {
   {
     int *p = new int[2];
     delete[] p;
     // CHECK-NEXT: PreCall (operator new[]) [CXXAllocatorCall]
     // CHECK-NEXT: PostCall (operator new[]) [CXXAllocatorCall]
     // CHECK-NEXT: PreStmt<CXXNewExpr>
     // CHECK-NEXT: PostStmt<CXXNewExpr>
     // CHECK-NEXT: PreStmt<CXXDeleteExpr>
+    // CHECK-NEXT: PostStmt<CXXDeleteExpr>
+    // CHECK-NEXT: PreCall (operator delete[]) [CXXDeallocatorCall]
+    // CHECK-NEXT: PostCall (operator delete[]) [CXXDeallocatorCall]
 
     p = new int[2];
     operator delete[](p, 23542368);
     // CHECK-NEXT: PreCall (operator new[]) [CXXAllocatorCall]
     // CHECK-NEXT: PostCall (operator new[]) [CXXAllocatorCall]
     // CHECK-NEXT: PreStmt<CXXNewExpr>
     // CHECK-NEXT: PostStmt<CXXNewExpr>
     // CHECK-NEXT: PreCall (operator delete[]) [SimpleFunctionCall]

clang/unittests/StaticAnalyzer/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_unittest(StaticAnalysisTests
   AnalyzerOptionsTest.cpp
   CallDescriptionTest.cpp
+  CallEventTest.cpp
   StoreTest.cpp
   RegisterCustomCheckersTest.cpp
   SymbolReaperTest.cpp
   )
 
 clang_target_link_libraries(StaticAnalysisTests
   PRIVATE
   clangBasic

clang/unittests/StaticAnalyzer/CallEventTest.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "CheckerRegistration.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+void reportBug(const CheckerBase *Checker, const CallEvent &Call,
+               CheckerContext &C, StringRef WarningMsg) {
+  C.getBugReporter().EmitBasicReport(
+      nullptr, Checker, "", categories::LogicError, WarningMsg,
+      PathDiagnosticLocation(Call.getOriginExpr(), C.getSourceManager(),
+                             C.getLocationContext()),
+      {});
+}
+
+class CXXDeallocatorChecker : public Checker<check::PreCall> {
+  std::unique_ptr<BuiltinBug> BT_uninitField;
+
+public:
+  CXXDeallocatorChecker()
+      : BT_uninitField(new BuiltinBug(this, "CXXDeallocator")) {}
+
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const {
+    const auto *DC = dyn_cast<CXXDeallocatorCall>(&Call);
+    if (!DC) {
+      return;
+    }
+
+    SmallString<100> WarningBuf;
+    llvm::raw_svector_ostream WarningOS(WarningBuf);
+    WarningOS << "NumArgs: " << DC->getNumArgs();
+
+    reportBug(this, *DC, C, WarningBuf);
+  }
+};
+
+void addCXXDeallocatorChecker(AnalysisASTConsumer &AnalysisConsumer,
+                              AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"test.CXXDeallocator", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+    Registry.addChecker<CXXDeallocatorChecker>("test.CXXDeallocator",
+                                               "Description", "");
+  });
+}
+
+// TODO: What we should really be testing here is all the different varieties
+// of delete operators, and wether the retrieval of their arguments works as
+// intended. At the time of writing this file, CXXDeallocatorCall doesn't pick
+// up on much of those due to the AST not containing CXXDeleteExpr for most of
+// the standard/custom deletes.
+TEST(CXXDeallocatorCall, SimpleDestructor) {
+  std::string Diags;
+  EXPECT_TRUE(runCheckerOnCode<addCXXDeallocatorChecker>(R"(
+    struct A {};
+
+    void f() {
+      A *a = new A;
+      delete a;
+    }
+  )",
+                                                         Diags));
+  EXPECT_EQ(Diags, "test.CXXDeallocator:NumArgs: 1\n");
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang