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

Re: [PATCH] x86: conditionalize workaround for build issue with GNU ld 2.37


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Mon, 13 Sep 2021 16:33:00 +0200
  • 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; bh=ctat5Gg+/Z8V4ZI07tGQbtU60i88SPRxZBQcf/3yWwc=; b=ePSBfGjS2v186w9FzqBXvMn00GojuI3+UQAVB+HxLJMwquwT7xl1bXa49U7JScyWxmsw0Cbkas/WdjKUPe7NBPRYcKBzVQ9j9EwW4SlEuAo42v6AET1KuHSeMpP4NKWbzE/vbkcM9Dwo7HCYz4keboPwc4zyhfZxqlnvlUaveeHTonQhsHiduWuWbPf+rU19LdCN5WTLsFlczTnXYohx0xrszndNInq68r/WoDwcpxHbi8s3vAV/aih856lm5cO0PIT1JNTcWokqA7zUTnfHTMZ7gj1ql3ehuqK6SXXeb7d3NaJ5/vbkT5Puov+x7+hxzwTCfk5x3Y9H1ozjIjf4AA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XN0qeSVCaMfclGnqeSPe62B/U0a3hnHDHDEw9qXPGYhRi3ZSfCrrr3GvchntzOeDZM0WEj/S9QgtyL0z4dbExJCpKcTc87NJfXzal1QhVsZbYgUfpEW+JjUAmQ4Za20JbdNlOjvPApO0zXoqP/fvEfS8Yv2EWPurk6C1enyiolJBusgYK8sVvWpoNeMsZuKIK8rjHxOAyoYIHinUUwcg/cBXZ5hLtU63+B0VWSf0YViMMvIOrZAlqZIFUmdG8SG1s8MvPUYEY08pSa8DkLxiSbcP7D3ifX8s/m3xPRpmvrs/tnpQw9yASC3ItipuaX+MoPLrbdMuVSN6iO7Su6d6gg==
  • Authentication-results: esa3.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: Mon, 13 Sep 2021 14:33:20 +0000
  • Ironport-data: A9a23:5JKQAqs1gd3tMzVn384k2kR+O+fnVE9ZMUV32f8akzHdYApBsoF/q tZmKWCBM6mNNjOge9AiPIS09EIHucLVn4UyGgJs/i4zRC9G+JbJXdiXEBz9bniYRiHhoOOLz Cm8hv3odp1coqr0/0/1WlTZQP0VOZigHtIQMsadUsxKbVIiGHpJZS5LwbZj29Y52IPhWWthh PupyyHhEA79s9JLGjp8B5Kr8HuDa9yr5Vv0FnRnDRx6lAe2e0s9VfrzFonoR5fMeaFGH/bSe gr25OrRElU1XfsaIojNfr7TKiXmS1NJVOSEoiI+t6OK2nCuqsGuu0qS2TV1hUp/0l20c95NJ Npl7rfoSToDMPD3iukWdxdfHgheBL0aweqSSZS/mZT7I0zudnLtx7NlDV0sPJ1e8eFyaY1M3 aVGcnZXNEnF3r/ohuLgIgVvrp1LwM3DJoQQt2sm1TjEJf0nXYrCU+PB4towMDIY25oQRaiHP 5RxhTxHTw7LRhpzJUwrFI9hpe2QmEnHQRdAkQfAzUYwyzeKl1EguFT3C/LKfvSaSMMTmVyXz krW8mK8DhwEOdi3zTue7mnqluLJhTn8Wo8ZCPu/7PECqF+Zy3EXCRYWfUCmuvT/gUm7M++zM GRNpHBo9/JrshX2EJ+tBHVUvUJooDZHcIJQLsc0tTqfx6Hw7QGUL0g9XmJ4PYlOWNANedA66 rOYt4q3XmU16+DEFS31GqS89mzpaHNMRYMWTWpdF1JUvYO7yG0mpk+XFr5e/LiJYsoZ8N0a6 wuDqjQ3z54XhNQCv0lQ1QGa22/wznQlowhc2+k2Yo5HxlgiDGJGT9bxgbQ+0RqnBNzFJmRtR FBex6CjABkmVPlhbhBhpdkw8EyBva7ZYFUwfmKD77F+rm/wqhZPjKh74S1kJVcBD/vojQTBO ReJ0SsIvcc7FCLzMcdfPtLgY+x3nPOIPYm0CZjpgi9mP8EZmPmvp3o1OyZ9HgnFzSARrE3IE c7LL5/zXCdFUP4PIfjfb7517ILHDxsWnAv7bZv60w6mwfyZYnuUQq0CK1yAcqYy66bsnekf2 4wHXydT4xkAAuD4fAfN9osfcQIDIXQhXMikoM1LbO+TZAFhHTh5WfPWxLogfa1jnrhUybiUr i3sBBcAxQqtn2DDJCWLdmtnNOHlU6FgoC9pJicrJ1uphSQuON798KcFepIrVrA77+g/n+VsR vwIdpzYUPRCQzjK4RoHapz5oNAwfRinn1vWbSGkfCI+b9hrQAmQoo3oeQ7m9S8vCCurtJRh/ +38h12DGZdaHlZsFsfbbv6r3midh3lFlbIgRVbML/lSZF7orNpgJRvug6JlOMoLMxjCmGeXj l7EHRcCqODRiIYp692V17ucpoKkHuYiTEpXG27XseS/OSXApzfxxIZBVKCDfCzHVXOy86KnP L0Hw/b5OfwBvVBLr4sjTOo7kfNgv4Pi9+1A0wBpPHTXdFD6WLpvL06P0dRLqqAQlKRSvhG7W x7X99RXUVlT1BgJzLLFyNIZU9m+
  • Ironport-hdrordr: A9a23:hk6j4aM3I61s28BcT1D155DYdb4zR+YMi2TDiHofdfUFSKClfp 6V8cjztSWUtN4QMEtQ/uxoHJPwO080kqQFnLX5XI3SJzUO3VHHEGgM1/qB/9SNIVyaygcZ79 YdT0EcMqyAMbEZt7eC3ODQKb9Jq7PmgcPY9ds2jU0dNT2CA5sQkTuRYTzrdHGeKjM2YabQQ/ Gnl7V6TnebCDkqR/X+IkNAc/nIptXNmp6jSRkaByQ/4A3LqT+z8rb1HzWRwx9bClp0sPgf2F mAtza8yrSosvm9xBOZ/2jP765OkN+k7tdYHsSDhuUcNz2poAe1Y4ZKXaGEoVkO0a2SwWdvtO OJjwYrPsx15X+UVmapoSH10w2l6zoq42+K8y7RvVLT5ejCAB4qActIgoxUNjHD7VA7gd162K VXm0qEqpt+F3r77WTAzumNcysvulu/oHIkn+JWpWdYS5EiZLhYqpFa1F9JEa0HADnx5OkcYa hT5fnnlbRrmG6hHjXkVjEF+q3pYp1zJGbJfqE6gL3X79AM90oJiHfxx6Qk7z49HdwGOt95D0 mtCNUdqFh0dL5lUUtKPpZ2fSKGMB2/ffvyChPmHb3GLtBNB5ufke+83F0KjNvaD6DgiqFCwa j8bA==
  • Ironport-sdr: wV9EQ54t5JNj1L3zVWB1W4IH6mGj/xkjhDLcts7qCwcJZ+GSP2vBVgyrWnOUwnCJin6L3D/YIS 6VNn0X04yi4tkUqQtG0bG7UtmTiWE4+jYHlx7cXbnrnVEanKYWyhr0cnUpXnOoF0Ac/ebjmFNq UW/2pRINmwSLDo1u2RMameRMh70bHsMBWH9urJdjtPuUUa2/bnEcriYTlCwdXVGsQyTihJLZ6b KTJsgvd/moQOfQNrIz8jloZmjC/XCr3i0xwPeKPE7Dwd9BO74sVs9SKXrNPQ33fKVHgYaaHHWZ jFTvmATyEwzvdXzgkky7GpGM
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, Sep 13, 2021 at 04:05:15PM +0200, Jan Beulich wrote:
> On 13.09.2021 15:37, Roger Pau Monné wrote:
> > On Thu, Sep 09, 2021 at 04:35:49PM +0200, Jan Beulich wrote:
> >> I haven't been able to find an environment where I could actually try
> >> with lld (ld.lld); all testing was with GNU ld (ld.bfd).
> > 
> > Thanks for fixing this. I've been able to test with LLVM ld and the
> > workaround is fine.
> 
> Oh, good, thanks for trying this out.
> 
> >> --- a/xen/arch/x86/Makefile
> >> +++ b/xen/arch/x86/Makefile
> >> @@ -92,10 +92,16 @@ efi-$(CONFIG_PV_SHIM_EXCLUSIVE) :=
> >>  
> >>  ifneq ($(build_id_linker),)
> >>  notes_phdrs = --notes
> >> +# Determine whether to engage a workaround for GNU ld 2.37.
> >> +build-id-ld-good = $(shell echo 'void test(void) {}' \
> >> +                           | $(CC) $(XEN_CFLAGS) -o .check.o -c -x c - 
> >> 2>.check.err \
> >> +                           && $(LD) -T check.lds -o .check.elf .check.o 
> >> 2>>.check.err \
> >> +                           && echo y)
> > 
> > Do we want to make this a Kconfig option (ie: LD_UNQUOTED_DASH) and
> > then use is here?
> > 
> > We already have compiler and assembler checks in x86/Kconfig, so it
> > would seem more natural to place it there.
> 
> The question of whether to record tool chain capabilities in .config
> is still pending. I'm not convinced this is a good idea, Andrew keeps
> shouting me out for that, and an actual discussion doesn't really
> happen. Yet unlike back at the time when I first raised my concern,
> Anthony meanwhile supports me in at least the question (to Andrew) of
> when such a discussion would have happened: Neither of us is aware,
> yet Andrew claims it did happen, but so far didn't point out where
> one could read about what was discussed and decided there.
> 
> For the few uses we've accumulated I gave (if at all) an ack for
> things happening under some sort of pressure, with the request that
> aformentioned discussion would happen subsequently (and, depending on
> outcome, these would be converted to another approach if need be). I
> have meanwhile realized that it was a mistake to allow such things in
> on this basis - the more of them we gain, the more I'm hearing "we've
> already got some".

I see, that's not an ideal situation from a review PoV, as we don't
have a clear placement for those and that will just cause confusion
(and more work since there are potentially two places to check).

What's the benefit of placing the checks in Kconfig instead of the
Makefiles?

As I understand Kconfig is run only once, while the Makefile could run
the check multiple times.

The downfall of having them in .config is that .config could suddenly
change as tools are updated or as it's moved around different systems
(yet .config already contains specific compiler version information).

> >>  else
> >>  ifeq ($(CONFIG_PVH_GUEST),y)
> >>  notes_phdrs = --notes
> >>  endif
> >> +build-id-ld-good := y
> >>  endif
> > 
> > I also wonder whether we need to make the quoting tied to the usage of
> > build-id. I guess we don't add sections with dashes and instead
> > use underscores, but it might be prudent to always quote to be on the
> > safe side if dashes are not supported.
> 
> If quoting was uniformly supported, I might have considered that. But
> it not being uniformly supported is the reason for this change in the
> first place. Hence I'd prefer to generalize this only if really needed.

OK, FE. As said I don't we would deliberately add sections with
slashes.

> >> --- /dev/null
> >> +++ b/xen/arch/x86/check.lds
> > 
> > I would maybe name this check-dash.lds, in case we need to add more ld
> > build tests.
> 
> I sincerely hope it was a one-off that a binutils release got cut with
> this sort of a supposedly prominent bug. Considering that the dash is
> merely what we're after in this specific case, but breakage was wider
> (presumably about any printable char that's not alnum or underscore),
> I'd consider check-dash too specific a name. You only say "maybe"; if
> you were sufficiently convinced, this is an adjustment I'd be willing
> to make. Yet even better would be if I / we could just be done with
> this.

Ack, we can always rename at a later point if there's a need to,
hopefully we are not being too optimistic.

Thanks, Roger.



 


Rackspace

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