https://github.com/T-Gruber updated
https://github.com/llvm/llvm-project/pull/129016
>From 79d8f061476c6ba21bf48f55597eaaef345c2e80 Mon Sep 17 00:00:00 2001
From: "tobias.gruber" <tobias.gru...@concentrio.io>
Date: Wed, 26 Feb 2025 18:00:21 +0100
Subject: [PATCH 1/5] Trigger checkLocation for RHS of copy construction
---
.../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 250 +++++++++---------
1 file changed, 121 insertions(+), 129 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index f7020da2e6da2..fbb5e316bb068 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -69,6 +69,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr,
ExplodedNode *Pred,
assert(ThisRD);
SVal V = Call.getArgSVal(0);
+ const Expr *VExpr = Call.getArgExpr(0);
// If the value being copied is not unknown, load from its location to get
// an aggregate rvalue.
@@ -76,7 +77,11 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr,
ExplodedNode *Pred,
V = Pred->getState()->getSVal(*L);
else
assert(V.isUnknownOrUndef());
- evalBind(Dst, CallExpr, Pred, ThisVal, V, true);
+
+ ExplodedNodeSet Tmp;
+ evalLocation(Tmp, CallExpr, VExpr, Pred, Pred->getState(), V, true);
+ for (ExplodedNode *N : Tmp)
+ evalBind(Dst, CallExpr, N, ThisVal, V, true);
PostStmt PS(CallExpr, LCtx);
for (ExplodedNode *N : Dst) {
@@ -141,10 +146,9 @@ SVal ExprEngine::computeObjectUnderConstruction(
if (Init->isBaseInitializer()) {
const auto *ThisReg = cast<SubRegion>(ThisVal.getAsRegion());
const CXXRecordDecl *BaseClass =
- Init->getBaseClass()->getAsCXXRecordDecl();
- const auto *BaseReg =
- MRMgr.getCXXBaseObjectRegion(BaseClass, ThisReg,
- Init->isBaseVirtual());
+ Init->getBaseClass()->getAsCXXRecordDecl();
+ const auto *BaseReg = MRMgr.getCXXBaseObjectRegion(
+ BaseClass, ThisReg, Init->isBaseVirtual());
return SVB.makeLoc(BaseReg);
}
if (Init->isDelegatingInitializer())
@@ -183,7 +187,7 @@ SVal ExprEngine::computeObjectUnderConstruction(
return loc::MemRegionVal(R);
}
- return V;
+ return V;
}
// TODO: Detect when the allocator returns a null pointer.
// Constructor shall not be called in this case.
@@ -405,99 +409,99 @@ ProgramStateRef
ExprEngine::updateObjectsUnderConstruction(
case ConstructionContext::SimpleVariableKind: {
const auto *DSCC = cast<VariableConstructionContext>(CC);
return addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, V);
- }
- case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
- case ConstructionContext::SimpleConstructorInitializerKind: {
- const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
- const auto *Init = ICC->getCXXCtorInitializer();
- // Base and delegating initializers handled above
- assert(Init->isAnyMemberInitializer() &&
- "Base and delegating initializers should have been handled by"
- "computeObjectUnderConstruction()");
- return addObjectUnderConstruction(State, Init, LCtx, V);
- }
- case ConstructionContext::NewAllocatedObjectKind: {
+ }
+ case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
+ case ConstructionContext::SimpleConstructorInitializerKind: {
+ const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
+ const auto *Init = ICC->getCXXCtorInitializer();
+ // Base and delegating initializers handled above
+ assert(Init->isAnyMemberInitializer() &&
+ "Base and delegating initializers should have been handled by"
+ "computeObjectUnderConstruction()");
+ return addObjectUnderConstruction(State, Init, LCtx, V);
+ }
+ case ConstructionContext::NewAllocatedObjectKind: {
+ return State;
+ }
+ case ConstructionContext::SimpleReturnedValueKind:
+ case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
+ const StackFrameContext *SFC = LCtx->getStackFrame();
+ const LocationContext *CallerLCtx = SFC->getParent();
+ if (!CallerLCtx) {
+ // No extra work is necessary in top frame.
return State;
}
- case ConstructionContext::SimpleReturnedValueKind:
- case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
- const StackFrameContext *SFC = LCtx->getStackFrame();
- const LocationContext *CallerLCtx = SFC->getParent();
- if (!CallerLCtx) {
- // No extra work is necessary in top frame.
- return State;
- }
- auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
- .getAs<CFGCXXRecordTypedCall>();
- assert(RTC && "Could not have had a target region without it");
- if (isa<BlockInvocationContext>(CallerLCtx)) {
- // Unwrap block invocation contexts. They're mostly part of
- // the current stack frame.
- CallerLCtx = CallerLCtx->getParent();
- assert(!isa<BlockInvocationContext>(CallerLCtx));
- }
-
- return updateObjectsUnderConstruction(V,
- cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
- RTC->getConstructionContext(), CallOpts);
- }
- case ConstructionContext::ElidedTemporaryObjectKind: {
- assert(AMgr.getAnalyzerOptions().ShouldElideConstructors);
- if (!CallOpts.IsElidableCtorThatHasNotBeenElided) {
- const auto *TCC = cast<ElidedTemporaryObjectConstructionContext>(CC);
- State = updateObjectsUnderConstruction(
- V, TCC->getConstructorAfterElision(), State, LCtx,
- TCC->getConstructionContextAfterElision(), CallOpts);
-
- // Remember that we've elided the constructor.
- State = addObjectUnderConstruction(
- State, TCC->getConstructorAfterElision(), LCtx, V);
-
- // Remember that we've elided the destructor.
- if (const auto *BTE = TCC->getCXXBindTemporaryExpr())
- State = elideDestructor(State, BTE, LCtx);
-
- // Instead of materialization, shamelessly return
- // the final object destination.
- if (const auto *MTE = TCC->getMaterializedTemporaryExpr())
- State = addObjectUnderConstruction(State, MTE, LCtx, V);
-
- return State;
- }
- // If we decided not to elide the constructor, proceed as if
- // it's a simple temporary.
- [[fallthrough]];
+ auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
+ .getAs<CFGCXXRecordTypedCall>();
+ assert(RTC && "Could not have had a target region without it");
+ if (isa<BlockInvocationContext>(CallerLCtx)) {
+ // Unwrap block invocation contexts. They're mostly part of
+ // the current stack frame.
+ CallerLCtx = CallerLCtx->getParent();
+ assert(!isa<BlockInvocationContext>(CallerLCtx));
}
- case ConstructionContext::SimpleTemporaryObjectKind: {
- const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
+
+ return updateObjectsUnderConstruction(
+ V, cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
+ RTC->getConstructionContext(), CallOpts);
+ }
+ case ConstructionContext::ElidedTemporaryObjectKind: {
+ assert(AMgr.getAnalyzerOptions().ShouldElideConstructors);
+ if (!CallOpts.IsElidableCtorThatHasNotBeenElided) {
+ const auto *TCC = cast<ElidedTemporaryObjectConstructionContext>(CC);
+ State = updateObjectsUnderConstruction(
+ V, TCC->getConstructorAfterElision(), State, LCtx,
+ TCC->getConstructionContextAfterElision(), CallOpts);
+
+ // Remember that we've elided the constructor.
+ State = addObjectUnderConstruction(
+ State, TCC->getConstructorAfterElision(), LCtx, V);
+
+ // Remember that we've elided the destructor.
if (const auto *BTE = TCC->getCXXBindTemporaryExpr())
- State = addObjectUnderConstruction(State, BTE, LCtx, V);
+ State = elideDestructor(State, BTE, LCtx);
+ // Instead of materialization, shamelessly return
+ // the final object destination.
if (const auto *MTE = TCC->getMaterializedTemporaryExpr())
State = addObjectUnderConstruction(State, MTE, LCtx, V);
return State;
}
- case ConstructionContext::LambdaCaptureKind: {
- const auto *LCC = cast<LambdaCaptureConstructionContext>(CC);
+ // If we decided not to elide the constructor, proceed as if
+ // it's a simple temporary.
+ [[fallthrough]];
+ }
+ case ConstructionContext::SimpleTemporaryObjectKind: {
+ const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
+ if (const auto *BTE = TCC->getCXXBindTemporaryExpr())
+ State = addObjectUnderConstruction(State, BTE, LCtx, V);
- // If we capture and array, we want to store the super region, not a
- // sub-region.
- if (const auto *EL = dyn_cast_or_null<ElementRegion>(V.getAsRegion()))
- V = loc::MemRegionVal(EL->getSuperRegion());
+ if (const auto *MTE = TCC->getMaterializedTemporaryExpr())
+ State = addObjectUnderConstruction(State, MTE, LCtx, V);
- return addObjectUnderConstruction(
- State, {LCC->getLambdaExpr(), LCC->getIndex()}, LCtx, V);
- }
- case ConstructionContext::ArgumentKind: {
- const auto *ACC = cast<ArgumentConstructionContext>(CC);
- if (const auto *BTE = ACC->getCXXBindTemporaryExpr())
- State = addObjectUnderConstruction(State, BTE, LCtx, V);
+ return State;
+ }
+ case ConstructionContext::LambdaCaptureKind: {
+ const auto *LCC = cast<LambdaCaptureConstructionContext>(CC);
- return addObjectUnderConstruction(
- State, {ACC->getCallLikeExpr(), ACC->getIndex()}, LCtx, V);
- }
+ // If we capture and array, we want to store the super region, not a
+ // sub-region.
+ if (const auto *EL = dyn_cast_or_null<ElementRegion>(V.getAsRegion()))
+ V = loc::MemRegionVal(EL->getSuperRegion());
+
+ return addObjectUnderConstruction(
+ State, {LCC->getLambdaExpr(), LCC->getIndex()}, LCtx, V);
+ }
+ case ConstructionContext::ArgumentKind: {
+ const auto *ACC = cast<ArgumentConstructionContext>(CC);
+ if (const auto *BTE = ACC->getCXXBindTemporaryExpr())
+ State = addObjectUnderConstruction(State, BTE, LCtx, V);
+
+ return addObjectUnderConstruction(
+ State, {ACC->getCallLikeExpr(), ACC->getIndex()}, LCtx, V);
+ }
}
llvm_unreachable("Unhandled construction context!");
}
@@ -526,8 +530,7 @@ bindRequiredArrayElementToEnvironment(ProgramStateRef State,
loc::MemRegionVal(ElementRegion));
}
-void ExprEngine::handleConstructor(const Expr *E,
- ExplodedNode *Pred,
+void ExprEngine::handleConstructor(const Expr *E, ExplodedNode *Pred,
ExplodedNodeSet &destNodes) {
const auto *CE = dyn_cast<CXXConstructExpr>(E);
const auto *CIE = dyn_cast<CXXInheritedCtorInitExpr>(E);
@@ -541,16 +544,16 @@ void ExprEngine::handleConstructor(const Expr *E,
if (CE) {
if (std::optional<SVal> ElidedTarget =
getObjectUnderConstruction(State, CE, LCtx)) {
- // We've previously modeled an elidable constructor by pretending that
- // it in fact constructs into the correct target. This constructor can
- // therefore be skipped.
- Target = *ElidedTarget;
- StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx);
- State = finishObjectConstruction(State, CE, LCtx);
- if (auto L = Target.getAs<Loc>())
- State = State->BindExpr(CE, LCtx, State->getSVal(*L, CE->getType()));
- Bldr.generateNode(CE, Pred, State);
- return;
+ // We've previously modeled an elidable constructor by pretending that
+ // it in fact constructs into the correct target. This constructor can
+ // therefore be skipped.
+ Target = *ElidedTarget;
+ StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx);
+ State = finishObjectConstruction(State, CE, LCtx);
+ if (auto L = Target.getAs<Loc>())
+ State = State->BindExpr(CE, LCtx, State->getSVal(*L, CE->getType()));
+ Bldr.generateNode(CE, Pred, State);
+ return;
}
}
@@ -648,8 +651,7 @@ void ExprEngine::handleConstructor(const Expr *E,
[[fallthrough]];
case CXXConstructionKind::Delegating: {
const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
- Loc ThisPtr = getSValBuilder().getCXXThis(CurCtor,
- LCtx->getStackFrame());
+ Loc ThisPtr = getSValBuilder().getCXXThis(CurCtor, LCtx->getStackFrame());
SVal ThisVal = State->getSVal(ThisPtr);
if (CK == CXXConstructionKind::Delegating) {
@@ -718,8 +720,8 @@ void ExprEngine::handleConstructor(const Expr *E,
}
ExplodedNodeSet DstPreCall;
- getCheckerManager().runCheckersForPreCall(DstPreCall, PreInitialized,
- *Call, *this);
+ getCheckerManager().runCheckersForPreCall(DstPreCall, PreInitialized, *Call,
+ *this);
ExplodedNodeSet DstEvaluated;
@@ -782,9 +784,8 @@ void ExprEngine::handleConstructor(const Expr *E,
// If there were other constructors called for object-type arguments
// of this constructor, clean them up.
ExplodedNodeSet DstPostCall;
- getCheckerManager().runCheckersForPostCall(DstPostCall,
- DstPostArgumentCleanup,
- *Call, *this);
+ getCheckerManager().runCheckersForPostCall(
+ DstPostCall, DstPostArgumentCleanup, *Call, *this);
getCheckerManager().runCheckersForPostStmt(destNodes, DstPostCall, E, *this);
}
@@ -800,12 +801,9 @@ void ExprEngine::VisitCXXInheritedCtorInitExpr(
handleConstructor(CE, Pred, Dst);
}
-void ExprEngine::VisitCXXDestructor(QualType ObjectType,
- const MemRegion *Dest,
- const Stmt *S,
- bool IsBaseDtor,
- ExplodedNode *Pred,
- ExplodedNodeSet &Dst,
+void ExprEngine::VisitCXXDestructor(QualType ObjectType, const MemRegion *Dest,
+ const Stmt *S, bool IsBaseDtor,
+ ExplodedNode *Pred, ExplodedNodeSet &Dst,
EvalCallOptions &CallOpts) {
assert(S && "A destructor without a trigger!");
const LocationContext *LCtx = Pred->getLocationContext();
@@ -840,8 +838,8 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
} else {
static SimpleProgramPointTag T("ExprEngine", "SkipInvalidDestructor");
NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
- Bldr.generateSink(Pred->getLocation().withTag(&T),
- Pred->getState(), Pred);
+ Bldr.generateSink(Pred->getLocation().withTag(&T), Pred->getState(),
+ Pred);
return;
}
}
@@ -855,16 +853,14 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
"Error evaluating destructor");
ExplodedNodeSet DstPreCall;
- getCheckerManager().runCheckersForPreCall(DstPreCall, Pred,
- *Call, *this);
+ getCheckerManager().runCheckersForPreCall(DstPreCall, Pred, *Call, *this);
ExplodedNodeSet DstInvalidated;
StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
for (ExplodedNode *N : DstPreCall)
defaultEvalCall(Bldr, N, *Call, CallOpts);
- getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated,
- *Call, *this);
+ getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated, *Call,
*this);
}
void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
@@ -880,8 +876,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr
*CNE,
CEMgr.getCXXAllocatorCall(CNE, State, LCtx, getCFGElementRef());
ExplodedNodeSet DstPreCall;
- getCheckerManager().runCheckersForPreCall(DstPreCall, Pred,
- *Call, *this);
+ getCheckerManager().runCheckersForPreCall(DstPreCall, Pred, *Call, *this);
ExplodedNodeSet DstPostCall;
StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
@@ -940,7 +935,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr
*CNE,
}
void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
- ExplodedNodeSet &Dst) {
+ ExplodedNodeSet &Dst) {
// FIXME: Much of this should eventually migrate to CXXAllocatorCall.
// Also, we need to decide how allocators actually work -- they're not
// really part of the CXXNewExpr because they happen BEFORE the
@@ -1112,15 +1107,13 @@ void ExprEngine::VisitCXXCatchStmt(const CXXCatchStmt
*CS, ExplodedNode *Pred,
}
void ExprEngine::VisitCXXThisExpr(const CXXThisExpr *TE, ExplodedNode *Pred,
- ExplodedNodeSet &Dst) {
+ ExplodedNodeSet &Dst) {
StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
// Get the this object region from StoreManager.
const LocationContext *LCtx = Pred->getLocationContext();
- const MemRegion *R =
- svalBuilder.getRegionManager().getCXXThisRegion(
- getContext().getCanonicalType(TE->getType()),
- LCtx);
+ const MemRegion *R = svalBuilder.getRegionManager().getCXXThisRegion(
+ getContext().getCanonicalType(TE->getType()), LCtx);
ProgramStateRef state = Pred->getState();
SVal V = state->getSVal(loc::MemRegionVal(R));
@@ -1132,8 +1125,8 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE,
ExplodedNode *Pred,
const LocationContext *LocCtxt = Pred->getLocationContext();
// Get the region of the lambda itself.
- const MemRegion *R = svalBuilder.getRegionManager().getCXXTempObjectRegion(
- LE, LocCtxt);
+ const MemRegion *R =
+ svalBuilder.getRegionManager().getCXXTempObjectRegion(LE, LocCtxt);
SVal V = loc::MemRegionVal(R);
ProgramStateRef State = Pred->getState();
@@ -1193,9 +1186,8 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE,
ExplodedNode *Pred,
ExplodedNodeSet Tmp;
StmtNodeBuilder Bldr(Pred, Tmp, *currBldrCtx);
// FIXME: is this the right program point kind?
- Bldr.generateNode(LE, Pred,
- State->BindExpr(LE, LocCtxt, LambdaRVal),
- nullptr, ProgramPoint::PostLValueKind);
+ Bldr.generateNode(LE, Pred, State->BindExpr(LE, LocCtxt, LambdaRVal),
nullptr,
+ ProgramPoint::PostLValueKind);
// FIXME: Move all post/pre visits to ::Visit().
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, LE, *this);
>From 1bad0e4094e87401b550a90922626d341ba2547c Mon Sep 17 00:00:00 2001
From: "tobias.gruber" <tobias.gru...@concentrio.io>
Date: Thu, 27 Feb 2025 07:26:23 +0100
Subject: [PATCH 2/5] Unittest for default copy construction
---
.../StaticAnalyzer/ExprEngineVisitTest.cpp | 54 +++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
index a8579f9d0f90c..19de9919701a3 100644
--- a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
+++ b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
@@ -12,6 +12,8 @@
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/Support/Casting.h"
#include "gtest/gtest.h"
using namespace clang;
@@ -27,6 +29,14 @@ void emitErrorReport(CheckerContext &C, const BugType &Bug,
}
}
+inline std::string getMemRegionName(const SVal &Val) {
+ if (auto MemVal = llvm::dyn_cast<loc::MemRegionVal>(Val))
+ return MemVal->getRegion()->getDescriptiveName(false);
+ if (auto ComVal = llvm::dyn_cast<nonloc::LazyCompoundVal>(Val))
+ return ComVal->getRegion()->getDescriptiveName(false);
+ return "";
+}
+
#define CREATE_EXPR_ENGINE_CHECKER(CHECKER_NAME, CALLBACK, STMT_TYPE,
\
BUG_NAME)
\
class CHECKER_NAME : public Checker<check::CALLBACK<STMT_TYPE>> {
\
@@ -44,6 +54,21 @@ CREATE_EXPR_ENGINE_CHECKER(ExprEngineVisitPreChecker,
PreStmt, GCCAsmStmt,
CREATE_EXPR_ENGINE_CHECKER(ExprEngineVisitPostChecker, PostStmt, GCCAsmStmt,
"GCCAsmStmtBug")
+class MemAccessChecker : public Checker<check::Location, check::Bind> {
+public:
+ void checkLocation(const SVal &Loc, bool IsLoad, const Stmt *S,
+ CheckerContext &C) const {
+ emitErrorReport(C, Bug, "checkLocation: Loc = " + getMemRegionName(Loc));
+ }
+
+ void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const {
+ emitErrorReport(C, Bug, "checkBind: Loc = " + getMemRegionName(Loc));
+ }
+
+private:
+ const BugType Bug{this, "MemAccess"};
+};
+
void addExprEngineVisitPreChecker(AnalysisASTConsumer &AnalysisConsumer,
AnalyzerOptions &AnOpts) {
AnOpts.CheckersAndPackages = {{"ExprEngineVisitPreChecker", true}};
@@ -62,6 +87,15 @@ void addExprEngineVisitPostChecker(AnalysisASTConsumer
&AnalysisConsumer,
});
}
+void addMemAccessChecker(AnalysisASTConsumer &AnalysisConsumer,
+ AnalyzerOptions &AnOpts) {
+ AnOpts.CheckersAndPackages = {{"MemAccessChecker", true}};
+ AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+ Registry.addChecker<MemAccessChecker>("MemAccessChecker", "Desc",
+ "DocsURI");
+ });
+}
+
TEST(ExprEngineVisitTest, checkPreStmtGCCAsmStmt) {
std::string Diags;
EXPECT_TRUE(runCheckerOnCode<addExprEngineVisitPreChecker>(R"(
@@ -84,4 +118,24 @@ TEST(ExprEngineVisitTest, checkPostStmtGCCAsmStmt) {
EXPECT_EQ(Diags, "ExprEngineVisitPostChecker: checkPostStmt<GCCAsmStmt>\n");
}
+TEST(ExprEngineVisitTest, checkLocationAndBind) {
+ std::string Diags;
+ EXPECT_TRUE(runCheckerOnCode<addMemAccessChecker>(R"(
+ class MyClass{
+ public:
+ int Value;
+ };
+ extern MyClass MyClassWrite, MyClassRead;
+ void top() {
+ MyClassWrite = MyClassRead;
+ }
+ )",
+ Diags));
+
+ std::string RHSMsg = "checkLocation: Loc = MyClassRead";
+ std::string LHSMsg = "checkBind: Loc = MyClassWrite";
+ EXPECT_NE(Diags.find(RHSMsg), std::string::npos);
+ EXPECT_NE(Diags.find(LHSMsg), std::string::npos);
+}
+
} // namespace
>From cfa8d3491f8d436530c6c995ed07412623809590 Mon Sep 17 00:00:00 2001
From: "tobias.gruber" <tobias.gru...@concentrio.io>
Date: Thu, 27 Feb 2025 07:50:54 +0100
Subject: [PATCH 3/5] Undo format changes
---
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index fbb5e316bb068..f9b0f9d8dd93e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -1191,4 +1191,4 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE,
ExplodedNode *Pred,
// FIXME: Move all post/pre visits to ::Visit().
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, LE, *this);
-}
+}
\ No newline at end of file
>From 57b4517b29bd6d8d7ba2fafa94b6b7b17494050b Mon Sep 17 00:00:00 2001
From: "tobias.gruber" <tobias.gru...@concentrio.io>
Date: Thu, 27 Feb 2025 07:57:36 +0100
Subject: [PATCH 4/5] Undo format changes
---
.../lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 245 +++++++++---------
1 file changed, 129 insertions(+), 116 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index f9b0f9d8dd93e..47888cf9689c7 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -77,7 +77,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr,
ExplodedNode *Pred,
V = Pred->getState()->getSVal(*L);
else
assert(V.isUnknownOrUndef());
-
+
ExplodedNodeSet Tmp;
evalLocation(Tmp, CallExpr, VExpr, Pred, Pred->getState(), V, true);
for (ExplodedNode *N : Tmp)
@@ -146,9 +146,10 @@ SVal ExprEngine::computeObjectUnderConstruction(
if (Init->isBaseInitializer()) {
const auto *ThisReg = cast<SubRegion>(ThisVal.getAsRegion());
const CXXRecordDecl *BaseClass =
- Init->getBaseClass()->getAsCXXRecordDecl();
- const auto *BaseReg = MRMgr.getCXXBaseObjectRegion(
- BaseClass, ThisReg, Init->isBaseVirtual());
+ Init->getBaseClass()->getAsCXXRecordDecl();
+ const auto *BaseReg =
+ MRMgr.getCXXBaseObjectRegion(BaseClass, ThisReg,
+ Init->isBaseVirtual());
return SVB.makeLoc(BaseReg);
}
if (Init->isDelegatingInitializer())
@@ -187,7 +188,7 @@ SVal ExprEngine::computeObjectUnderConstruction(
return loc::MemRegionVal(R);
}
- return V;
+ return V;
}
// TODO: Detect when the allocator returns a null pointer.
// Constructor shall not be called in this case.
@@ -409,99 +410,99 @@ ProgramStateRef
ExprEngine::updateObjectsUnderConstruction(
case ConstructionContext::SimpleVariableKind: {
const auto *DSCC = cast<VariableConstructionContext>(CC);
return addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, V);
- }
- case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
- case ConstructionContext::SimpleConstructorInitializerKind: {
- const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
- const auto *Init = ICC->getCXXCtorInitializer();
- // Base and delegating initializers handled above
- assert(Init->isAnyMemberInitializer() &&
- "Base and delegating initializers should have been handled by"
- "computeObjectUnderConstruction()");
- return addObjectUnderConstruction(State, Init, LCtx, V);
- }
- case ConstructionContext::NewAllocatedObjectKind: {
- return State;
- }
- case ConstructionContext::SimpleReturnedValueKind:
- case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
- const StackFrameContext *SFC = LCtx->getStackFrame();
- const LocationContext *CallerLCtx = SFC->getParent();
- if (!CallerLCtx) {
- // No extra work is necessary in top frame.
- return State;
}
-
- auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
- .getAs<CFGCXXRecordTypedCall>();
- assert(RTC && "Could not have had a target region without it");
- if (isa<BlockInvocationContext>(CallerLCtx)) {
- // Unwrap block invocation contexts. They're mostly part of
- // the current stack frame.
- CallerLCtx = CallerLCtx->getParent();
- assert(!isa<BlockInvocationContext>(CallerLCtx));
+ case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
+ case ConstructionContext::SimpleConstructorInitializerKind: {
+ const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
+ const auto *Init = ICC->getCXXCtorInitializer();
+ // Base and delegating initializers handled above
+ assert(Init->isAnyMemberInitializer() &&
+ "Base and delegating initializers should have been handled by"
+ "computeObjectUnderConstruction()");
+ return addObjectUnderConstruction(State, Init, LCtx, V);
}
+ case ConstructionContext::NewAllocatedObjectKind: {
+ return State;
+ }
+ case ConstructionContext::SimpleReturnedValueKind:
+ case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
+ const StackFrameContext *SFC = LCtx->getStackFrame();
+ const LocationContext *CallerLCtx = SFC->getParent();
+ if (!CallerLCtx) {
+ // No extra work is necessary in top frame.
+ return State;
+ }
- return updateObjectsUnderConstruction(
- V, cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
- RTC->getConstructionContext(), CallOpts);
- }
- case ConstructionContext::ElidedTemporaryObjectKind: {
- assert(AMgr.getAnalyzerOptions().ShouldElideConstructors);
- if (!CallOpts.IsElidableCtorThatHasNotBeenElided) {
- const auto *TCC = cast<ElidedTemporaryObjectConstructionContext>(CC);
- State = updateObjectsUnderConstruction(
- V, TCC->getConstructorAfterElision(), State, LCtx,
- TCC->getConstructionContextAfterElision(), CallOpts);
-
- // Remember that we've elided the constructor.
- State = addObjectUnderConstruction(
- State, TCC->getConstructorAfterElision(), LCtx, V);
+ auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
+ .getAs<CFGCXXRecordTypedCall>();
+ assert(RTC && "Could not have had a target region without it");
+ if (isa<BlockInvocationContext>(CallerLCtx)) {
+ // Unwrap block invocation contexts. They're mostly part of
+ // the current stack frame.
+ CallerLCtx = CallerLCtx->getParent();
+ assert(!isa<BlockInvocationContext>(CallerLCtx));
+ }
- // Remember that we've elided the destructor.
+ return updateObjectsUnderConstruction(V,
+ cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
+ RTC->getConstructionContext(), CallOpts);
+ }
+ case ConstructionContext::ElidedTemporaryObjectKind: {
+ assert(AMgr.getAnalyzerOptions().ShouldElideConstructors);
+ if (!CallOpts.IsElidableCtorThatHasNotBeenElided) {
+ const auto *TCC = cast<ElidedTemporaryObjectConstructionContext>(CC);
+ State = updateObjectsUnderConstruction(
+ V, TCC->getConstructorAfterElision(), State, LCtx,
+ TCC->getConstructionContextAfterElision(), CallOpts);
+
+ // Remember that we've elided the constructor.
+ State = addObjectUnderConstruction(
+ State, TCC->getConstructorAfterElision(), LCtx, V);
+
+ // Remember that we've elided the destructor.
+ if (const auto *BTE = TCC->getCXXBindTemporaryExpr())
+ State = elideDestructor(State, BTE, LCtx);
+
+ // Instead of materialization, shamelessly return
+ // the final object destination.
+ if (const auto *MTE = TCC->getMaterializedTemporaryExpr())
+ State = addObjectUnderConstruction(State, MTE, LCtx, V);
+
+ return State;
+ }
+ // If we decided not to elide the constructor, proceed as if
+ // it's a simple temporary.
+ [[fallthrough]];
+ }
+ case ConstructionContext::SimpleTemporaryObjectKind: {
+ const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
if (const auto *BTE = TCC->getCXXBindTemporaryExpr())
- State = elideDestructor(State, BTE, LCtx);
+ State = addObjectUnderConstruction(State, BTE, LCtx, V);
- // Instead of materialization, shamelessly return
- // the final object destination.
if (const auto *MTE = TCC->getMaterializedTemporaryExpr())
State = addObjectUnderConstruction(State, MTE, LCtx, V);
return State;
}
- // If we decided not to elide the constructor, proceed as if
- // it's a simple temporary.
- [[fallthrough]];
- }
- case ConstructionContext::SimpleTemporaryObjectKind: {
- const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
- if (const auto *BTE = TCC->getCXXBindTemporaryExpr())
- State = addObjectUnderConstruction(State, BTE, LCtx, V);
-
- if (const auto *MTE = TCC->getMaterializedTemporaryExpr())
- State = addObjectUnderConstruction(State, MTE, LCtx, V);
-
- return State;
- }
- case ConstructionContext::LambdaCaptureKind: {
- const auto *LCC = cast<LambdaCaptureConstructionContext>(CC);
+ case ConstructionContext::LambdaCaptureKind: {
+ const auto *LCC = cast<LambdaCaptureConstructionContext>(CC);
- // If we capture and array, we want to store the super region, not a
- // sub-region.
- if (const auto *EL = dyn_cast_or_null<ElementRegion>(V.getAsRegion()))
- V = loc::MemRegionVal(EL->getSuperRegion());
+ // If we capture and array, we want to store the super region, not a
+ // sub-region.
+ if (const auto *EL = dyn_cast_or_null<ElementRegion>(V.getAsRegion()))
+ V = loc::MemRegionVal(EL->getSuperRegion());
- return addObjectUnderConstruction(
- State, {LCC->getLambdaExpr(), LCC->getIndex()}, LCtx, V);
- }
- case ConstructionContext::ArgumentKind: {
- const auto *ACC = cast<ArgumentConstructionContext>(CC);
- if (const auto *BTE = ACC->getCXXBindTemporaryExpr())
- State = addObjectUnderConstruction(State, BTE, LCtx, V);
+ return addObjectUnderConstruction(
+ State, {LCC->getLambdaExpr(), LCC->getIndex()}, LCtx, V);
+ }
+ case ConstructionContext::ArgumentKind: {
+ const auto *ACC = cast<ArgumentConstructionContext>(CC);
+ if (const auto *BTE = ACC->getCXXBindTemporaryExpr())
+ State = addObjectUnderConstruction(State, BTE, LCtx, V);
- return addObjectUnderConstruction(
- State, {ACC->getCallLikeExpr(), ACC->getIndex()}, LCtx, V);
- }
+ return addObjectUnderConstruction(
+ State, {ACC->getCallLikeExpr(), ACC->getIndex()}, LCtx, V);
+ }
}
llvm_unreachable("Unhandled construction context!");
}
@@ -530,7 +531,8 @@ bindRequiredArrayElementToEnvironment(ProgramStateRef State,
loc::MemRegionVal(ElementRegion));
}
-void ExprEngine::handleConstructor(const Expr *E, ExplodedNode *Pred,
+void ExprEngine::handleConstructor(const Expr *E,
+ ExplodedNode *Pred,
ExplodedNodeSet &destNodes) {
const auto *CE = dyn_cast<CXXConstructExpr>(E);
const auto *CIE = dyn_cast<CXXInheritedCtorInitExpr>(E);
@@ -544,16 +546,16 @@ void ExprEngine::handleConstructor(const Expr *E,
ExplodedNode *Pred,
if (CE) {
if (std::optional<SVal> ElidedTarget =
getObjectUnderConstruction(State, CE, LCtx)) {
- // We've previously modeled an elidable constructor by pretending that
- // it in fact constructs into the correct target. This constructor can
- // therefore be skipped.
- Target = *ElidedTarget;
- StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx);
- State = finishObjectConstruction(State, CE, LCtx);
- if (auto L = Target.getAs<Loc>())
- State = State->BindExpr(CE, LCtx, State->getSVal(*L, CE->getType()));
- Bldr.generateNode(CE, Pred, State);
- return;
+ // We've previously modeled an elidable constructor by pretending that
+ // it in fact constructs into the correct target. This constructor can
+ // therefore be skipped.
+ Target = *ElidedTarget;
+ StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx);
+ State = finishObjectConstruction(State, CE, LCtx);
+ if (auto L = Target.getAs<Loc>())
+ State = State->BindExpr(CE, LCtx, State->getSVal(*L, CE->getType()));
+ Bldr.generateNode(CE, Pred, State);
+ return;
}
}
@@ -651,7 +653,8 @@ void ExprEngine::handleConstructor(const Expr *E,
ExplodedNode *Pred,
[[fallthrough]];
case CXXConstructionKind::Delegating: {
const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
- Loc ThisPtr = getSValBuilder().getCXXThis(CurCtor, LCtx->getStackFrame());
+ Loc ThisPtr = getSValBuilder().getCXXThis(CurCtor,
+ LCtx->getStackFrame());
SVal ThisVal = State->getSVal(ThisPtr);
if (CK == CXXConstructionKind::Delegating) {
@@ -720,8 +723,8 @@ void ExprEngine::handleConstructor(const Expr *E,
ExplodedNode *Pred,
}
ExplodedNodeSet DstPreCall;
- getCheckerManager().runCheckersForPreCall(DstPreCall, PreInitialized, *Call,
- *this);
+ getCheckerManager().runCheckersForPreCall(DstPreCall, PreInitialized,
+ *Call, *this);
ExplodedNodeSet DstEvaluated;
@@ -784,8 +787,9 @@ void ExprEngine::handleConstructor(const Expr *E,
ExplodedNode *Pred,
// If there were other constructors called for object-type arguments
// of this constructor, clean them up.
ExplodedNodeSet DstPostCall;
- getCheckerManager().runCheckersForPostCall(
- DstPostCall, DstPostArgumentCleanup, *Call, *this);
+ getCheckerManager().runCheckersForPostCall(DstPostCall,
+ DstPostArgumentCleanup,
+ *Call, *this);
getCheckerManager().runCheckersForPostStmt(destNodes, DstPostCall, E, *this);
}
@@ -801,9 +805,12 @@ void ExprEngine::VisitCXXInheritedCtorInitExpr(
handleConstructor(CE, Pred, Dst);
}
-void ExprEngine::VisitCXXDestructor(QualType ObjectType, const MemRegion *Dest,
- const Stmt *S, bool IsBaseDtor,
- ExplodedNode *Pred, ExplodedNodeSet &Dst,
+void ExprEngine::VisitCXXDestructor(QualType ObjectType,
+ const MemRegion *Dest,
+ const Stmt *S,
+ bool IsBaseDtor,
+ ExplodedNode *Pred,
+ ExplodedNodeSet &Dst,
EvalCallOptions &CallOpts) {
assert(S && "A destructor without a trigger!");
const LocationContext *LCtx = Pred->getLocationContext();
@@ -838,8 +845,8 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
const MemRegion *Dest,
} else {
static SimpleProgramPointTag T("ExprEngine", "SkipInvalidDestructor");
NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
- Bldr.generateSink(Pred->getLocation().withTag(&T), Pred->getState(),
- Pred);
+ Bldr.generateSink(Pred->getLocation().withTag(&T),
+ Pred->getState(), Pred);
return;
}
}
@@ -853,14 +860,16 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
const MemRegion *Dest,
"Error evaluating destructor");
ExplodedNodeSet DstPreCall;
- getCheckerManager().runCheckersForPreCall(DstPreCall, Pred, *Call, *this);
+ getCheckerManager().runCheckersForPreCall(DstPreCall, Pred,
+ *Call, *this);
ExplodedNodeSet DstInvalidated;
StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
for (ExplodedNode *N : DstPreCall)
defaultEvalCall(Bldr, N, *Call, CallOpts);
- getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated, *Call,
*this);
+ getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated,
+ *Call, *this);
}
void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
@@ -876,7 +885,8 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr
*CNE,
CEMgr.getCXXAllocatorCall(CNE, State, LCtx, getCFGElementRef());
ExplodedNodeSet DstPreCall;
- getCheckerManager().runCheckersForPreCall(DstPreCall, Pred, *Call, *this);
+ getCheckerManager().runCheckersForPreCall(DstPreCall, Pred,
+ *Call, *this);
ExplodedNodeSet DstPostCall;
StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
@@ -935,7 +945,7 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr
*CNE,
}
void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
- ExplodedNodeSet &Dst) {
+ ExplodedNodeSet &Dst) {
// FIXME: Much of this should eventually migrate to CXXAllocatorCall.
// Also, we need to decide how allocators actually work -- they're not
// really part of the CXXNewExpr because they happen BEFORE the
@@ -1107,13 +1117,15 @@ void ExprEngine::VisitCXXCatchStmt(const CXXCatchStmt
*CS, ExplodedNode *Pred,
}
void ExprEngine::VisitCXXThisExpr(const CXXThisExpr *TE, ExplodedNode *Pred,
- ExplodedNodeSet &Dst) {
+ ExplodedNodeSet &Dst) {
StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
// Get the this object region from StoreManager.
const LocationContext *LCtx = Pred->getLocationContext();
- const MemRegion *R = svalBuilder.getRegionManager().getCXXThisRegion(
- getContext().getCanonicalType(TE->getType()), LCtx);
+ const MemRegion *R =
+ svalBuilder.getRegionManager().getCXXThisRegion(
+ getContext().getCanonicalType(TE->getType()),
+ LCtx);
ProgramStateRef state = Pred->getState();
SVal V = state->getSVal(loc::MemRegionVal(R));
@@ -1125,8 +1137,8 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE,
ExplodedNode *Pred,
const LocationContext *LocCtxt = Pred->getLocationContext();
// Get the region of the lambda itself.
- const MemRegion *R =
- svalBuilder.getRegionManager().getCXXTempObjectRegion(LE, LocCtxt);
+ const MemRegion *R = svalBuilder.getRegionManager().getCXXTempObjectRegion(
+ LE, LocCtxt);
SVal V = loc::MemRegionVal(R);
ProgramStateRef State = Pred->getState();
@@ -1186,8 +1198,9 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE,
ExplodedNode *Pred,
ExplodedNodeSet Tmp;
StmtNodeBuilder Bldr(Pred, Tmp, *currBldrCtx);
// FIXME: is this the right program point kind?
- Bldr.generateNode(LE, Pred, State->BindExpr(LE, LocCtxt, LambdaRVal),
nullptr,
- ProgramPoint::PostLValueKind);
+ Bldr.generateNode(LE, Pred,
+ State->BindExpr(LE, LocCtxt, LambdaRVal),
+ nullptr, ProgramPoint::PostLValueKind);
// FIXME: Move all post/pre visits to ::Visit().
getCheckerManager().runCheckersForPostStmt(Dst, Tmp, LE, *this);
>From 0af9569813f0f9e402372a1dde0e28ce9b7ff921 Mon Sep 17 00:00:00 2001
From: "tobias.gruber" <tobias.gru...@concentrio.io>
Date: Thu, 27 Feb 2025 08:22:47 +0100
Subject: [PATCH 5/5] Fix formatting
---
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 47888cf9689c7..0dbeee10ec20e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -77,7 +77,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr,
ExplodedNode *Pred,
V = Pred->getState()->getSVal(*L);
else
assert(V.isUnknownOrUndef());
-
+
ExplodedNodeSet Tmp;
evalLocation(Tmp, CallExpr, VExpr, Pred, Pred->getState(), V, true);
for (ExplodedNode *N : Tmp)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
