Author: Gábor Tóthvári Date: 2026-08-03T15:13:33+02:00 New Revision: 53ee7b167d8aee0a75c1332ca4a6aa037e0869a0
URL: https://github.com/llvm/llvm-project/commit/53ee7b167d8aee0a75c1332ca4a6aa037e0869a0 DIFF: https://github.com/llvm/llvm-project/commit/53ee7b167d8aee0a75c1332ca4a6aa037e0869a0.diff LOG: [NFC][analyzer] Eliminate NodeBuilder from ExprEngine visit methods and from their utility methods (#212186) This patch eliminates the remaining uses of the class `NodeBuilder` from the `ExprEngine::Visit*` methods and from their utility methods such as `evalLocation`, `evalLoad`, `CreateCXXTemporaryObject` and `handleConstructor`. Added: Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp clang/test/Analysis/misc-ps.m Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index bd50674fa428b..7fe6e59a3679d 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -679,7 +679,7 @@ class ExprEngine { ProgramStateRef handleLValueBitCast(ProgramStateRef state, const Expr *Ex, const StackFrame *SF, QualType T, QualType ExTy, const CastExpr *CastE, - NodeBuilder &Bldr, ExplodedNode *Pred); + ExplodedNodeSet &Dst, ExplodedNode *Pred); public: SVal evalBinOp(ProgramStateRef ST, BinaryOperator::Opcode Op, diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 07597769009ba..6373e9aafe97b 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -3347,7 +3347,6 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred, for (const auto I : CheckedSet) VisitCommonDeclRefExpr(M, Member, I, EvalSet); } else { - ExplodedNodeSet Tmp; for (const auto I : CheckedSet) { ProgramStateRef state = I->getState(); @@ -3408,10 +3407,7 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred, EvalSet.insert(Engine.makeNodeWithBinding( I, M, L, state, ProgramPoint::PostLValueKind)); } else { - // FIXME: When evalLoad no longer uses NodeBuilders, eliminate Tmp and - // pass EvalSet as the first argument of evalLoad. - evalLoad(Tmp, M, M, I, state, L); - EvalSet.insert(Tmp); + evalLoad(EvalSet, M, M, I, state, L); } } } @@ -3636,9 +3632,10 @@ void ExprEngine::evalLoad(ExplodedNodeSet &Dst, if (Tmp.empty()) return; - NodeBuilder Bldr(Tmp, Dst, *currBldrCtx); - if (location.isUndef()) + if (location.isUndef()) { + Dst.insert(Tmp); return; + } // Proceed with the load. for (const auto I : Tmp) { @@ -3651,29 +3648,26 @@ void ExprEngine::evalLoad(ExplodedNodeSet &Dst, V = state->getSVal(location.castAs<Loc>(), LoadTy); } - Bldr.generateNode(NodeEx, I, - state->BindExpr(BoundEx, I->getStackFrame(), V), tag, - ProgramPoint::PostLoadKind); + const auto *SF = I->getStackFrame(); + PostLoad Loc(NodeEx, SF, tag); + Dst.insert(Engine.makeNode(Loc, state->BindExpr(BoundEx, SF, V), I)); } } -void ExprEngine::evalLocation(ExplodedNodeSet &Dst, - const Stmt *NodeEx, - const Stmt *BoundEx, - ExplodedNode *Pred, - ProgramStateRef state, - SVal location, +void ExprEngine::evalLocation(ExplodedNodeSet &Dst, const Stmt *NodeEx, + const Stmt *BoundEx, ExplodedNode *Pred, + ProgramStateRef state, SVal location, bool isLoad) { - NodeBuilder BldrTop(Pred, Dst, *currBldrCtx); // Early checks for performance reason. if (location.isUnknown()) { + Dst.insert(Pred); return; } ExplodedNodeSet Src; - BldrTop.takeNodes(Pred); - NodeBuilder Bldr(Pred, Src, *currBldrCtx); - if (Pred->getState() != state) { + if (Pred->getState() == state) { + Src.insert(Pred); + } else { // Associate this new state with an ExplodedNode. // FIXME: If I pass null tag, the graph is incorrect, e.g for // int *p; @@ -3684,12 +3678,14 @@ void ExprEngine::evalLocation(ExplodedNodeSet &Dst, // "Variable 'p' initialized to a null pointer value" static SimpleProgramPointTag tag(TagProviderName, "Location"); - Bldr.generateNode(NodeEx, Pred, state, &tag); + PostStmt Loc(NodeEx, Pred->getStackFrame(), &tag); + Src.insert(Engine.makeNode(Loc, state, Pred)); } + ExplodedNodeSet Tmp; getCheckerManager().runCheckersForLocation(Tmp, Src, location, isLoad, NodeEx, BoundEx, *this); - BldrTop.addNodes(Tmp); + Dst.insert(Tmp); } std::pair<const ProgramPointTag *, const ProgramPointTag *> diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index fa177fc2835f0..6127328cefe23 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -224,7 +224,7 @@ void ExprEngine::VisitBlockExpr(const BlockExpr *BE, ExplodedNode *Pred, ProgramStateRef ExprEngine::handleLValueBitCast(ProgramStateRef state, const Expr *Ex, const StackFrame *SF, QualType T, QualType ExTy, - const CastExpr *CastE, NodeBuilder &Bldr, + const CastExpr *CastE, ExplodedNodeSet &Dst, ExplodedNode *Pred) { if (T->isLValueReferenceType()) { assert(!CastE->getType()->isLValueReferenceType()); @@ -245,7 +245,7 @@ ExprEngine::handleLValueBitCast(ProgramStateRef state, const Expr *Ex, if (V.isUnknown() && !OrigV.isUnknown()) { state = escapeValues(state, OrigV, PSK_EscapeOther); } - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makePostStmtNode(CastE, state, Pred)); return state; } @@ -277,7 +277,6 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, } // Simulate the operation that actually casts the original value to a new // value of the destination type : - NodeBuilder Bldr(DstEvalLoc, Dst, *currBldrCtx); for (ExplodedNode *Node : DstEvalLoc) { ProgramStateRef State = Node->getState(); @@ -292,8 +291,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, CastedV = svalBuilder.evalCast(svalBuilder.simplifySVal(State, OrigV), CastE->getType(), Ex->getType()); } - State = State->BindExpr(CastE, SF, CastedV); - Bldr.generateNode(CastE, Node, State); + Dst.insert(Engine.makeNodeWithBinding(Node, CastE, CastedV)); } return; } @@ -305,7 +303,6 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, if (const ExplicitCastExpr *ExCast=dyn_cast_or_null<ExplicitCastExpr>(CastE)) T = ExCast->getTypeAsWritten(); - NodeBuilder Bldr(DstPreStmt, Dst, *currBldrCtx); for (ExplodedNode *Pred : DstPreStmt) { ProgramStateRef state = Pred->getState(); const StackFrame *SF = Pred->getStackFrame(); @@ -315,6 +312,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, case CK_LValueToRValueBitCast: llvm_unreachable("LValueToRValue casts handled earlier."); case CK_ToVoid: + Dst.insert(Pred); continue; // The analyzer doesn't do anything special with these casts, // since it understands retain/release semantics already. @@ -339,8 +337,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, ProgramStateRef state = Pred->getState(); const StackFrame *SF = Pred->getStackFrame(); SVal V = state->getSVal(Ex, SF); - state = state->BindExpr(CastE, SF, V); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, V)); continue; } case CK_MemberPointerToBoolean: @@ -350,12 +347,11 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, if (PTMSV) V = svalBuilder.makeTruthVal(!PTMSV->isNullMemberPointer(), ExTy); if (V.isUndef() || PTMSV) { - state = state->BindExpr(CastE, SF, V); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, V)); continue; } // Explicitly proceed with default handler for this case cascade. - state = handleLValueBitCast(state, Ex, SF, T, ExTy, CastE, Bldr, Pred); + state = handleLValueBitCast(state, Ex, SF, T, ExTy, CastE, Dst, Pred); continue; } case CK_Dependent: @@ -367,12 +363,11 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, case CK_PointerToIntegral: { SVal V = state->getSVal(Ex, SF); if (isa<nonloc::PointerToMember>(V)) { - state = state->BindExpr(CastE, SF, UnknownVal()); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, UnknownVal())); continue; } // Explicitly proceed with default handler for this case cascade. - state = handleLValueBitCast(state, Ex, SF, T, ExTy, CastE, Bldr, Pred); + state = handleLValueBitCast(state, Ex, SF, T, ExTy, CastE, Dst, Pred); continue; } case CK_IntegralToBoolean: @@ -403,7 +398,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, case CK_FixedPointToBoolean: case CK_FixedPointToIntegral: case CK_IntegralToFixedPoint: { - state = handleLValueBitCast(state, Ex, SF, T, ExTy, CastE, Bldr, Pred); + state = handleLValueBitCast(state, Ex, SF, T, ExTy, CastE, Dst, Pred); continue; } case CK_IntegralCast: { @@ -413,8 +408,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, V = svalBuilder.evalCast(V, T, ExTy); else V = svalBuilder.evalIntegralCast(state, V, T, ExTy); - state = state->BindExpr(CastE, SF, V); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, V)); continue; } case CK_DerivedToBase: @@ -422,8 +416,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, // For DerivedToBase cast, delegate to the store manager. SVal val = state->getSVal(Ex, SF); val = getStoreManager().evalDerivedToBase(val, CastE); - state = state->BindExpr(CastE, SF, val); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, val)); continue; } // Handle C++ dyn_cast. @@ -449,7 +442,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, if (T->isReferenceType()) { // A bad_cast exception is thrown if input value is a reference. // Currently, we model this, by generating a sink. - Bldr.generateSink(CastE, Pred, state); + Engine.makePostStmtNode(CastE, state, Pred, /*MarkAsSink=*/true); continue; } else { // If the cast fails on a pointer, bind to 0. @@ -467,7 +460,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, // Else, bind to the derived region value. state = state->BindExpr(CastE, SF, val); } - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makePostStmtNode(CastE, state, Pred)); continue; } case CK_BaseToDerived: { @@ -487,20 +480,17 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, /*symbolTag=*/nullptr, getCFGElementRef(), SF, resultType, getNumVisitedCurrent()); } - state = state->BindExpr(CastE, SF, val); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, val)); continue; } case CK_NullToPointer: { SVal V = svalBuilder.makeNullWithType(CastE->getType()); - state = state->BindExpr(CastE, SF, V); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, V)); continue; } case CK_NullToMemberPointer: { SVal V = svalBuilder.getMemberPointer(nullptr); - state = state->BindExpr(CastE, SF, V); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, V)); continue; } case CK_DerivedToBaseMemberPointer: @@ -511,8 +501,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, SVal CastedPTMSV = svalBuilder.makePointerToMember(getBasicVals().accumCXXBase( CastE->path(), *PTMSV, CastE->getCastKind())); - state = state->BindExpr(CastE, SF, CastedPTMSV); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, CastedPTMSV)); continue; } // Explicitly proceed with default handler for this case cascade. @@ -532,8 +521,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, SVal result = svalBuilder.conjureSymbolVal( /*symbolTag=*/nullptr, getCFGElementRef(), SF, resultType, getNumVisitedCurrent()); - state = state->BindExpr(CastE, SF, result); - Bldr.generateNode(CastE, Pred, state); + Dst.insert(Engine.makeNodeWithBinding(Pred, CastE, result)); continue; } } @@ -543,8 +531,6 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, void ExprEngine::VisitCompoundLiteralExpr(const CompoundLiteralExpr *CL, ExplodedNode *Pred, ExplodedNodeSet &Dst) { - NodeBuilder B(Pred, Dst, *currBldrCtx); - ProgramStateRef State = Pred->getState(); const StackFrame *SF = Pred->getStackFrame(); @@ -562,7 +548,7 @@ void ExprEngine::VisitCompoundLiteralExpr(const CompoundLiteralExpr *CL, V = CLLoc; } - B.generateNode(CL, Pred, State->BindExpr(CL, SF, V)); + Dst.insert(Engine.makeNodeWithBinding(Pred, CL, V, State)); } void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred, @@ -614,7 +600,6 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred, getCheckerManager().runCheckersForPreStmt(dstPreVisit, Pred, DS, *this); ExplodedNodeSet dstEvaluated; - NodeBuilder B(dstPreVisit, dstEvaluated, *currBldrCtx); for (ExplodedNodeSet::iterator I = dstPreVisit.begin(), E = dstPreVisit.end(); I!=E; ++I) { ExplodedNode *N = *I; @@ -633,7 +618,7 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred, state = finishObjectConstruction(state, DS, SF); // We constructed the object directly in the variable. // No need to bind anything. - B.generateNode(DS, UpdatedN, state); + dstEvaluated.insert(Engine.makePostStmtNode(DS, state, UpdatedN)); } else { // Recover some path-sensitivity if a scalar value evaluated to // UnknownVal. @@ -648,19 +633,16 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred, getNumVisitedCurrent()); } - - B.takeNodes(UpdatedN); - ExplodedNodeSet Dst2; - evalBind(Dst2, DS, UpdatedN, state->getLValue(VD, SF), InitVal, true); - B.addNodes(Dst2); + evalBind(dstEvaluated, DS, UpdatedN, state->getLValue(VD, SF), InitVal, + true); } } else { - B.generateNode(DS, N, state); + dstEvaluated.insert(Engine.makePostStmtNode(DS, state, N)); } } - getCheckerManager().runCheckersForPostStmt(Dst, B.getResults(), DS, *this); + getCheckerManager().runCheckersForPostStmt(Dst, dstEvaluated, DS, *this); } void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred, @@ -683,7 +665,6 @@ void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred, assert(B->getOpcode() == BO_LAnd || B->getOpcode() == BO_LOr); - NodeBuilder Bldr(Pred, Dst, *currBldrCtx); ProgramStateRef state = Pred->getState(); if (B->getType()->isVectorType()) { @@ -692,7 +673,7 @@ void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred, // logical operators on vectors are not short-circuit. Currently they are // modeled as short-circuit in Clang CFG but this is incorrect. // Do not set the value for the expression. It'd be UnknownVal by default. - Bldr.generateNode(B, Pred, state); + Dst.insert(Engine.makePostStmtNode(B, state, Pred)); return; } @@ -704,7 +685,7 @@ void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred, (void) P; if (N->pred_size() != 1) { // We failed to track back where we came from. - Bldr.generateNode(B, Pred, state); + Dst.insert(Engine.makePostStmtNode(B, state, Pred)); return; } N = *N->pred_begin(); @@ -712,7 +693,7 @@ void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred, if (N->pred_size() != 1) { // We failed to track back where we came from. - Bldr.generateNode(B, Pred, state); + Dst.insert(Engine.makePostStmtNode(B, state, Pred)); return; } @@ -752,7 +733,7 @@ void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred, svalBuilder.makeZeroVal(RHS->getType()), B->getType()); } } - Bldr.generateNode(B, Pred, state->BindExpr(B, Pred->getStackFrame(), X)); + Dst.insert(Engine.makeNodeWithBinding(Pred, B, X)); } void ExprEngine::VisitGuardedExpr(const Expr *Ex, @@ -762,7 +743,6 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex, ExplodedNodeSet &Dst) { assert(L && R); - NodeBuilder B(Pred, Dst, *currBldrCtx); ProgramStateRef state = Pred->getState(); const StackFrame *SF = Pred->getStackFrame(); const CFGBlock *SrcBlock = nullptr; @@ -816,13 +796,11 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex, getNumVisitedCurrent()); // Generate a new node with the binding from the appropriate path. - B.generateNode(Ex, Pred, state->BindExpr(Ex, SF, V, true)); + Dst.insert(Engine.makeNodeWithBinding(Pred, Ex, V)); } -void ExprEngine:: -VisitOffsetOfExpr(const OffsetOfExpr *OOE, - ExplodedNode *Pred, ExplodedNodeSet &Dst) { - NodeBuilder B(Pred, Dst, *currBldrCtx); +void ExprEngine::VisitOffsetOfExpr(const OffsetOfExpr *OOE, ExplodedNode *Pred, + ExplodedNodeSet &Dst) { Expr::EvalResult Result; if (OOE->EvaluateAsInt(Result, getContext())) { APSInt IV = Result.Val.getInt(); @@ -830,13 +808,13 @@ VisitOffsetOfExpr(const OffsetOfExpr *OOE, assert(OOE->getType()->castAs<BuiltinType>()->isInteger()); assert(IV.isSigned() == OOE->getType()->isSignedIntegerType()); SVal X = svalBuilder.makeIntVal(IV); - B.generateNode(OOE, Pred, - Pred->getState()->BindExpr(OOE, Pred->getStackFrame(), X)); + Dst.insert(Engine.makeNodeWithBinding(Pred, OOE, X)); + } else { + // FIXME: Handle the case where __builtin_offsetof is not a constant. + Dst.insert(Pred); } - // FIXME: Handle the case where __builtin_offsetof is not a constant. } - void ExprEngine:: VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *Ex, ExplodedNode *Pred, @@ -846,8 +824,6 @@ VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *Ex, getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, Ex, *this); ExplodedNodeSet EvalSet; - NodeBuilder Bldr(CheckedSet, EvalSet, *currBldrCtx); - QualType T = Ex->getTypeOfArgument(); for (ExplodedNode *N : CheckedSet) { @@ -858,11 +834,13 @@ VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *Ex, // FIXME: Add support for VLA type arguments and VLA expressions. // When that happens, we should probably refactor VLASizeChecker's code. + EvalSet.insert(N); continue; } else if (T->getAs<ObjCObjectType>()) { // Some code tries to take the sizeof an ObjCObjectType, relying that // the compiler has laid out its representation. Just report Unknown // for these. + EvalSet.insert(N); continue; } } @@ -870,11 +848,8 @@ VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *Ex, APSInt Value = Ex->EvaluateKnownConstInt(getContext()); CharUnits amt = CharUnits::fromQuantity(Value.getZExtValue()); - ProgramStateRef state = N->getState(); - state = state->BindExpr( - Ex, N->getStackFrame(), - svalBuilder.makeIntVal(amt.getQuantity(), Ex->getType())); - Bldr.generateNode(Ex, N, state); + SVal V = svalBuilder.makeIntVal(amt.getQuantity(), Ex->getType()); + EvalSet.insert(Engine.makeNodeWithBinding(N, Ex, V)); } getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, Ex, *this); @@ -1032,7 +1007,6 @@ void ExprEngine::VisitIncrementDecrementOperator(const UnaryOperator* U, evalLoad(Tmp, U, Ex, Pred, state, loc); ExplodedNodeSet Dst2; - NodeBuilder Bldr(Tmp, Dst2, *currBldrCtx); for (ExplodedNode *N : Tmp) { state = N->getState(); assert(SF == N->getStackFrame()); @@ -1043,11 +1017,7 @@ void ExprEngine::VisitIncrementDecrementOperator(const UnaryOperator* U, state = state->BindExpr(U, SF, V2_untested); // Perform the store, so that the uninitialized value detection happens. - Bldr.takeNodes(N); - ExplodedNodeSet Dst3; - evalStore(Dst3, U, Ex, N, state, loc, V2_untested); - Bldr.addNodes(Dst3); - + evalStore(Dst2, U, Ex, N, state, loc, V2_untested); continue; } DefinedSVal V2 = V2_untested.castAs<DefinedSVal>(); @@ -1111,10 +1081,7 @@ void ExprEngine::VisitIncrementDecrementOperator(const UnaryOperator* U, state = state->BindExpr(U, SF, U->isPostfix() ? V2 : Result); // Perform the store. - Bldr.takeNodes(N); - ExplodedNodeSet Dst3; - evalStore(Dst3, U, Ex, N, state, loc, Result); - Bldr.addNodes(Dst3); + evalStore(Dst2, U, Ex, N, state, loc, Result); } Dst.insert(Dst2); } diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 9c10bcbdc4c0d..c9ef6df71abc5 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -33,13 +33,12 @@ using namespace ento; void ExprEngine::CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME, ExplodedNode *Pred, ExplodedNodeSet &Dst) { - NodeBuilder Bldr(Pred, Dst, *currBldrCtx); const Expr *tempExpr = ME->getSubExpr()->IgnoreParens(); ProgramStateRef state = Pred->getState(); const StackFrame *SF = Pred->getStackFrame(); state = createTemporaryRegionIfNeeded(state, SF, tempExpr, ME); - Bldr.generateNode(ME, Pred, state); + Dst.insert(Engine.makePostStmtNode(ME, state, Pred)); } void ExprEngine::performTrivialCopy(ExplodedNodeSet &Dst, ExplodedNode *Pred, @@ -523,9 +522,8 @@ bindRequiredArrayElementToEnvironment(ProgramStateRef State, return State->BindExpr(Ctor->getArg(0), SF, loc::MemRegionVal(ElementRegion)); } -void ExprEngine::handleConstructor(const Expr *E, - ExplodedNode *Pred, - ExplodedNodeSet &destNodes) { +void ExprEngine::handleConstructor(const Expr *E, ExplodedNode *Pred, + ExplodedNodeSet &Dst) { const auto *CE = dyn_cast<CXXConstructExpr>(E); const auto *CIE = dyn_cast<CXXInheritedCtorInitExpr>(E); assert(CE || CIE); @@ -542,11 +540,10 @@ void ExprEngine::handleConstructor(const Expr *E, // it in fact constructs into the correct target. This constructor can // therefore be skipped. Target = *ElidedTarget; - NodeBuilder Bldr(Pred, destNodes, *currBldrCtx); State = finishObjectConstruction(State, CE, SF); if (auto L = Target.getAs<Loc>()) State = State->BindExpr(CE, SF, State->getSVal(*L, CE->getType())); - Bldr.generateNode(CE, Pred, State); + Dst.insert(Engine.makePostStmtNode(CE, State, Pred)); return; } } @@ -583,10 +580,10 @@ void ExprEngine::handleConstructor(const Expr *E, // No element construction will happen in a 0 size array. if (isZeroSizeArray()) { - NodeBuilder Bldr(Pred, destNodes, *currBldrCtx); static SimpleProgramPointTag T{"ExprEngine", "Skipping 0 size array construction"}; - Bldr.generateNode(CE, Pred, State, &T); + PostStmt Loc(CE, Pred->getStackFrame(), &T); + Dst.insert(Engine.makeNode(Loc, State, Pred)); return; } @@ -664,10 +661,7 @@ void ExprEngine::handleConstructor(const Expr *E, if (State != Pred->getState()) { static SimpleProgramPointTag T("ExprEngine", "Prepare for object construction"); - ExplodedNodeSet DstPrepare; - NodeBuilder BldrPrepare(Pred, DstPrepare, *currBldrCtx); - Pred = - BldrPrepare.generateNode(E, Pred, State, &T, ProgramPoint::PreStmtKind); + Pred = Engine.makeNode(PreStmt(E, SF, &T), State, Pred); if (!Pred) return; } @@ -686,7 +680,6 @@ void ExprEngine::handleConstructor(const Expr *E, ExplodedNodeSet PreInitialized; if (CE) { // FIXME: Is it possible and/or useful to do this before PreStmt? - NodeBuilder Bldr(DstPreVisit, PreInitialized, *currBldrCtx); for (ExplodedNode *N : DstPreVisit) { ProgramStateRef State = N->getState(); if (CE->requiresZeroInitialization()) { @@ -709,8 +702,8 @@ void ExprEngine::handleConstructor(const Expr *E, State = State->bindDefaultZero(Target, SF); } - Bldr.generateNode(CE, N, State, /*tag=*/nullptr, - ProgramPoint::PreStmtKind); + PreStmt P(CE, N->getStackFrame(), /*tag=*/nullptr); + PreInitialized.insert(Engine.makeNode(P, State, N)); } } else { PreInitialized = DstPreVisit; @@ -743,7 +736,6 @@ void ExprEngine::handleConstructor(const Expr *E, // later (for life-time extended temporaries) -- but avoids infeasible // paths when no-return temporary destructors are used for assertions. ExplodedNodeSet DstEvaluatedPostProcessed; - NodeBuilder Bldr(DstEvaluated, DstEvaluatedPostProcessed, *currBldrCtx); const AnalysisDeclContext *ADC = SF->getAnalysisDeclContext(); if (!ADC->getCFGBuildOptions().AddTemporaryDtors) { if (llvm::isa_and_nonnull<CXXTempObjectRegion, @@ -763,7 +755,7 @@ void ExprEngine::handleConstructor(const Expr *E, "We should not have inlined this constructor!"); for (ExplodedNode *N : DstEvaluated) { - Bldr.generateSink(E, N, N->getState()); + Engine.makePostStmtNode(E, N->getState(), N, /*MarkAsSink=*/true); } // There is no need to run the PostCall and PostStmt checker @@ -773,6 +765,7 @@ void ExprEngine::handleConstructor(const Expr *E, } } + DstEvaluatedPostProcessed.insert(DstEvaluated); ExplodedNodeSet DstPostArgumentCleanup; for (ExplodedNode *I : DstEvaluatedPostProcessed) finishArgumentConstruction(DstPostArgumentCleanup, I, *Call); @@ -783,7 +776,7 @@ void ExprEngine::handleConstructor(const Expr *E, getCheckerManager().runCheckersForPostCall(DstPostCall, DstPostArgumentCleanup, *Call, *this); - getCheckerManager().runCheckersForPostStmt(destNodes, DstPostCall, E, *this); + getCheckerManager().runCheckersForPostStmt(Dst, DstPostCall, E, *this); } void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE, @@ -821,8 +814,7 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType, // FIXME: PostImplicitCall with a null decl may crash elsewhere anyway. PostImplicitCall PP(/*Decl=*/nullptr, S->getEndLoc(), SF, getCFGElementRef(), &T); - NodeBuilder Bldr(Pred, Dst, *currBldrCtx); - Bldr.generateNode(PP, Pred->getState(), Pred); + Dst.insert(Engine.makeNode(PP, Pred->getState(), Pred)); return; } @@ -837,9 +829,8 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType, Dest = MRMgr.getCXXTempObjectRegion(E, Pred->getStackFrame()); } else { static SimpleProgramPointTag T("ExprEngine", "SkipInvalidDestructor"); - NodeBuilder Bldr(Pred, Dst, *currBldrCtx); - Bldr.generateSink(Pred->getLocation().withTag(&T), - Pred->getState(), Pred); + Engine.makeNode(Pred->getLocation().withTag(&T), Pred->getState(), Pred, + /*MarkAsSink=*/true); return; } } @@ -897,7 +888,6 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, // Store return value of operator new() for future use, until the actual // CXXNewExpr gets processed. ExplodedNodeSet DstPostValue; - NodeBuilder ValueBldr(DstPostCall, DstPostValue, *currBldrCtx); 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 @@ -930,8 +920,8 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, State = State->assume(RetVal.castAs<DefinedOrUnknownSVal>(), true); } - ValueBldr.generateNode(CNE, I, - addObjectUnderConstruction(State, CNE, SF, RetVal)); + DstPostValue.insert(Engine.makePostStmtNode( + CNE, addObjectUnderConstruction(State, CNE, SF, RetVal), I)); } ExplodedNodeSet DstPostPostCallCallback; @@ -1003,8 +993,6 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, State = State->assume(*dSymVal, true); } - NodeBuilder Bldr(Pred, Dst, *currBldrCtx); - SVal Result = symVal; if (CNE->isArray()) { @@ -1027,23 +1015,19 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, // If the array is list initialized, we bind the initializer list to the // memory region here, otherwise we would lose it. if (isInitList) { - Bldr.takeNodes(Pred); - Pred = Bldr.generateNode(CNE, Pred, State); + Pred = Engine.makePostStmtNode(CNE, State, Pred); SVal V = State->getSVal(Init, SF); ExplodedNodeSet evaluated; evalBind(evaluated, CNE, Pred, Result, V, true); - Bldr.takeNodes(Pred); - Bldr.addNodes(evaluated); - + assert(evaluated.size() == 1); Pred = *evaluated.begin(); State = Pred->getState(); } } - State = State->BindExpr(CNE, Pred->getStackFrame(), Result); - Bldr.generateNode(CNE, Pred, State); + Dst.insert(Engine.makeNodeWithBinding(Pred, CNE, Result, State)); return; } @@ -1059,8 +1043,8 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, } // Bind the address of the object, then check to see if we cached out. - State = State->BindExpr(CNE, SF, Result); - ExplodedNode *NewN = Bldr.generateNode(CNE, Pred, State); + ExplodedNode *NewN = Engine.makeNodeWithBinding(Pred, CNE, Result, State); + Dst.insert(NewN); if (!NewN) return; @@ -1068,8 +1052,8 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, // initializer. Copy the value over. if (const Expr *Init = CNE->getInitializer()) { if (!isa<CXXConstructExpr>(Init)) { - assert(Bldr.getResults().size() == 1); - Bldr.takeNodes(NewN); + assert(Dst.size() == 1); + Dst.erase(NewN); evalBind(Dst, CNE, NewN, Result, State->getSVal(Init, SF), /*FirstInit=*/IsStandardGlobalOpNewFunction); } @@ -1115,14 +1099,11 @@ void ExprEngine::VisitCXXCatchStmt(const CXXCatchStmt *CS, ExplodedNode *Pred, ProgramStateRef state = Pred->getState(); state = state->bindLoc(state->getLValue(VD, SF), V, SF); - NodeBuilder Bldr(Pred, Dst, *currBldrCtx); - Bldr.generateNode(CS, Pred, state); + Dst.insert(Engine.makePostStmtNode(CS, state, Pred)); } void ExprEngine::VisitCXXThisExpr(const CXXThisExpr *TE, ExplodedNode *Pred, - ExplodedNodeSet &Dst) { - NodeBuilder Bldr(Pred, Dst, *currBldrCtx); - + ExplodedNodeSet &Dst) { // Get the this object region from StoreManager. const StackFrame *SF = Pred->getStackFrame(); const MemRegion *R = svalBuilder.getRegionManager().getCXXThisRegion( @@ -1130,7 +1111,7 @@ void ExprEngine::VisitCXXThisExpr(const CXXThisExpr *TE, ExplodedNode *Pred, ProgramStateRef state = Pred->getState(); SVal V = state->getSVal(loc::MemRegionVal(R)); - Bldr.generateNode(TE, Pred, state->BindExpr(TE, SF, V)); + Dst.insert(Engine.makeNodeWithBinding(Pred, TE, V)); } void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred, @@ -1195,14 +1176,12 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred, // to be an RValue. SVal LambdaRVal = State->getSVal(R); - ExplodedNodeSet Tmp; - NodeBuilder Bldr(Pred, Tmp, *currBldrCtx); // FIXME: is this the right program point kind? - Bldr.generateNode(LE, Pred, State->BindExpr(LE, SF, LambdaRVal), nullptr, - ProgramPoint::PostLValueKind); + ExplodedNode *N = Engine.makeNodeWithBinding(Pred, LE, LambdaRVal, State, + ProgramPoint::PostLValueKind); // FIXME: Move all post/pre visits to ::Visit(). - getCheckerManager().runCheckersForPostStmt(Dst, Tmp, LE, *this); + getCheckerManager().runCheckersForPostStmt(Dst, N, LE, *this); } void ExprEngine::VisitAttributedStmt(const AttributedStmt *A, diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m index c22e0dbb6137d..e6f665d68a8d6 100644 --- a/clang/test/Analysis/misc-ps.m +++ b/clang/test/Analysis/misc-ps.m @@ -622,6 +622,15 @@ void test_offsetof_4(void) { *p = 0xDEADBEEF; // expected-warning{{Dereference of null pointer}} } +// Test when __builtin_offsetof is not constant-foldable. +struct test_offsetof_5_struct { int a; int arr[10]; }; +int test_offsetof_5(int i) { + int *p = 0; + unsigned long off = __builtin_offsetof(struct test_offsetof_5_struct, arr[i]); + (void)off; + return *p; // expected-warning{{Dereference of null pointer}} +} + // "nil receiver" false positive: make tracking of the MemRegion for 'self' // path-sensitive @interface RDar6829164 : NSObject { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
