[analyzer] Inline operator delete when MayInlineCXXAllocator is set.

This patch restores the symmetry between how operator new and operator delete
are handled by also inlining the content of operator delete when possible.

Patch by Fred Tingaud.

Reviewed By: martong

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

diff --git a/clang/include/clang/Analysis/ConstructionContext.h b/clang/include/clang/Analysis/ConstructionContext.h
index 4fa5c8b..a437160 100644 (file)
--- a/clang/include/clang/Analysis/ConstructionContext.h
+++ b/clang/include/clang/Analysis/ConstructionContext.h
@@ -120,7 +120,8 @@ public:
   ConstructionContextItem(const Expr *E, unsigned Index)
       : Data(E), Kind(ArgumentKind), Index(Index) {
     assert(isa<CallExpr>(E) || isa<CXXConstructExpr>(E) ||
-           isa<CXXInheritedCtorInitExpr>(E) || isa<ObjCMessageExpr>(E));
+           isa<CXXDeleteExpr>(E) || isa<CXXInheritedCtorInitExpr>(E) ||
+           isa<ObjCMessageExpr>(E));
   }
 
   ConstructionContextItem(const CXXCtorInitializer *Init)

diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index 7b976dd..be356b5 100644 (file)
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -128,7 +128,8 @@ ANALYZER_OPTION(bool, MayInlineCXXStandardLibrary, "c++-stdlib-inlining",
                 true)
 
 ANALYZER_OPTION(bool, MayInlineCXXAllocator, "c++-allocator-inlining",
-                "Whether or not allocator call may be considered for inlining.",
+                "Whether or not allocator and deallocator calls may be "
+                "considered for inlining.",
                 true)
 
 ANALYZER_OPTION(

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index bfaeb06..fba9aa7 100644 (file)
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1276,7 +1276,7 @@ public:
   getCaller(const StackFrameContext *CalleeCtx, ProgramStateRef State);
 
   /// Gets a call event for a function call, Objective-C method call,
-  /// or a 'new' call.
+  /// a 'new', or a 'delete' call.
   CallEventRef<>
   getCall(const Stmt *S, ProgramStateRef State,
           const LocationContext *LC);

diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index ae46709..1e61c54 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -1408,6 +1408,8 @@ CallEventRef<> CallEventManager::getCall(const Stmt *S, ProgramStateRef State,
     return getSimpleCall(CE, State, LC);
   } else if (const auto *NE = dyn_cast<CXXNewExpr>(S)) {
     return getCXXAllocatorCall(NE, State, LC);
+  } else if (const auto *DE = dyn_cast<CXXDeleteExpr>(S)) {
+    return getCXXDeallocatorCall(DE, State, LC);
   } else if (const auto *ME = dyn_cast<ObjCMessageExpr>(S)) {
     return getObjCMethodCall(ME, State, LC);
   } else {

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 0a6127d..6d979da 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -945,8 +945,17 @@ void ExprEngine::VisitCXXDeleteExpr(const CXXDeleteExpr *CDE,
 
   ExplodedNodeSet DstPreCall;
   getCheckerManager().runCheckersForPreCall(DstPreCall, Pred, *Call, *this);
+  ExplodedNodeSet DstPostCall;
 
-  getCheckerManager().runCheckersForPostCall(Dst, DstPreCall, *Call, *this);
+  if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) {
+    StmtNodeBuilder Bldr(DstPreCall, DstPostCall, *currBldrCtx);
+    for (ExplodedNode *I : DstPreCall) {
+      defaultEvalCall(Bldr, I, *Call);
+    }
+  } else {
+    DstPostCall = DstPreCall;
+  }
+  getCheckerManager().runCheckersForPostCall(Dst, DstPostCall, *Call, *this);
 }
 
 void ExprEngine::VisitCXXCatchStmt(const CXXCatchStmt *CS, ExplodedNode *Pred,

diff --git a/clang/test/Analysis/cxxnewexpr-callback-inline.cpp b/clang/test/Analysis/cxxnewexpr-callback-inline.cpp
deleted file mode 100644 (file)
index c823de8..0000000
--- a/clang/test/Analysis/cxxnewexpr-callback-inline.cpp
+++ /dev/null
@@ -1,32 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config c++-allocator-inlining=true,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true %s 2>&1 | FileCheck %s
-
-#include "Inputs/system-header-simulator-cxx.h"
-
-namespace std {
-  void *malloc(size_t);
-}
-
-void *operator new(size_t size) { return std::malloc(size); }
-
-struct S {
-  S() {}
-};
-
-void foo();
-
-void test() {
-  S *s = new S();
-  foo();
-}
-
-// CHECK:      PreCall (operator new)
-// CHECK-NEXT: PreCall (std::malloc)
-// CHECK-NEXT: PostCall (std::malloc)
-// CHECK-NEXT: PostCall (operator new)
-// CHECK-NEXT: NewAllocator
-// CHECK-NEXT: PreCall (S::S)
-// CHECK-NEXT: PostCall (S::S)
-// CHECK-NEXT: PreStmt<CXXNewExpr>
-// CHECK-NEXT: PostStmt<CXXNewExpr>
-// CHECK-NEXT: PreCall (foo)
-// CHECK-NEXT: PostCall (foo)

diff --git a/clang/test/Analysis/cxxnewexpr-callback-noinline.cpp b/clang/test/Analysis/cxxnewexpr-callback-noinline.cpp
deleted file mode 100644 (file)
index 87edeac..0000000
--- a/clang/test/Analysis/cxxnewexpr-callback-noinline.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config c++-allocator-inlining=false,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true %s 2>&1 | FileCheck %s
-
-#include "Inputs/system-header-simulator-cxx.h"
-
-namespace std {
-  void *malloc(size_t);
-}
-
-void *operator new(size_t size) { return std::malloc(size); }
-
-struct S {
-  S() {}
-};
-
-void foo();
-
-void test() {
-  S *s = new S();
-  foo();
-}
-
-// CHECK:      PreCall (S::S)
-// CHECK-NEXT: PostCall (S::S)
-// CHECK-NEXT: PreStmt<CXXNewExpr>
-// CHECK-NEXT: PostStmt<CXXNewExpr>
-// CHECK-NEXT: PreCall (foo)
-// CHECK-NEXT: PostCall (foo)
-// CHECK-NEXT: PreCall (std::malloc)
-// CHECK-NEXT: PostCall (std::malloc)

diff --git a/clang/test/Analysis/cxxnewexpr-callback.cpp b/clang/test/Analysis/cxxnewexpr-callback.cpp
new file mode 100644 (file)
index 0000000..fe7a9ff
--- /dev/null
+++ b/clang/test/Analysis/cxxnewexpr-callback.cpp
@@ -0,0 +1,63 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config c++-allocator-inlining=true,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true %s 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-INLINE
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config c++-allocator-inlining=false,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true %s 2>&1 | FileCheck %s  --check-prefixes=CHECK,CHECK-NO-INLINE
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+namespace std {
+void *malloc(size_t);
+void free(void *);
+} // namespace std
+
+void *operator new(size_t size) { return std::malloc(size); }
+void operator delete(void *ptr) { std::free(ptr); }
+
+struct S {
+  S() {}
+  ~S() {}
+};
+
+void foo();
+
+void test() {
+  S *s = new S();
+  foo();
+  delete s;
+}
+
+/*
+void test() {
+  S *s = new S();
+// CHECK-INLINE:      PreCall (operator new)
+// CHECK-INLINE-NEXT: PreCall (std::malloc)
+// CHECK-INLINE-NEXT: PostCall (std::malloc)
+// CHECK-INLINE-NEXT: PostCall (operator new)
+// CHECK-INLINE-NEXT: NewAllocator
+// CHECK-NO-INLINE: PreCall (S::S)
+// CHECK-INLINE-NEXT: PreCall (S::S)
+// CHECK-NEXT: PostCall (S::S)
+// CHECK-NEXT: PreStmt<CXXNewExpr>
+// CHECK-NEXT: PostStmt<CXXNewExpr>
+  foo();
+// CHECK-NEXT: PreCall (foo)
+// CHECK-NEXT: PostCall (foo)
+  delete s;
+// CHECK-NEXT: PreCall (S::~S)
+// CHECK-NEXT: PostCall (S::~S)
+// CHECK-NEXT: PreCall (operator delete)
+// CHECK-INLINE-NEXT: PreCall (std::free)
+// CHECK-INLINE-NEXT: PostCall (std::free)
+// CHECK-NEXT: PostCall (operator delete)
+}
+
+void operator delete(void *ptr) {
+  std::free(ptr);
+// CHECK-NO-INLINE-NEXT: PreCall (std::free)
+// CHECK-NO-INLINE-NEXT: PostCall (std::free)
+}
+
+void *operator new(size_t size) {
+  return std::malloc(size);
+// CHECK-NO-INLINE-NEXT: PreCall (std::malloc)
+// CHECK-NO-INLINE-NEXT: PostCall (std::malloc)
+}
+*/

diff --git a/clang/test/Analysis/dtor.cpp b/clang/test/Analysis/dtor.cpp
index 2e984fc..7935fac 100644 (file)
--- a/clang/test/Analysis/dtor.cpp
+++ b/clang/test/Analysis/dtor.cpp
@@ -570,3 +570,32 @@ void testAutoDtor() {
   // no-crash
 }
 } // namespace dtor_over_loc_concrete_int
+
+// Test overriden new/delete operators
+struct CustomOperators {
+  void *operator new(size_t count) {
+    return malloc(count);
+  }
+
+  void operator delete(void *addr) {
+    free(addr);
+  }
+
+private:
+  int i;
+};
+
+void compliant() {
+  auto *a = new CustomOperators();
+  delete a;
+}
+
+void overrideLeak() {
+  auto *a = new CustomOperators();
+} // expected-warning{{Potential leak of memory pointed to by 'a'}}
+
+void overrideDoubleDelete() {
+  auto *a = new CustomOperators();
+  delete a;
+  delete a; // expected-warning@581 {{Attempt to free released memory}}
+}