Clang arm64 build is broken

Clang arm64 build is broken

[Andrey Konovalov]

Hi Marc!

Your recent commit [1] broke clang build on arm64. The issue is that
clang doesn't know about the "S" asm constraint. I reported this to
clang [2], and hopefully this will get fixed. In the meantime, would
it possible to work around using the "S" constraint in the kernel?

While we're here, regarding the other issue with kvm [3], I didn't
receive any comments as to whether it makes sense to send the fix that
adds -fno-jump-tables flag when building kvm with clang.

Thanks!

[1] https://github.com/torvalds/linux/commit/44a497abd621a71c645f06d3d545ae2f46448830

[2] https://github.com/ClangBuiltLinux/linux/issues/13

[3] https://lkml.org/lkml/2018/3/16/476

Re: Clang arm64 build is broken

[Marc Zyngier]

Hi Andrey.

On 19/04/18 18:06, Andrey Konovalov wrote:
> Hi Marc!
> 
> Your recent commit [1] broke clang build on arm64.

Or rather, it has uncovered yet another missing feature in clang! ;-)

> The issue is that
> clang doesn't know about the "S" asm constraint. I reported this to
> clang [2], and hopefully this will get fixed. In the meantime, would
> it possible to work around using the "S" constraint in the kernel?

I have no idea, I've never used clang to build the kernel. Clang isn't
really supported to build the arm64 kernel anyway (as you mention
below), and working around clang deficiencies would mean that we leave
with the workaround forever. I'd rather enable clang once it is at
feature parity with GCC.

> While we're here, regarding the other issue with kvm [3], I didn't
> receive any comments as to whether it makes sense to send the fix that
> adds -fno-jump-tables flag when building kvm with clang.

Is that the only thing missing? Are you sure that there is no other way
for clang to generate absolute addresses that will then lead to a crash?
Again, I'd rather make sure we have the full picture.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: Clang arm64 build is broken

[Andrey Konovalov]

On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> The issue is that
>> clang doesn't know about the "S" asm constraint. I reported this to
>> clang [2], and hopefully this will get fixed. In the meantime, would
>> it possible to work around using the "S" constraint in the kernel?
>
> I have no idea, I've never used clang to build the kernel. Clang isn't
> really supported to build the arm64 kernel anyway (as you mention
> below), and working around clang deficiencies would mean that we leave
> with the workaround forever. I'd rather enable clang once it is at
> feature parity with GCC.

The fact that there are some existing issues with building arm64
kernel with clang doesn't sound like a good justification for adding
new issues :)

However in this case I do believe that this is more of a bug in clang
that should be fixed.

>> While we're here, regarding the other issue with kvm [3], I didn't
>> receive any comments as to whether it makes sense to send the fix that
>> adds -fno-jump-tables flag when building kvm with clang.
>
> Is that the only thing missing? Are you sure that there is no other way
> for clang to generate absolute addresses that will then lead to a crash?
> Again, I'd rather make sure we have the full picture.

Well, I have tried applying that patch and running kvm tests that I
could find [1], and they passed (actually I think there was an issue
with one of them, but I saw the same thing when I tried running them
on a kernel built with GCC).

[1] https://www.linux-kvm.org/page/KVM-unit-tests

Re: Clang arm64 build is broken

[Marc Zyngier]

[fixing up Christoffer's address]

On 20/04/18 15:59, Andrey Konovalov wrote:
> On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> The issue is that
>>> clang doesn't know about the "S" asm constraint. I reported this to
>>> clang [2], and hopefully this will get fixed. In the meantime, would
>>> it possible to work around using the "S" constraint in the kernel?
>>
>> I have no idea, I've never used clang to build the kernel. Clang isn't
>> really supported to build the arm64 kernel anyway (as you mention
>> below), and working around clang deficiencies would mean that we leave
>> with the workaround forever. I'd rather enable clang once it is at
>> feature parity with GCC.
> 
> The fact that there are some existing issues with building arm64
> kernel with clang doesn't sound like a good justification for adding
> new issues :)

Once clang is in a state where it actually compiles a full-featured
kernel that can be subsequently booted, I'll definitely care about it,
and will make sure it doesn't break. In the meantime, I'm targeting GCC,
which is the only sane option for arm64 at the moment. Waiting for clang
to fixed is not an option.

> However in this case I do believe that this is more of a bug in clang
> that should be fixed.

Absolutely. Lack of GCC compatibility is what impairs the adoption of clang.

>>> While we're here, regarding the other issue with kvm [3], I didn't
>>> receive any comments as to whether it makes sense to send the fix that
>>> adds -fno-jump-tables flag when building kvm with clang.
>>
>> Is that the only thing missing? Are you sure that there is no other way
>> for clang to generate absolute addresses that will then lead to a crash?
>> Again, I'd rather make sure we have the full picture.
> 
> Well, I have tried applying that patch and running kvm tests that I
> could find [1], and they passed (actually I think there was an issue
> with one of them, but I saw the same thing when I tried running them
> on a kernel built with GCC).
> 
> [1] https://www.linux-kvm.org/page/KVM-unit-tests

Which issue? Maybe it'd be worth reporting it?

To the original discussion about absolute addresses: I'm not so much
looking for additional testing (I trust that you've tested the patch
before sending it). I'm more after a statement from the LLVM folks
indicating in which conditions the compiler will emit absolute addresses
instead of PC-relative ones, and whether there is a way to make sure
that we always get the latter. If '-fno-jump-tables' is the only missing
flag, great. Otherwise, I'd like to know.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: Clang arm64 build is broken

[Mark Rutland]

Hi Andrey,

On Fri, Apr 20, 2018 at 04:59:35PM +0200, Andrey Konovalov wrote:
> On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >> The issue is that
> >> clang doesn't know about the "S" asm constraint. I reported this to
> >> clang [2], and hopefully this will get fixed. In the meantime, would
> >> it possible to work around using the "S" constraint in the kernel?
> >
> > I have no idea, I've never used clang to build the kernel. Clang isn't
> > really supported to build the arm64 kernel anyway (as you mention
> > below), and working around clang deficiencies would mean that we leave
> > with the workaround forever. I'd rather enable clang once it is at
> > feature parity with GCC.
> 
> The fact that there are some existing issues with building arm64
> kernel with clang doesn't sound like a good justification for adding
> new issues :)

I appreciate this is somewhat frustrating, but every feature where clang
is not at parity with GCC is effectively a functional regression for us.

Recently, the code that clang hasn't liked happens to be security
critical, and it is somewhat difficult to justify making that code more
complex to cater for a compiler that we know has outstanding issues with
features we rely upon.

Which is to say, I'm not sure that there's much justification either
way. We really need clang to be at feature parity with GCC for it to be
considered supported.

It would be great if some effort could be focussed on bringing clang to
feature parity with GCC before implementing new clang-specific features.

> However in this case I do believe that this is more of a bug in clang
> that should be fixed.

Just to check: does clang implement the rest of the AArch64 machine
constraints [1]?

We're liable to use more of them in future, and we should aim for parity
now so that we don't fall into the same trap in future.

> >> While we're here, regarding the other issue with kvm [3], I didn't
> >> receive any comments as to whether it makes sense to send the fix that
> >> adds -fno-jump-tables flag when building kvm with clang.
> >
> > Is that the only thing missing? Are you sure that there is no other way
> > for clang to generate absolute addresses that will then lead to a crash?
> > Again, I'd rather make sure we have the full picture.
> 
> Well, I have tried applying that patch and running kvm tests that I
> could find [1], and they passed (actually I think there was an issue
> with one of them, but I saw the same thing when I tried running them
> on a kernel built with GCC).

I think what Marc wants is a statement as to whether -fno-jump-tables is
sufficient to ensure that clang will not try to use absolute addressing,
based on an understanding of the AArch64 LLVM backend rather than test
cases.

For example, could any other pass result in the use of an absolute
address? Or are jump tables the *only* reason that clang would try to
use an absolute address.

Are there other options that we might need to pass?

Are there any options that we can pass to forbid absolute addressing?

Thanks,
Mark.

[1] https://gcc.gnu.org/onlinedocs/gccint/Machine-Constraints.html

Re: Clang arm64 build is broken

[Ard Biesheuvel]

On 20 April 2018 at 17:38, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Andrey,
>
> On Fri, Apr 20, 2018 at 04:59:35PM +0200, Andrey Konovalov wrote:
>> On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> >> The issue is that
>> >> clang doesn't know about the "S" asm constraint. I reported this to
>> >> clang [2], and hopefully this will get fixed. In the meantime, would
>> >> it possible to work around using the "S" constraint in the kernel?
>> >
>> > I have no idea, I've never used clang to build the kernel. Clang isn't
>> > really supported to build the arm64 kernel anyway (as you mention
>> > below), and working around clang deficiencies would mean that we leave
>> > with the workaround forever. I'd rather enable clang once it is at
>> > feature parity with GCC.
>>
>> The fact that there are some existing issues with building arm64
>> kernel with clang doesn't sound like a good justification for adding
>> new issues :)
>
> I appreciate this is somewhat frustrating, but every feature where clang
> is not at parity with GCC is effectively a functional regression for us.
>
> Recently, the code that clang hasn't liked happens to be security
> critical, and it is somewhat difficult to justify making that code more
> complex to cater for a compiler that we know has outstanding issues with
> features we rely upon.
>
> Which is to say, I'm not sure that there's much justification either
> way. We really need clang to be at feature parity with GCC for it to be
> considered supported.
>
> It would be great if some effort could be focussed on bringing clang to
> feature parity with GCC before implementing new clang-specific features.
>
>> However in this case I do believe that this is more of a bug in clang
>> that should be fixed.
>
> Just to check: does clang implement the rest of the AArch64 machine
> constraints [1]?
>
> We're liable to use more of them in future, and we should aim for parity
> now so that we don't fall into the same trap in future.
>
>> >> While we're here, regarding the other issue with kvm [3], I didn't
>> >> receive any comments as to whether it makes sense to send the fix that
>> >> adds -fno-jump-tables flag when building kvm with clang.
>> >
>> > Is that the only thing missing? Are you sure that there is no other way
>> > for clang to generate absolute addresses that will then lead to a crash?
>> > Again, I'd rather make sure we have the full picture.
>>
>> Well, I have tried applying that patch and running kvm tests that I
>> could find [1], and they passed (actually I think there was an issue
>> with one of them, but I saw the same thing when I tried running them
>> on a kernel built with GCC).
>
> I think what Marc wants is a statement as to whether -fno-jump-tables is
> sufficient to ensure that clang will not try to use absolute addressing,
> based on an understanding of the AArch64 LLVM backend rather than test
> cases.
>
> For example, could any other pass result in the use of an absolute
> address? Or are jump tables the *only* reason that clang would try to
> use an absolute address.
>
> Are there other options that we might need to pass?
>
> Are there any options that we can pass to forbid absolute addressing?
>

One thing to note here is that it is not generally possible to inhibit
all absolute references, given that statically initialized pointer
variables will result in R_AARCH64_ABS64 relocations in any case. So
what we are after (and we should align this with the GCC team as well)
is a code model option or some other flag that prevents the compiler
from *generating code* that relies on absolute addressing, given that
we have no control over this (whereas it is feasible to avoid
statically initialized pointer variables in code that is expected to
execute at a memory offset that is different from the kernel's runtime
memory offset)

Re: Clang arm64 build is broken

[Nick Desaulniers]

On Fri, Apr 20, 2018 at 7:59 AM Andrey Konovalov <andreyknvl@google.com>
wrote:
> On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier <marc.zyngier@arm.com>
wrote:
> >> The issue is that
> >> clang doesn't know about the "S" asm constraint. I reported this to
> >> clang [2], and hopefully this will get fixed. In the meantime, would
> >> it possible to work around using the "S" constraint in the kernel?
> >
> > I have no idea, I've never used clang to build the kernel. Clang isn't
> > really supported to build the arm64 kernel anyway (as you mention
> > below), and working around clang deficiencies would mean that we leave
> > with the workaround forever. I'd rather enable clang once it is at
> > feature parity with GCC.

> The fact that there are some existing issues with building arm64
> kernel with clang doesn't sound like a good justification for adding
> new issues :)

> However in this case I do believe that this is more of a bug in clang
> that should be fixed.

Just to follow up with this thread;

Support for "S" constraints is being (re-)added to Clang in:
https://reviews.llvm.org/D46745

-- 
Thanks,
~Nick Desaulniers

Re: Clang arm64 build is broken

[Andrey Konovalov]

On Mon, May 14, 2018 at 6:24 PM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Fri, Apr 20, 2018 at 7:59 AM Andrey Konovalov <andreyknvl@google.com>
> wrote:
>> On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier <marc.zyngier@arm.com>
> wrote:
>> >> The issue is that
>> >> clang doesn't know about the "S" asm constraint. I reported this to
>> >> clang [2], and hopefully this will get fixed. In the meantime, would
>> >> it possible to work around using the "S" constraint in the kernel?
>> >
>> > I have no idea, I've never used clang to build the kernel. Clang isn't
>> > really supported to build the arm64 kernel anyway (as you mention
>> > below), and working around clang deficiencies would mean that we leave
>> > with the workaround forever. I'd rather enable clang once it is at
>> > feature parity with GCC.
>
>> The fact that there are some existing issues with building arm64
>> kernel with clang doesn't sound like a good justification for adding
>> new issues :)
>
>> However in this case I do believe that this is more of a bug in clang
>> that should be fixed.
>
> Just to follow up with this thread;
>
> Support for "S" constraints is being (re-)added to Clang in:
> https://reviews.llvm.org/D46745

Hi Nick!

I can confirm that the latest clang (which includes this patch) is
able to build the kernel with CONFIG_KVM enabled.

Thanks!

Re: Clang arm64 build is broken

[Nick Desaulniers]

On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier <marc.zyngier@arm.com> wrote:
> Clang isn't
> really supported to build the arm64 kernel anyway

Can you expand on this?  There are millions of arm64 devices shipping with
Clang built Linux kernels.

--
Thanks,
~Nick Desaulniers

Re: Clang arm64 build is broken

[Marc Zyngier]

On 20/04/18 17:30, Nick Desaulniers wrote:
> On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Clang isn't
>> really supported to build the arm64 kernel anyway
> 
> Can you expand on this?  There are millions of arm64 devices shipping with
> Clang built Linux kernels.

How many of these devices run a full-featured mainline kernel?

Sure, Android is building the kernel with clang, but that's with a pile
of out of tree patches. At that point, I'm not sure we're talking about
the same arm64 kernel.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: Clang arm64 build is broken

[Nick Desaulniers]

On Fri, Apr 20, 2018 at 9:36 AM Marc Zyngier <marc.zyngier@arm.com> wrote:

> On 20/04/18 17:30, Nick Desaulniers wrote:
> > On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier <marc.zyngier@arm.com>
wrote:
> >> Clang isn't
> >> really supported to build the arm64 kernel anyway
> >
> > Can you expand on this?  There are millions of arm64 devices shipping
with
> > Clang built Linux kernels.

> How many of these devices run a full-featured mainline kernel?

> Sure, Android is building the kernel with clang, but that's with a pile
> of out of tree patches. At that point, I'm not sure we're talking about
> the same arm64 kernel.

Do you recommend that we only work with ARM licensed SoC vendors with no
out of tree patches?

--
Thanks,
~Nick Desaulniers

Re: Clang arm64 build is broken

[Marc Zyngier]

On 20/04/18 17:43, Nick Desaulniers wrote:
> On Fri, Apr 20, 2018 at 9:36 AM Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> On 20/04/18 17:30, Nick Desaulniers wrote:
>>> On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier <marc.zyngier@arm.com>
> wrote:
>>>> Clang isn't
>>>> really supported to build the arm64 kernel anyway
>>>
>>> Can you expand on this?  There are millions of arm64 devices shipping
> with
>>> Clang built Linux kernels.
> 
>> How many of these devices run a full-featured mainline kernel?
> 
>> Sure, Android is building the kernel with clang, but that's with a pile
>> of out of tree patches. At that point, I'm not sure we're talking about
>> the same arm64 kernel.
> 
> Do you recommend that we only work with ARM licensed SoC vendors with no
> out of tree patches?

I mean the clang-related patches which are not upstream. We can ignore
SoC support code for this discussion.

The android clang patches try to create the illusion that clang is
usable for the kernel. It is not. It misses fundamental functionality
that either leads to performance degradation (lack of asm goto) or
prevents the implementation of security fixes (see the recent discussion
about SMCCC 1.1 and the top of this very thread).

I'd rather recommend that clang is brought up to the level of GCC so
that we don't have to continuously have that discussion. Give us that
compiler, and we'll make sure the arm64 doesn't break.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...