[llvm-branch-commits] [clang] Thread Safety Analysis: Track try-acquired capabilities as a ternary try-held state (PR #220635)

[Ziqing Luo via llvm-branch-commits]

ziqingluo-90 wrote:

Thank you @vtjnash. I was working locally on the same thing with the same idea, but slowly because I have other major work. So I'm more than happy to see this effort and will review your PRs.

I haven't read the code yet. I have some comments and questions about the design, which overall aligns with what I have in mind.

> A capability now has a ternary per-program-point state: not-held, try-held, or held.

This is exactly what I planned to propose. Though, I prefer to call "try-held" something else like "unknown", "maybe-held", or "conditionally-held". The reason is that this new state value represents the join of `held` and `not-held`, and `held U not-held = try-held` sounds a bit unnatural to me.
As a quick justification note, this third value implicitly exists in the current implementation already. It is not explicitly spelled out in the code because TSA reports a warning immediately when held joins not-held.

>A held/try-held join difference is ignored only when the terminator
re-branches on the same call that created both facts. Origins merged
from different calls, or overwritten by an acquire or assert, are
cleared: such facts are conservatively never resolved by a branch
again.

Not sure if I understand this one correctly. The existing special handling of re-branches on the same try-lock call is a workaround for the incomplete try-lock analysis. With the new design, we should be able to remove the workaround.



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