[analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

[Szelethus]

Summary

If you remember the mail [1] I sent out about how I envision the future of the already existing checkers to look dependencywise, one my main points was that no checker that emits diagnostics should be a dependency. This is more problematic for some checkers (ahem, RetainCount [2]) more than for others, like this one.

The MallocChecker family is a mostly big monolithic modeling class some small reporting checkers that only come to action when we are constructing a warning message, after the actual bug was detected. The implication of this is that NewDeleteChecker doesn't really do anything to depend on, so this change was relatively simple.

The only thing that complicates this change is that FreeMemAux (MallocCheckers method that models general memory deallocation) returns after calling a bug reporting method, regardless whether the report was ever emitted (which may not always happen, for instance, if the checker responsible for the report isn't enabled). This return unfortunately happens before cleaning up the maps in the GDM keeping track of the state of symbols (whether they are released, whether that release was successful, etc). What this means is that upon disabling some checkers, we would never clean up the map and that could've lead to false positives, e.g.:

error: 'warning' diagnostics seen but not expected: 
  File clang/test/Analysis/NewDelete-intersections.mm Line 66: Potential leak of memory pointed to by 'p'
  File clang/test/Analysis/NewDelete-intersections.mm Line 73: Potential leak of memory pointed to by 'p'
  File clang/test/Analysis/NewDelete-intersections.mm Line 77: Potential leak of memory pointed to by 'p'

error: 'warning' diagnostics seen but not expected: 
  File clang/test/Analysis/NewDelete-checker-test.cpp Line 111: Undefined or garbage value returned to caller
  File clang/test/Analysis/NewDelete-checker-test.cpp Line 200: Potential leak of memory pointed to by 'p'

error: 'warning' diagnostics seen but not expected: 
  File clang/test/Analysis/new.cpp Line 137: Potential leak of memory pointed to by 'x'

There two possible approaches I had in mind:

• Make bug reporting methods of MallocChecker returns whether they succeeded, and proceed with the rest of FreeMemAux if not,
• Halt execution with a sink node upon failure. I decided to go with this, as described in the code.

As you can see from the removed/changed test files, before the big checker dependency effort landed, there were tests to check for all the weird configurations of enabled/disabled checkers and their messy interactions, I largely repurposed these.

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-August/063070.html
[2] http://lists.llvm.org/pipermail/cfe-dev/2019-August/063205.html

[Szelethus]

Btw this branch is about 7 months old, and I just now remembered why I haven't rushed publishing it.

My ultimate idea (as opposed to the two detailed in the summary) was to introduce a subchecker system in MallocChecker, which also strongly ties into D67336. The core of the checker (unix.DynamicMemoryModeling) would be responsible for dispatching from regular checker callbacks upon seeing a call to a memory management function, and providing the general utilities for modeling it. Smaller subcheckers would then use this interface to implement the modeling of specific functions. For instance, the we could create NewDeleteChecker, a subchecker of DynamicMemoryModeling, which would get control when stumbling upon a call to a new/delete operator.

This would allow us to spread the implementation into smaller files, which is great, but upon taking a look at D68165, you can see that the bulk of the code isn't really in these modeling functions. For such a change to really make sense, the responsibility of bug emission (like ReportFunctionPointerFree) should be moved into the subcheckers as well. This would achieve my goal of smaller, self-contained subcheckers implementing something that we previously did with giants like MallocChecker is now. As to how I should actually pull this off (make an event about memory being deallocated to the subcheckers, make them check for bugs? make an event about finding a leak, double delete, etc?) is a different issue, but I'll see what makes more sense, if I choose to go forth with this project.

I do now believe that though that all of these changes would be orthogonal to this one. Just thought I'd share :^).

[balazske]

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

195

if (!State)
  State = getState();

is better? (I put the same comment into some (other?) patch already but maybe it disappeared?)

[Szelethus]

Gentle ping :^)

[martong]

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

194

I like the joke, but this comment does not have any value, could you please remove?

195

+1 for balazske's suggestion.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

1744–1745

This sentence is too bloated, I'd rather remove it.

AFAIU, the general rule of thumb is that if we have found a bug then we terminate further analysis, period. This is independent of whether we emit the warning or not, that actually depends on whether the corresponding subchecker is enabled or not.

2024

This seems to be inverse logic to me.
I'd expect that in a function called Report... we do stuff that is related to reporting only. That is why I think it would be better to have the condition and addSink before calling Report.... That way reporting and modeling would be even more separated.

[Szelethus]

Fix according to reviewer comments! Thanks!

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

195

Landed this part of the change in D77866.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

2024

Very good point, but I think the actual problem lies in the name of the method. ReportBadFree in particular is called from multiple places, and I like how this is the function that takes over once we find a bug, because a bad free (which in this context means the deallocation of a non-heap allocated object) can never be a non-fatal error, so it make sense that the sink is solved here.

[martong]

Looks good.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

2024

Okay, makes sense, the new name fits this.

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

 def NewDeleteChecker : Checker<"NewDelete">,
   HelpText<"Check for double-free and use-after-free problems. Traces memory "
            "managed by new/delete.">,
   Dependencies<[DynamicMemoryModeling]>,
   Documentation<HasDocumentation>;
 
 def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">,
   HelpText<"Check for memory leaks. Traces memory managed by new/delete.">,
-  Dependencies<[NewDeleteChecker]>,
+  Dependencies<[DynamicMemoryModeling]>,
   Documentation<HasDocumentation>;
 
 def PlacementNewChecker : Checker<"PlacementNew">,
   HelpText<"Check if default placement new is provided with pointers to "
            "sufficient storage capacity">,
   Dependencies<[NewDeleteChecker]>,
   Documentation<HasDocumentation>;
 

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

public:
 
   /// Generates a new transition with the given predecessor.
   /// Allows checkers to generate a chain of nodes.
   ///
   /// @param State The state of the generated node.
   /// @param Pred The transition will be generated from the specified Pred node
   ///             to the newly generated node.
   /// @param Tag The tag to uniquely identify the creation site.
-  ExplodedNode *addTransition(ProgramStateRef State,
-                              ExplodedNode *Pred,
+  ExplodedNode *addTransition(ProgramStateRef State, ExplodedNode *Pred,
                               const ProgramPointTag *Tag = nullptr) {
     return addTransitionImpl(State, false, Pred, Tag);
   }
 
   /// Generate a sink node. Generating a sink stops exploration of the
   /// given path. To create a sink node for the purpose of reporting an error,
   /// checkers should use generateErrorNode() instead.
   ExplodedNode *generateSink(ProgramStateRef State, ExplodedNode *Pred,
                              const ProgramPointTag *Tag = nullptr) {
     return addTransitionImpl(State ? State : getState(), true, Pred, Tag);
   }
 
+  /// Add a sink node to the current path of execution, halting analysis.
+  void addSink(ProgramStateRef State = nullptr,
+               const ProgramPointTag *Tag = nullptr) {
+    // Say this 3 times fast.
+    State = State ? State : getState();
+    addTransition(State, generateSink(State, getPredecessor()));
+  }
+
   /// Generate a transition to a node that will be used to report
   /// an error. This node will be a sink. That is, it will stop exploration of
   /// the given path.
   ///
   /// @param State The state of the generated node.
   /// @param Tag The tag to uniquely identify the creation site. If null,
   ///        the default tag for the checker will be used.
   ExplodedNode *generateErrorNode(ProgramStateRef State = nullptr,

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

ProgramStateRef MallocChecker::FreeMemAux(
   // Unknown values could easily be okay
   // Undefined values are handled elsewhere
   if (ArgVal.isUnknownOrUndef())
     return nullptr;
 
   const MemRegion *R = ArgVal.getAsRegion();
   const Expr *ParentExpr = Call.getOriginExpr();
 
+  // NOTE: There is a philosophical question to be answered when we detect a
+  // bug, but the checker under whose name we would emit the error is disabled.
+  // Generally speaking, the MallocChecker family is an integral part of the
+  // Static Analyzer, and disabling any part of it should only be done under
+  // exceptional circumstances, such as frequent false positives. If this is the
+  // case, we can reasonably believe that there are serious faults in our
+  // understanding of the source code, and even if we don't emit an warning, we
+  // should terminate further analysis with a sink node.
+
   // Nonlocs can't be freed, of course.
   // Non-region locations (labels and fixed addresses) also shouldn't be freed.
   if (!R) {
     // Exception:
     // If the macro ZERO_SIZE_PTR is defined, this could be a kernel source
     // code. In that case, the ZERO_SIZE_PTR defines a special value used for a
     // zero-sized memory block which is allowed to be freed, despite not being a
     // null pointer.
bool MallocChecker::SummarizeRegion(raw_ostream &os,
   }
   }
 }
 
 void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
                                   SourceRange Range, const Expr *DeallocExpr,
                                   AllocationFamily Family) const {
 
-  if (!ChecksEnabled[CK_MallocChecker] &&
-      !ChecksEnabled[CK_NewDeleteChecker])
+  if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) {
+    C.addSink();
     return;
+  }
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
   if (!CheckKind.hasValue())
     return;
 
   if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_BadFree[*CheckKind])
       BT_BadFree[*CheckKind].reset(new BugType(
void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal,
                                      SourceRange Range) const {
 
   Optional<MallocChecker::CheckKind> CheckKind;
 
   if (ChecksEnabled[CK_MallocChecker])
     CheckKind = CK_MallocChecker;
   else if (ChecksEnabled[CK_MismatchedDeallocatorChecker])
     CheckKind = CK_MismatchedDeallocatorChecker;
-  else
+  else {
+    C.addSink();
     return;
+  }
 
   if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_FreeAlloca[*CheckKind])
       BT_FreeAlloca[*CheckKind].reset(new BugType(
           CheckNames[*CheckKind], "Free alloca()", categories::MemoryError));
 
     auto R = std::make_unique<PathSensitiveBugReport>(
         *BT_FreeAlloca[*CheckKind],
         "Memory allocated by alloca() should not be deallocated", N);
     R->markInteresting(ArgVal.getAsRegion());
     R->addRange(Range);
     C.emitReport(std::move(R));
   }
 }
 
 void MallocChecker::ReportMismatchedDealloc(CheckerContext &C,
                                             SourceRange Range,
                                             const Expr *DeallocExpr,
                                             const RefState *RS,
                                             SymbolRef Sym,
                                             bool OwnershipTransferred) const {
 
-  if (!ChecksEnabled[CK_MismatchedDeallocatorChecker])
+  if (!ChecksEnabled[CK_MismatchedDeallocatorChecker]) {
+    C.addSink();
     return;
+  }
 
   if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_MismatchedDealloc)
       BT_MismatchedDealloc.reset(
           new BugType(CheckNames[CK_MismatchedDeallocatorChecker],
                       "Bad deallocator", categories::MemoryError));
 
     SmallString<100> buf;
void MallocChecker::ReportMismatchedDealloc(CheckerContext &C,
   }
 }
 
 void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
                                      SourceRange Range, const Expr *DeallocExpr,
                                      AllocationFamily Family,
                                      const Expr *AllocExpr) const {
 
-  if (!ChecksEnabled[CK_MallocChecker] &&
-      !ChecksEnabled[CK_NewDeleteChecker])
+  if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) {
+    C.addSink();
     return;
+  }
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
   if (!CheckKind.hasValue())
     return;
 
   ExplodedNode *N = C.generateErrorNode();
   if (!N)
     return;
void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
   R->markInteresting(MR->getBaseRegion());
   R->addRange(Range);
   C.emitReport(std::move(R));
 }
 
 void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range,
                                        SymbolRef Sym) const {
 
-  if (!ChecksEnabled[CK_MallocChecker] &&
-      !ChecksEnabled[CK_NewDeleteChecker] &&
-      !ChecksEnabled[CK_InnerPointerChecker])
+  if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker] &&
+      !ChecksEnabled[CK_InnerPointerChecker]) {
+    C.addSink();
     return;
+  }
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
   if (!CheckKind.hasValue())
     return;
 
   if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_UseFree[*CheckKind])
       BT_UseFree[*CheckKind].reset(new BugType(
if (ExplodedNode *N = C.generateErrorNode()) {
     C.emitReport(std::move(R));
   }
 }
 
 void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range,
                                      bool Released, SymbolRef Sym,
                                      SymbolRef PrevSym) const {
 
-  if (!ChecksEnabled[CK_MallocChecker] &&
-      !ChecksEnabled[CK_NewDeleteChecker])
+  if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) {
+    C.addSink();
     return;
+  }
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
   if (!CheckKind.hasValue())
     return;
 
   if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_DoubleFree[*CheckKind])
       BT_DoubleFree[*CheckKind].reset(new BugType(
if (PrevSym)
       R->markInteresting(PrevSym);
     R->addVisitor(std::make_unique<MallocBugVisitor>(Sym));
     C.emitReport(std::move(R));
   }
 }
 
 void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const {
 
-  if (!ChecksEnabled[CK_NewDeleteChecker])
+  if (!ChecksEnabled[CK_NewDeleteChecker]) {
+    C.addSink();
     return;
+  }
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
   if (!CheckKind.hasValue())
     return;
 
   if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_DoubleDelete)
       BT_DoubleDelete.reset(new BugType(CheckNames[CK_NewDeleteChecker],
                                         "Double delete",
                                         categories::MemoryError));
 
     auto R = std::make_unique<PathSensitiveBugReport>(
         *BT_DoubleDelete, "Attempt to delete released memory", N);
 
     R->markInteresting(Sym);
     R->addVisitor(std::make_unique<MallocBugVisitor>(Sym));
     C.emitReport(std::move(R));
   }
 }
 
 void MallocChecker::ReportUseZeroAllocated(CheckerContext &C,
                                            SourceRange Range,
                                            SymbolRef Sym) const {
 
-  if (!ChecksEnabled[CK_MallocChecker] &&
-      !ChecksEnabled[CK_NewDeleteChecker])
+  if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) {
+    C.addSink();
     return;
+  }
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
 
   if (!CheckKind.hasValue())
     return;
 
   if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_UseZerroAllocated[*CheckKind])
if (ExplodedNode *N = C.generateErrorNode()) {
     C.emitReport(std::move(R));
   }
 }
 
 void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
                                               SourceRange Range,
                                               const Expr *FreeExpr,
                                               AllocationFamily Family) const {
-  if (!ChecksEnabled[CK_MallocChecker])
+  if (!ChecksEnabled[CK_MallocChecker]) {
+    C.addSink();
     return;
+  }
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
   if (!CheckKind.hasValue())
     return;
 
   if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_BadFree[*CheckKind])
       BT_BadFree[*CheckKind].reset(new BugType(

clang/test/Analysis/Malloc+NewDelete_intersections.cpp

-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete -std=c++11 -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -std=c++11 -verify %s
-
-typedef __typeof(sizeof(int)) size_t;
-void *malloc(size_t);
-void free(void *);
-
-//-------------------------------------------------------------------
-// Check that unix.Malloc + cplusplus.NewDelete does not enable
-// warnings produced by unix.MismatchedDeallocator.
-//-------------------------------------------------------------------
-void testMismatchedDeallocator() {
-  int *p = (int *)malloc(sizeof(int));
-  delete p;
-} // expected-warning{{Potential leak of memory pointed to by 'p'}}

clang/test/Analysis/NewDelete-checker-test.cpp

-// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks %s \
+// RUN:   -verify=expected,newdelete \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=cplusplus.NewDelete
 //
-// RUN: %clang_analyze_cc1 -DLEAKS -std=c++11 -fblocks -verify %s \
+// RUN: %clang_analyze_cc1 -DLEAKS -std=c++11 -fblocks %s \
+// RUN:   -verify=expected,newdelete,leak \
 // RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.NewDelete \
 // RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks
 //
-// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks %s \
+// RUN:   -verify=expected,newdelete \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=cplusplus.NewDelete \
 // RUN:   -analyzer-config c++-allocator-inlining=true
 //
-// RUN: %clang_analyze_cc1 -DLEAKS -std=c++11 -fblocks -verify %s \
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN:   -verify=expected,newdelete,leak \
 // RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.NewDelete \
 // RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks \
 // RUN:   -analyzer-config c++-allocator-inlining=true
 //
-// RUN: %clang_analyze_cc1 -DTEST_INLINABLE_ALLOCATORS \
-// RUN:   -std=c++11 -fblocks -verify %s \
-// RUN:   -analyzer-checker=core \
-// RUN:   -analyzer-checker=cplusplus.NewDelete
-//
-// RUN: %clang_analyze_cc1 -DLEAKS -DTEST_INLINABLE_ALLOCATORS \
-// RUN:   -std=c++11 -fblocks -verify %s \
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN:   -verify=expected,leak \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks
-//
-// RUN: %clang_analyze_cc1 -DTEST_INLINABLE_ALLOCATORS \
-// RUN:   -std=c++11 -fblocks -verify %s \
-// RUN:   -analyzer-checker=core \
-// RUN:   -analyzer-checker=cplusplus.NewDelete \
-// RUN:   -analyzer-config c++-allocator-inlining=true
-//
-// RUN: %clang_analyze_cc1 -DLEAKS -DTEST_INLINABLE_ALLOCATORS \
-// RUN:   -std=c++11 -fblocks -verify %s \
-// RUN:   -analyzer-checker=core \
-// RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks \
-// RUN:   -analyzer-config c++-allocator-inlining=true
 
 #include "Inputs/system-header-simulator-cxx.h"
 
 typedef __typeof__(sizeof(int)) size_t;
 extern "C" void *malloc(size_t);
 extern "C" void free (void* ptr);
 int *global;
 
 //------------------
 // check for leaks
 //------------------
 
 //----- Standard non-placement operators
 void testGlobalOpNew() {
   void *p = operator new(0);
-}
-#ifdef LEAKS
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
+} // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 void testGlobalOpNewArray() {
   void *p = operator new[](0);
-}
-#ifdef LEAKS
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
+} // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 void testGlobalNewExpr() {
   int *p = new int;
-}
-#ifdef LEAKS
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
+} // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 void testGlobalNewExprArray() {
   int *p = new int[0];
-}
-#ifdef LEAKS
-// expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
-#endif
+} // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 //----- Standard nothrow placement operators
 void testGlobalNoThrowPlacementOpNewBeforeOverload() {
   void *p = operator new(0, std::nothrow);
-}
-#ifdef LEAKS
-#ifndef TEST_INLINABLE_ALLOCATORS
-// expected-warning@-3{{Potential leak of memory pointed to by 'p'}}
-#endif
-#endif
+} // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 void testGlobalNoThrowPlacementExprNewBeforeOverload() {
   int *p = new(std::nothrow) int;
-}
-#ifdef LEAKS
-#ifndef TEST_INLINABLE_ALLOCATORS
-// expected-warning@-3{{Potential leak of memory pointed to by 'p'}}
-#endif
-#endif
+} // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 //----- Standard pointer placement operators
 void testGlobalPointerPlacementNew() {
   int i;
 
   void *p1 = operator new(0, &i); // no warn
 
   void *p2 = operator new[](0, &i); // no warn

 }
 
 //-----------------------------------------
 // check for usage of zero-allocated memory
 //-----------------------------------------
 
 void testUseZeroAlloc1() {
   int *p = (int *)operator new(0);
-  *p = 1; // expected-warning {{Use of zero-allocated memory}}
+  *p = 1; // newdelete-warning {{Use of zero-allocated memory}}
   delete p;
 }
 
 int testUseZeroAlloc2() {
   int *p = (int *)operator new[](0);
-  return p[0]; // expected-warning {{Use of zero-allocated memory}}
+  return p[0]; // newdelete-warning {{Use of zero-allocated memory}}
   delete[] p;
 }
 
 void f(int);
 
 void testUseZeroAlloc3() {
   int *p = new int[0];
-  f(*p); // expected-warning {{Use of zero-allocated memory}}
+  f(*p); // newdelete-warning {{Use of zero-allocated memory}}
   delete[] p;
 }
 
 //---------------
 // other checks
 //---------------
 
 class SomeClass {
 public:
   void f(int *p);
 };
 
 void f(int *p1, int *p2 = 0, int *p3 = 0);
 void g(SomeClass &c, ...);
 
 void testUseFirstArgAfterDelete() {
   int *p = new int;
   delete p;
-  f(p); // expected-warning{{Use of memory after it is freed}}
+  f(p); // newdelete-warning{{Use of memory after it is freed}}
 }
 
 void testUseMiddleArgAfterDelete(int *p) {
   delete p;
-  f(0, p); // expected-warning{{Use of memory after it is freed}}
+  f(0, p); // newdelete-warning{{Use of memory after it is freed}}
 }
 
 void testUseLastArgAfterDelete(int *p) {
   delete p;
-  f(0, 0, p); // expected-warning{{Use of memory after it is freed}}
+  f(0, 0, p); // newdelete-warning{{Use of memory after it is freed}}
 }
 
 void testUseSeveralArgsAfterDelete(int *p) {
   delete p;
-  f(p, p, p); // expected-warning{{Use of memory after it is freed}}
+  f(p, p, p); // newdelete-warning{{Use of memory after it is freed}}
 }
 
 void testUseRefArgAfterDelete(SomeClass &c) {
   delete &c;
-  g(c); // expected-warning{{Use of memory after it is freed}}
+  g(c); // newdelete-warning{{Use of memory after it is freed}}
 }
 
 void testVariadicArgAfterDelete() {
   SomeClass c;
   int *p = new int;
   delete p;
-  g(c, 0, p); // expected-warning{{Use of memory after it is freed}}
+  g(c, 0, p); // newdelete-warning{{Use of memory after it is freed}}
 }
 
 void testUseMethodArgAfterDelete(int *p) {
   SomeClass *c = new SomeClass;
   delete p;
-  c->f(p); // expected-warning{{Use of memory after it is freed}}
+  c->f(p); // newdelete-warning{{Use of memory after it is freed}}
 }
 
 void testUseThisAfterDelete() {
   SomeClass *c = new SomeClass;
   delete c;
-  c->f(0); // expected-warning{{Use of memory after it is freed}}
+  c->f(0); // newdelete-warning{{Use of memory after it is freed}}
 }
 
 void testDoubleDelete() {
   int *p = new int;
   delete p;
-  delete p; // expected-warning{{Attempt to free released memory}}
+  delete p; // newdelete-warning{{Attempt to free released memory}}
 }
 
 void testExprDeleteArg() {
   int i;
-  delete &i; // expected-warning{{Argument to 'delete' is the address of the local variable 'i', which is not memory allocated by 'new'}}
+  delete &i; // newdelete-warning{{Argument to 'delete' is the address of the local variable 'i', which is not memory allocated by 'new'}}
 }
 
 void testExprDeleteArrArg() {
   int i;
-  delete[] &i; // expected-warning{{Argument to 'delete[]' is the address of the local variable 'i', which is not memory allocated by 'new[]'}}
+  delete[] & i; // newdelete-warning{{Argument to 'delete[]' is the address of the local variable 'i', which is not memory allocated by 'new[]'}}
 }
 
 void testAllocDeallocNames() {
   int *p = new(std::nothrow) int[1];
   delete[] (++p);
-#ifndef TEST_INLINABLE_ALLOCATORS
-  // expected-warning@-2{{Argument to 'delete[]' is offset by 4 bytes from the start of memory allocated by 'new[]'}}
-#endif
+  // newdelete-warning@-1{{Argument to 'delete[]' is offset by 4 bytes from the start of memory allocated by 'new[]'}}
 }
 
 //--------------------------------
 // Test escape of newed const pointer. Note, a const pointer can be deleted.
 //--------------------------------
 struct StWithConstPtr {
   const int *memp;
 };
public:
   int *x;
   DerefClass() {}
   ~DerefClass() {*x = 1;}
 };
 
 void testDoubleDeleteClassInstance() {
   DerefClass *foo = new DerefClass();
   delete foo;
-  delete foo; // expected-warning {{Attempt to delete released memory}}
+  delete foo; // newdelete-warning {{Attempt to delete released memory}}
 }
 
 class EmptyClass{
 public:
   EmptyClass() {}
   ~EmptyClass() {}
 };
 
 void testDoubleDeleteEmptyClass() {
   EmptyClass *foo = new EmptyClass();
   delete foo;
-  delete foo;  // expected-warning {{Attempt to delete released memory}}
+  delete foo; // newdelete-warning {{Attempt to delete released memory}}
 }
 
 struct Base {
   virtual ~Base() {}
 };
 
 struct Derived : Base {
 };

clang/test/Analysis/NewDelete-intersections.mm

-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -std=c++11 -DLEAKS -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -std=c++11 -DLEAKS -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks %s \
+// RUN:  -verify=newdelete \
+// RUN:  -analyzer-checker=core \
+// RUN:  -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -std=c++11 -DLEAKS -fblocks %s \
+// RUN:   -verify=leak \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks
+
+// leak-no-diagnostics
+
+// RUN: %clang_analyze_cc1 -std=c++11 -DLEAKS -fblocks %s \
+// RUN:   -verify=mismatch \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.MismatchedDeallocator
+
 #include "Inputs/system-header-simulator-cxx.h"
 #include "Inputs/system-header-simulator-objc.h"
 
 typedef __typeof__(sizeof(int)) size_t;
 extern "C" void *malloc(size_t);
 extern "C" void *alloca(size_t);
 extern "C" void free(void *);
 
-//----------------------------------------------------------------------------
-// Check for intersections with unix.Malloc and unix.MallocWithAnnotations 
-// checkers bounded with cplusplus.NewDelete.
-//----------------------------------------------------------------------------
-
-//----- malloc()/free() are subjects of unix.Malloc and unix.MallocWithAnnotations
 void testMallocFreeNoWarn() {
   int i;
   free(&i); // no warn
 
   int *p1 = (int *)malloc(sizeof(int));
   free(++p1); // no warn
 
   int *p2 = (int *)malloc(sizeof(int));
   free(p2);
   free(p2); // no warn
 
   int *p3 = (int *)malloc(sizeof(int)); // no warn
 
   int *p4 = (int *)malloc(sizeof(int));
   free(p4);
   int j = *p4; // no warn
 
   int *p5 = (int *)alloca(sizeof(int));
   free(p5); // no warn
 }
 
 void testDeleteMalloced() {
   int *p1 = (int *)malloc(sizeof(int));
-  delete p1; // no warn
+  delete p1;
+  // mismatch-warning@-1{{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 
   int *p2 = (int *)__builtin_alloca(sizeof(int));
   delete p2; // no warn
 } 
 
 void testUseZeroAllocatedMalloced() {
   int *p1 = (int *)malloc(0);
   *p1 = 1; // no warn
 }
 
 //----- Test free standard new
 void testFreeOpNew() {
   void *p = operator new(0);
   free(p);
+  // mismatch-warning@-1{{Memory allocated by operator new should be deallocated by 'delete', not free()}}
 }
-#ifdef LEAKS
-// expected-warning@-2 {{Potential leak of memory pointed to by 'p'}}
-#endif
 
 void testFreeNewExpr() {
   int *p = new int;
   free(p);
+  // mismatch-warning@-1{{Memory allocated by 'new' should be deallocated by 'delete', not free()}}
+  free(p);
 }
-#ifdef LEAKS
-// expected-warning@-2 {{Potential leak of memory pointed to by 'p'}}
-#endif
 
 void testObjcFreeNewed() {
   int *p = new int;
   NSData *nsdata = [NSData dataWithBytesNoCopy:p length:sizeof(int) freeWhenDone:1];
-#ifdef LEAKS
-  // expected-warning@-2 {{Potential leak of memory pointed to by 'p'}}
-#endif
+  // mismatch-warning@-1{{+dataWithBytesNoCopy:length:freeWhenDone: cannot take ownership of memory allocated by 'new'}}
 }
 
 void testFreeAfterDelete() {
   int *p = new int;  
   delete p;
-  free(p); // expected-warning{{Use of memory after it is freed}}
+  free(p); // newdelete-warning{{Use of memory after it is freed}}
 }
 
 void testStandardPlacementNewAfterDelete() {
   int *p = new int;  
   delete p;
-  p = new(p) int; // expected-warning{{Use of memory after it is freed}}
+  p = new (p) int; // newdelete-warning{{Use of memory after it is freed}}
 }

clang/test/Analysis/new.cpp

 
 void testUseAfter(int *p) {
   SomeClass *c = new SomeClass;
   free(p);
   c->f(p); // expected-warning{{Use of memory after it is freed}}
   delete c;
 }
 
-//--------------------------------------------------------------------
-// Check for intersection with other checkers from MallocChecker.cpp 
-// bounded with unix.Malloc
-//--------------------------------------------------------------------
-
 // new/delete oparators are subjects of cplusplus.NewDelete.
 void testNewDeleteNoWarn() {
   int i;
   delete &i; // no-warning
 
   int *p1 = new int;
   delete ++p1; // no-warning
 
   int *p2 = new int;
   delete p2;
   delete p2; // no-warning
 
   int *p3 = new int; // no-warning
 }
 
-// unix.Malloc does not know about operators new/delete.
 void testDeleteMallocked() {
   int *x = (int *)malloc(sizeof(int));
-  delete x; // FIXME: Should detect pointer escape and keep silent after 'delete' is modeled properly.
-} // expected-warning{{Potential leak of memory pointed to by 'x'}}
+  // unix.MismatchedDeallocator would catch this, but we're not testing it here.
+  delete x;
+}
 
 void testDeleteOpAfterFree() {
   int *p = (int *)malloc(sizeof(int));
   free(p);
   operator delete(p); // expected-warning{{Use of memory after it is freed}}
 }
 
 void testDeleteAfterFree() {