Re: Thread Safety Analysis and the Linux kernel

Peter Zijlstra
2025-03-07T14:13:13Z

On Fri, Mar 07, 2025 at 08:59:50AM +0100, Peter Zijlstra wrote:
> On Thu, Mar 06, 2025 at 10:18:32PM +0000, Puchert, Aaron wrote:
>
> > > Users would typically look like:
> > >
> > > try_to_wake_up(p, state)
> > > {
> > > struct rq *rq;
> > >
> > > scoped_guard (raw_spinlock_irqsave, &p->pi_lock) {
> > > if (!ttwu_state_match(p, state))
> > > break;
> > >
> > > rq = __task_rq_lock(p);
> > > // go enqueue task
> > > raw_spin_rq_unlock(rq);
> > > }
> > > }
> >
> > Can the return value be used as an initializer by moving the
> > declaration into the scoped_guard block? Or do you have a style guide
> > that wants all declarations at the beginning of a block?
>
> Yeah, we have a style guide that strongly suggests variables are
> declared at the start. In fact, we used to have
> -Wdeclaration-after-statement and only (finally) got rid of it in order
> to allow for these scope guards.
>
> > We track capabilities as symbolic expressions, so something like
> > "rq->__lock" in this case. If there is an assignment to "rq", that
> > changes the meaning of the symbolic expression. The object referred to
> > by the expression is then no longer reachable. Currently we don't look
> > at assignments at all when it comes to tracking capabilities. We don't
> > even warn, it's simply documented as not being supported. We only look
> > at initializers, as in alias analysis. Having a separate variable
> > being initialized with the return value gives us a unique name for the
> > return value, which is good when we're working with symbolic
> > expressions.
> >
> > If there are cases where assignment is really needed, we can also
> > check to which extent we can rewrite expressions or warn when the
> > objects they point to become unreachable. (In the example that would
> > be the case if someone assigned to "rq" again after the call to
> > "__task_rq_lock ".)
>
> Right. I suspect we might need this if we want to minimize code churn.

There is also a case where a guard wraps the pointer in a struct;
because we need to carry extra state, like IRQ flags and the like, or
because the lock doesn't have a native type (RCU).

In this case the constructor things will return this structure and we'll
get a local copy on stack, a pointer to which will then be handed to the
destructor thing.

It would be very nice if it could understand that pattern too.