[clang] f2be30d - [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall
Kirstóf Umann via cfe-commits
cfe-commits at lists.llvm.org
Tue May 19 15:56:20 PDT 2020
Author: Kirstóf Umann
Date: 2020-05-20T00:56:10+02:00
New Revision: f2be30def37d248a7666cfac3c922ca9db308c2a
URL: https://github.com/llvm/llvm-project/commit/f2be30def37d248a7666cfac3c922ca9db308c2a
DIFF: https://github.com/llvm/llvm-project/commit/f2be30def37d248a7666cfac3c922ca9db308c2a.diff
LOG: [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall
Party based on this thread:
http://lists.llvm.org/pipermail/cfe-dev/2020-February/064754.html.
This patch merges two of CXXAllocatorCall's parameters, so that we are able to
supply a CallEvent object to check::NewAllocatorCall (see the description of
D75430 to see why this would be great).
One of the things mentioned by @NoQ was the following:
I think at this point we might actually do a good job sorting out this
check::NewAllocator issue because we have a "separate" "Environment" to hold
the other SVal, which is "objects under construction"! - so we should probably
simply teach CXXAllocatorCall to extract the value from the
objects-under-construction trait of the program state and we're good.
I had MallocChecker in my crosshair for now, so I admittedly threw together
something as a proof of concept. Now that I know that this effort is worth
pursuing though, I'll happily look for a solution better then demonstrated in
this patch.
Differential Revision: https://reviews.llvm.org/D75431
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/Checker.h
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index 0c7acdbc3a97..fdba49664615 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -285,9 +285,9 @@ class BranchCondition {
class NewAllocator {
template <typename CHECKER>
- static void _checkNewAllocator(void *checker, const CXXNewExpr *NE,
- SVal Target, CheckerContext &C) {
- ((const CHECKER *)checker)->checkNewAllocator(NE, Target, C);
+ static void _checkNewAllocator(void *checker, const CXXAllocatorCall &Call,
+ CheckerContext &C) {
+ ((const CHECKER *)checker)->checkNewAllocator(Call, C);
}
public:
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index f34f5c239290..820ccfceecf6 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -37,6 +37,7 @@ class TranslationUnitDecl;
namespace ento {
class AnalysisManager;
+class CXXAllocatorCall;
class BugReporter;
class CallEvent;
class CheckerBase;
@@ -361,11 +362,9 @@ class CheckerManager {
ExprEngine &Eng);
/// Run checkers between C++ operator new and constructor calls.
- void runCheckersForNewAllocator(const CXXNewExpr *NE, SVal Target,
- ExplodedNodeSet &Dst,
- ExplodedNode *Pred,
- ExprEngine &Eng,
- bool wasInlined = false);
+ void runCheckersForNewAllocator(const CXXAllocatorCall &Call,
+ ExplodedNodeSet &Dst, ExplodedNode *Pred,
+ ExprEngine &Eng, bool wasInlined = false);
/// Run checkers for live symbols.
///
@@ -506,7 +505,7 @@ class CheckerManager {
CheckerFn<void (const Stmt *, CheckerContext &)>;
using CheckNewAllocatorFunc =
- CheckerFn<void (const CXXNewExpr *, SVal, CheckerContext &)>;
+ CheckerFn<void(const CXXAllocatorCall &Call, CheckerContext &)>;
using CheckDeadSymbolsFunc =
CheckerFn<void (SymbolReaper &, CheckerContext &)>;
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 871875b45b1b..8b84b79651bd 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -39,6 +39,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/ErrorHandling.h"
@@ -1010,6 +1011,12 @@ class CXXAllocatorCall : public AnyFunctionCall {
return getOriginExpr()->getOperatorNew();
}
+ SVal getObjectUnderConstruction() const {
+ return ExprEngine::getObjectUnderConstruction(getState(), getOriginExpr(),
+ getLocationContext())
+ .getValue();
+ }
+
/// Number of non-placement arguments to the call. It is equal to 2 for
/// C++17 aligned operator new() calls that have alignment implicitly
/// passed as the second argument, and to 1 for other operator new() calls.
diff --git a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
index f44a40d8bd15..479e50b80867 100644
--- a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
@@ -165,7 +165,7 @@ class AnalysisOrderChecker
llvm::errs() << "EndAnalysis\n";
}
- void checkNewAllocator(const CXXNewExpr *CNE, SVal Target,
+ void checkNewAllocator(const CXXAllocatorCall &Call,
CheckerContext &C) const {
if (isCallbackEnabled(C, "NewAllocator"))
llvm::errs() << "NewAllocator\n";
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index d108f88776d4..edcee6e2d052 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -315,8 +315,8 @@ class MallocChecker
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
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 checkNewAllocator(const CXXAllocatorCall &Call, 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;
@@ -1357,11 +1357,12 @@ void MallocChecker::checkPostStmt(const CXXNewExpr *NE,
}
}
-void MallocChecker::checkNewAllocator(const CXXNewExpr *NE, SVal Target,
+void MallocChecker::checkNewAllocator(const CXXAllocatorCall &Call,
CheckerContext &C) const {
if (!C.wasInlined) {
- processNewAllocation(NE, C, Target,
- (NE->isArray() ? AF_CXXNewArray : AF_CXXNew));
+ processNewAllocation(
+ Call.getOriginExpr(), C, Call.getObjectUnderConstruction(),
+ (Call.getOriginExpr()->isArray() ? AF_CXXNewArray : AF_CXXNew));
}
}
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index 17c66cd23279..98504006030a 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -243,7 +243,7 @@ void CheckerManager::runCheckersForObjCMessage(ObjCMessageVisitKind visitKind,
const ObjCMethodCall &msg,
ExprEngine &Eng,
bool WasInlined) {
- auto &checkers = getObjCMessageCheckers(visitKind);
+ const auto &checkers = getObjCMessageCheckers(visitKind);
CheckObjCMessageContext C(visitKind, checkers, msg, Eng, WasInlined);
expandGraphWithCheckers(C, Dst, Src);
}
@@ -507,35 +507,38 @@ namespace {
using CheckersTy = std::vector<CheckerManager::CheckNewAllocatorFunc>;
const CheckersTy &Checkers;
- const CXXNewExpr *NE;
- SVal Target;
+ const CXXAllocatorCall &Call;
bool WasInlined;
ExprEngine &Eng;
- CheckNewAllocatorContext(const CheckersTy &Checkers, const CXXNewExpr *NE,
- SVal Target, bool WasInlined, ExprEngine &Eng)
- : Checkers(Checkers), NE(NE), Target(Target), WasInlined(WasInlined),
- Eng(Eng) {}
+ CheckNewAllocatorContext(const CheckersTy &Checkers,
+ const CXXAllocatorCall &Call, bool WasInlined,
+ ExprEngine &Eng)
+ : Checkers(Checkers), Call(Call), WasInlined(WasInlined), Eng(Eng) {}
CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); }
CheckersTy::const_iterator checkers_end() { return Checkers.end(); }
void runChecker(CheckerManager::CheckNewAllocatorFunc checkFn,
NodeBuilder &Bldr, ExplodedNode *Pred) {
- ProgramPoint L = PostAllocatorCall(NE, Pred->getLocationContext());
+ ProgramPoint L =
+ PostAllocatorCall(Call.getOriginExpr(), Pred->getLocationContext());
CheckerContext C(Bldr, Eng, Pred, L, WasInlined);
- checkFn(NE, Target, C);
+ checkFn(cast<CXXAllocatorCall>(*Call.cloneWithState(Pred->getState())),
+ C);
}
};
} // namespace
-void CheckerManager::runCheckersForNewAllocator(
- const CXXNewExpr *NE, SVal Target, ExplodedNodeSet &Dst, ExplodedNode *Pred,
- ExprEngine &Eng, bool WasInlined) {
+void CheckerManager::runCheckersForNewAllocator(const CXXAllocatorCall &Call,
+ ExplodedNodeSet &Dst,
+ ExplodedNode *Pred,
+ ExprEngine &Eng,
+ bool WasInlined) {
ExplodedNodeSet Src;
Src.insert(Pred);
- CheckNewAllocatorContext C(NewAllocatorCheckers, NE, Target, WasInlined, Eng);
+ CheckNewAllocatorContext C(NewAllocatorCheckers, Call, WasInlined, Eng);
expandGraphWithCheckers(C, Dst, Src);
}
@@ -651,7 +654,7 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst,
const ExplodedNodeSet &Src,
const CallEvent &Call,
ExprEngine &Eng) {
- for (const auto Pred : Src) {
+ for (auto *const Pred : Src) {
bool anyEvaluated = false;
ExplodedNodeSet checkDst;
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 7171dd220485..aee8286b9c72 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -577,9 +577,10 @@ void ExprEngine::handleConstructor(const Expr *E,
// paths when no-return temporary destructors are used for assertions.
const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
- if (TargetRegion && isa<CXXTempObjectRegion>(TargetRegion) &&
+ if (llvm::isa_and_nonnull<CXXTempObjectRegion>(TargetRegion) &&
cast<CXXConstructorDecl>(Call->getDecl())
- ->getParent()->isAnyDestructorNoReturn()) {
+ ->getParent()
+ ->isAnyDestructorNoReturn()) {
// If we've inlined the constructor, then DstEvaluated would be empty.
// In this case we still want a sink, which could be implemented
@@ -603,7 +604,7 @@ void ExprEngine::handleConstructor(const Expr *E,
}
ExplodedNodeSet DstPostArgumentCleanup;
- for (auto I : DstEvaluated)
+ for (ExplodedNode *I : DstEvaluated)
finishArgumentConstruction(DstPostArgumentCleanup, I, *Call);
// If there were other constructors called for object-type arguments
@@ -712,7 +713,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
ExplodedNodeSet DstPostCall;
StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
- for (auto I : DstPreCall) {
+ for (ExplodedNode *I : DstPreCall) {
// FIXME: Provide evalCall for checkers?
defaultEvalCall(CallBldr, I, *Call);
}
@@ -722,7 +723,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
// CXXNewExpr gets processed.
ExplodedNodeSet DstPostValue;
StmtNodeBuilder ValueBldr(DstPostCall, DstPostValue, *currBldrCtx);
- for (auto I : DstPostCall) {
+ for (ExplodedNode *I : DstPostCall) {
// FIXME: Because CNE serves as the "call site" for the allocator (due to
// lack of a better expression in the AST), the conjured return value symbol
// is going to be of the same type (C++ object pointer type). Technically
@@ -756,10 +757,8 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
ExplodedNodeSet DstPostPostCallCallback;
getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback,
DstPostValue, *Call, *this);
- for (auto I : DstPostPostCallCallback) {
- getCheckerManager().runCheckersForNewAllocator(
- CNE, *getObjectUnderConstruction(I->getState(), CNE, LCtx), Dst, I,
- *this);
+ for (ExplodedNode *I : DstPostPostCallCallback) {
+ getCheckerManager().runCheckersForNewAllocator(*Call, Dst, I, *this);
}
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 2ac0160e15c2..da7b5d9efa47 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -21,6 +21,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/Statistic.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/SaveAndRestore.h"
@@ -325,17 +326,14 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState);
ExplodedNodeSet DstPostCall;
- if (const CXXNewExpr *CNE = dyn_cast_or_null<CXXNewExpr>(CE)) {
+ if (llvm::isa_and_nonnull<CXXNewExpr>(CE)) {
ExplodedNodeSet DstPostPostCallCallback;
getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback,
CEENode, *UpdatedCall, *this,
/*wasInlined=*/true);
- for (auto I : DstPostPostCallCallback) {
+ for (ExplodedNode *I : DstPostPostCallCallback) {
getCheckerManager().runCheckersForNewAllocator(
- CNE,
- *getObjectUnderConstruction(I->getState(), CNE,
- calleeCtx->getParent()),
- DstPostCall, I, *this,
+ cast<CXXAllocatorCall>(*UpdatedCall), DstPostCall, I, *this,
/*wasInlined=*/true);
}
} else {
@@ -591,7 +589,7 @@ void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
// If there were other constructors called for object-type arguments
// of this call, clean them up.
ExplodedNodeSet dstArgumentCleanup;
- for (auto I : dstCallEvaluated)
+ for (ExplodedNode *I : dstCallEvaluated)
finishArgumentConstruction(dstArgumentCleanup, I, Call);
ExplodedNodeSet dstPostCall;
@@ -605,7 +603,7 @@ void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
// Run pointerEscape callback with the newly conjured symbols.
SmallVector<std::pair<SVal, SVal>, 8> Escaped;
- for (auto I : dstPostCall) {
+ for (ExplodedNode *I : dstPostCall) {
NodeBuilder B(I, Dst, *currBldrCtx);
ProgramStateRef State = I->getState();
Escaped.clear();
@@ -743,7 +741,7 @@ ExprEngine::mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred,
const ConstructionContext *CC = CCE ? CCE->getConstructionContext()
: nullptr;
- if (CC && isa<NewAllocatedObjectConstructionContext>(CC) &&
+ if (llvm::isa_and_nonnull<NewAllocatedObjectConstructionContext>(CC) &&
!Opts.MayInlineCXXAllocator)
return CIP_DisallowedOnce;
More information about the cfe-commits
mailing list