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

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

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@xxxxxxxxxx>
> > ---
> > 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.