Author: Donát Nagy Date: 2026-08-27T19:56:17+02:00 New Revision: 11d5d799507f704d76df795b53837755290cc794
URL: https://github.com/llvm/llvm-project/commit/11d5d799507f704d76df795b53837755290cc794 DIFF: https://github.com/llvm/llvm-project/commit/11d5d799507f704d76df795b53837755290cc794.diff LOG: [analyzer] Do not attempt transition from sink node (#218970) The method `CheckerContext::addSink`, which was added in 2020 by commit 1c8f999e0b59731a4214f76528f83e4196e1fcc3 contained the following code: ``` addTransition(State, generateSink(State, getPredecessor())) ``` At first glance this looks as if this generates a sink then transitions TO that sink node; but in fact `generateSink` already adds the freshly generated sink node to the graph, and then `addTransition` tries to add a transition FROM the sink to a freshly generated non-sink node. Note that `ExplodedNode::addPredecessor` (which is called by `makeNode` to connect the freshly created node to its parent) asserts that the parent node is not a sink -- but this assertion is narrowly dodged because the early return at the beginning of `CheckerContext::addTransitionImpl` is practically always [1] triggered by the `addTransition` call in `addSink`. Due to this early return `makeNode` is never reached and `addTransition` does nothing. This commit removes the pointless and logically unjustified `addTransition` call from `addSink`. This probably won't have any functional impact, but could in theory prevent crashes in very rare corner cases e.g. related to `PosteriorlyOverconstrained` states. After this change it would be important to clean up the `CheckerContext` interface, where currently multiple methods fulfill the same role (e.g. `generateSink`, `addSink` and `generateErrorNode` are all redundant) and the names of `addTransition` vs `generateSink` do not reflect their relationship. ------ [1] The early return in `addTransitionImpl` is triggered if (1) the state is equal to `CheckerContext::getState()`, (2) `Tag` is null and (3) `MarkAsSink` is false (which always holds in the `addTransition` call). Callers of `addSink` practically always pass a state that is equal to the initial state (with the possible exception of `PosteriorlyOverconstrained` states and one corner case in CStringChecker.cpp which is probably never reached) and never specify a non-null `Tag`, so these happen to be true in practically all calls of `addSink`. Added: Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h index 6463bad3de6ab..331a163be443f 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -220,7 +220,7 @@ class CheckerContext { const ProgramPointTag *Tag = nullptr) { if (!State) State = getState(); - addTransition(State, generateSink(State, getPredecessor())); + generateSink(State, getPredecessor()); } /// Generate a transition to a node that will be used to report _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
