Re: [PATCH] docs/memory-barriers.txt: volatile is not a barrier() substitute

[Ard Biesheuvel <ardb@kernel.org>]

On Wed, 2 Feb 2022 at 04:26, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Tue, Feb 1, 2022 at 2:15 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 1 Feb 2022 at 20:40, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > >
> > > On Tue, Feb 1, 2022 at 1:32 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Mon, 31 Jan 2022 at 23:53, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > > >
> > > > > +     around an asm statement so long as clobbers are not violated. For example,
> > > > > +
> > > > > +       asm volatile ("");
> > > > > +       flag = true;
> > > > > +
> > > > > +     May be modified by the compiler to:
> > > > > +
> > > > > +       flag = true;
> > > > > +       asm volatile ("");
> > > > > +
> > > > > +     Marking an asm statement as volatile is not a substitute for barrier(),
> > > > > +     and is implicit for asm goto statements and asm statements that do not
> > > > > +     have outputs (like the above example). Prefer either:
> > > > > +
> > > > > +       asm ("":::"memory");
> > > > > +       flag = true;
> > > > > +
> > > > > +     Or:
> > > > > +
> > > > > +       asm ("");
> > > > > +       barrier();
> > > > > +       flag = true;
> > > > > +
> > > >
> > > > I would expect the memory clobber to only hazard against the
> > > > assignment of flag if it results in a store, but looking at your
> > > > Godbolt example, this appears to apply even if flag is kept in a
> > > > register.
> > > >
> > > > Is that behavior documented/codified anywhere? Or are we relying on
> > > > compiler implementation details here?
> > >
> > > https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile
> > > "Note that the compiler can move even volatile asm instructions
> > > relative to other code, including across jump instructions."
> > >
> >
> > That doesn't really answer my question. We are documenting here that
> > asm volatile does not prevent reordering but non-volatile asm with a
> > "memory" clobber does, and even prevents reordering of instructions
> > that do not modify memory to begin with.
> >
> > Why is it justified to rely on this undocumented behavior?
>
> I see your point.  You're right, I couldn't find anywhere where such
> behavior was specified.  So the suggestion to use barrier() would rely
> on unspecified behavior and should not be suggested.
>
> Probably worth still mentioning that `volatile` qualifying an asm
> statement doesn't prevent such reordering in this document somehow,
> and perhaps that it's (currently) unspecified whether a barrier() can
> prevent re-ordering with regards to non-memory-modifying instructions.

Yes, and to correct myself here, I meant 'instructions that do not
/reference/ memory to begin with.

I suppose the problem here is that the distinction only matters for
things like objtool obsessing about whether or not instructions look
unreachable, and relying on asm volatile() to insert markers into the
object code.

So it seems we need a stronger clobber, one which avoids any kind of
reordering at the instruction level.