diff mbox series

[RFC] x86/lld: fix symbol map generation

Message ID 20220502152020.19768-1-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show
Series [RFC] x86/lld: fix symbol map generation | expand

Commit Message

Roger Pau Monné May 2, 2022, 3:20 p.m. UTC
The symbol map generation (and thus the debug info attached to Xen) is
partially broken when using LLVM LD.  That's due to LLD converting
almost all symbols from global to local in the last linking step, and
thus confusing tools/symbols into adding a file prefix to all text
symbols, the results looks like:

Xen call trace:
   [<ffff82d040449fe8>] R xxhash64.c#__start_xen+0x3938/0x39c0
   [<ffff82d040203734>] F __high_start+0x94/0xa0

In order to workaround this create a list of global symbols prior to
the linking step, and use objcopy to convert the symbols in the final
binary back to global before processing with tools/symbols.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
I haven't found a way to prevent LLD from converting the symbols, so
I've come up with this rather crappy workaround.

Not applied to EFI, partially because I don't have an environment with
LLD capable of generating the EFI binary.

Obtaining the global symbol list could likely be a target on itself,
if it is to be shared between the ELF and the EFI binary generation.
---
 xen/arch/x86/Makefile | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

Comments

Jan Beulich May 3, 2022, 8:17 a.m. UTC | #1
On 02.05.2022 17:20, Roger Pau Monne wrote:
> The symbol map generation (and thus the debug info attached to Xen) is
> partially broken when using LLVM LD.  That's due to LLD converting
> almost all symbols from global to local in the last linking step, and

I'm puzzled by "almost" - is there a pattern of which ones aren't
converted?

Also "last linking step" is ambiguous, as we link three binaries and
aiui the issue is present on every of these passes. May I suggest
"... when linking actual executables" or (still somewhat ambiguous)
"... when linking final binaries"?

> thus confusing tools/symbols into adding a file prefix to all text
> symbols, the results looks like:
> 
> Xen call trace:
>    [<ffff82d040449fe8>] R xxhash64.c#__start_xen+0x3938/0x39c0
>    [<ffff82d040203734>] F __high_start+0x94/0xa0
> 
> In order to workaround this create a list of global symbols prior to
> the linking step, and use objcopy to convert the symbols in the final
> binary back to global before processing with tools/symbols.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> I haven't found a way to prevent LLD from converting the symbols, so
> I've come up with this rather crappy workaround.

Perhaps a map file (like we use for shared libraries in tools/) would
allow doing so? But of course this would want to be machine-generated,
not manually maintained.

Have you gained any insight into _why_ they are doing what they do?

> Not applied to EFI, partially because I don't have an environment with
> LLD capable of generating the EFI binary.
> 
> Obtaining the global symbol list could likely be a target on itself,
> if it is to be shared between the ELF and the EFI binary generation.

If, as the last paragraph of the description is worded, you did this
just once (as a prereq), I could see this working. Otherwise (as you
have it now, with it done 3 times) it would first require splitting
the linking rules into many separate ones (which has been the plan
anyway, but so far I didn't get to it).

> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -134,24 +134,34 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>  
>  $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
> +	# Dump global text symbols before the linking step
> +	$(NM) -pa --format=bsd $< | awk '{ if($$2 == "T") print $$3}' \
> +	    > $(@D)/.$(@F).global-syms
>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
> -	    $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0
> +	    $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0.tmp
> +	# LLVM LD has converted global symbols into local ones as part of the
> +	# linking step, convert those back to global before using tools/symbols.
> +	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms \
> +	    $(@D)/.$(@F).0.tmp $(@D)/.$(@F).0
>  	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
>  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>  		>$(@D)/.$(@F).0.S
>  	$(MAKE) $(build)=$(@D) $(@D)/.$(@F).0.o
>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
> -	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
> +	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1.tmp
> +	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms \
> +	    $(@D)/.$(@F).1.tmp $(@D)/.$(@F).1
>  	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
>  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \
>  		>$(@D)/.$(@F).1.S
>  	$(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o
>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
> -	    $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@
> +	    $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@.tmp
> +	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms $@.tmp $@

Is this very useful? It only affects ...

>  	$(NM) -pa --format=sysv $(@D)/$(@F) \
>  		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
>  		>$(@D)/$(@F).map

... the actual map file; what's in the binary and in this map file doesn't
depend on local vs global anymore (and you limit this to text symbols
anyway; I wonder in how far livepatching might also be affected by the
same issue with data symbols).

In any event I would like to ask that the objcopy invocations be tied to
lld being in use. No matter that it shouldn't, objcopy can alter binaries
even if no actual change is being made (I've just recently observed this
with xen.efi, see the thread rooted at "EFI: strip xen.efi when putting it
on the EFI partition", and recall that at least for GNU binutils objcopy
and strip are effectively [almost] the same binary).

Jan
Roger Pau Monné May 3, 2022, 9:15 a.m. UTC | #2
On Tue, May 03, 2022 at 10:17:44AM +0200, Jan Beulich wrote:
> On 02.05.2022 17:20, Roger Pau Monne wrote:
> > The symbol map generation (and thus the debug info attached to Xen) is
> > partially broken when using LLVM LD.  That's due to LLD converting
> > almost all symbols from global to local in the last linking step, and
> 
> I'm puzzled by "almost" - is there a pattern of which ones aren't
> converted?

This is the list of the ones that aren't converted:

__x86_indirect_thunk_r11
s3_resume
start
__image_base__
__high_start
wakeup_stack
wakeup_stack_start
handle_exception
dom_crash_sync_extable
common_interrupt
__x86_indirect_thunk_rbx
__x86_indirect_thunk_rcx
__x86_indirect_thunk_rax
__x86_indirect_thunk_rdx
__x86_indirect_thunk_rbp
__x86_indirect_thunk_rsi
__x86_indirect_thunk_rdi
__x86_indirect_thunk_r8
__x86_indirect_thunk_r9
__x86_indirect_thunk_r10
__x86_indirect_thunk_r12
__x86_indirect_thunk_r13
__x86_indirect_thunk_r14
__x86_indirect_thunk_r15

I assume there's some kind of pattern, but I haven't yet been able to
spot where triggers the conversion from global to local in lld.

> Also "last linking step" is ambiguous, as we link three binaries and
> aiui the issue is present on every of these passes. May I suggest
> "... when linking actual executables" or (still somewhat ambiguous)
> "... when linking final binaries"?
> 
> > thus confusing tools/symbols into adding a file prefix to all text
> > symbols, the results looks like:
> > 
> > Xen call trace:
> >    [<ffff82d040449fe8>] R xxhash64.c#__start_xen+0x3938/0x39c0
> >    [<ffff82d040203734>] F __high_start+0x94/0xa0
> > 
> > In order to workaround this create a list of global symbols prior to
> > the linking step, and use objcopy to convert the symbols in the final
> > binary back to global before processing with tools/symbols.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > I haven't found a way to prevent LLD from converting the symbols, so
> > I've come up with this rather crappy workaround.
> 
> Perhaps a map file (like we use for shared libraries in tools/) would
> allow doing so? But of course this would want to be machine-generated,
> not manually maintained.
> 
> Have you gained any insight into _why_ they are doing what they do?

I've informally asked on IRC but got no reply.  I've now created this:

https://discourse.llvm.org/t/conversion-of-text-symbols-from-global-to-local

> > Not applied to EFI, partially because I don't have an environment with
> > LLD capable of generating the EFI binary.
> > 
> > Obtaining the global symbol list could likely be a target on itself,
> > if it is to be shared between the ELF and the EFI binary generation.
> 
> If, as the last paragraph of the description is worded, you did this
> just once (as a prereq), I could see this working.

Yes, my comment was about splitting the:

$(NM) -pa --format=bsd $< | awk '{ if($$2 == "T") print $$3}' \
      > $(@D)/.$(@F).global-syms

rune into a separate $(TARGET)-syms.global-syms target or some such.
Not sure it's really worth it.

> Otherwise (as you
> have it now, with it done 3 times) it would first require splitting
> the linking rules into many separate ones (which has been the plan
> anyway, but so far I didn't get to it).
> 
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -134,24 +134,34 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
> >  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
> >  
> >  $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
> > +	# Dump global text symbols before the linking step
> > +	$(NM) -pa --format=bsd $< | awk '{ if($$2 == "T") print $$3}' \
> > +	    > $(@D)/.$(@F).global-syms
> >  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
> > -	    $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0
> > +	    $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0.tmp
> > +	# LLVM LD has converted global symbols into local ones as part of the
> > +	# linking step, convert those back to global before using tools/symbols.
> > +	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms \
> > +	    $(@D)/.$(@F).0.tmp $(@D)/.$(@F).0
> >  	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
> >  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
> >  		>$(@D)/.$(@F).0.S
> >  	$(MAKE) $(build)=$(@D) $(@D)/.$(@F).0.o
> >  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
> > -	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
> > +	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1.tmp
> > +	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms \
> > +	    $(@D)/.$(@F).1.tmp $(@D)/.$(@F).1
> >  	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
> >  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \
> >  		>$(@D)/.$(@F).1.S
> >  	$(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o
> >  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
> > -	    $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@
> > +	    $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@.tmp
> > +	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms $@.tmp $@
> 
> Is this very useful? It only affects ...
> 
> >  	$(NM) -pa --format=sysv $(@D)/$(@F) \
> >  		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
> >  		>$(@D)/$(@F).map
> 
> ... the actual map file; what's in the binary and in this map file doesn't
> depend on local vs global anymore (and you limit this to text symbols
> anyway; I wonder in how far livepatching might also be affected by the
> same issue with data symbols).

If I don't add this step then the map file will also end up with lines
like:

0xffff82d0405b6968 b lib/xxhash64.c#iommuv2_enabled
0xffff82d0405b6970 b lib/xxhash64.c#nr_ioapic_sbdf
0xffff82d0405b6980 b lib/xxhash64.c#ioapic_sbdf

I see the same happen with other non-text symbols, so I would likely
need to extend the fixing to preserve all global symbols from the
input file, not just text ones.

> In any event I would like to ask that the objcopy invocations be tied to
> lld being in use. No matter that it shouldn't, objcopy can alter binaries
> even if no actual change is being made (I've just recently observed this
> with xen.efi, see the thread rooted at "EFI: strip xen.efi when putting it
> on the EFI partition", and recall that at least for GNU binutils objcopy
> and strip are effectively [almost] the same binary).

Right, that's fine.  I would still hope to find a better solution,
this is quite crappy IMO.

Thanks, Roger.
Jan Beulich May 3, 2022, 4:06 p.m. UTC | #3
On 03.05.2022 11:15, Roger Pau Monné wrote:
> On Tue, May 03, 2022 at 10:17:44AM +0200, Jan Beulich wrote:
>> On 02.05.2022 17:20, Roger Pau Monne wrote:
>>> The symbol map generation (and thus the debug info attached to Xen) is
>>> partially broken when using LLVM LD.  That's due to LLD converting
>>> almost all symbols from global to local in the last linking step, and
>>
>> I'm puzzled by "almost" - is there a pattern of which ones aren't
>> converted?
> 
> This is the list of the ones that aren't converted:
> 
> __x86_indirect_thunk_r11
> s3_resume
> start
> __image_base__
> __high_start
> wakeup_stack
> wakeup_stack_start
> handle_exception
> dom_crash_sync_extable
> common_interrupt
> __x86_indirect_thunk_rbx
> __x86_indirect_thunk_rcx
> __x86_indirect_thunk_rax
> __x86_indirect_thunk_rdx
> __x86_indirect_thunk_rbp
> __x86_indirect_thunk_rsi
> __x86_indirect_thunk_rdi
> __x86_indirect_thunk_r8
> __x86_indirect_thunk_r9
> __x86_indirect_thunk_r10
> __x86_indirect_thunk_r12
> __x86_indirect_thunk_r13
> __x86_indirect_thunk_r14
> __x86_indirect_thunk_r15
> 
> I assume there's some kind of pattern, but I haven't yet been able to
> spot where triggers the conversion from global to local in lld.

At least this looks to all be symbols defined in assembly files, which
don't have a C-visible declaration.

>>> Not applied to EFI, partially because I don't have an environment with
>>> LLD capable of generating the EFI binary.
>>>
>>> Obtaining the global symbol list could likely be a target on itself,
>>> if it is to be shared between the ELF and the EFI binary generation.
>>
>> If, as the last paragraph of the description is worded, you did this
>> just once (as a prereq), I could see this working.
> 
> Yes, my comment was about splitting the:
> 
> $(NM) -pa --format=bsd $< | awk '{ if($$2 == "T") print $$3}' \
>       > $(@D)/.$(@F).global-syms
> 
> rune into a separate $(TARGET)-syms.global-syms target or some such.
> Not sure it's really worth it.

Probably indeed only when splitting up the rule as a whole.

>>> --- a/xen/arch/x86/Makefile
>>> +++ b/xen/arch/x86/Makefile
>>> @@ -134,24 +134,34 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
>>>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>>>  
>>>  $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
>>> +	# Dump global text symbols before the linking step
>>> +	$(NM) -pa --format=bsd $< | awk '{ if($$2 == "T") print $$3}' \
>>> +	    > $(@D)/.$(@F).global-syms
>>>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
>>> -	    $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0
>>> +	    $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0.tmp
>>> +	# LLVM LD has converted global symbols into local ones as part of the
>>> +	# linking step, convert those back to global before using tools/symbols.
>>> +	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms \
>>> +	    $(@D)/.$(@F).0.tmp $(@D)/.$(@F).0
>>>  	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
>>>  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
>>>  		>$(@D)/.$(@F).0.S
>>>  	$(MAKE) $(build)=$(@D) $(@D)/.$(@F).0.o
>>>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
>>> -	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
>>> +	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1.tmp
>>> +	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms \
>>> +	    $(@D)/.$(@F).1.tmp $(@D)/.$(@F).1
>>>  	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
>>>  		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \
>>>  		>$(@D)/.$(@F).1.S
>>>  	$(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o
>>>  	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
>>> -	    $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@
>>> +	    $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@.tmp
>>> +	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms $@.tmp $@
>>
>> Is this very useful? It only affects ...
>>
>>>  	$(NM) -pa --format=sysv $(@D)/$(@F) \
>>>  		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
>>>  		>$(@D)/$(@F).map
>>
>> ... the actual map file; what's in the binary and in this map file doesn't
>> depend on local vs global anymore (and you limit this to text symbols
>> anyway; I wonder in how far livepatching might also be affected by the
>> same issue with data symbols).
> 
> If I don't add this step then the map file will also end up with lines
> like:
> 
> 0xffff82d0405b6968 b lib/xxhash64.c#iommuv2_enabled
> 0xffff82d0405b6970 b lib/xxhash64.c#nr_ioapic_sbdf
> 0xffff82d0405b6980 b lib/xxhash64.c#ioapic_sbdf
> 
> I see the same happen with other non-text symbols, so I would likely
> need to extend the fixing to preserve all global symbols from the
> input file, not just text ones.

Oh, I see - yes, this wants avoiding.

Jan
Roger Pau Monné May 5, 2022, 8:39 a.m. UTC | #4
On Tue, May 03, 2022 at 10:17:44AM +0200, Jan Beulich wrote:
> On 02.05.2022 17:20, Roger Pau Monne wrote:
> > The symbol map generation (and thus the debug info attached to Xen) is
> > partially broken when using LLVM LD.  That's due to LLD converting
> > almost all symbols from global to local in the last linking step, and
> 
> I'm puzzled by "almost" - is there a pattern of which ones aren't
> converted?
> 
> Also "last linking step" is ambiguous, as we link three binaries and
> aiui the issue is present on every of these passes. May I suggest
> "... when linking actual executables" or (still somewhat ambiguous)
> "... when linking final binaries"?
> 
> > thus confusing tools/symbols into adding a file prefix to all text
> > symbols, the results looks like:
> > 
> > Xen call trace:
> >    [<ffff82d040449fe8>] R xxhash64.c#__start_xen+0x3938/0x39c0
> >    [<ffff82d040203734>] F __high_start+0x94/0xa0
> > 
> > In order to workaround this create a list of global symbols prior to
> > the linking step, and use objcopy to convert the symbols in the final
> > binary back to global before processing with tools/symbols.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > I haven't found a way to prevent LLD from converting the symbols, so
> > I've come up with this rather crappy workaround.
> 
> Perhaps a map file (like we use for shared libraries in tools/) would
> allow doing so? But of course this would want to be machine-generated,
> not manually maintained.
> 
> Have you gained any insight into _why_ they are doing what they do?

So after raising this in the LLVM LD forum, I was told this behavior
is due to the spec:

"A hidden symbol contained in a relocatable object must be either
removed or converted to STB_LOCAL binding by the link-editor when the
relocatable object is included in an executable file or shared
object."

Then I did some search myself and found that you raised the same with
GNU ld not doing the conversion:

https://sourceware.org/bugzilla/show_bug.cgi?id=12374

So it seems LLVM LD goes a bit further than GNU ld and also changes
the binding of symbols in the .symtab.  I'm not sure I would consider
the behavior of either linkers wrong.

As a test I've attempted to disable the hidden visibility setting we
set in compiler.h, just to realize that parts of our code do rely on
having hidden visibility.  That's the bug and alternative constructs
that use the "i" asm constrain with function pointers.  That's only
possible in the absence of a GOT or PLT table:

https://godbolt.org/z/jK3bq4fhe

So I think the way to fix this would be to set the visibility to
protected instead of hidden, and then to also make the setting of the
visibility unconditional: the compiler not supporting -fvisibility and
Xen not setting it will just lead to compiler errors further on during
the build process.

Thanks, Roger.
Jan Beulich May 5, 2022, 11:44 a.m. UTC | #5
On 05.05.2022 10:39, Roger Pau Monné wrote:
> On Tue, May 03, 2022 at 10:17:44AM +0200, Jan Beulich wrote:
>> On 02.05.2022 17:20, Roger Pau Monne wrote:
>>> The symbol map generation (and thus the debug info attached to Xen) is
>>> partially broken when using LLVM LD.  That's due to LLD converting
>>> almost all symbols from global to local in the last linking step, and
>>
>> I'm puzzled by "almost" - is there a pattern of which ones aren't
>> converted?
>>
>> Also "last linking step" is ambiguous, as we link three binaries and
>> aiui the issue is present on every of these passes. May I suggest
>> "... when linking actual executables" or (still somewhat ambiguous)
>> "... when linking final binaries"?
>>
>>> thus confusing tools/symbols into adding a file prefix to all text
>>> symbols, the results looks like:
>>>
>>> Xen call trace:
>>>    [<ffff82d040449fe8>] R xxhash64.c#__start_xen+0x3938/0x39c0
>>>    [<ffff82d040203734>] F __high_start+0x94/0xa0
>>>
>>> In order to workaround this create a list of global symbols prior to
>>> the linking step, and use objcopy to convert the symbols in the final
>>> binary back to global before processing with tools/symbols.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> ---
>>> I haven't found a way to prevent LLD from converting the symbols, so
>>> I've come up with this rather crappy workaround.
>>
>> Perhaps a map file (like we use for shared libraries in tools/) would
>> allow doing so? But of course this would want to be machine-generated,
>> not manually maintained.
>>
>> Have you gained any insight into _why_ they are doing what they do?
> 
> So after raising this in the LLVM LD forum, I was told this behavior
> is due to the spec:
> 
> "A hidden symbol contained in a relocatable object must be either
> removed or converted to STB_LOCAL binding by the link-editor when the
> relocatable object is included in an executable file or shared
> object."
> 
> Then I did some search myself and found that you raised the same with
> GNU ld not doing the conversion:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=12374

Hmm, interesting. Too long ago to remember, but yes.

> So it seems LLVM LD goes a bit further than GNU ld and also changes
> the binding of symbols in the .symtab.  I'm not sure I would consider
> the behavior of either linkers wrong.

I agree (taking into account Alan's comment in the bug report above).

> As a test I've attempted to disable the hidden visibility setting we
> set in compiler.h, just to realize that parts of our code do rely on
> having hidden visibility.  That's the bug and alternative constructs
> that use the "i" asm constrain with function pointers.  That's only
> possible in the absence of a GOT or PLT table:
> 
> https://godbolt.org/z/jK3bq4fhe

Right, -fpic would then also need to go away.

> So I think the way to fix this would be to set the visibility to
> protected instead of hidden,

Originally, when we introduced the use of hidden, it was pretty clear
that protected would suffice, but using hidden seemed more logical.
Now that we have a reason where hidden ends up being too strict, I
agree we can switch to protected.

> and then to also make the setting of the
> visibility unconditional: the compiler not supporting -fvisibility and
> Xen not setting it will just lead to compiler errors further on during
> the build process.

I guess this being conditional pre-dates our requiring of gcc 4.1,
as that version looks to support both the command line option and
the pragma we use. So switching to making this unconditional ought to
be fine.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 177a2ff742..f3817827bc 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -134,24 +134,34 @@  $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
+	# Dump global text symbols before the linking step
+	$(NM) -pa --format=bsd $< | awk '{ if($$2 == "T") print $$3}' \
+	    > $(@D)/.$(@F).global-syms
 	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
-	    $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0
+	    $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0.tmp
+	# LLVM LD has converted global symbols into local ones as part of the
+	# linking step, convert those back to global before using tools/symbols.
+	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms \
+	    $(@D)/.$(@F).0.tmp $(@D)/.$(@F).0
 	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
 		>$(@D)/.$(@F).0.S
 	$(MAKE) $(build)=$(@D) $(@D)/.$(@F).0.o
 	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
-	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
+	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1.tmp
+	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms \
+	    $(@D)/.$(@F).1.tmp $(@D)/.$(@F).1
 	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
 		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \
 		>$(@D)/.$(@F).1.S
 	$(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o
 	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
-	    $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@
+	    $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@.tmp
+	$(OBJCOPY) --globalize-symbols=$(@D)/.$(@F).global-syms $@.tmp $@
 	$(NM) -pa --format=sysv $(@D)/$(@F) \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
 		>$(@D)/$(@F).map
-	rm -f $(@D)/.$(@F).[0-9]* $(@D)/..$(@F).[0-9]*
+	rm -f $(@D)/.$(@F).[0-9]* $(@D)/..$(@F).[0-9]* $(@D)/.$(@F).global-syms
 ifeq ($(CONFIG_XEN_IBT),y)
 	$(SHELL) $(srctree)/tools/check-endbr.sh $@
 endif
@@ -266,6 +276,7 @@  $(obj)/xen.lds $(obj)/efi.lds: $(src)/xen.lds.S FORCE
 clean-files := \
     include/asm/asm-macros.* \
     $(objtree)/.xen-syms.[0-9]* \
+    $(objtree)/.xen-syms.global-syms \
     $(objtree)/.xen.elf32 \
     $(objtree)/.xen.efi.[0-9]* \
     efi/*.efi