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

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

On Fri, Mar 04, 2022 at 09:02:08AM +0100, Jan Beulich wrote:
> On 03.03.2022 16:09, Roger Pau Monné wrote:
> > On Thu, Mar 03, 2022 at 01:17:03PM +0100, Jan Beulich wrote:
> >> On 03.03.2022 12:19, Roger Pau Monné wrote:
> >>> 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.
> >>
> >> Hmm, that's ugly. How do I recognize LLVM ld? I can't simply use a
> >> pre-processor conditional keying off of __clang__, as that used as the
> >> compiler doesn't mean their ld is also in use (typically the case on
> >> Linux).
> > 
> > Hard to tell, `ld -v` for LLD will typically contain '^LLD' I think,
> > but I don't really like matching on human readable output like this.
> 
> Same here. But Linux'es ld-version.sh looks to be doing just that.

OK, so be it then. We can always improve afterwards, as I don't really
have any better suggestion ATM.

> >> I also don't want to add these uniformly, for now knowing what
> >> side effects their mentioning might have with GNU ld.
> > 
> > Wouldn't it be fine to just place them at the end, just like it's
> > done by default by ld?
> > 
> > Are you worried about not getting the proper type if mentioned in the
> > linker script?
> 
> I'm worried of about any kind of anomaly that could be caused by
> mentioning sections which a linker doesn't expect to be named in
> a script. That's hardly something they would even test their
> linkers against.

I've raised a bug with LLD:

https://github.com/llvm/llvm-project/issues/54194

To see whether this behavior is intended.

> >>>> --- 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?
> >>
> >> Well. I've tried to stay away from complaining if people introduce
> >> new tool chain capability checks in Kconfig. But I'm not going to
> >> add any myself (unless things would become really inconsistent) up
> >> and until we have actually properly discussed the upsides and
> >> downsides of either model. Doing this via email (see the "Kconfig
> >> vs tool chain capabilities" thread started in August 2020) has
> >> proven to not lead anywhere. I'm really hoping that we can finally
> >> sort this in Bukarest.
> >>
> >>> 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.
> >>
> >> Not for XEN_LDFLAGS at least, and undesirable for EFI_LDFLAGS. See
> >> the respective post-commit message remark.
> > 
> > But the calls to LD in order to generate $(TARGET)-syms do not use -r,
> > and are all using the linker script, so it should be fine to use
> > --orphan-handling=warn there?
> 
> But XEN_LDFLAGS is also used elsewhere together with -r. (Whether
> that's actually correct is a different question.)
> 
> > Could we do something like:
> > 
> > $(TARGET)-syms: XEN_LDFLAGS += ...
> > 
> > And similar for $(TARGET).efi?
> 
> Yes, this ought to be possible, but would again lead to the option
> being passed on all three linking stages instead of just the final
> one. When there are many warnings (e.g. because of the same kind of
> section appearing many times), it's not helpful to see the flood
> three times (or likely even six times, once for xen-syms and once
> for xen.efi).

OK, I think our build system is already quite chatty, so wouldn't
really care about seeing repeated messages there. We can find a way to
generalize passing options to the final linker step if/when we need to
add more.

I'm fine with doing the LLD fixup as a separate patch, so:

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

My personal preference would also be for placing the ld option check
in Kconfig, but I'm not going to insist.

Thanks, Roger.