Author: Donát Nagy Date: 2026-05-12T13:08:37+02:00 New Revision: 215bd25a7bec346445402ae6535d42d71bf2cc18
URL: https://github.com/llvm/llvm-project/commit/215bd25a7bec346445402ae6535d42d71bf2cc18 DIFF: https://github.com/llvm/llvm-project/commit/215bd25a7bec346445402ae6535d42d71bf2cc18.diff LOG: [analyzer] Clean up evalBind, fix bad logic (#196313) This commit refactors `ExprEngine::evalBind` to eliminate the use of a `NodeBuilder` and fix incorrect logic that was apparently introduced because the `NodeBuilder` had obfuscated the underlying set operations. In the special case when the engine is binding to an `Unknown` or `Undefined` memory location, with the old code on each execution path _either_ only the `check::Bind` checkers _or_ only the pointer escape checkers were invoked. This commit ensures that on each execution path _both_ the `check::Bind` checkers _and then_ the pointer escape checkers get a chance to activate. I'm pretty sure that the bad logic did not cause incorrect behavior of the analyzer, because there are no `checkBind` checkers that generate non-sink transitions when the location is `Unknown` or `Undefined`. I also added an assertion that the location argument of `evalBind` cannot be a `NonLoc`, because this is a common sense precondition, seems to be actually true and makes it easier to reason about the behavior of this function. Added: Modified: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 1efe7e6f84b23..123207f4a1e0d 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -3713,52 +3713,38 @@ ExprEngine::notifyCheckersOfPointerEscape(ProgramStateRef State, } /// evalBind - Handle the semantics of binding a value to a specific location. -/// This method is used by evalStore and (soon) VisitDeclStmt, and others. +/// This method is used by evalStore, VisitDeclStmt, and others. void ExprEngine::evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE, - ExplodedNode *Pred, SVal location, SVal Val, + ExplodedNode *Pred, SVal Location, SVal Val, bool AtDeclInit, const ProgramPoint *PP) { + // It may be a Loc, UnknownVal or perhaps UndefinedVal. + assert(!isa<NonLoc>(Location) && "evalBind location should not be NonLoc!"); + const LocationContext *LC = Pred->getLocationContext(); - PostStmt PS(StoreE, LC); + PostStmt DefaultPP(StoreE, LC); if (!PP) - PP = &PS; + PP = &DefaultPP; // Do a previsit of the bind. ExplodedNodeSet CheckedSet; - getCheckerManager().runCheckersForBind(CheckedSet, Pred, location, Val, + getCheckerManager().runCheckersForBind(CheckedSet, Pred, Location, Val, StoreE, AtDeclInit, *this, *PP); - NodeBuilder Bldr(CheckedSet, Dst, *currBldrCtx); - - // If the location is not a 'Loc', it will already be handled by - // the checkers. There is nothing left to do. - if (!isa<Loc>(location)) { - const ProgramPoint L = PostStore(StoreE, LC, /*Loc*/nullptr, - /*tag*/nullptr); - ProgramStateRef state = Pred->getState(); - state = processPointerEscapedOnBind(state, location, Val, LC); - Bldr.generateNode(L, state, Pred); - return; - } - - for (const auto PredI : CheckedSet) { - ProgramStateRef state = PredI->getState(); - - state = processPointerEscapedOnBind(state, location, Val, LC); + for (ExplodedNode *PredI : CheckedSet) { + ProgramStateRef State = PredI->getState(); - // When binding the value, pass on the hint that this is a initialization. - // For initializations, we do not need to inform clients of region - // changes. - state = state->bindLoc(location.castAs<Loc>(), Val, LC, - /* notifyChanges = */ !AtDeclInit); + // Check and record that 'Val' may escape: + State = processPointerEscapedOnBind(State, Location, Val, LC); - const MemRegion *LocReg = nullptr; - if (std::optional<loc::MemRegionVal> LocRegVal = - location.getAs<loc::MemRegionVal>()) { - LocReg = LocRegVal->getRegion(); + if (auto AsLoc = Location.getAs<Loc>()) { + // When binding the value, pass on the hint that this is a + // initialization. For initializations, we do not need to inform clients + // of region changes. + State = State->bindLoc(*AsLoc, Val, LC, /*notifyChanges=*/!AtDeclInit); } - const ProgramPoint L = PostStore(StoreE, LC, LocReg, nullptr); - Bldr.generateNode(L, state, PredI); + PostStore PS(StoreE, LC, Location.getAsRegion(), /*tag=*/nullptr); + Dst.insert(Engine.makeNode(PS, State, PredI)); } } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
