Re: [PATCH v2 06/34] cleanup: Basic compatibility with capability analysis

[Peter Zijlstra <peterz@infradead.org>]

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.