Szelethus created this revision.
Szelethus added reviewers: balazske, martong, NoQ, xazax.hun, rnkovacs,
dcoughlin, baloghadamsoftware.
Herald added subscribers: cfe-commits, steakhal, Charusso, gamesh411, dkrupp,
donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.
Herald added a project: clang.
Szelethus added a parent revision: D68165: [analyzer][MallocChecker][NFC] Split
checkPostCall up, deploy CallDescriptionMap.
This is my first ever patch touching `ExprEngine`, sorry if the code causes
lasting damage to the reader.
One of the pain points in simplifying `MallocChecker`s interface by gradually
changing to `CallEvent` is that a variety of C++ allocation and deallocation
functionalities are modeled through `preStmt<...>` where `CallEvent` is
unavailable, and a single one of these callbacks can prevent a mass parameter
change.
This patch introduces a new `CallEvent`, `CXXDeallocatorCall`, which happens
//after// `preStmt<CXXDeleteExpr>`, and can completely replace that callback as
demonstrated. Note how you can retrieve this call through `preCall`, yet there
is no `postCall` to pair it with -- the reason behind this is that neither does
`preStmt<CXXDeleteExpr>` have a `postStmt<CXXDeleteExpr>` pair. But I have no
clue why that is :^)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75430
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/operator-delete-analysis-order.cpp
Index: clang/test/Analysis/operator-delete-analysis-order.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/operator-delete-analysis-order.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=debug.AnalysisOrder \
+// RUN: -analyzer-config debug.AnalysisOrder:PreStmtCXXDeleteExpr=true \
+// RUN: -analyzer-config debug.AnalysisOrder:PostStmtCXXDeleteExpr=true \
+// RUN: -analyzer-config debug.AnalysisOrder:PreCall=true \
+// RUN: -analyzer-config debug.AnalysisOrder:PostCall=true \
+// RUN: 2>&1 | FileCheck %s
+
+// expected-no-diagnostics
+
+namespace fundamental_dealloc {
+void f() {
+ int *p = new int;
+ delete p;
+}
+} // namespace fundamental_dealloc
+
+namespace record_dealloc {
+struct S {
+ int x, y;
+};
+
+void f() {
+ S *s = new S;
+ delete s;
+}
+} // namespace record_dealloc
+
+namespace nontrivial_destructor {
+struct NontrivialDtor {
+ int x, y;
+
+ ~NontrivialDtor() {
+ // Casually mine bitcoin.
+ }
+};
+
+void f() {
+ NontrivialDtor *s = new NontrivialDtor;
+ delete s;
+}
+} // namespace nontrivial_destructor
+
+// Mind that the results here are in reverse order compared how functions are
+// defined in this file.
+
+// CHECK: PreCall (operator new)
+// CHECK-NEXT: PostCall (operator new)
+// CHECK-NEXT: PreCall (nontrivial_destructor::NontrivialDtor::NontrivialDtor)
+// CHECK-NEXT: PostCall (nontrivial_destructor::NontrivialDtor::NontrivialDtor)
+// CHECK-NEXT: PreCall (nontrivial_destructor::NontrivialDtor::~NontrivialDtor)
+// CHECK-NEXT: PostCall (nontrivial_destructor::NontrivialDtor::~NontrivialDtor)
+// CHECK-NEXT: PreStmt<CXXDeleteExpr>
+// CHECK-NEXT: PreCall (operator delete)
+
+// CHECK: PreCall (operator new)
+// CHECK-NEXT: PostCall (operator new)
+// CHECK-NEXT: PreCall (record_dealloc::S::S)
+// CHECK-NEXT: PostCall (record_dealloc::S::S)
+// CHECK-NEXT: PreStmt<CXXDeleteExpr>
+// CHECK-NEXT: PreCall (operator delete)
+
+// CHECK: PreCall (operator new)
+// CHECK-NEXT: PostCall (operator new)
+// CHECK-NEXT: PreStmt<CXXDeleteExpr>
+// CHECK-NEXT: PreCall (operator delete)
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -36,17 +36,22 @@
// CHECK-NEXT: deadcode.DeadStores:WarnForDeadNestedAssignments = true
// CHECK-NEXT: debug.AnalysisOrder:* = false
// CHECK-NEXT: debug.AnalysisOrder:Bind = false
+// CHECK-NEXT: debug.AnalysisOrder:EndAnalysis = false
// CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
// CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
// CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
// CHECK-NEXT: debug.AnalysisOrder:PostCall = false
// CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXConstructExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXDeleteExpr = false
// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
// CHECK-NEXT: debug.AnalysisOrder:PostStmtCastExpr = false
// CHECK-NEXT: debug.AnalysisOrder:PostStmtOffsetOfExpr = false
// CHECK-NEXT: debug.AnalysisOrder:PreCall = false
// CHECK-NEXT: debug.AnalysisOrder:PreStmtArraySubscriptExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtCXXConstructExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtCXXDeleteExpr = false
// CHECK-NEXT: debug.AnalysisOrder:PreStmtCXXNewExpr = false
// CHECK-NEXT: debug.AnalysisOrder:PreStmtCastExpr = false
// CHECK-NEXT: debug.AnalysisOrder:PreStmtOffsetOfExpr = false
@@ -100,4 +105,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 97
+// CHECK-NEXT: num-entries = 102
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -846,9 +846,11 @@
void ExprEngine::VisitCXXDeleteExpr(const CXXDeleteExpr *CDE,
ExplodedNode *Pred, ExplodedNodeSet &Dst) {
- StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
- ProgramStateRef state = Pred->getState();
- Bldr.generateNode(CDE, Pred, state);
+
+ CallEventManager &CEMgr = getStateManager().getCallEventManager();
+ CallEventRef<CXXDeallocatorCall> Call = CEMgr.getCXXDeallocatorCall(
+ CDE, Pred->getState(), Pred->getLocationContext());
+ getCheckerManager().runCheckersForPreCall(Dst, Pred, *Call, *this);
}
void ExprEngine::VisitCXXCatchStmt(const CXXCatchStmt *CS,
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -281,8 +281,8 @@
check::ConstPointerEscape, check::PreStmt<ReturnStmt>,
check::EndFunction, check::PreCall, check::PostCall,
check::PostStmt<CXXNewExpr>, check::NewAllocator,
- check::PreStmt<CXXDeleteExpr>, check::PostStmt<BlockExpr>,
- check::PostObjCMessage, check::Location, eval::Assume> {
+ check::PostStmt<BlockExpr>, check::PostObjCMessage,
+ check::Location, eval::Assume> {
public:
/// In pessimistic mode, the checker assumes that it does not know which
/// functions might free the memory.
@@ -314,7 +314,6 @@
void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const;
void checkNewAllocator(const CXXNewExpr *NE, SVal Target,
CheckerContext &C) const;
- void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
@@ -1378,25 +1377,6 @@
return State;
}
-void MallocChecker::checkPreStmt(const CXXDeleteExpr *DE,
- CheckerContext &C) const {
-
- if (!ChecksEnabled[CK_NewDeleteChecker])
- if (SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol())
- checkUseAfterFree(Sym, C, DE->getArgument());
-
- if (!isStandardNewDelete(DE->getOperatorDelete()))
- return;
-
- ProgramStateRef State = C.getState();
- bool IsKnownToBeAllocated;
- State = FreeMemAux(C, DE->getArgument(), DE, State,
- /*Hold*/ false, IsKnownToBeAllocated,
- (DE->isArrayForm() ? AF_CXXNewArray : AF_CXXNew));
-
- C.addTransition(State);
-}
-
static bool isKnownDeallocObjCMethodName(const ObjCMethodCall &Call) {
// If the first selector piece is one of the names below, assume that the
// object takes ownership of the memory, promising to eventually deallocate it
@@ -2580,7 +2560,27 @@
void MallocChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
- if (const CXXDestructorCall *DC = dyn_cast<CXXDestructorCall>(&Call)) {
+ if (const auto *DC = dyn_cast<CXXDeallocatorCall>(&Call)) {
+ const CXXDeleteExpr *DE = DC->getOriginExpr();
+
+ if (!ChecksEnabled[CK_NewDeleteChecker])
+ if (SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol())
+ checkUseAfterFree(Sym, C, DE->getArgument());
+
+ if (!isStandardNewDelete(DC->getDecl()))
+ return;
+
+ ProgramStateRef State = C.getState();
+ bool IsKnownToBeAllocated;
+ State = FreeMemAux(C, DE->getArgument(), DE, State,
+ /*Hold*/ false, IsKnownToBeAllocated,
+ (DE->isArrayForm() ? AF_CXXNewArray : AF_CXXNew));
+
+ C.addTransition(State);
+ return;
+ }
+
+ if (const auto *DC = dyn_cast<CXXDestructorCall>(&Call)) {
SymbolRef Sym = DC->getCXXThisVal().getAsSymbol();
if (!Sym || checkDoubleDelete(Sym, C))
return;
Index: clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
@@ -33,18 +33,23 @@
check::PostStmt<ArraySubscriptExpr>,
check::PreStmt<CXXNewExpr>,
check::PostStmt<CXXNewExpr>,
+ check::PreStmt<CXXDeleteExpr>,
+ check::PostStmt<CXXDeleteExpr>,
+ check::PreStmt<CXXConstructExpr>,
+ check::PostStmt<CXXConstructExpr>,
check::PreStmt<OffsetOfExpr>,
check::PostStmt<OffsetOfExpr>,
check::PreCall,
check::PostCall,
check::EndFunction,
+ check::EndAnalysis,
check::NewAllocator,
check::Bind,
check::PointerEscape,
check::RegionChanges,
check::LiveSymbols> {
- bool isCallbackEnabled(AnalyzerOptions &Opts, StringRef CallbackName) const {
+ bool isCallbackEnabled(const AnalyzerOptions &Opts, StringRef CallbackName) const {
return Opts.getCheckerBooleanOption(this, "*") ||
Opts.getCheckerBooleanOption(this, CallbackName);
}
@@ -95,6 +100,26 @@
llvm::errs() << "PostStmt<CXXNewExpr>\n";
}
+ void checkPreStmt(const CXXDeleteExpr *NE, CheckerContext &C) const {
+ if (isCallbackEnabled(C, "PreStmtCXXDeleteExpr"))
+ llvm::errs() << "PreStmt<CXXDeleteExpr>\n";
+ }
+
+ void checkPostStmt(const CXXDeleteExpr *NE, CheckerContext &C) const {
+ if (isCallbackEnabled(C, "PostStmtCXXDeleteExpr"))
+ llvm::errs() << "PostStmt<CXXDeleteExpr>\n";
+ }
+
+ void checkPreStmt(const CXXConstructExpr *NE, CheckerContext &C) const {
+ if (isCallbackEnabled(C, "PreStmtCXXConstructExpr"))
+ llvm::errs() << "PreStmt<CXXConstructExpr>\n";
+ }
+
+ void checkPostStmt(const CXXConstructExpr *NE, CheckerContext &C) const {
+ if (isCallbackEnabled(C, "PostStmtCXXConstructExpr"))
+ llvm::errs() << "PostStmt<CXXConstructExpr>\n";
+ }
+
void checkPreStmt(const OffsetOfExpr *OOE, CheckerContext &C) const {
if (isCallbackEnabled(C, "PreStmtOffsetOfExpr"))
llvm::errs() << "PreStmt<OffsetOfExpr>\n";
@@ -140,6 +165,13 @@
}
}
+ void checkEndAnalysis(ExplodedGraph &G,
+ BugReporter &BR,
+ ExprEngine &Eng) const {
+ if (isCallbackEnabled(BR.getAnalyzerOptions(), "EndAnalysis"))
+ llvm::errs() << "EndAnalysis\n";
+ }
+
void checkNewAllocator(const CXXNewExpr *CNE, SVal Target,
CheckerContext &C) const {
if (isCallbackEnabled(C, "NewAllocator"))
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
@@ -64,6 +64,7 @@
CE_END_CXX_INSTANCE_CALLS = CE_CXXDestructor,
CE_CXXConstructor,
CE_CXXAllocator,
+ CE_CXXDeallocator,
CE_BEG_FUNCTION_CALLS = CE_Function,
CE_END_FUNCTION_CALLS = CE_CXXAllocator,
CE_Block,
@@ -929,6 +930,44 @@
}
};
+/// Represents the memory deallocation call in a C++ delete-expression.
+///
+/// This is a call to "operator delete".
+class CXXDeallocatorCall : public AnyFunctionCall {
+ friend class CallEventManager;
+
+protected:
+ CXXDeallocatorCall(const CXXDeleteExpr *E, ProgramStateRef St,
+ const LocationContext *LCtx)
+ : AnyFunctionCall(E, St, LCtx) {}
+ CXXDeallocatorCall(const CXXDeallocatorCall &Other) = default;
+
+ void cloneTo(void *Dest) const override {
+ new (Dest) CXXDeallocatorCall(*this);
+ }
+
+public:
+ virtual const CXXDeleteExpr *getOriginExpr() const {
+ return cast<CXXDeleteExpr>(AnyFunctionCall::getOriginExpr());
+ }
+
+ const FunctionDecl *getDecl() const override {
+ return getOriginExpr()->getOperatorDelete();
+ }
+
+ unsigned getNumArgs() const override { return getDecl()->getNumParams(); }
+
+ const Expr *getArgExpr(unsigned Index = 0) const override {
+ return getOriginExpr()->getArgument();
+ }
+
+ Kind getKind() const override { return CE_CXXDeallocator; }
+
+ static bool classof(const CallEvent *CE) {
+ return CE->getKind() == CE_CXXDeallocator;
+ }
+};
+
/// Represents the ways an Objective-C message send can occur.
//
// Note to maintainers: OCM_Message should always be last, since it does not
@@ -1244,6 +1283,12 @@
const LocationContext *LCtx) {
return create<CXXAllocatorCall>(E, State, LCtx);
}
+
+ CallEventRef<CXXDeallocatorCall>
+ getCXXDeallocatorCall(const CXXDeleteExpr *E, ProgramStateRef State,
+ const LocationContext *LCtx) {
+ return create<CXXDeallocatorCall>(E, State, LCtx);
+ }
};
template <typename T>
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1268,6 +1268,30 @@
"false",
Released,
Hide>,
+ CmdLineOption<Boolean,
+ "PreStmtCXXDeleteExpr",
+ "",
+ "false",
+ Released,
+ Hide>,
+ CmdLineOption<Boolean,
+ "PostStmtCXXDeleteExpr",
+ "",
+ "false",
+ Released,
+ Hide>,
+ CmdLineOption<Boolean,
+ "PreStmtCXXConstructExpr",
+ "",
+ "false",
+ Released,
+ Hide>,
+ CmdLineOption<Boolean,
+ "PostStmtCXXConstructExpr",
+ "",
+ "false",
+ Released,
+ Hide>,
CmdLineOption<Boolean,
"PreStmtOffsetOfExpr",
"",
@@ -1298,6 +1322,12 @@
"false",
Released,
Hide>,
+ CmdLineOption<Boolean,
+ "EndAnalysis",
+ "",
+ "false",
+ Released,
+ Hide>,
CmdLineOption<Boolean,
"NewAllocator",
"",
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
