Author: Donát Nagy Date: 2025-11-03T13:51:14+01:00 New Revision: ca19ab97b9cb0104965319dda9400ad4955662ae
URL: https://github.com/llvm/llvm-project/commit/ca19ab97b9cb0104965319dda9400ad4955662ae DIFF: https://github.com/llvm/llvm-project/commit/ca19ab97b9cb0104965319dda9400ad4955662ae.diff LOG: [NFCI][analyzer] invalidateRegions: require explicit State (#164434) The method `CallEvent::invalidateRegions()` takes (an unsigned `BlockCount` and) a `ProgramStateRef` which was previously defaulted to `nullptr` -- and when the state argument was `nullptr`, the method used the "inner" state of the `CallEvent` as a starting point for the invalidation. My recent commit 0f6f13bdccd3345522b7d38294a72b802b6ffc28 turned the "inner" state of the `CallEvent` into a hidden `protected` implementation detail; so this commit follows that direction by removing the "defaults to the inner state" behavior from `invalidateRegions` to avoid exposing the inner state through this channel. The method `CallEvent::invalidateRegions()` was only called in two locations, only one of those was relying on the existence of the default argument value, and even there it was easy to explicitly pass the `State` object. This commit also eliminates a footgun: with the old logic, if some code had tried to pass a state explicitly (presumably because the "inner" state of the call was obsolete) but that "new" state happened to be `nullptr`, then `invalidateRegions` would have silently used the inner state attached to the call. However, I reviewed the call sites and don't think that it was actually possible to reach this buggy execution path. Added: Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h clang/lib/StaticAnalyzer/Core/CallEvent.cpp clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h index 4aee16576ebd1..66da79970ca19 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -372,12 +372,10 @@ class CallEvent { ProgramPoint getProgramPoint(bool IsPreVisit = false, const ProgramPointTag *Tag = nullptr) const; - /// Returns a new state with all argument regions invalidated. - /// - /// This accepts an alternate state in case some processing has already - /// occurred. + /// Invalidates the regions (arguments, globals, special regions like 'this') + /// that may have been written by this call, returning the updated state. ProgramStateRef invalidateRegions(unsigned BlockCount, - ProgramStateRef Orig = nullptr) const; + ProgramStateRef State) const; using FrameBindingTy = std::pair<SVal, SVal>; using BindingsTy = SmallVectorImpl<FrameBindingTy>; diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index 62460cc6f5b19..d04c827ce1391 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -230,13 +230,11 @@ static void findPtrToConstParams(llvm::SmallSet<unsigned, 4> &PreserveArgs, } ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount, - ProgramStateRef Orig) const { - ProgramStateRef Result = (Orig ? Orig : getState()); - + ProgramStateRef State) const { // Don't invalidate anything if the callee is marked pure/const. - if (const Decl *callee = getDecl()) - if (callee->hasAttr<PureAttr>() || callee->hasAttr<ConstAttr>()) - return Result; + if (const Decl *Callee = getDecl()) + if (Callee->hasAttr<PureAttr>() || Callee->hasAttr<ConstAttr>()) + return State; SmallVector<SVal, 8> ValuesToInvalidate; RegionAndSymbolInvalidationTraits ETraits; @@ -278,10 +276,10 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount, // Invalidate designated regions using the batch invalidation API. // NOTE: Even if RegionsToInvalidate is empty, we may still invalidate // global variables. - return Result->invalidateRegions(ValuesToInvalidate, getCFGElementRef(), - BlockCount, getLocationContext(), - /*CausedByPointerEscape*/ true, - /*Symbols=*/nullptr, this, &ETraits); + return State->invalidateRegions(ValuesToInvalidate, getCFGElementRef(), + BlockCount, getLocationContext(), + /*CausedByPointerEscape*/ true, + /*Symbols=*/nullptr, this, &ETraits); } ProgramPoint CallEvent::getProgramPoint(bool IsPreVisit, diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 75d7e265af0f3..00e3ef8311919 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -1013,7 +1013,7 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, // FIXME: Once we figure out how we want allocators to work, // we should be using the usual pre-/(default-)eval-/post-call checkers // here. - State = Call->invalidateRegions(blockCount); + State = Call->invalidateRegions(blockCount, State); if (!State) return; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
