[PATCH v2] x86: avoid SORT_BY_INIT_PRIORITY with old GNU ld

Jan Beulich posted 1 patch 2 years, 1 month ago
Failed in applying to current master (apply log)
There is a newer version of this series
[PATCH v2] x86: avoid SORT_BY_INIT_PRIORITY with old GNU ld
Posted by Jan Beulich 2 years, 1 month ago
Support for this construct was added in 2.22 only. Avoid the need to
introduce logic to probe for linker script capabilities by (ab)using the
probe for a command line option having appeared at about the same time.

Fixes: 4b7fd8153ddf ("x86: fold sections in final binaries")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Always define HAVE_LD_SORT_BY_INIT_PRIORITY when using LLVM ld.

--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -73,6 +73,16 @@ ifeq ($(CONFIG_UBSAN),y)
 $(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
 endif
 
+ifeq ($(call success,$(LD) --version | head -n 1 | grep -q "GNU ld"),y)
+# While not much better than going by raw GNU ld version, utilize that the
+# feature we're after has appeared in the same release as the
+# --print-output-format command line option.
+AFLAGS-$(call ld-option,--print-output-format) += -DHAVE_LD_SORT_BY_INIT_PRIORITY
+else
+# Assume all versions of LLD support this.
+AFLAGS += -DHAVE_LD_SORT_BY_INIT_PRIORITY
+endif
+
 ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
 
 efi-check := arch/x86/efi/check
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -30,6 +30,10 @@ ENTRY(start_pa)
 # define SECTION_ALIGN PAGE_SIZE
 #endif
 
+#ifndef HAVE_LD_SORT_BY_INIT_PRIORITY
+# define SORT_BY_INIT_PRIORITY SORT
+#endif
+
 OUTPUT_FORMAT(FORMAT, FORMAT, FORMAT)
 
 OUTPUT_ARCH(i386:x86-64)
Re: [PATCH v2] x86: avoid SORT_BY_INIT_PRIORITY with old GNU ld
Posted by Roger Pau Monné 2 years, 1 month ago
On Fri, Mar 11, 2022 at 02:28:40PM +0100, Jan Beulich wrote:
> Support for this construct was added in 2.22 only. Avoid the need to
> introduce logic to probe for linker script capabilities by (ab)using the
> probe for a command line option having appeared at about the same time.
> 
> Fixes: 4b7fd8153ddf ("x86: fold sections in final binaries")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Always define HAVE_LD_SORT_BY_INIT_PRIORITY when using LLVM ld.
> 
> --- a/xen/arch/x86/arch.mk
> +++ b/xen/arch/x86/arch.mk
> @@ -73,6 +73,16 @@ ifeq ($(CONFIG_UBSAN),y)
>  $(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
>  endif
>  
> +ifeq ($(call success,$(LD) --version | head -n 1 | grep -q "GNU ld"),y)

You are not going to like this, but I think this should live in
xen/Kconfig together with CC_IS_{GCC,CLANG}, ie: LD_IS_GNU or similar.

It's possible we will need this in the future in other places, so
having it in Kconfig makes sense.

> +# While not much better than going by raw GNU ld version, utilize that the
> +# feature we're after has appeared in the same release as the
> +# --print-output-format command line option.
> +AFLAGS-$(call ld-option,--print-output-format) += -DHAVE_LD_SORT_BY_INIT_PRIORITY

That's fine to have here IMO.

Thanks, Roger.
Re: [PATCH v2] x86: avoid SORT_BY_INIT_PRIORITY with old GNU ld
Posted by Jan Beulich 2 years, 1 month ago
On 11.03.2022 15:34, Roger Pau Monné wrote:
> On Fri, Mar 11, 2022 at 02:28:40PM +0100, Jan Beulich wrote:
>> Support for this construct was added in 2.22 only. Avoid the need to
>> introduce logic to probe for linker script capabilities by (ab)using the
>> probe for a command line option having appeared at about the same time.
>>
>> Fixes: 4b7fd8153ddf ("x86: fold sections in final binaries")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> v2: Always define HAVE_LD_SORT_BY_INIT_PRIORITY when using LLVM ld.
>>
>> --- a/xen/arch/x86/arch.mk
>> +++ b/xen/arch/x86/arch.mk
>> @@ -73,6 +73,16 @@ ifeq ($(CONFIG_UBSAN),y)
>>  $(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
>>  endif
>>  
>> +ifeq ($(call success,$(LD) --version | head -n 1 | grep -q "GNU ld"),y)
> 
> You are not going to like this, but I think this should live in
> xen/Kconfig together with CC_IS_{GCC,CLANG}, ie: LD_IS_GNU or similar.
> 
> It's possible we will need this in the future in other places, so
> having it in Kconfig makes sense.

Despite me not liking this (until, as said elsewhere, we've properly
settled on either approach) I did actually consider doing like you
suggest. But: I would have to introduce there more than I need here,
just for consistency's sake, and I wouldn't have a way to test the
LLD part of it (I did check - none of the distros where I chose to
install Clang offer the linker). I realize I could ask you to help
with the testing, but then the first point would remain. I'd prefer
if for this simple build fix it was okay to go the old fashioned
route ...

Jan
Re: [PATCH v2] x86: avoid SORT_BY_INIT_PRIORITY with old GNU ld
Posted by Roger Pau Monné 2 years, 1 month ago
On Fri, Mar 11, 2022 at 03:55:57PM +0100, Jan Beulich wrote:
> On 11.03.2022 15:34, Roger Pau Monné wrote:
> > On Fri, Mar 11, 2022 at 02:28:40PM +0100, Jan Beulich wrote:
> >> Support for this construct was added in 2.22 only. Avoid the need to
> >> introduce logic to probe for linker script capabilities by (ab)using the
> >> probe for a command line option having appeared at about the same time.
> >>
> >> Fixes: 4b7fd8153ddf ("x86: fold sections in final binaries")
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> ---
> >> v2: Always define HAVE_LD_SORT_BY_INIT_PRIORITY when using LLVM ld.
> >>
> >> --- a/xen/arch/x86/arch.mk
> >> +++ b/xen/arch/x86/arch.mk
> >> @@ -73,6 +73,16 @@ ifeq ($(CONFIG_UBSAN),y)
> >>  $(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
> >>  endif
> >>  
> >> +ifeq ($(call success,$(LD) --version | head -n 1 | grep -q "GNU ld"),y)
> > 
> > You are not going to like this, but I think this should live in
> > xen/Kconfig together with CC_IS_{GCC,CLANG}, ie: LD_IS_GNU or similar.
> > 
> > It's possible we will need this in the future in other places, so
> > having it in Kconfig makes sense.
> 
> Despite me not liking this (until, as said elsewhere, we've properly
> settled on either approach) I did actually consider doing like you
> suggest. But: I would have to introduce there more than I need here,
> just for consistency's sake, and I wouldn't have a way to test the
> LLD part of it (I did check - none of the distros where I chose to
> install Clang offer the linker). I realize I could ask you to help
> with the testing, but then the first point would remain. I'd prefer
> if for this simple build fix it was okay to go the old fashioned
> route ...

I would be fine with you just introducing LD_IS_GNU. That's all we
need so far. We can introduce LD_IS_LLVM if/when required. I prefer
that asymmetry rather than doing the detection here.

Thanks, Roger.

Re: [PATCH v2] x86: avoid SORT_BY_INIT_PRIORITY with old GNU ld
Posted by Jan Beulich 2 years, 1 month ago
On 11.03.2022 16:07, Roger Pau Monné wrote:
> On Fri, Mar 11, 2022 at 03:55:57PM +0100, Jan Beulich wrote:
>> On 11.03.2022 15:34, Roger Pau Monné wrote:
>>> On Fri, Mar 11, 2022 at 02:28:40PM +0100, Jan Beulich wrote:
>>>> Support for this construct was added in 2.22 only. Avoid the need to
>>>> introduce logic to probe for linker script capabilities by (ab)using the
>>>> probe for a command line option having appeared at about the same time.
>>>>
>>>> Fixes: 4b7fd8153ddf ("x86: fold sections in final binaries")
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> v2: Always define HAVE_LD_SORT_BY_INIT_PRIORITY when using LLVM ld.
>>>>
>>>> --- a/xen/arch/x86/arch.mk
>>>> +++ b/xen/arch/x86/arch.mk
>>>> @@ -73,6 +73,16 @@ ifeq ($(CONFIG_UBSAN),y)
>>>>  $(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
>>>>  endif
>>>>  
>>>> +ifeq ($(call success,$(LD) --version | head -n 1 | grep -q "GNU ld"),y)
>>>
>>> You are not going to like this, but I think this should live in
>>> xen/Kconfig together with CC_IS_{GCC,CLANG}, ie: LD_IS_GNU or similar.
>>>
>>> It's possible we will need this in the future in other places, so
>>> having it in Kconfig makes sense.
>>
>> Despite me not liking this (until, as said elsewhere, we've properly
>> settled on either approach) I did actually consider doing like you
>> suggest. But: I would have to introduce there more than I need here,
>> just for consistency's sake, and I wouldn't have a way to test the
>> LLD part of it (I did check - none of the distros where I chose to
>> install Clang offer the linker). I realize I could ask you to help
>> with the testing, but then the first point would remain. I'd prefer
>> if for this simple build fix it was okay to go the old fashioned
>> route ...
> 
> I would be fine with you just introducing LD_IS_GNU. That's all we
> need so far. We can introduce LD_IS_LLVM if/when required. I prefer
> that asymmetry rather than doing the detection here.

Yet I'm not happy to go this route. I'm only willing to do this
consistently, but that in turn not without us having formally sat down
and discussed the overall model to use. The only short term alternative
I see is to go back to SORT() unilaterally, hoping that for now
different priorities won't be encountered.

Jan