Re: Thread Safety Analysis and the Linux kernel

[Peter Zijlstra <peterz@infradead.org>]

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.