Re: [PATCH] build/xen: fix symbol generation with LLVM LD

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, May 06, 2022 at 03:31:12PM +0200, Roger Pau Monné wrote:
> On Fri, May 06, 2022 at 02:56:56PM +0200, Jan Beulich wrote:
> > On 05.05.2022 16:21, Roger Pau Monne wrote:
> > > --- a/xen/include/xen/compiler.h
> > > +++ b/xen/include/xen/compiler.h
> > > @@ -125,10 +125,11 @@
> > >  #define __must_be_array(a) \
> > >    BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), 
> > > typeof(&a[0])))
> > >  
> > > -#ifdef CONFIG_CC_HAS_VISIBILITY_ATTRIBUTE
> > > -/* Results in more efficient PIC code (no indirections through GOT or 
> > > PLT). */
> > > -#pragma GCC visibility push(hidden)
> > > -#endif
> > > +/*
> > > + * Results in more efficient PIC code (no indirections through GOT or 
> > > PLT)
> > > + * and is also required by some of the assembly constructs.
> > > + */
> > > +#pragma GCC visibility push(protected)
> > >  
> > >  /* Make the optimizer believe the variable can be manipulated 
> > > arbitrarily. */
> > >  #define OPTIMIZER_HIDE_VAR(var) __asm__ ( "" : "+g" (var) )
> > 
> > This has failed my pre-push build test, with massive amounts of errors
> > about asm() constraints in the alternative call infrastructure. This
> > was with gcc 11.3.0.
> 
> Hm, great. I guess I will have to use protected with clang and hidden
> with gcc then, for lack of a better solution.
> 
> I'm slightly confused as to why my godbolt example:
> 
> https://godbolt.org/z/chTnMWxeP
> 
> Seems to work with gcc 11 then.  I will have to investigate a bit I
> think.

So it seems the problem is explicitly with constructs like:

void (*foo)(void);

void test(void)
{
    asm volatile (".long [addr]" :: [addr] "i" (&(foo)));
}

See:

https://godbolt.org/z/TYqeGdWsn

AFAICT gcc will consider the function pointer foo to go through the
GOT/PLT redirection table, while clang will not.  I think gcc behavior
is correct because in theory foo could be set from a different module?
protect only guarantees that references to local functions cannot be
overwritten, but not external ones.

I don't really see a good way to fix this, rather that setting
different visibilities based on the compiler.  clang would use
protected and gcc would use hidden.  I think it's unlikely to have a
toolstack setup to use gcc as the compiler and LLVM LD as the
linker, which would be the problematic configuration, and even in that
case it's kind of a cosmetic issue with symbol resolution, binary
output from the linker would still be correct.

Let me know if that seems acceptable.

Thanks, Roger.