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

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

[Jan Beulich]

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

[Roger Pau Monné]

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

[Jan Beulich]

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

[Roger Pau Monné]

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

[Jan Beulich]

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

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

[Roger Pau Monné]

On Mon, Mar 14, 2022 at 08:38:52AM +0100, Jan Beulich wrote:
> 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.

Would you be fine if it was a patch of mine that introduces
LD_IS_{GNU,LLVM} to xen/Kconfig (Acked by someone else) and then you
use it here?

I realize this is tricking you, but I would rather get this unblocked
if possible.

While I can agree to test for features in Makefiles closer to the
place where they are used, I don't like doing the version checks in
Makefiles, much less arch-specific ones.

Thanks, Roger.

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

[Jan Beulich]

On 14.03.2022 10:05, Roger Pau Monné wrote:
> On Mon, Mar 14, 2022 at 08:38:52AM +0100, Jan Beulich wrote:
>> 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.
> 
> Would you be fine if it was a patch of mine that introduces
> LD_IS_{GNU,LLVM} to xen/Kconfig (Acked by someone else) and then you
> use it here?
> 
> I realize this is tricking you, but I would rather get this unblocked
> if possible.

Well, yes, if the construct had been there, I certainly would have used it
(somewhat hesitantly maybe).

Jan