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

[Jameson Nash via llvm-branch-commits]

vtjnash wrote:

For a real-world data point (with help from Claude to test and format this): this catches an annotation inaccuracy in today's Linux kernel.

I ran the kernel's Clang context analysis (`CONFIG_WARN_CONTEXT_ANALYSIS=y`, which enables
`-Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta`) over every
context-analysis-enabled subsystem of torvalds/linux @ 89a312991dc6 (v7.3-rc1), comparing
clang at this PR against its base. This PR adds exactly one new kernel diagnostic:

```
kernel/locking/mutex.c:1165:2: warning: acquiring mutex 'lock' that may already be held [-Wthread-safety-analysis]
 1165 |         __acquire(lock);
      |         ^
./include/linux/compiler-context-analysis.h:375:23: note: expanded from macro '__acquire'
  375 | #define __acquire(x)            __acquire_ctx_lock(x)
      |                                 ^
kernel/locking/mutex.c:1164:2: note: mutex acquired here
 1164 |         __mutex_lock(lock, TASK_UNINTERRUPTIBLE, 0, NULL, _RET_IP_);
      |         ^
```

The flagged code is the annotate-the-slowpath wrapper
([kernel/locking/mutex.c#L1160-L1166](https://github.com/torvalds/linux/blob/89a312991dc6e638a36adc43ccb91dbc25504c04/kernel/locking/mutex.c#L1160-L1166)):

```c
static noinline void __sched
__mutex_lock_slowpath(struct mutex *lock)
      __acquires(lock)
{
      __mutex_lock(lock, TASK_UNINTERRUPTIBLE, 0, NULL, _RET_IP_);
      __acquire(lock);
}
```

Per the kernel's own annotations, this double-acquires:

* `__mutex_lock()` is annotated `__cond_acquires(0, lock)`
  ([mutex.c#L816-L821](https://github.com/torvalds/linux/blob/89a312991dc6e638a36adc43ccb91dbc25504c04/kernel/locking/mutex.c#L816-L821)),
  which desugars to `try_acquire_capability(0, lock)`
  ([compiler-context-analysis.h#L303-L326](https://github.com/torvalds/linux/blob/89a312991dc6e638a36adc43ccb91dbc25504c04/include/linux/compiler-context-analysis.h#L303-L326)) —
  a *conditional* acquisition whose result line 1164 discards.
* `__acquire(lock)` then performs a second, unconditional acquisition: it resolves to an
  inline helper carrying a real `acquire_capability` attribute
  ([compiler-context-analysis.h#L375](https://github.com/torvalds/linux/blob/89a312991dc6e638a36adc43ccb91dbc25504c04/include/linux/compiler-context-analysis.h#L375),
  helpers defined at [#L112-L131](https://github.com/torvalds/linux/blob/89a312991d4/include/linux/compiler-context-analysis.h#L112-L131)).

Before this change, an unbranched try-acquire left no trace in the lockset, so the
inconsistency was invisible; with the try-held state, the `__acquire` on the next line is
correctly recognized as acquiring a capability that may already be held — note the "may",
which is this PR's try-held variant of the double-lock diagnostic.

The call in fact always succeeds for `TASK_UNINTERRUPTIBLE`, but the annotations cannot
express that. The clean fix is to assert on the trylock result instead of re-annotate
e.g.:

```c
      BUG_ON(__mutex_lock(lock, TASK_UNINTERRUPTIBLE, 0, NULL, _RET_IP_));
```

(Compile-verified against the same tree and flags: with this change mutex.o builds with zero thread-safety warnings).

The failure path is an abort and the surviving path proves the zero (success) result, so the
analysis resolves the try-held fact to a definitely-held one — no second acquisition
annotation needed, and the fact keeps its identity so the release pairing implied by
`__acquires(lock)` on the wrapper stays intact. (`__assume_ctx_lock()` would also silence
the warning, but an assumed capability discards the release annotations, so asserting on
the result is the more faithful fix.)

For context: across the whole 11-commit stack this is one of only three new diagnoses on
the kernel corpus (the other two, in `drivers/nvme/host/ioctl.c`, come from the
unchecked-result `[-Wthread-safety-beta]` diagnostic later in the stack which traces to
`__cond_releases()` desugaring into a `__releases(x) __try_acquires_ctx_lock(0, x)`.
The callers in that file assume that the release was guaranteed – which it is, but not for reasons that the analyzer would be able to observe related to the specific values being passed to the caller of the function that unlocks the lock. A BUG_ON here also will silence it. I'll make a full version of this comment when making that commit into a PR.)
Every other commit in the stack is diagnostic-neutral on the linux kernel.

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