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

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

On Sun, May 08, 2022 at 10:34:43AM +0200, Jan Beulich wrote:
> On 06.05.2022 17:35, Roger Pau Monné wrote:
> > 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.
> 
> Right, since there's no way to tell the compiler that the symbol will
> be resolved in the same "module".
> 
> > 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.
> 
> If gcc's behavior is indeed correct, then moving to protected with
> clang would set us up for going through GOT/PLT there - either right
> away (if the implement this like gcc), or once they correct their
> behavior. I don't think we want that. Therefore I think we want to
> alter visibility between compilation and linking (i.e. presumably
> right in prelink.o), going from compile-time hidden to link-time
> protected. That would likely be closer to what your original patch
> did (sadly there's no "convert <visibility1> to <visibility2> option
> to objcopy, and making it have one wouldn't really help us here;
> it's also not clear to me whether llvm comes with its own objcopy,
> or whether they re-use GNU's).

So I've raised the difference in protected behavior between gcc and
clang:

https://discourse.llvm.org/t/gcc-vs-clang-differences-in-protected-visibility-implementation

It's no clear to me whether clang would switch it's implementation,
but it also seems fragile to rely on global protected function
pointers not going through the GOT.

Do you have any recommendation as to how to change symbol visibility?
I've been looking at objcopy, but I don't seem to find a way to do
it.

Thanks, Roger.