[clang] 1ec1cdc - [analyzer] Inline operator delete when MayInlineCXXAllocator is set.

Tomasz KamiƄski via cfe-commits cfe-commits at lists.llvm.org
Mon May 9 06:48:02 PDT 2022


Author: Fred Tingaud
Date: 2022-05-09T15:44:33+02:00
New Revision: 1ec1cdcfb49aed24a634999ab90c4feb48100c3e

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

LOG: [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

Added: 
    clang/test/Analysis/cxxnewexpr-callback.cpp

Modified: 
    clang/include/clang/Analysis/ConstructionContext.h
    clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
    clang/lib/StaticAnalyzer/Core/CallEvent.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    clang/test/Analysis/dtor.cpp

Removed: 
    clang/test/Analysis/cxxnewexpr-callback-inline.cpp
    clang/test/Analysis/cxxnewexpr-callback-noinline.cpp


################################################################################
diff  --git a/clang/include/clang/Analysis/ConstructionContext.h b/clang/include/clang/Analysis/ConstructionContext.h
index 4fa5c8b454a03..a437160e07789 100644
--- a/clang/include/clang/Analysis/ConstructionContext.h
+++ b/clang/include/clang/Analysis/ConstructionContext.h
@@ -120,7 +120,8 @@ class ConstructionContextItem {
   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 7b976ddeaba24..be356b55a08c7 100644
--- 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 bfaeb06951d76..fba9aa722e1f7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1276,7 +1276,7 @@ class CallEventManager {
   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 ae46709340d3a..1e61c54a57cd3 100644
--- 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 0a6127d800152..6d979da2755fa 100644
--- 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
index c823de85821d3..0000000000000
--- 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
index 87edeac3fd580..0000000000000
--- 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
index 0000000000000..fe7a9fffad93d
--- /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 2e984fca39cb7..7935faca868ad 100644
--- 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 at 581 {{Attempt to free released memory}}
+}


        


More information about the cfe-commits mailing list