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

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

On Thu, Sep 09, 2021 at 04:35:49PM +0200, Jan Beulich wrote:
> While LLVM's lld is supposed to be a drop-in replacement for GNU ld [1],
> it appears to not understand quoted section names as operands to e.g.
> ADDR(). Therefore the original workaround broke the build in
> environments where ld is actually LLVM's, like on FreeBSD.
> 
> Fixes: 58ad654ebce7 ("x86: work around build issue with GNU ld 2.37")
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> [1] 
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flld.llvm.org%2F&amp;data=04%7C01%7Croger.pau%40citrix.com%7C07abc3240fc14a3f095408d9739f3eb3%7C335836de42ef43a2b145348c2ee9ca5b%7C0%7C0%7C637667950073151096%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Sa0BBWP2cWOhWsB3b4FQIZ1J3KvfWP%2BnINiqyXdChD0%3D&amp;reserved=0
> ---
> 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.

> --- a/.gitignore
> +++ b/.gitignore
> @@ -306,6 +306,7 @@
>  xen/.config.old
>  xen/.xen.elf32
>  xen/System.map
> +xen/arch/x86/.check.*
>  xen/arch/x86/asm-macros.i
>  xen/arch/x86/boot/mkelf32
>  xen/arch/x86/boot/cmdline.S
> --- 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.

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

>  
>  ifdef CONFIG_LIVEPATCH
> @@ -291,6 +297,10 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
>       $(call move-if-changed,$@.new,$@)
>  
>  efi.lds: AFLAGS-y += -DEFI
> +xen.lds: Makefile
> +ifneq ($(build-id-ld-good),y)
> +xen.lds: CFLAGS-y += -DQUOTE_SECTION_NAMES
> +endif
>  xen.lds efi.lds: xen.lds.S
>       $(CPP) -P $(call cpp_flags,$(a_flags)) -MQ $@ -o $@ $<
>  
> @@ -302,7 +312,7 @@ hweight.o: CFLAGS-y += $(foreach reg,cx
>  
>  .PHONY: clean
>  clean::
> -     rm -f *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
> +     rm -f ???.lds *.new .check.* boot/*.o boot/*~ boot/core boot/mkelf32
>       rm -f asm-macros.i $(BASEDIR)/include/asm-x86/asm-macros.*
>       rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d $(BASEDIR)/.xen.elf32
>       rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/mkreloc
> --- /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.

Thanks, Roger.