frederic-tingaud-sonarsource created this revision.
frederic-tingaud-sonarsource added a reviewer: dcoughlin.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy,
Szelethus, a.sidorin, baloghadamsoftware.
Herald added a project: All.
frederic-tingaud-sonarsource requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Currently, when encountering an overridden new operator, the StaticAnalyzer
will inline it when possible, while an overridden delete operator will never
be, which leads us to false positives like the following:
struct CustomOperators {
void *operator new(size_t count) {
return malloc(count);
}
void operator delete(void *addr) {
free(addr);
}
};
void compliant() {
auto *a = new CustomOperators();
delete a; // warning{{Potential leak of memory pointed to by 'a'}}
}
This patch restores the symmetry between how operator new and operator delete
are handled by also inlining the content of operator delete when possible.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124845
Files:
clang/include/clang/Analysis/ConstructionContext.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/test/Analysis/cxxnewexpr-callback-inline.cpp
clang/test/Analysis/cxxnewexpr-callback-noinline.cpp
clang/test/Analysis/dtor.cpp
Index: clang/test/Analysis/dtor.cpp
===================================================================
--- clang/test/Analysis/dtor.cpp
+++ clang/test/Analysis/dtor.cpp
@@ -570,3 +570,32 @@
// 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}}
+}
Index: clang/test/Analysis/cxxnewexpr-callback-noinline.cpp
===================================================================
--- clang/test/Analysis/cxxnewexpr-callback-noinline.cpp
+++ clang/test/Analysis/cxxnewexpr-callback-noinline.cpp
@@ -3,13 +3,16 @@
#include "Inputs/system-header-simulator-cxx.h"
namespace std {
- void *malloc(size_t);
-}
+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();
@@ -17,13 +20,35 @@
void test() {
S *s = new S();
foo();
+ delete s;
}
+/*
+void test() {
+ S *s = new S();
// CHECK: 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-NEXT: PostCall (operator delete)
+}
+
+void operator delete(void *ptr) {
+ std::free(ptr);
+// CHECK-NEXT: PreCall (std::free)
+// CHECK-NEXT: PostCall (std::free)
+}
+
+void *operator new(size_t size) {
+ return std::malloc(size);
// CHECK-NEXT: PreCall (std::malloc)
// CHECK-NEXT: PostCall (std::malloc)
+}
+*/
Index: clang/test/Analysis/cxxnewexpr-callback-inline.cpp
===================================================================
--- clang/test/Analysis/cxxnewexpr-callback-inline.cpp
+++ clang/test/Analysis/cxxnewexpr-callback-inline.cpp
@@ -4,12 +4,15 @@
namespace std {
void *malloc(size_t);
+ void free(void *);
}
void *operator new(size_t size) { return std::malloc(size); }
+void operator delete(void *ptr) { std::free(ptr); }
struct S {
S() {}
+ ~S() {}
};
void foo();
@@ -17,8 +20,12 @@
void test() {
S *s = new S();
foo();
+ delete s;
}
+/*
+void test() {
+ S *s = new S();
// CHECK: PreCall (operator new)
// CHECK-NEXT: PreCall (std::malloc)
// CHECK-NEXT: PostCall (std::malloc)
@@ -28,5 +35,15 @@
// 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-NEXT: PreCall (std::free)
+// CHECK-NEXT: PostCall (std::free)
+// CHECK-NEXT: PostCall (operator delete)
+}
+*/
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -945,8 +945,17 @@
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,
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -1408,6 +1408,8 @@
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 {
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1276,7 +1276,7 @@
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);
Index: clang/include/clang/Analysis/ConstructionContext.h
===================================================================
--- clang/include/clang/Analysis/ConstructionContext.h
+++ clang/include/clang/Analysis/ConstructionContext.h
@@ -120,7 +120,8 @@
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)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
