MaskRay added a comment. In D72222#1838988 <https://reviews.llvm.org/D72222#1838988>, @mrutland wrote:
> In D72222#1837536 <https://reviews.llvm.org/D72222#1837536>, @MaskRay wrote: > > > > > > > > > For `BTI c` issue, GCC has several releases that do not work with > > -mbranch-protection=bti. The Linux kernel has to develop some mechanism to > > detect the undesirable placement of `bti c`, if there are > > -mbranch-protection=bti users. So I don't think that inconsistency in clang > > 10.0.0 with GCC will be a problem. > > Speaking as the person implementing the Linux side of things, I think that > will be a problem. Kernel-side we want consistency across compilers. > > For Linux we were planning to do a very simple build-time test to rule out > compilers with the broken behaviour. We would expect functional compilers to > have a consistent layout for a given combination of options, and we would > consider this layout to be ABI. > > The compile time check would compile a trivial test file, e.g. > > void function(void) { } > > > ... with flags: > > -fpatchable-function-entry=2 -mbranch-protection=bti > > > ... and if the function entry point is a NOP, mark that compiler as broken. > In practice, this will mean that the kernel build system will disable BTI > support for those broken compilers. > > Trying to support different layout approaches within Linux will add a fair > amount of unnecessary complexity which we cannot contain in one place, and > makes it more likely that support for either case will be broken in future. > > For the M=0 case, I would prefer to avoid runtime detection unless really > necessary. > > For the M!=0 case, which I believe Linux will need to use in the near future, > I realise that a runtime check will be necessary to detect the absence of a > BTI. Regardless, consistent behaviour across compilers will make this > significantly simpler and less error-prone. > > Thanks, > Mark. When -mbranch-protection=bti is used, due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92424 (existing GCC releases have the issue), Linux/arch/arm64 has to make more checks to detect the combination. I changed `nop; nop; bti c` to `bti c; nop; nop` with 2 commits not included in LLVM release/10.x (a72d15e37c5e066f597f13a8ba60aff214ac992d <https://reviews.llvm.org/rGa72d15e37c5e066f597f13a8ba60aff214ac992d> and 9a24488cb67a90f889529987275c5e411ce01dda <https://reviews.llvm.org/rG9a24488cb67a90f889529987275c5e411ce01dda>). They make Clang's M!=0 placement (.Lpatch; nop; func: bti c; nop) consistent with GCC. If @hans allows me to cherry pick them to release/10.x, I'll be happy to do that. For the M=0 case, Clang does (`func: .Lpatch; bti c; nop; nop`), which is inconsistent with GCC (`func: bti c; .Lpatch: nop; nop`). I'll be happy to try updating the placement of .Lpatch if future M>0 usage does not obsolete M=0 usage (a proof that the proposed GCC layout is indeed superior, not just for simplicity for a not-so-common configuration), otherwise I would feel the clang side is just making changes to appease GCC/Linux compatibility, which would make me sad. I shall also mention that we are essentially making decisions for x86 people's endbr32/endbr64. I hope you can engage in x86 maintainers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72222/new/ https://reviews.llvm.org/D72222 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
