On 16.05.2022 10:01, Roger Pau Monné wrote: > 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.
I agree, and I don't view it as likely that they would change their code. GNU objcopy offers a separate step for that conversion (--localize-hidden), but .. > 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. ... kind of unexpectedly doesn't offer means to alter visibility. So I guess you/we need to turn back to your original RFC approach, no matter that it wasn't really pretty. Jan
