Re: [PATCH RFC] x86/lld: fix symbol map generation

[Jan Beulich <jbeulich@xxxxxxxx>]

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