Re: Thread Safety Analysis and the Linux kernel

Peter Zijlstra
2025-03-07T08:00:01Z

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.