Re: Clang arm64 build is broken
On Mon, May 14, 2018 at 6:24 PM, Nick Desaulniers wrote: > On Fri, Apr 20, 2018 at 7:59 AM Andrey Konovalov > wrote: >> On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier > 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! ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On Fri, Apr 20, 2018 at 7:59 AM Andrey Konovalov wrote: > On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier 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 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On Fri, Apr 20, 2018 at 9:36 AM Marc Zyngier wrote: > On 20/04/18 17:30, Nick Desaulniers wrote: > > On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier 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 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier 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 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On 20/04/18 17:43, Nick Desaulniers wrote: > On Fri, Apr 20, 2018 at 9:36 AM Marc Zyngier wrote: > >> On 20/04/18 17:30, Nick Desaulniers wrote: >>> On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier > 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... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On 20/04/18 17:30, Nick Desaulniers wrote: > On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier 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... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On 20 April 2018 at 17:38, Mark Rutland 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 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) ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
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 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 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
[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 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... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier 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 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
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... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Clang arm64 build is broken
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 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm