2017-08-10 20:13:27

by Laura Abbott

[permalink] [raw]
Subject: New assembler warnings with binutils 2.29

Fedora rawhide recently upgraded to binutils 2.29 and this seems
to produce new warnings:

./arch/arm64/include/asm/assembler.h: Assembler messages:
./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to redefine built-in register 'lr'

This is

/*
* Register aliases.
*/
lr .req x30 // link register

https://kojipkgs.fedoraproject.org//packages/kernel/4.13.0/0.rc2.git2.1.fc27/data/logs/aarch64/build.log
has an example log. As far as I can tell this doesn't affect anything at runtime.

Any ideas?

Thanks,
Laura


2017-08-11 09:22:19

by Catalin Marinas

[permalink] [raw]
Subject: Re: New assembler warnings with binutils 2.29

On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
> Fedora rawhide recently upgraded to binutils 2.29 and this seems
> to produce new warnings:
>
> ./arch/arm64/include/asm/assembler.h: Assembler messages:
> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to redefine built-in register 'lr'
>
> This is
>
> /*
> * Register aliases.
> */
> lr .req x30 // link register

Strange, does gas now think 'lr' is a general purpose register (aliased
to x30)? It never was and IIRC the toolchain people many years ago
refused to add it, hence the alias above in the kernel. I wonder if they
added 'fp' as well...

We could remove the alias and replace all 'lr' instances with 'x30'
throughout the kernel (no too many) or we add some #ifdef around the
above based on the binutils version.

--
Catalin

2017-08-11 09:26:08

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: New assembler warnings with binutils 2.29

On 11 August 2017 at 10:22, Catalin Marinas <[email protected]> wrote:
> On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
>> Fedora rawhide recently upgraded to binutils 2.29 and this seems
>> to produce new warnings:
>>
>> ./arch/arm64/include/asm/assembler.h: Assembler messages:
>> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to redefine built-in register 'lr'
>>
>> This is
>>
>> /*
>> * Register aliases.
>> */
>> lr .req x30 // link register
>
> Strange, does gas now think 'lr' is a general purpose register (aliased
> to x30)? It never was and IIRC the toolchain people many years ago
> refused to add it, hence the alias above in the kernel. I wonder if they
> added 'fp' as well...
>
> We could remove the alias and replace all 'lr' instances with 'x30'
> throughout the kernel (no too many) or we add some #ifdef around the
> above based on the binutils version.
>

This is annoying. Replacing x30 with lr achieves the opposite of the
intent of the binutils change. And using #ifdefs is inaccurate,
because you can't really test the binutils version only the GCC
version, and those are not tightly coupled.

Can you .unreq it?

2017-08-11 09:34:39

by Arnd Bergmann

[permalink] [raw]
Subject: Re: New assembler warnings with binutils 2.29

On Fri, Aug 11, 2017 at 11:26 AM, Ard Biesheuvel
<[email protected]> wrote:
> On 11 August 2017 at 10:22, Catalin Marinas <[email protected]> wrote:
>> On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
>>> Fedora rawhide recently upgraded to binutils 2.29 and this seems
>>> to produce new warnings:
>>>
>>> ./arch/arm64/include/asm/assembler.h: Assembler messages:
>>> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to redefine built-in register 'lr'
>>>
>>> This is
>>>
>>> /*
>>> * Register aliases.
>>> */
>>> lr .req x30 // link register
>>
>> Strange, does gas now think 'lr' is a general purpose register (aliased
>> to x30)? It never was and IIRC the toolchain people many years ago
>> refused to add it, hence the alias above in the kernel. I wonder if they
>> added 'fp' as well...
>>
>> We could remove the alias and replace all 'lr' instances with 'x30'
>> throughout the kernel (no too many) or we add some #ifdef around the
>> above based on the binutils version.
>>
>
> This is annoying. Replacing x30 with lr achieves the opposite of the
> intent of the binutils change. And using #ifdefs is inaccurate,
> because you can't really test the binutils version only the GCC
> version, and those are not tightly coupled.
>
> Can you .unreq it?

adding the author of the change to cc

https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=62e20ed45e3da5f3ba695e4ee109317668180fe6

There probably was some reasoning behind the change and an
intended method for using it.

Arnd

2017-08-11 10:17:05

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: New assembler warnings with binutils 2.29

On 11 August 2017 at 10:34, Arnd Bergmann <[email protected]> wrote:
> On Fri, Aug 11, 2017 at 11:26 AM, Ard Biesheuvel
> <[email protected]> wrote:
>> On 11 August 2017 at 10:22, Catalin Marinas <[email protected]> wrote:
>>> On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
>>>> Fedora rawhide recently upgraded to binutils 2.29 and this seems
>>>> to produce new warnings:
>>>>
>>>> ./arch/arm64/include/asm/assembler.h: Assembler messages:
>>>> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to redefine built-in register 'lr'
>>>>
>>>> This is
>>>>
>>>> /*
>>>> * Register aliases.
>>>> */
>>>> lr .req x30 // link register
>>>
>>> Strange, does gas now think 'lr' is a general purpose register (aliased
>>> to x30)? It never was and IIRC the toolchain people many years ago
>>> refused to add it, hence the alias above in the kernel. I wonder if they
>>> added 'fp' as well...
>>>
>>> We could remove the alias and replace all 'lr' instances with 'x30'
>>> throughout the kernel (no too many) or we add some #ifdef around the
>>> above based on the binutils version.
>>>
>>
>> This is annoying. Replacing x30 with lr achieves the opposite of the

Of course, I meant replacing lr with x30 in our code.

>> intent of the binutils change. And using #ifdefs is inaccurate,
>> because you can't really test the binutils version only the GCC
>> version, and those are not tightly coupled.
>>
>> Can you .unreq it?
>
> adding the author of the change to cc
>
> https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=62e20ed45e3da5f3ba695e4ee109317668180fe6
>
> There probably was some reasoning behind the change and an
> intended method for using it.
>
> Arnd

2017-08-14 08:59:07

by Catalin Marinas

[permalink] [raw]
Subject: Re: New assembler warnings with binutils 2.29

On Fri, Aug 11, 2017 at 10:26:06AM +0100, Ard Biesheuvel wrote:
> On 11 August 2017 at 10:22, Catalin Marinas <[email protected]> wrote:
> > On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
> >> Fedora rawhide recently upgraded to binutils 2.29 and this seems
> >> to produce new warnings:
> >>
> >> ./arch/arm64/include/asm/assembler.h: Assembler messages:
> >> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to redefine built-in register 'lr'
> >>
> >> This is
> >>
> >> /*
> >> * Register aliases.
> >> */
> >> lr .req x30 // link register
> >
> > Strange, does gas now think 'lr' is a general purpose register (aliased
> > to x30)? It never was and IIRC the toolchain people many years ago
> > refused to add it, hence the alias above in the kernel. I wonder if they
> > added 'fp' as well...
> >
> > We could remove the alias and replace all 'lr' instances with 'x30'
> > throughout the kernel (no too many) or we add some #ifdef around the
> > above based on the binutils version.
>
> This is annoying. Replacing lr with x30 achieves the opposite of the
> intent of the binutils change. And using #ifdefs is inaccurate,
> because you can't really test the binutils version only the GCC
> version, and those are not tightly coupled.
>
> Can you .unreq it?

Not really, with an older binutils I get:

arch/arm64/include/asm/assembler.h:125: Error: unknown register alias 'lr'

I personally consider this a binutils bug. After 6+ years (probably not
all public) of building the kernel just fine, all of a sudden certain
strings became reserved in gas. Three options:

a) binutils reverts the change
b) we stop using lr etc. in Linux for good (since you can't tell which
gas supports them)
c) we replace .req with #define in the kernel

My preference is (a) but we can go for (c) as being more under our
control.

--
Catalin

2017-08-14 09:55:59

by Ramana Radhakrishnan

[permalink] [raw]
Subject: Re: New assembler warnings with binutils 2.29

On Mon, Aug 14, 2017 at 09:59:02AM +0100, Catalin Marinas wrote:
> On Fri, Aug 11, 2017 at 10:26:06AM +0100, Ard Biesheuvel wrote:
> > On 11 August 2017 at 10:22, Catalin Marinas <[email protected]> wrote:
> > > On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
> > >> Fedora rawhide recently upgraded to binutils 2.29 and this seems
> > >> to produce new warnings:
> > >>
> > >> ./arch/arm64/include/asm/assembler.h: Assembler messages:
> > >> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to redefine built-in register 'lr'
> > >>
> > >> This is
> > >>
> > >> /*
> > >> * Register aliases.
> > >> */
> > >> lr .req x30 // link register
> > >
> > > Strange, does gas now think 'lr' is a general purpose register (aliased
> > > to x30)? It never was and IIRC the toolchain people many years ago
> > > refused to add it, hence the alias above in the kernel. I wonder if they
> > > added 'fp' as well...

I don't remember the reasons why it was refused in the past. Asking
around I can't seem to find anyone who remembers why either and it would
be much quicker to fix the problem in binutils than any of this
archeology :)

> > >
> > > We could remove the alias and replace all 'lr' instances with 'x30'
> > > throughout the kernel (no too many) or we add some #ifdef around the
> > > above based on the binutils version.
> >
> > This is annoying. Replacing lr with x30 achieves the opposite of the
> > intent of the binutils change. And using #ifdefs is inaccurate,
> > because you can't really test the binutils version only the GCC
> > version, and those are not tightly coupled.
> >
> > Can you .unreq it?
>
> Not really, with an older binutils I get:
>
> arch/arm64/include/asm/assembler.h:125: Error: unknown register alias 'lr'
>
> I personally consider this a binutils bug. After 6+ years (probably not
> all public) of building the kernel just fine, all of a sudden certain
> strings became reserved in gas. Three options:

IIRC the ABI states that LR is an alias for X30, FP is an alias for X29,
IP0 an alias for X16 and IP1 an alias for X17.

The warning is unfortunate - looking at the implementation of the
.req directive in gas it's something controlled in the aarch64 backend
and thus we could change gas to make LR (and friends) an internal alias
thus removing the warning.

Any attempts to realias these to the correct meaning as per the AAPCS
continues to work from my limited testing of a prototype patch so far.

I'll do a test run and post it on the lists and point you guys to it
(probably now after lunch as I have a couple of meetings to run to) so
that you could also test it and let us know if there are any issues.


regards
Ramana

2017-08-14 10:34:51

by Catalin Marinas

[permalink] [raw]
Subject: Re: New assembler warnings with binutils 2.29

On Mon, Aug 14, 2017 at 10:55:48AM +0100, Ramana Radhakrishnan wrote:
> On Mon, Aug 14, 2017 at 09:59:02AM +0100, Catalin Marinas wrote:
> > On Fri, Aug 11, 2017 at 10:26:06AM +0100, Ard Biesheuvel wrote:
> > > On 11 August 2017 at 10:22, Catalin Marinas <[email protected]> wrote:
> > > > On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
> > > >> Fedora rawhide recently upgraded to binutils 2.29 and this seems
> > > >> to produce new warnings:
> > > >>
> > > >> ./arch/arm64/include/asm/assembler.h: Assembler messages:
> > > >> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to redefine built-in register 'lr'
> > > >>
> > > >> This is
> > > >>
> > > >> /*
> > > >> * Register aliases.
> > > >> */
> > > >> lr .req x30 // link register
> > > >
> > > > Strange, does gas now think 'lr' is a general purpose register (aliased
> > > > to x30)? It never was and IIRC the toolchain people many years ago
> > > > refused to add it, hence the alias above in the kernel. I wonder if they
> > > > added 'fp' as well...
>
> I don't remember the reasons why it was refused in the past. Asking around I
> can't seem to find anyone who remembers why either and it would be much
> quicker to fix the problem in binutils than any of this archeology :)

I don't have the old emails around either ;).

> > > > We could remove the alias and replace all 'lr' instances with 'x30'
> > > > throughout the kernel (no too many) or we add some #ifdef around the
> > > > above based on the binutils version.
> > > > This is annoying. Replacing lr with x30 achieves the opposite of the
> > > intent of the binutils change. And using #ifdefs is inaccurate,
> > > because you can't really test the binutils version only the GCC
> > > version, and those are not tightly coupled.
> > > > Can you .unreq it?
> >
> > Not really, with an older binutils I get:
> >
> > arch/arm64/include/asm/assembler.h:125: Error: unknown register alias 'lr'
> >
> > I personally consider this a binutils bug. After 6+ years (probably not
> > all public) of building the kernel just fine, all of a sudden certain
> > strings became reserved in gas. Three options:
>
> IIRC the ABI states that LR is an alias for X30, FP is an alias for X29,
> IP0 an alias for X16 and IP1 an alias for X17.

It's unfortunate that binutils never define them, so we ended up with
.req in the kernel.

> The warning is unfortunate - looking at the implementation of the
> .req directive in gas it's something controlled in the aarch64 backend
> and thus we could change gas to make LR (and friends) an internal alias thus
> removing the warning.
>
> Any attempts to realias these to the correct meaning as per the AAPCS
> continues to work from my limited testing of a prototype patch so far.

This would be great. Thanks Ramana!

--
Catalin

2017-08-14 14:45:46

by Ramana Radhakrishnan

[permalink] [raw]
Subject: Re: New assembler warnings with binutils 2.29

On Mon, Aug 14, 2017 at 11:34:45AM +0100, Catalin Marinas wrote:
> >
> > Any attempts to realias these to the correct meaning as per the AAPCS
> > continues to work from my limited testing of a prototype patch so far.
>
> This would be great. Thanks Ramana!

I've submitted the patch upstream and would appreciate any testing of kernel
builds to double check that this fixes the issue at hand.

https://sourceware.org/ml/binutils/2017-08/msg00160.html


Ramana

>
> --
> Catalin

2017-08-17 18:22:11

by Laura Abbott

[permalink] [raw]
Subject: Re: New assembler warnings with binutils 2.29

On 08/14/2017 07:45 AM, Ramana Radhakrishnan wrote:
> On Mon, Aug 14, 2017 at 11:34:45AM +0100, Catalin Marinas wrote:
>>>
>>> Any attempts to realias these to the correct meaning as per the AAPCS
>>> continues to work from my limited testing of a prototype patch so far.
>>
>> This would be great. Thanks Ramana!
>
> I've submitted the patch upstream and would appreciate any testing of kernel
> builds to double check that this fixes the issue at hand.
>
> https://sourceware.org/ml/binutils/2017-08/msg00160.html

I verified on a local build that this makes the warnings go away.

Thanks,
Laura

>
>
> Ramana