https://github.com/T-Gruber created https://github.com/llvm/llvm-project/pull/129016
The triggered callbacks for the default copy constructed instance and the instance used for initialization now behave in the same way. The LHS already calls checkBind. To keep this consistent, checkLocation is now triggered accordingly for the RHS. Further details on the previous discussion: https://discourse.llvm.org/t/checklocation-for-implicitcastexpr-of-kind-ck-noop/84729 >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/4] 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/4] 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/4] 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/4] 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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
