[compiler-rt] [scudo] Add missing thread-safety analysis annotations. (PR #68072)

Aaron Puchert via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 7 08:29:58 PDT 2023


aaronpuchert wrote:

>From the looks of it, `ASSERT_CAPABILITY` should be dropped in all of these functions. It should perhaps have been called `ASSERTS_CAPABILITY` to avoid misunderstanding.

> My understanding is that `ASSERT_CAPABILITY` is a purely dynamic check, and does not imply `REQUIRES`. Adding @aaronpuchert for confirmation.

Correct, it's for cases where we can't statically prove that the lock is held, so you can add a runtime check as “escape hatch“. The compiler will assume that the lock is held starting from the call to an assert-annotated function.

> Just read the description again and I found that I misunderstood its meaning.
> 
> ```
> ... Presence of this annotation causes the analysis to assume the capability is held after calls to the annotated function ...
> ```

The documentation is tricky, we already had some discussions in [D87629](https://reviews.llvm.org/D87629).

> I thought it asserted _before_ the calling of the function.

Either way, we currently consider the attribute only when such a function is called, not within its own implementation. That's because we can't generally see assertions (e.g. with `-DNDEBUG`), and generally such a function doesn't do more than asserting.

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


More information about the llvm-commits mailing list