Szelethus created this revision.
Szelethus added reviewers: NoQ, baloghadamsoftware, balazske, martong,
dcoughlin, xazax.hun, rnkovacs.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, steakhal, Charusso, gamesh411, dkrupp,
donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.
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 <https://reviews.llvm.org/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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75431
Files:
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
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -10,17 +10,18 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/AST/Decl.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "PrettyStackTraceLocationContext.h"
#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/Analysis/Analyses/LiveVariables.h"
#include "clang/Analysis/ConstructionContext.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#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/SaveAndRestore.h"
using namespace clang;
@@ -324,17 +325,14 @@
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 {
@@ -590,7 +588,7 @@
// 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;
@@ -604,7 +602,7 @@
// 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();
@@ -742,7 +740,7 @@
const ConstructionContext *CC = CCE ? CCE->getConstructionContext()
: nullptr;
- if (CC && isa<NewAllocatedObjectConstructionContext>(CC) &&
+ if (llvm::isa_and_nonnull<NewAllocatedObjectConstructionContext>(CC) &&
!Opts.MayInlineCXXAllocator)
return CIP_DisallowedOnce;
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -561,7 +561,7 @@
const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
const MemRegion *Target = Call->getCXXThisVal().getAsRegion();
- if (Target && isa<CXXTempObjectRegion>(Target) &&
+ if (llvm::isa_and_nonnull<CXXTempObjectRegion>(Target) &&
Call->getDecl()->getParent()->isAnyDestructorNoReturn()) {
// If we've inlined the constructor, then DstEvaluated would be empty.
@@ -586,7 +586,7 @@
}
ExplodedNodeSet DstPostArgumentCleanup;
- for (auto I : DstEvaluated)
+ for (ExplodedNode *I : DstEvaluated)
finishArgumentConstruction(DstPostArgumentCleanup, I, *Call);
// If there were other constructors called for object-type arguments
@@ -683,7 +683,7 @@
ExplodedNodeSet DstPostCall;
StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
- for (auto I : DstPreCall) {
+ for (ExplodedNode *I : DstPreCall) {
// FIXME: Provide evalCall for checkers?
defaultEvalCall(CallBldr, I, *Call);
}
@@ -693,7 +693,7 @@
// 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
@@ -727,10 +727,8 @@
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);
}
}
Index: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -243,7 +243,7 @@
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,37 @@
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(Call, 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 +653,7 @@
const ExplodedNodeSet &Src,
const CallEvent &Call,
ExprEngine &Eng) {
- for (const auto Pred : Src) {
+ for (auto *const Pred : Src) {
bool anyEvaluated = false;
ExplodedNodeSet checkDst;
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -312,8 +312,7 @@
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 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;
@@ -1323,11 +1322,12 @@
}
}
-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(C.getState()),
+ (Call.getOriginExpr()->isArray() ? AF_CXXNewArray : AF_CXXNew));
}
}
Index: clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
@@ -172,7 +172,7 @@
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";
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
@@ -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"
@@ -204,6 +205,10 @@
return Origin.dyn_cast<const Decl *>();
}
+ SValBuilder &getSValBuilder() const {
+ return State->getStateManager().getSValBuilder();
+ }
+
/// The state in which the call is being evaluated.
const ProgramStateRef &getState() const {
return State;
@@ -897,6 +902,12 @@
return getOriginExpr()->getOperatorNew();
}
+ SVal getObjectUnderConstruction(ProgramStateRef State) const {
+ return ExprEngine::getObjectUnderConstruction(State, 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.
Index: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -36,6 +36,7 @@
namespace ento {
class AnalysisManager;
+class CXXAllocatorCall;
class BugReporter;
class CallEvent;
class CheckerBase;
@@ -327,11 +328,9 @@
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.
///
@@ -472,7 +471,7 @@
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 &)>;
Index: clang/include/clang/StaticAnalyzer/Core/Checker.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -285,9 +285,9 @@
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:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
