[llvm-branch-commits] [clang] efd1a8e - [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

Kirstóf Umann via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue May 26 15:08:35 PDT 2020


Author: Kristóf Umann
Date: 2020-05-27T00:03:53+02:00
New Revision: efd1a8e66eaa13afff709ebf16ff6280caa82ead

URL: https://github.com/llvm/llvm-project/commit/efd1a8e66eaa13afff709ebf16ff6280caa82ead
DIFF: https://github.com/llvm/llvm-project/commit/efd1a8e66eaa13afff709ebf16ff6280caa82ead.diff

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

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

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

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
    clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
    clang/test/Analysis/NewDelete-checker-test.cpp
    clang/test/Analysis/NewDelete-intersections.mm
    clang/test/Analysis/new.cpp

Removed: 
    clang/test/Analysis/Malloc+NewDelete_intersections.cpp


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index bc4b7d00e2d4..2ba3881c6135 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -556,13 +556,13 @@ def NewDeleteChecker : Checker<"NewDelete">,
 
 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]>,
+  Dependencies<[DynamicMemoryModeling]>,
   Documentation<HasDocumentation>;
 
 def CXXSelfAssignmentChecker : Checker<"SelfAssignment">,

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index a7c62a7e8046..fa69bc253fbd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -684,41 +684,42 @@ class MallocChecker
   static bool SummarizeValue(raw_ostream &os, SVal V);
   static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
 
-  void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
-                     const Expr *DeallocExpr, AllocationFamily Family) const;
+  void HandleNonHeapDealloc(CheckerContext &C, SVal ArgVal, SourceRange Range,
+                            const Expr *DeallocExpr,
+                            AllocationFamily Family) const;
 
-  void ReportFreeAlloca(CheckerContext &C, SVal ArgVal,
+  void HandleFreeAlloca(CheckerContext &C, SVal ArgVal,
                         SourceRange Range) const;
 
-  void ReportMismatchedDealloc(CheckerContext &C, SourceRange Range,
+  void HandleMismatchedDealloc(CheckerContext &C, SourceRange Range,
                                const Expr *DeallocExpr, const RefState *RS,
                                SymbolRef Sym, bool OwnershipTransferred) const;
 
-  void ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
+  void HandleOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
                         const Expr *DeallocExpr, AllocationFamily Family,
                         const Expr *AllocExpr = nullptr) const;
 
-  void ReportUseAfterFree(CheckerContext &C, SourceRange Range,
+  void HandleUseAfterFree(CheckerContext &C, SourceRange Range,
                           SymbolRef Sym) const;
 
-  void ReportDoubleFree(CheckerContext &C, SourceRange Range, bool Released,
+  void HandleDoubleFree(CheckerContext &C, SourceRange Range, bool Released,
                         SymbolRef Sym, SymbolRef PrevSym) const;
 
-  void ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const;
+  void HandleDoubleDelete(CheckerContext &C, SymbolRef Sym) const;
 
-  void ReportUseZeroAllocated(CheckerContext &C, SourceRange Range,
-                              SymbolRef Sym) const;
+  void HandleUseZeroAlloc(CheckerContext &C, SourceRange Range,
+                          SymbolRef Sym) const;
 
-  void ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
-                                 SourceRange Range, const Expr *FreeExpr,
-                                 AllocationFamily Family) const;
+  void HandleFunctionPtrFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
+                             const Expr *FreeExpr,
+                             AllocationFamily Family) const;
 
   /// Find the location of the allocation for Sym on the path leading to the
   /// exploded node N.
   static LeakInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
                                     CheckerContext &C);
 
-  void reportLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const;
+  void HandleLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const;
 
   /// Test if value in ArgVal equals to value in macro `ZERO_SIZE_PTR`.
   bool isArgZERO_SIZE_PTR(ProgramStateRef State, CheckerContext &C,
@@ -1743,6 +1744,15 @@ ProgramStateRef MallocChecker::FreeMemAux(
   const MemRegion *R = ArgVal.getAsRegion();
   const Expr *ParentExpr = Call.getOriginExpr();
 
+  // NOTE: We detected a bug, but the checker under whose name we would emit the
+  // error could be 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) {
@@ -1752,7 +1762,8 @@ ProgramStateRef MallocChecker::FreeMemAux(
     // zero-sized memory block which is allowed to be freed, despite not being a
     // null pointer.
     if (Family != AF_Malloc || !isArgZERO_SIZE_PTR(State, C, ArgVal))
-      ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
+      HandleNonHeapDealloc(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
+                           Family);
     return nullptr;
   }
 
@@ -1760,7 +1771,8 @@ ProgramStateRef MallocChecker::FreeMemAux(
 
   // Blocks might show up as heap data, but should not be free()d
   if (isa<BlockDataRegion>(R)) {
-    ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
+    HandleNonHeapDealloc(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
+                         Family);
     return nullptr;
   }
 
@@ -1778,9 +1790,10 @@ ProgramStateRef MallocChecker::FreeMemAux(
     // False negatives are better than false positives.
 
     if (isa<AllocaRegion>(R))
-      ReportFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
+      HandleFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
     else
-      ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
+      HandleNonHeapDealloc(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
+                           Family);
 
     return nullptr;
   }
@@ -1802,14 +1815,14 @@ ProgramStateRef MallocChecker::FreeMemAux(
 
     // Memory returned by alloca() shouldn't be freed.
     if (RsBase->getAllocationFamily() == AF_Alloca) {
-      ReportFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
+      HandleFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
       return nullptr;
     }
 
     // Check for double free first.
     if ((RsBase->isReleased() || RsBase->isRelinquished()) &&
         !didPreviousFreeFail(State, SymBase, PreviousRetStatusSymbol)) {
-      ReportDoubleFree(C, ParentExpr->getSourceRange(), RsBase->isReleased(),
+      HandleDoubleFree(C, ParentExpr->getSourceRange(), RsBase->isReleased(),
                        SymBase, PreviousRetStatusSymbol);
       return nullptr;
 
@@ -1821,8 +1834,8 @@ ProgramStateRef MallocChecker::FreeMemAux(
       // Check if an expected deallocation function matches the real one.
       bool DeallocMatchesAlloc = RsBase->getAllocationFamily() == Family;
       if (!DeallocMatchesAlloc) {
-        ReportMismatchedDealloc(C, ArgExpr->getSourceRange(),
-                                ParentExpr, RsBase, SymBase, Hold);
+        HandleMismatchedDealloc(C, ArgExpr->getSourceRange(), ParentExpr,
+                                RsBase, SymBase, Hold);
         return nullptr;
       }
 
@@ -1833,7 +1846,7 @@ ProgramStateRef MallocChecker::FreeMemAux(
           !Offset.hasSymbolicOffset() &&
           Offset.getOffset() != 0) {
         const Expr *AllocExpr = cast<Expr>(RsBase->getStmt());
-        ReportOffsetFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
+        HandleOffsetFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
                          Family, AllocExpr);
         return nullptr;
       }
@@ -1841,8 +1854,8 @@ ProgramStateRef MallocChecker::FreeMemAux(
   }
 
   if (SymBase->getType()->isFunctionPointerType()) {
-    ReportFunctionPointerFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
-                              Family);
+    HandleFunctionPtrFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
+                          Family);
     return nullptr;
   }
 
@@ -2009,13 +2022,15 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os,
   }
 }
 
-void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
-                                  SourceRange Range, const Expr *DeallocExpr,
-                                  AllocationFamily Family) const {
+void MallocChecker::HandleNonHeapDealloc(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())
@@ -2055,7 +2070,7 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
   }
 }
 
-void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal,
+void MallocChecker::HandleFreeAlloca(CheckerContext &C, SVal ArgVal,
                                      SourceRange Range) const {
 
   Optional<MallocChecker::CheckKind> CheckKind;
@@ -2064,8 +2079,10 @@ void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal,
     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])
@@ -2081,15 +2098,16 @@ void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal,
   }
 }
 
-void MallocChecker::ReportMismatchedDealloc(CheckerContext &C,
+void MallocChecker::HandleMismatchedDealloc(CheckerContext &C,
                                             SourceRange Range,
                                             const Expr *DeallocExpr,
-                                            const RefState *RS,
-                                            SymbolRef Sym,
+                                            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)
@@ -2137,14 +2155,15 @@ void MallocChecker::ReportMismatchedDealloc(CheckerContext &C,
   }
 }
 
-void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
+void MallocChecker::HandleOffsetFree(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())
@@ -2194,13 +2213,14 @@ void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
   C.emitReport(std::move(R));
 }
 
-void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range,
+void MallocChecker::HandleUseAfterFree(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())
@@ -2232,13 +2252,14 @@ void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range,
   }
 }
 
-void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range,
+void MallocChecker::HandleDoubleFree(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())
@@ -2263,10 +2284,12 @@ void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range,
   }
 }
 
-void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const {
+void MallocChecker::HandleDoubleDelete(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())
@@ -2287,13 +2310,13 @@ void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const {
   }
 }
 
-void MallocChecker::ReportUseZeroAllocated(CheckerContext &C,
-                                           SourceRange Range,
-                                           SymbolRef Sym) const {
+void MallocChecker::HandleUseZeroAlloc(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);
 
@@ -2318,12 +2341,14 @@ void MallocChecker::ReportUseZeroAllocated(CheckerContext &C,
   }
 }
 
-void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
-                                              SourceRange Range,
-                                              const Expr *FreeExpr,
-                                              AllocationFamily Family) const {
-  if (!ChecksEnabled[CK_MallocChecker])
+void MallocChecker::HandleFunctionPtrFree(CheckerContext &C, SVal ArgVal,
+                                          SourceRange Range,
+                                          const Expr *FreeExpr,
+                                          AllocationFamily Family) const {
+  if (!ChecksEnabled[CK_MallocChecker]) {
+    C.addSink();
     return;
+  }
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
   if (!CheckKind.hasValue())
@@ -2521,7 +2546,7 @@ MallocChecker::LeakInfo MallocChecker::getAllocationSite(const ExplodedNode *N,
   return LeakInfo(AllocNode, ReferenceRegion);
 }
 
-void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N,
+void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N,
                                CheckerContext &C) const {
 
   if (!ChecksEnabled[CK_MallocChecker] &&
@@ -2637,7 +2662,7 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
     if (N) {
       for (SmallVectorImpl<SymbolRef>::iterator
            I = Errors.begin(), E = Errors.end(); I != E; ++I) {
-        reportLeak(*I, N, C);
+        HandleLeak(*I, N, C);
       }
     }
   }
@@ -2822,7 +2847,7 @@ bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
                                       const Stmt *S) const {
 
   if (isReleased(Sym, C)) {
-    ReportUseAfterFree(C, S->getSourceRange(), Sym);
+    HandleUseAfterFree(C, S->getSourceRange(), Sym);
     return true;
   }
 
@@ -2835,17 +2860,17 @@ void MallocChecker::checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C,
 
   if (const RefState *RS = C.getState()->get<RegionState>(Sym)) {
     if (RS->isAllocatedOfSizeZero())
-      ReportUseZeroAllocated(C, RS->getStmt()->getSourceRange(), Sym);
+      HandleUseZeroAlloc(C, RS->getStmt()->getSourceRange(), Sym);
   }
   else if (C.getState()->contains<ReallocSizeZeroSymbols>(Sym)) {
-    ReportUseZeroAllocated(C, S->getSourceRange(), Sym);
+    HandleUseZeroAlloc(C, S->getSourceRange(), Sym);
   }
 }
 
 bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const {
 
   if (isReleased(Sym, C)) {
-    ReportDoubleDelete(C, Sym);
+    HandleDoubleDelete(C, Sym);
     return true;
   }
   return false;

diff  --git a/clang/test/Analysis/Malloc+NewDelete_intersections.cpp b/clang/test/Analysis/Malloc+NewDelete_intersections.cpp
deleted file mode 100644
index 9140e1f4a372..000000000000
--- a/clang/test/Analysis/Malloc+NewDelete_intersections.cpp
+++ /dev/null
@@ -1,15 +0,0 @@
-// 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'}}

diff  --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp
index ba179749510c..f0d42171a875 100644
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -1,42 +1,31 @@
-// 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"
 
@@ -52,50 +41,28 @@ int *global;
 //----- Standard non-placement operators
 void testGlobalOpNew() {
   void *p = operator new(0);
-}
-#ifdef LEAKS
-// expected-warning at -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 at -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 at -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 at -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 at -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 at -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() {
@@ -135,13 +102,13 @@ void testNewInvalidationPlacement(PtrWrapper *w) {
 
 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;
 }
 
@@ -149,7 +116,7 @@ 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;
 }
 
@@ -168,70 +135,68 @@ 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 at -2{{Argument to 'delete[]' is offset by 4 bytes from the start of memory allocated by 'new[]'}}
-#endif
+  // newdelete-warning at -1{{Argument to 'delete[]' is offset by 4 bytes from the start of memory allocated by 'new[]'}}
 }
 
 //--------------------------------
@@ -408,7 +373,7 @@ class DerefClass{
 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{
@@ -420,7 +385,7 @@ class 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 {

diff  --git a/clang/test/Analysis/NewDelete-intersections.mm b/clang/test/Analysis/NewDelete-intersections.mm
index b3707858f00c..f01d62f8d365 100644
--- a/clang/test/Analysis/NewDelete-intersections.mm
+++ b/clang/test/Analysis/NewDelete-intersections.mm
@@ -1,7 +1,20 @@
-// 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"
 
@@ -10,12 +23,6 @@
 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
@@ -39,7 +46,8 @@ void testMallocFreeNoWarn() {
 
 void testDeleteMalloced() {
   int *p1 = (int *)malloc(sizeof(int));
-  delete p1; // no warn
+  delete p1;
+  // mismatch-warning at -1{{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 
   int *p2 = (int *)__builtin_alloca(sizeof(int));
   delete p2; // no warn
@@ -54,35 +62,30 @@ void testUseZeroAllocatedMalloced() {
 void testFreeOpNew() {
   void *p = operator new(0);
   free(p);
+  // mismatch-warning at -1{{Memory allocated by operator new should be deallocated by 'delete', not free()}}
 }
-#ifdef LEAKS
-// expected-warning at -2 {{Potential leak of memory pointed to by 'p'}}
-#endif
 
 void testFreeNewExpr() {
   int *p = new int;
   free(p);
+  // mismatch-warning at -1{{Memory allocated by 'new' should be deallocated by 'delete', not free()}}
+  free(p);
 }
-#ifdef LEAKS
-// expected-warning at -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 at -2 {{Potential leak of memory pointed to by 'p'}}
-#endif
+  // mismatch-warning at -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}}
 }

diff  --git a/clang/test/Analysis/new.cpp b/clang/test/Analysis/new.cpp
index 3384cfeb6141..2c3eb2825a6b 100644
--- a/clang/test/Analysis/new.cpp
+++ b/clang/test/Analysis/new.cpp
@@ -115,11 +115,6 @@ void testUseAfter(int *p) {
   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;
@@ -135,11 +130,11 @@ void testNewDeleteNoWarn() {
   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));


        


More information about the llvm-branch-commits mailing list