[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/build: use --orphan-handling linker option if available
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@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> 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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |