[clang] Thread Safety Analysis: Drop most call-based alias invalidation (PR #190154)

[Marco Elver via cfe-commits]

melver wrote:


> What I'm thinking now is to just fix this bug. It will solve the problem of #183640. And we can keep the invalidation mechanism for soundness.

I'm in favour. If we can keep *soundness* AND address the relevant false positives, I think it's overall the best option.

> [..] 
> What's tricky is that the `RELEASE` annotation, in contrast, should be interpreted at the pre-state. For `void g(Mutex * p) RELEASE(*p)`, it actually says "function `g` unlocks the object passed in by `p`". Function `g` may modify `*p`; thus, we do not know anything about `*p` after the call, but we do know that what `p` pointed to before the call has been unlocked.

Right.

> So TSA is currently doing it right for `RELEASE` but not `ACQUIRE`, though this did not matter when there was no invalidation mechanism.
> [...] 
> What TSA currently does wrong at `lock(&C)` is that the object pointed-to by `C` immediately before the call is locked, i.e., `*0` being locked; and then invalidates `C` to point to an object that is not locked. So we observe false alarms _"releasing mutex that was not held"_ and _"mutex is still held at the end of function"_.
> 
> Therefore, if we fix this bug, the problem of #183640 will be solved.

So effectively you're saying the right fix should be "evaluate ACQUIRE at post-state" to fix?  If you already have a patch to that effect, feel free to send - and I assume you suggest that I close this PR, or would you suggest we still drop some invalidation rules? Presumably not because that'd defeat the soundness argument.

https://github.com/llvm/llvm-project/pull/190154