On Tue, Mar 04, 2025 at 10:21:05AM +0100, Marco Elver wrote:
> Due to the scoped cleanup helpers used for lock guards wrapping
> acquire/release around their own constructors/destructors that store
> pointers to the passed locks in a separate struct, we currently cannot
> accurately annotate *destructors* which lock was released. While it's
> possible to annotate the constructor to say which lock was acquired,
> that alone would result in false positives claiming the lock was not
> released on function return.
>
> Instead, to avoid false positives, we can claim that the constructor
> "asserts" that the taken lock is held. This will ensure we can still
> benefit from the analysis where scoped guards are used to protect access
> to guarded variables, while avoiding false positives. The only downside
> are false negatives where we might accidentally lock the same lock
> again:
>
> raw_spin_lock(&my_lock);
> ...
> guard(raw_spinlock)(&my_lock); // no warning
>
> Arguably, lockdep will immediately catch issues like this.
>
> While Clang's analysis supports scoped guards in C++ [1], there's no way
> to apply this to C right now. Better support for Linux's scoped guard
> design could be added in future if deemed critical.
Would definitely be nice to have.
> @@ -383,6 +387,7 @@ static inline void
> *class_##_name##_lock_ptr(class_##_name##_t *_T) \
>
> #define __DEFINE_LOCK_GUARD_1(_name, _type, _lock) \
> static inline class_##_name##_t class_##_name##_constructor(_type *l)
> \
> + __no_capability_analysis __asserts_cap(l) \
> { \
> class_##_name##_t _t = { .lock = l }, *_T = &_t; \
> _lock; \
> @@ -391,6 +396,7 @@ static inline class_##_name##_t
> class_##_name##_constructor(_type *l) \
>
> #define __DEFINE_LOCK_GUARD_0(_name, _lock) \
> static inline class_##_name##_t class_##_name##_constructor(void) \
> + __no_capability_analysis \
Does this not need __asserts_cal(_lock) or somesuch?
GUARD_0 is the one used for RCU and preempt, rather sad if it doesn't
have annotations at all.
