https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59856
--- Comment #18 from Josh Triplett <josh at joshtriplett dot org> ---
(In reply to PaX Team from comment #17)
> (In reply to Josh Triplett from comment #16)
> > (In reply to PaX Team from comment #14)
> > > can you give me an example of this?
> >
> > Sparse warns if you can reach the same basic block with different context
> > values (for instance, with or without a lock held). That warning *may*
> > indicate a problem, but it can also just indicate code too intricate for
> > Sparse to understand.
>
> i see, so this would basically trigger every time a lock is taken/released
> based on a condition as in the example in comment #1?
Yes.
> or is there some
> deeper analysis done to avoid at least some false positives?
Nope. Sparse always generates a warning in such a case. Usually you can work
around that by simplifying your locking, which seems like a good idea
regardless.
> in any case, with my method this checking is pretty easy too since the SSA
> representation ensures that we get a phi for the local context tracking
> variable if different paths produce different values when a given basic
> block is reached. it also seems that gcc can eliminate at least some trivial
> false reports since none of your conditional locking examples produce a
> false positive or negative (however i expect real linux code to do worse).
I don't think it makes sense to issue "differing contexts for basic block"
warnings at all.
> > (In reply to Tom Tromey from comment #15)
> > > Anyway given that this is happening, I suggest closing this gcc bug.
> >
> > Have we completely ruled out the possibility of a native GCC implementation?
> > This really doesn't seem specific to the Linux kernel in any way; it'd be
> > nice if this could work in most programs. We could add annotations to
> > pthread locking functions, and any program using locking would benefit.
>
> i agree, this context attribute based invariant verification is very generic
> and could be worked into a general gcc feature. the plugin approach is a
> good way to prototype it and get it some real life use before it'd make into
> the compiler.
>
> speaking of generalization, the encoding of the lock expression (e.g., lock
> ptr) will have to change as it really doesn't seem feasible to refer to
> function arguments from an attribute.
Doesn't seem too unreasonable to handle, actually. When parsing an attribute,
refer back to the function arguments; the context expression must either use
those or globals.
> as long as we expect to feed the
> compiler only manual annotations, the only thing that matters is that the
> lock expression be something unique, so as a compromise i suggest to simply
> make it a string constant, which would both preserve context for a human
> programmer and be also easy for the compiler to derive a unique name from
> for the local variable tracking the given context. i.e., instead of
> __attribute__((context(lock_expr, in, out))) we'd have
> __attribute__((context("lock_expr", in, out))) and ditto for calling
> __context__().
That fails for a really common case that should generate a warning:
spin_lock(&some_lock);
/* Piles of code */
spin_unlock(&some_other_lock);
I don't think stringizing the lock argument and using that as the context makes
sense, either, as it need not textually match to semantically match.
