[clang] [analyzer] Avoid use of `CallEvent`s with obsolete state (PR #160707)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 8 05:54:30 PDT 2025
NagyDonat wrote:
> By making `getState()` private/protected you effectively prove it at compile time that the code no longer makes the mistake you're describing. I see this as a much better thing to do than making a runtime test. Like, why would we even be here if we didn't believe in the superiority of compile-time bug prevention? 😅
>
> Also we're not even trying to prevent the `CallEvent`'s inner state from going out of sync.
No, this commit _is_ trying to prevent the `CallEvent`'s inner state from going out of sinc.
I am proposing a hybrid approach:
1. This commit ensures that the inner state of the `CallEvent` is up-to-date (for the EvalCall and PointerEscape callbacks that are targeted by this commit).
2. The planned followup commit will make `getState()` protected to limit access to the inner state of `CallEvent` and ensure that the analyzer works correctly even if the inner state of the `CallEvent` is obsolete.
These two commits are mostly redundant with each other (either of them would fix 90% of the problems alone), but I see minor reasons for applying both of them:
- If we make `getState()` protected but don't apply this commit the method `CXXInstanceCall::getDeclForDynamicType()` and the analogous tricky Objective-C logic – which I don't understand – could theoretically leak the obsolete parts of the state attached to the `CallEvent`. (I don't know whether it actually leaks problematic parts of the state, but I cannot rule it out.)
- Also, I would prefer applying this commit from an aesthetic "Why not be accurate?" point of view – if we know the accurate state and can easily attach it to the `CallEvent`, then I prefer just attaching it instead of reasoning like "actually, the old state is inaccurate but not too inaccurate, so we can keep and use it..."
- If we apply this commit but don't make `getState()` protected, then state attached to the `CallEvent` will be up-to-date at the start of the the EvalCall and PointerEscape callbacks (btw it is also up-to-date at the start of Pre/PostCall even without this commit), but for the sake of elegance and consistency I still don't want to see checkers that get the state from the `CallEvent` instead of getting the same state from the `CheckerContext`.
https://github.com/llvm/llvm-project/pull/160707
More information about the cfe-commits
mailing list