[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Fri, 4 Mar 2022 10:19:34 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=9P1ZjLL1kVlW6J4Z49boFuPRdd4bDtOHBhk1MH43gNI=; b=QXA1mkzfEsXpeQiikuvis8BA2xzH7M945n3Ieg2hhjyE2UGSTZE+yO9oJ1Huy7gvBRVkgEMWCX/KNROoIcmUpm7SPgf90ERzjomk26ps8AvK0HIOTrO0jAYuZ3wcXhR3W5Ar324ewOSb1thcOoOzoL/XoeSettpjfTMwvkPiLxcjNCZ8qZ0eM95/qsFjRFkZUAQzD6T2S21Wf7AzNh0PJFOZusgnkLIFr2opFfqcwNqbiCYPD1UYreeM3CA26tuLq5ba+qv1tIZSFa1l7vQM8OjK2NDmiJ5N9kv33glRhq5zGXgrcrzkMrILJcxj80tGeqLLVWcB+ANYxnOo4/TQIA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=We9Ggm8ucBcz6DrRzgqQ0A04C4kYFyaWDyIHZAJVexwcEgcwOu5YcvrVqm2cpspejSs9udX/Nr/zmSL0cWTyiehH7CuQlpVNDT2JUvaOpuaK0HMadKksP7f/jqVGNsJFET21reOZTbvopGqjT4OO1lIBXysgr9qYMXrk9FBkngGRqze3VygXhGrfTlEw9WmvxcqWBe8h/LSJXNKJrhjfrTKnIBNA0FzGXsif0HeXnN27MOo+urtAHjiszPHgO3iuElS0wE99JaWP/6Bvd1Xebx03JCp7OQcuY2momKoxVtui8wy7L1qBXcsp5Se5Y3YmVtKSQid+isCsj2/Dd9v7IA==
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Fri, 04 Mar 2022 09:19:58 +0000
  • Ironport-data: A9a23:rj/1HKLQ/n1jcDp7FE+Rx5UlxSXFcZb7ZxGr2PjKsXjdYENS1zxWz msYCjiGa/6LZGT9KNh0YIW/8RsP68PTnIdgTwZlqX01Q3x08seUXt7xwmUcns+xwm8vaGo9s q3yv/GZdJhcokf0/0vrav67xZVF/fngqoDUUYYoAQgsA148IMsdoUg7wbRh2dY02YHR7z6l4 rseneWOYDdJ5BYsWo4kw/rrRMRH5amaVJsw5zTSVNgT1LPsvyB94KE3fMldG0DQUIhMdtNWc s6YpF2PEsE1yD92Yj+tuu6TnkTn2dc+NyDW4pZdc/DKbhSvOkXee0v0XRYRQR4/ttmHozx+4 PVDh8yATCUxBKj3tedAdxpYMQp3BoQTrdcrIVDn2SCS50jPcn+qyPRyFkAme4Yf/46bA0kXq 6ZecmpUKEne2aTmm9pXScE17ignBNPsM44F/Glp0BnSDOo8QICFSKLPjTNd9Glv1pgWTa6OD yYfQQplN0jBajkXAAZUEqA4hNaq2ELNbxQN/Tp5ooJoujOOnWSdyoPFL979atGMA8JPkS6wt m/Aumj0HBweHNie0iaetGKhgPfVmiH2U55UE6e3ntZoilCOwm0YCDUNSEC25/K+jyaDt8l3c hJOvHB09O5rqRLtHoKVswCETGCsnhAuC+pVMfUA7Tqgyffw4Aa7LWE/Z2sUADA5j/MeSTsv3 16PutrmAz1zrbGYIU6gGqeoQSCaYnZMczJbDcMQZU5cuoS4/tlv5v7aZos7SMaIYsvJ9SYcK txghAw3nP0tgMECzM1XFniX0mv39vAlouPYjzg7v15JDCskPOZJhKTysDA3CMqsyq7DFDFtW 1Bex6CjABgmV83lqcB0aLxl8EuVz/iEKibAplVkAoMs8T+gk1b6I9wOu2okdRwxbJpfEdMMX KM1kVkLjHO0FCH3BZKbnqrrU5h6pUQePY6Nug/ogipmPcEqKV7vENBGbk+MxWH9+HXAYolkU ap3hf2EVC5AYYw+lWLeb75EjdcDm3BvrUuOFMuT50n2jtKjiIu9FO5t3K2mNbtisstpYWz9r r5iCid9404HAb2mP3KOqtZ7wJJjBSFTOK0aYvd/L4arCgFnBHsgG7nWx7YgcJZihKNbiqHD+ XTVZ6OS4ACXaaHvQelSVk1eVQ==
  • Ironport-hdrordr: A9a23:MI0DSaEit74SxMQhpLqFCpHXdLJyesId70hD6qkvc3Nom52j+/ xGws536faVslcssHFJo6HmBEClewKnyXcT2/htAV7CZnichILMFu9fBOTZsl/d8kHFh4tgPO JbAtRD4b7LfClHZKTBkXCF+r8bqbHtmsDY5ts2jU0dNT2CA5sQkTuRYTzrdHGeKjM2YabQQ/ Gnl7V6TnebCDwqR/X+IkNAc/nIptXNmp6jSRkaByQ/4A3LqT+z8rb1HzWRwx9bClp0sP0f2F mAtza8yrSosvm9xBOZ/2jP765OkN+k7tdYHsSDhuUcNz2poAe1Y4ZKXaGEoVkO0aqSwWdvtO OJjwYrPsx15X+UVmapoSH10w2l6zoq42+K8y7uvVLT5ejCAB4qActIgoxUNjHD7VA7gd162K VXm0qEqpt+F3r77WvAzumNcysvulu/oHIkn+JWpWdYS5EiZLhYqpFa1F9JEa0HADnx5OkcYa VT5fnnlbdrmG6hHjDkVjEF+q3uYp1zJGbKfqE6gL3a79AM90oJjXfxx6Qk7wM9HdwGOtx5Dt //Q9dVfYF1P78rhJ1GdZU8qOuMexrwqEH3QSuvyWqOLtBzB5uKke+y3IkI
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.