diff mbox series

x86/build: use --orphan-handling linker option if available

Message ID 5c374fd3-017d-3c40-4ded-25a3b1c978a6@suse.com (mailing list archive)
State Superseded
Headers show
Series x86/build: use --orphan-handling linker option if available | expand

Commit Message

Jan Beulich March 2, 2022, 2:19 p.m. UTC
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@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
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.

Comments

Roger Pau Monné March 3, 2022, 11:19 a.m. UTC | #1
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@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> 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.
Jan Beulich March 3, 2022, 12:17 p.m. UTC | #2
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@citrix.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> 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). I also don't want to add these uniformly, for now knowing what
side effects their mentioning might have with GNU 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?

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.

>> --- 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?

I've done so, but I'm not sure if implicitly you mean to say that
LLVM ld does honor the directive when linking xen.efi? If that
wasn't the case, it would rather seem misleading to have "GNU"
there. Unless e.g. LLVM ld can't link xen.efi at all ...

Jan
Roger Pau Monné March 3, 2022, 3:09 p.m. UTC | #3
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@citrix.com>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> ---
> >> 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.

> 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?

> >> --- 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?

Could we do something like:

$(TARGET)-syms: XEN_LDFLAGS += ...

And similar for $(TARGET).efi?

> >> --- 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?
> 
> I've done so, but I'm not sure if implicitly you mean to say that
> LLVM ld does honor the directive when linking xen.efi? If that
> wasn't the case, it would rather seem misleading to have "GNU"
> there. Unless e.g. LLVM ld can't link xen.efi at all ...

So the one installed by default on FreeBSD doesn't have support for the
i386pep target emulation built in. AFAICT the EFI loader on FreeBDS is
built using a pre-made PE header and assembled together using objcopy.

Thanks, Roger.
Jan Beulich March 4, 2022, 8:02 a.m. UTC | #4
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@citrix.com>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> 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.

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

>>>> --- 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).

Jan
Roger Pau Monné March 4, 2022, 9:19 a.m. UTC | #5
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@citrix.com>
> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>>> ---
> >>>> 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@citrix.com>

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

Thanks, Roger.
Roger Pau Monné March 4, 2022, 9:31 a.m. UTC | #6
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@citrix.com>
> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>>> ---
> >>>> 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.
> 
> >> 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.

Just realized, in arch/x86/boot/build32.lds we already explicitly
handle .symtab, .shstrtab and .strtab for LLD, it was added by commit
10d27b48b5b in order to prevent LLD from complaining about discarding
those sections. So it should be safe to also do this handling in the
general linker script?

Thanks, Roger.
Jan Beulich March 4, 2022, 9:46 a.m. UTC | #7
On 04.03.2022 10:31, Roger Pau Monné wrote:
> 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@citrix.com>
>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>>> ---
>>>>>> 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.
>>
>>>> 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.
> 
> Just realized, in arch/x86/boot/build32.lds we already explicitly
> handle .symtab, .shstrtab and .strtab for LLD, it was added by commit
> 10d27b48b5b in order to prevent LLD from complaining about discarding
> those sections. So it should be safe to also do this handling in the
> general linker script?

I wouldn't want to infer such. What build32.lds is used for is very
simple input. It's a hint at best that it might be okay to use even
with GNU ld.

Jan
Jan Beulich March 7, 2022, 8:18 a.m. UTC | #8
On 04.03.2022 10:19, Roger Pau Monné wrote:
> 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@citrix.com>
>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>>> ---
>>>>>> 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@citrix.com>

Thanks. However, something is wrong here. Unlike in my local builds, the
pre-push build test I did after committing this triggered a massive amount
(tens of thousands) of objdump warnings:

CU at offset ... contains corrupt or unsupported version number: 0
Invalid pointer size (0) in compunit header, using 4 instead

Helpfully it doesn't say whether that's xen-syms, xen-efi, or both. I'll
have to investigate and fix; I can only guess at this point that this
might be triggered by a difference in .config, or be hidden by some
other change I have in my local tree.

Jan
Roger Pau Monné March 7, 2022, 11:06 a.m. UTC | #9
On Mon, Mar 07, 2022 at 09:18:42AM +0100, Jan Beulich wrote:
> On 04.03.2022 10:19, Roger Pau Monné wrote:
> > 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@citrix.com>
> >>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>>>>> ---
> >>>>>> 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.

Got a reply back from the LLD folks, and they consider the GNU ld
behavior quirky. Linux linker script does explicitly mention .symtab,
.strtab and shstrtab:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a840c4de56

So Xen should be safe to do the same.

> >>>>>> --- 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@citrix.com>
> 
> Thanks. However, something is wrong here. Unlike in my local builds, the
> pre-push build test I did after committing this triggered a massive amount
> (tens of thousands) of objdump warnings:
> 
> CU at offset ... contains corrupt or unsupported version number: 0
> Invalid pointer size (0) in compunit header, using 4 instead

That's weird, I wasn't aware we had any objdump calls after the final
image is linked.

> Helpfully it doesn't say whether that's xen-syms, xen-efi, or both. I'll
> have to investigate and fix; I can only guess at this point that this
> might be triggered by a difference in .config, or be hidden by some
> other change I have in my local tree.

Hm, I didn't see any of those when doing my test build on FreeBSD, but
didn't check with gcc.

Thanks, Roger.
Jan Beulich March 7, 2022, 11:20 a.m. UTC | #10
On 07.03.2022 12:06, Roger Pau Monné wrote:
> On Mon, Mar 07, 2022 at 09:18:42AM +0100, Jan Beulich wrote:
>> On 04.03.2022 10:19, Roger Pau Monné wrote:
>>> 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@citrix.com>
>>>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>>>>> ---
>>>>>>>> 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.
> 
> Got a reply back from the LLD folks, and they consider the GNU ld
> behavior quirky. Linux linker script does explicitly mention .symtab,
> .strtab and shstrtab:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a840c4de56
> 
> So Xen should be safe to do the same.

Ah yes, this is good enough proof for those present being (sufficiently)
benign to GNU ld (and being able to expect that this isn't going to
change). I'll get these added in v2.

>>>>>>>> --- 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@citrix.com>
>>
>> Thanks. However, something is wrong here. Unlike in my local builds, the
>> pre-push build test I did after committing this triggered a massive amount
>> (tens of thousands) of objdump warnings:
>>
>> CU at offset ... contains corrupt or unsupported version number: 0
>> Invalid pointer size (0) in compunit header, using 4 instead
> 
> That's weird, I wasn't aware we had any objdump calls after the final
> image is linked.

We've gained those recently, from check-endbr.sh.

Jan
diff mbox series

Patch

--- 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
+
 $(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
+ * order to record that we'd prefer these sections to not be loaded into memory.
+ */
+#define DECL_DEBUG(x, a) #x ALIGN(a) (NOLOAD) : { *(x) }
+#define DECL_DEBUG2(x, y, a) #x ALIGN(a) (NOLOAD) : { *(x) *(y) }
 
 ENTRY(efi_start)
 
@@ -19,6 +25,8 @@  ENTRY(efi_start)
 
 #define FORMAT "elf64-x86-64"
 #define DECL_SECTION(x) #x : AT(ADDR(#x) - __XEN_VIRT_START)
+#define DECL_DEBUG(x, a) #x 0 (NOLOAD) : { *(x) }
+#define DECL_DEBUG2(x, y, a) #x 0 (NOLOAD) : { *(x) *(y) }
 
 ENTRY(start_pa)
 
@@ -179,6 +187,13 @@  SECTIONS
 #endif
 #endif
 
+#ifndef EFI
+  /* Retain these just for the purpose of possible analysis tools. */
+  DECL_SECTION(.note) {
+       *(.note.*)
+  } PHDR(note) PHDR(text)
+#endif
+
   _erodata = .;
 
   . = ALIGN(SECTION_ALIGN);
@@ -266,6 +281,32 @@  SECTIONS
        __ctors_end = .;
   } PHDR(text)
 
+#ifndef EFI
+  /*
+   * With --orphan-sections=warn (or =error) we need to handle certain linker
+   * generated sections. These are all expected to be empty; respective
+   * ASSERT()s can be found towards the end of this file.
+   */
+  DECL_SECTION(.got) {
+       *(.got)
+  } PHDR(text)
+  DECL_SECTION(.got.plt) {
+       *(.got.plt)
+  } PHDR(text)
+  DECL_SECTION(.igot.plt) {
+       *(.igot.plt)
+  } PHDR(text)
+  DECL_SECTION(.iplt) {
+       *(.iplt)
+  } PHDR(text)
+  DECL_SECTION(.plt) {
+       *(.plt)
+  } PHDR(text)
+  DECL_SECTION(.rela) {
+       *(.rela.*)
+  } PHDR(text)
+#endif
+
   . = ALIGN(SECTION_ALIGN);
   __init_end = .;
   __2M_init_end = .;
@@ -321,71 +362,6 @@  SECTIONS
     *(.reloc)
     __base_relocs_end = .;
   }
-  /*
-   * Explicitly list debug section for the PE output so that they don't end
-   * up at VA 0 which is below image base and thus invalid. Also use the
-   * NOLOAD directive, despite currently ignored by ld for PE output, in
-   * order to record that we'd prefer these sections to not be loaded into
-   * memory.
-   *
-   * Note that we're past _end here, so if these sections get loaded they'll
-   * be discarded at runtime anyway.
-   */
-  .debug_abbrev ALIGN(1) (NOLOAD) : {
-     *(.debug_abbrev)
-  }
-  .debug_info ALIGN(1) (NOLOAD) : {
-    *(.debug_info)
-    *(.gnu.linkonce.wi.*)
-  }
-  .debug_types ALIGN(1) (NOLOAD) : {
-    *(.debug_types)
-  }
-  .debug_str ALIGN(1) (NOLOAD) : {
-    *(.debug_str)
-  }
-  .debug_line ALIGN(1) (NOLOAD) : {
-    *(.debug_line)
-    *(.debug_line.*)
-  }
-  .debug_line_str ALIGN(1) (NOLOAD) : {
-    *(.debug_line_str)
-  }
-  .debug_names ALIGN(4) (NOLOAD) : {
-    *(.debug_names)
-  }
-  .debug_frame ALIGN(4) (NOLOAD) : {
-    *(.debug_frame)
-  }
-  .debug_loc ALIGN(1) (NOLOAD) : {
-    *(.debug_loc)
-  }
-  .debug_loclists ALIGN(4) (NOLOAD) : {
-    *(.debug_loclists)
-  }
-  .debug_ranges ALIGN(8) (NOLOAD) : {
-    *(.debug_ranges)
-  }
-  .debug_rnglists ALIGN(4) (NOLOAD) : {
-    *(.debug_rnglists)
-  }
-  .debug_addr ALIGN(8) (NOLOAD) : {
-    *(.debug_addr)
-  }
-  .debug_aranges ALIGN(1) (NOLOAD) : {
-    *(.debug_aranges)
-  }
-  .debug_pubnames ALIGN(1) (NOLOAD) : {
-    *(.debug_pubnames)
-  }
-  .debug_pubtypes ALIGN(1) (NOLOAD) : {
-    *(.debug_pubtypes)
-  }
-  /* Trick the linker into setting the image size to no less than 16Mb. */
-  __image_end__ = .;
-  .pad ALIGN(__section_alignment__) : {
-    . = __image_end__ < __image_base__ + MB(16) ? ALIGN(MB(16)) : .;
-  }
 #elif defined(XEN_BUILD_EFI)
   /*
    * Due to the way EFI support is currently implemented, these two symbols
@@ -400,6 +376,42 @@  SECTIONS
   efi = .;
 #endif
 
+  /*
+   * Explicitly list debug sections, first of all to avoid these sections being
+   * viewed as "orphan" by the linker.
+   *
+   * For the PE output this is further necessary so that they don't end up at
+   * VA 0, which is below image base and thus invalid. Note that we're past
+   * _end here, so if these sections get loaded they'll be discarded at runtime
+   * anyway.
+   */
+  DECL_DEBUG(.debug_abbrev, 1)
+  DECL_DEBUG2(.debug_info, .gnu.linkonce.wi.*, 1)
+  DECL_DEBUG(.debug_types, 1)
+  DECL_DEBUG(.debug_str, 1)
+  DECL_DEBUG2(.debug_line, .debug_line.*, 1)
+  DECL_DEBUG(.debug_line_str, 1)
+  DECL_DEBUG(.debug_names, 4)
+  DECL_DEBUG(.debug_frame, 4)
+  DECL_DEBUG(.debug_loc, 1)
+  DECL_DEBUG(.debug_loclists, 4)
+  DECL_DEBUG(.debug_macinfo, 1)
+  DECL_DEBUG(.debug_macro, 1)
+  DECL_DEBUG(.debug_ranges, 8)
+  DECL_DEBUG(.debug_rnglists, 4)
+  DECL_DEBUG(.debug_addr, 8)
+  DECL_DEBUG(.debug_aranges, 1)
+  DECL_DEBUG(.debug_pubnames, 1)
+  DECL_DEBUG(.debug_pubtypes, 1)
+
+#ifdef EFI
+  /* Trick the linker into setting the image size to no less than 16Mb. */
+  __image_end__ = .;
+  .pad ALIGN(__section_alignment__) : {
+    . = __image_end__ < __image_base__ + MB(16) ? ALIGN(MB(16)) : .;
+  }
+#endif
+
 #ifdef CONFIG_HYPERV_GUEST
   hv_hcall_page = ABSOLUTE(HV_HCALL_PAGE - XEN_VIRT_START + __XEN_VIRT_START);
 #endif
@@ -418,8 +430,7 @@  SECTIONS
 #ifdef EFI
        *(.comment)
        *(.comment.*)
-       *(.note.Xen)
-       *(.note.gnu.*)
+       *(.note.*)
 #endif
   }
 
@@ -465,6 +476,15 @@  ASSERT(IS_ALIGNED(trampoline_end,   4),
 ASSERT(IS_ALIGNED(__bss_start,      8), "__bss_start misaligned")
 ASSERT(IS_ALIGNED(__bss_end,        8), "__bss_end misaligned")
 
+#ifndef EFI
+ASSERT(!SIZEOF(.got),      ".got non-empty")
+ASSERT(!SIZEOF(.got.plt),  ".got.plt non-empty")
+ASSERT(!SIZEOF(.igot.plt), ".igot.plt non-empty")
+ASSERT(!SIZEOF(.iplt),     ".iplt non-empty")
+ASSERT(!SIZEOF(.plt),      ".plt non-empty")
+ASSERT(!SIZEOF(.rela),     "leftover relocations")
+#endif
+
 ASSERT((trampoline_end - trampoline_start) < TRAMPOLINE_SPACE - MBI_SPACE_MIN,
     "not enough room for trampoline and mbi data")
 ASSERT((wakeup_stack - wakeup_stack_start) >= WAKEUP_STACK_MIN,