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). > 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. While likely moot with the above, I agree that we could make such an assumption if need be. Jan > Let me know if that seems acceptable. > > Thanks, Roger. >
