| Message ID | 20200619123550.48098-1-broonie@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 4dc9b282bf5fc80b1761bac467adf78cd417b777 |
| Headers | show |
| Series | [v2] arm64: Depend on newer binutils when building PAC | expand |
On Fri, Jun 19, 2020 at 5:35 AM Mark Brown <broonie@kernel.org> wrote: > > Versions of binutils prior to 2.33.1 don't understand the ELF notes that > are added by modern compilers to indicate the PAC and BTI options used > to build the code. This causes them to emit large numbers of warnings in > the form: > > aarch64-linux-gnu-nm: warning: .tmp_vmlinux.kallsyms2: unsupported GNU_PROPERTY_TYPE (5) type: 0xc0000000 > > during the kernel build which is currently causing quite a bit of > disruption for automated build testing using clang. > > In commit 15cd0e675f3f76b (arm64: Kconfig: ptrauth: Add binutils version > check to fix mismatch) we added a dependency on binutils to avoid this > issue when building with versions of GCC that emit the notes but did not > do so for clang as it was believed that the existing check for > .cfi_negate_ra_state was already requiring a new enough binutils. This > does not appear to be the case for some versions of binutils (eg, the > binutils in Debian 10) so instead refactor so we require a new enough ^ is Debian 10 what KernelCI is running, currently? > GNU binutils in all cases other than when we are using an old GCC > version that does not emit notes. > > Other, more exotic, combinations of tools are possible such as using > clang, lld and gas together are possible and may have further problems > but rather than adding further version checks it looks like the most > robust thing will be to just test that we can build cleanly with the > configured tools but that will require more review and discussion so do > this for now to address the immediate problem disrupting build testing. I think that's a fair compromise, in the interest of immediate relief to KernelCI testing. If we need to be more precise, we can look into feature testing all of the build tools (version checks would have to check versions for BOTH sets of GNU and LLVM tools, so feature checks might actually be simpler in that regard). I think we can cross that bridge another day, reprioritizing if we get a report of PAC not working for some particular mismatch of tools. Given unlimited CI resources, I would absolutely test random combinations of GNU and LLVM tools, but for now we're mostly testing one or the other. Thanks for the patch and the revision. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1054 > Reported-by: KernelCI <bot@kernelci.org> > Reported-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Mark Brown <broonie@kernel.org> > --- > v2: Update comment and change to check binutils version except when > we specifically know that the compiler doesn't emit notes. > > arch/arm64/Kconfig | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a4a094bedcb2..66dc41fd49f2 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1518,9 +1518,9 @@ config ARM64_PTR_AUTH > default y > depends on !KVM || ARM64_VHE > depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC > - # GCC 9.1 and later inserts a .note.gnu.property section note for PAC > + # Modern compilers insert a .note.gnu.property section note for PAC It can still be helpful to note compiler version numbers (GCC 9.1, clang-10). Someday those will be ancient history, and the kernel will move beyond support for those toolchain versions. At that point, having a comment makes it easy to `grep` for `gcc 9` and find all the places in the code that can be cleaned up or simplified. > # which is only understood by binutils starting with version 2.33.1. > - depends on !CC_IS_GCC || GCC_VERSION < 90100 || LD_VERSION >= 233010000 > + depends on LD_IS_LLD || LD_VERSION >= 233010000 || (CC_IS_GCC && GCC_VERSION < 90100) > depends on !CC_IS_CLANG || AS_HAS_CFI_NEGATE_RA_STATE > depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS) > help > -- > 2.20.1 >
On Fri, Jun 19, 2020 at 09:55:04AM -0700, Nick Desaulniers wrote: > On Fri, Jun 19, 2020 at 5:35 AM Mark Brown <broonie@kernel.org> wrote: > > binutils in Debian 10) so instead refactor so we require a new enough > ^ is Debian 10 what KernelCI is running, currently? The Docker images it uses for builds are Debian based so should be yeah (it's the current release) but I actually pulled that version number from my desktop where I reproduced the problem. > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Thanks. > > - # GCC 9.1 and later inserts a .note.gnu.property section note for PAC > > + # Modern compilers insert a .note.gnu.property section note for PAC > It can still be helpful to note compiler version numbers (GCC 9.1, > clang-10). Someday those will be ancient history, and the kernel will > move beyond support for those toolchain versions. At that point, > having a comment makes it easy to `grep` for `gcc 9` and find all the > places in the code that can be cleaned up or simplified. I figured that in this case it's more the binutils version that's the issue than the compiler.
On Fri, 19 Jun 2020 13:35:50 +0100, Mark Brown wrote: > Versions of binutils prior to 2.33.1 don't understand the ELF notes that > are added by modern compilers to indicate the PAC and BTI options used > to build the code. This causes them to emit large numbers of warnings in > the form: > > aarch64-linux-gnu-nm: warning: .tmp_vmlinux.kallsyms2: unsupported GNU_PROPERTY_TYPE (5) type: 0xc0000000 > > [...] Applied to arm64 (for-next/fixes), thanks! [1/1] arm64: Depend on newer binutils when building PAC https://git.kernel.org/arm64/c/4dc9b282bf5f Cheers,
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index a4a094bedcb2..66dc41fd49f2 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1518,9 +1518,9 @@ config ARM64_PTR_AUTH default y depends on !KVM || ARM64_VHE depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC - # GCC 9.1 and later inserts a .note.gnu.property section note for PAC + # Modern compilers insert a .note.gnu.property section note for PAC # which is only understood by binutils starting with version 2.33.1. - depends on !CC_IS_GCC || GCC_VERSION < 90100 || LD_VERSION >= 233010000 + depends on LD_IS_LLD || LD_VERSION >= 233010000 || (CC_IS_GCC && GCC_VERSION < 90100) depends on !CC_IS_CLANG || AS_HAS_CFI_NEGATE_RA_STATE depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS) help
Versions of binutils prior to 2.33.1 don't understand the ELF notes that are added by modern compilers to indicate the PAC and BTI options used to build the code. This causes them to emit large numbers of warnings in the form: aarch64-linux-gnu-nm: warning: .tmp_vmlinux.kallsyms2: unsupported GNU_PROPERTY_TYPE (5) type: 0xc0000000 during the kernel build which is currently causing quite a bit of disruption for automated build testing using clang. In commit 15cd0e675f3f76b (arm64: Kconfig: ptrauth: Add binutils version check to fix mismatch) we added a dependency on binutils to avoid this issue when building with versions of GCC that emit the notes but did not do so for clang as it was believed that the existing check for .cfi_negate_ra_state was already requiring a new enough binutils. This does not appear to be the case for some versions of binutils (eg, the binutils in Debian 10) so instead refactor so we require a new enough GNU binutils in all cases other than when we are using an old GCC version that does not emit notes. Other, more exotic, combinations of tools are possible such as using clang, lld and gas together are possible and may have further problems but rather than adding further version checks it looks like the most robust thing will be to just test that we can build cleanly with the configured tools but that will require more review and discussion so do this for now to address the immediate problem disrupting build testing. Link: https://github.com/ClangBuiltLinux/linux/issues/1054 Reported-by: KernelCI <bot@kernelci.org> Reported-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- v2: Update comment and change to check binutils version except when we specifically know that the compiler doesn't emit notes. arch/arm64/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)