diff mbox series

x86: work around build issue with GNU ld 2.37

Message ID 2e0beb7e-022f-efb3-3adc-4877c60bfeba@suse.com (mailing list archive)
State New
Headers show
Series x86: work around build issue with GNU ld 2.37 | expand

Commit Message

Jan Beulich July 22, 2021, 9:20 a.m. UTC
I suspect it is commit 40726f16a8d7 ("ld script expression parsing")
which broke the hypervisor build, by no longer accepting section names
with a dash in them inside ADDR() (and perhaps other script directives
expecting just a section name, not an expression): .note.gnu.build-id
is such a section.

Quoting all section names passed to ADDR() via DECL_SECTION() works
around the regression.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments

Andrew Cooper July 27, 2021, 12:33 p.m. UTC | #1
On 22/07/2021 10:20, Jan Beulich wrote:
> I suspect it is commit 40726f16a8d7 ("ld script expression parsing")
> which broke the hypervisor build, by no longer accepting section names
> with a dash in them inside ADDR() (and perhaps other script directives
> expecting just a section name, not an expression): .note.gnu.build-id
> is such a section.

Are binutils going to fix their testing to reduce the number of serious
regressions they're releasing?

>
> Quoting all section names passed to ADDR() via DECL_SECTION() works
> around the regression.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I guess we've got no choice.  Acked-by: Andrew Cooper
<andrew.cooper3@citrix.com>

>
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -18,7 +18,7 @@ ENTRY(efi_start)
>  #else /* !EFI */
>  
>  #define FORMAT "elf64-x86-64"
> -#define DECL_SECTION(x) x : AT(ADDR(x) - __XEN_VIRT_START)
> +#define DECL_SECTION(x) x : AT(ADDR(#x) - __XEN_VIRT_START)
>  
>  ENTRY(start_pa)
>  
>
Jan Beulich Aug. 3, 2021, 6:37 a.m. UTC | #2
On 27.07.2021 14:33, Andrew Cooper wrote:
> On 22/07/2021 10:20, Jan Beulich wrote:
>> I suspect it is commit 40726f16a8d7 ("ld script expression parsing")
>> which broke the hypervisor build, by no longer accepting section names
>> with a dash in them inside ADDR() (and perhaps other script directives
>> expecting just a section name, not an expression): .note.gnu.build-id
>> is such a section.
> 
> Are binutils going to fix their testing to reduce the number of serious
> regressions they're releasing?

To be honest - I simply don't know.

>> Quoting all section names passed to ADDR() via DECL_SECTION() works
>> around the regression.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> I guess we've got no choice.  Acked-by: Andrew Cooper
> <andrew.cooper3@citrix.com>

Thanks. I see you've committed this already.

Jan
Andrew Cooper Aug. 10, 2021, 8:33 a.m. UTC | #3
On 03/08/2021 07:37, Jan Beulich wrote:
> On 27.07.2021 14:33, Andrew Cooper wrote:
>> On 22/07/2021 10:20, Jan Beulich wrote:
>>> I suspect it is commit 40726f16a8d7 ("ld script expression parsing")
>>> which broke the hypervisor build, by no longer accepting section names
>>> with a dash in them inside ADDR() (and perhaps other script directives
>>> expecting just a section name, not an expression): .note.gnu.build-id
>>> is such a section.
>> Are binutils going to fix their testing to reduce the number of serious
>> regressions they're releasing?
> To be honest - I simply don't know.
>
>>> Quoting all section names passed to ADDR() via DECL_SECTION() works
>>> around the regression.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> I guess we've got no choice.  Acked-by: Andrew Cooper
>> <andrew.cooper3@citrix.com>
> Thanks. I see you've committed this already.

Actually, it unilaterally breaks FreeBSD builds: 
https://cirrus-ci.com/task/5417332467040256

I'm not sure why my build tests didn't notice, but obviously this patch
isn't a workable option.

~Andrew
Jan Beulich Aug. 10, 2021, 8:50 a.m. UTC | #4
On 10.08.2021 10:33, Andrew Cooper wrote:
> On 03/08/2021 07:37, Jan Beulich wrote:
>> On 27.07.2021 14:33, Andrew Cooper wrote:
>>> On 22/07/2021 10:20, Jan Beulich wrote:
>>>> I suspect it is commit 40726f16a8d7 ("ld script expression parsing")
>>>> which broke the hypervisor build, by no longer accepting section names
>>>> with a dash in them inside ADDR() (and perhaps other script directives
>>>> expecting just a section name, not an expression): .note.gnu.build-id
>>>> is such a section.
>>> Are binutils going to fix their testing to reduce the number of serious
>>> regressions they're releasing?
>> To be honest - I simply don't know.
>>
>>>> Quoting all section names passed to ADDR() via DECL_SECTION() works
>>>> around the regression.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> I guess we've got no choice.  Acked-by: Andrew Cooper
>>> <andrew.cooper3@citrix.com>
>> Thanks. I see you've committed this already.
> 
> Actually, it unilaterally breaks FreeBSD builds: 
> https://cirrus-ci.com/task/5417332467040256
> 
> I'm not sure why my build tests didn't notice, but obviously this patch
> isn't a workable option.

I'm confused: Is the tool called "ld" there something that's not only not
GNU ld, but not even compatible with GNU ld? (Iirc clang's linker is named
differently. Or maybe that's just on Linux? In any event I've just checked
with clang5 [on Linux], and the build worked fine there. But this looks to
be using GNU ld irrespective of the compiler choice, and I also don't seem
to have anything named "llvm-ld" on that system, despite there being a lot
of other "llvm-*".)

Jan
Jan Beulich Aug. 16, 2021, 3:43 p.m. UTC | #5
On 10.08.2021 10:50, Jan Beulich wrote:
> On 10.08.2021 10:33, Andrew Cooper wrote:
>> On 03/08/2021 07:37, Jan Beulich wrote:
>>> On 27.07.2021 14:33, Andrew Cooper wrote:
>>>> On 22/07/2021 10:20, Jan Beulich wrote:
>>>>> I suspect it is commit 40726f16a8d7 ("ld script expression parsing")
>>>>> which broke the hypervisor build, by no longer accepting section names
>>>>> with a dash in them inside ADDR() (and perhaps other script directives
>>>>> expecting just a section name, not an expression): .note.gnu.build-id
>>>>> is such a section.
>>>> Are binutils going to fix their testing to reduce the number of serious
>>>> regressions they're releasing?
>>> To be honest - I simply don't know.
>>>
>>>>> Quoting all section names passed to ADDR() via DECL_SECTION() works
>>>>> around the regression.
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> I guess we've got no choice.  Acked-by: Andrew Cooper
>>>> <andrew.cooper3@citrix.com>
>>> Thanks. I see you've committed this already.
>>
>> Actually, it unilaterally breaks FreeBSD builds: 
>> https://cirrus-ci.com/task/5417332467040256
>>
>> I'm not sure why my build tests didn't notice, but obviously this patch
>> isn't a workable option.
> 
> I'm confused: Is the tool called "ld" there something that's not only not
> GNU ld, but not even compatible with GNU ld? (Iirc clang's linker is named
> differently. Or maybe that's just on Linux? In any event I've just checked
> with clang5 [on Linux], and the build worked fine there. But this looks to
> be using GNU ld irrespective of the compiler choice, and I also don't seem
> to have anything named "llvm-ld" on that system, despite there being a lot
> of other "llvm-*".)

So I've looked at their man pages. From version 11 to version 12 the
linker changed from GNU ld to LLVM's. Interestingly the respective man
page says

     ld.lld is a drop-in replacement for the GNU BFD and gold linkers.	It ac-
     cepts most	of the same command line arguments and linker scripts as GNU
     linkers.

To me "most of the same" isn't quite enough to claim "drop-in
replacement", but I guess that's just me ... Anyway - short of Roger
being around to ask, I'll try to locate a means to tell the two apart
from a linker script.

Jan
diff mbox series

Patch

--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -18,7 +18,7 @@  ENTRY(efi_start)
 #else /* !EFI */
 
 #define FORMAT "elf64-x86-64"
-#define DECL_SECTION(x) x : AT(ADDR(x) - __XEN_VIRT_START)
+#define DECL_SECTION(x) x : AT(ADDR(#x) - __XEN_VIRT_START)
 
 ENTRY(start_pa)