Re: [PATCH, -v2] compiler/gcc4: Add quirk for 'asm goto' miscompilation bug

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Oct 10, 2013 at 01:56:17PM +0200, Peter Zijlstra wrote:
> On Thu, Oct 10, 2013 at 10:55:06AM +0200, Ingo Molnar wrote:
> > +/*
> > + * GCC 'asm goto' miscompiles certain code sequences:
> > + *
> > + *   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
> > + *
> > + * Work it around via quirk suggested by Jakub Jelinek.
> > + * Fixed in GCC 4.8.2 and later versions.
> > + */
> > +#if GCC_VERSION <= 40801
> 
> We didn't do version checks for CC_HAVE_ASM_GOTO because of vendor
> backports; can't we detect this in the same way?

The problem is that it will be harder to check for this as compile time only
check, and for runtime check you'd need to have the assembly string for
every architecture and you couldn't do it for cross-compiling anyway.
For compile time only check, it wouldn't be 100% reliable, you could e.g.
check for that using -S -O2 -xc - -o - on:
int
foo (int a, int b)
{
  if (a)
    return -3;
  asm volatile goto ("asm volatile goto to %l[lab]" : : "m" (b) : "memory" : lab);
  return 0;
lab:
  return 0;
}
and use awk on the resulting assembly to find out if the
asm volatile goto to (.*)$
string, then skip lines starting in column 0 with an
assembly comment character(s) (#, %, //, not sure if those 3 are all you can
see) and check that the first non-skipped line starts with the string matching
(.*) earlier followed by : (or perhaps skip other labels too?).
That said, the check could fail even in fixed gccs, so perhaps you want to
combine that with both version check and test, if version is >= 4.8.3
(note, while I hope it will be fixed in 4.8.2 release, people using
prerelease compilers would still have __GNUC_PATCHLEVEL__ == 2, at least
in upstream gcc (e.g. in Fedora/RHEL we patch down the patchlevel version,
so that __GNUC_PATCHLEVEL__ is 2 only for GCC release x.y.2 and following
snapshots, while upstream bumps patchlevel immediately after a release is
made), even with gcc containing that bug.  So for >= 4.8.3 just assume no
workaround is needed, otherwise scan assembly.

> 
> > +# define __asm_goto(vol, x...) do { asm vol goto(x); asm (""); } while (0)
> > +#else
> > +# define __asm_goto(vol, x...) do { asm vol goto(x); } while (0)
> > +#endif
> 
> This places the asm("") in the fallthrough case; but Jakub wrote:
> 
> > @@ -8,6 +8,7 @@ foo (int a, int b)
> >    asm volatile goto ("bts $1, %0; jc %l[lab]" : : "m" (b) : "memory" : lab);
> >    return 0;
> >  lab:
> > +  asm ("");
> >    return 0;
> >  }
> 
> Which places the asm ("") after the label, these two are not the same.

See the follow-up mails, I think placing it immediately after asm goto might
be better.

	Jakub