Re: [PATCH] x86/build: use --orphan-handling linker option if available

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

On Wed, Mar 02, 2022 at 03:19:35PM +0100, Jan Beulich wrote:
> As was e.g. making necessary 4b7fd8153ddf ("x86: fold sections in final
> binaries"), arbitrary sections appearing without our linker script
> placing them explicitly can be a problem. Have the linker make us aware
> of such sections, so we would know that the script needs adjusting.
> 
> To deal with the resulting warnings:
> - Retain .note.* explicitly for ELF, and discard all of them (except the
>   earlier consumed .note.gnu.build-id) for PE/COFF.
> - Have explicit statements for .got, .plt, and alike and add assertions
>   that they're empty. No output sections will be created for these as
>   long as they remain empty (or else the assertions would cause early
>   failure anyway).
> - Collect all .rela.* into a single section, with again an assertion
>   added for the resulting section to be empty.
> - Extend the enumerating of .debug_* to ELF. Note that for Clang adding
>   of .debug_macinfo is necessary. Amend this by its Dwarf5 counterpart,
>   .debug_macro, then as well (albeit more may need adding for full
>   coverage).
> 
> Suggested-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> I would have wanted to make this generic (by putting it in
> xen/Makefile), but the option cannot be added to LDFLAGS, or else
> there'll be a flood of warnings with $(LD) -r. (Besides, adding to
> LDFLAGS would mean use of the option on every linker pass rather than
> just the last one.)
> 
> Retaining of .note in xen-syms is under question. Plus if we want to
> retain all notes, the question is whether they wouldn't better go into
> .init.rodata. But .note.gnu.build-id shouldn't move there, and when
> notes are discontiguous all intermediate space will also be assigned to
> the NOTE segment, thus making the contents useless for tools going just
> by program headers.
> 
> Newer Clang may require yet more .debug_* to be added. I've only played
> with versions 5 and 7 so far.
> 
> Unless we would finally drop all mentioning of Stabs sections, we may
> want to extend to there what is done for Dwarf here (allowing the EFI
> conditional around the section to also go away).
> 
> See also https://sourceware.org/pipermail/binutils/2022-March/119922.html.

LLD 13.0.0 also warns about:

ld: warning: <internal>:(.symtab) is being placed in '.symtab'
ld: warning: <internal>:(.shstrtab) is being placed in '.shstrtab'
ld: warning: <internal>:(.strtab) is being placed in '.strtab'

So seeing your mail where you mention GNU ld not needing those, I
think we would need to add them anyway for LLVM ld.

> 
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -120,6 +120,8 @@ syms-warn-dup-y := --warn-dup
>  syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
>  syms-warn-dup-$(CONFIG_ENFORCE_UNIQUE_SYMBOLS) := --error-dup
>  
> +orphan-handling-$(call ld-option,--orphan-handling=warn) += 
> --orphan-handling=warn

Might be better to place in xen/Kconfig with the CC checks?

I'm also wondering whether we could add the flag here into XEN_LDFLAGS
and EFI_LDFLAGS, as those options are only used together with the
linker script in the targets on the Makefile.

>  $(TARGET): TMP = $(@D)/.$(@F).elf32
>  $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32
>       $(obj)/boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) 
> $(XEN_IMG_OFFSET) \
> @@ -146,7 +148,7 @@ $(TARGET)-syms: $(BASEDIR)/prelink.o $(o
>               >$(@D)/.$(@F).1.S
>       $(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o
>       $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
> -         $(@D)/.$(@F).1.o -o $@
> +         $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@
>       $(NM) -pa --format=sysv $(@D)/$(@F) \
>               | $(BASEDIR)/tools/symbols --all-symbols --xensyms --sysv 
> --sort \
>               >$(@D)/$(@F).map
> @@ -220,7 +222,7 @@ endif
>               | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort 
> >$(@D)/.$(@F).1s.S
>       $(MAKE) $(build)=$(@D) .$(@F).1r.o .$(@F).1s.o
>       $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds -N $< \
> -                     $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(note_file_option) 
> -o $@
> +           $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(orphan-handling-y) 
> $(note_file_option) -o $@
>       $(NM) -pa --format=sysv $(@D)/$(@F) \
>               | $(BASEDIR)/tools/symbols --all-symbols --xensyms --sysv 
> --sort >$(@D)/$(@F).map
>       rm -f $(@D)/.$(@F).[0-9]* $(@D)/..$(@F).[0-9]*
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -12,6 +12,12 @@
>  #undef __XEN_VIRT_START
>  #define __XEN_VIRT_START __image_base__
>  #define DECL_SECTION(x) x :
> +/*
> + * Use the NOLOAD directive, despite currently ignored by ld for PE output, 
> in

Would you mind adding GNU ld here to avoid confusion?

Thanks, Roger.