Skip to contentSkip to conversation

arm64: Restrict ARM64_BTI_KERNEL to clang 12.0.0 and newer

PATCH Superseded Nathan Chancellor 3 participants 6 messages Jul 9, 2021 – Jul 13, 2021 arm64

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 arm64: Restrict ARM64_BTI_KERNEL to clang 12.0.0 and newer

Commit 97fed779f2a6 ("arm64: bti: Provide Kconfig for kernel mode BTI") disabled CONFIG_ARM64_BTI_KERNEL when CONFIG_GCOV_KERNEL was enabled and compilign with clang because of warnings that occur because LLVM was not emitting PAC/BTI instructions for compiler generated functions.

This was an adequate fix to avoid the warnings with allmodconfig until commit 51c2ee6d121c ("Kconfig: Introduce ARCH_WANTS_NO_INSTR and CC_HAS_NO_PROFILE_FN_ATTR"), which prevented CONFIG_GCOV_KERNEL from being selected with clang 12.0.0 and older because it does not support the no_profile_instrument_function attribute. As a result, CONFIG_ARM64_BTI_KERNEL gets enabled and there are more warnings of this nature.

Rather than play whack-a-mole with the config options, just update this condition to require clang 12.0.0, which should have all of the issues ironed out.

---
 arch/arm64/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/Kconfig +2 -1
index e07e7de9ac49..c2127595980f 100644
@@ -1605,7 +1605,8 @@ config ARM64_BTI_KERNEL
 	depends on CC_HAS_BRANCH_PROT_PAC_RET_BTI
 	# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697
 	depends on !CC_IS_GCC || GCC_VERSION >= 100100
-	depends on !(CC_IS_CLANG && GCOV_KERNEL)
+	# https://reviews.llvm.org/D85649
+	depends on !CC_IS_CLANG || CLANG_VERSION >= 120000
 	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
 	help
 	  Build the kernel with Branch Target Identification annotations

base-commit: f55966571d5eb2876a11e48e798b4592fa1ffbb7

Signature · 1 line
2.32.0.93.g670b81a890
maintainer
#1 lore raw

Hi Nathan,

Quoted 3 lines · Nathan Chancellor

Commit 97fed779f2a6 ("arm64: bti: Provide Kconfig for kernel mode BTI") disabled CONFIG_ARM64_BTI_KERNEL when CONFIG_GCOV_KERNEL was enabled and compilign with clang because of warnings that occur because LLVM was not

Typo: "compilign"

Quoted 9 lines

emitting PAC/BTI instructions for compiler generated functions.

This was an adequate fix to avoid the warnings with allmodconfig until commit 51c2ee6d121c ("Kconfig: Introduce ARCH_WANTS_NO_INSTR and CC_HAS_NO_PROFILE_FN_ATTR"), which prevented CONFIG_GCOV_KERNEL from being selected with clang 12.0.0 and older because it does not support the no_profile_instrument_function attribute. As a result, CONFIG_ARM64_BTI_KERNEL gets enabled and there are more warnings of this nature.

I'm half asleep today, but I'm struggling to follow what GCOV_KERNEL has to do with the warnings here. Prior to 51c2ee6d121c, you could still go and turn that option off and so wouldn't the same warnings be triggered in that configuration?

In other words, I think there are two aspects to this patch:

1. Removing the (now useless) !GCOV_KERNEL dependency
2. Requiring Clang >= version 12

but the commit message doesn't really say where these warnings are coming from or why Clang 12 gets rid of them.

Thanks,

Will

#2 lore raw
Quoted 20 lines · Will Deacon

Hi Nathan,

Quoted 3 lines · Nathan Chancellor

Commit 97fed779f2a6 ("arm64: bti: Provide Kconfig for kernel mode BTI") disabled CONFIG_ARM64_BTI_KERNEL when CONFIG_GCOV_KERNEL was enabled and compilign with clang because of warnings that occur because LLVM was not

Typo: "compilign"

Quoted 9 lines

emitting PAC/BTI instructions for compiler generated functions.

This was an adequate fix to avoid the warnings with allmodconfig until commit 51c2ee6d121c ("Kconfig: Introduce ARCH_WANTS_NO_INSTR and CC_HAS_NO_PROFILE_FN_ATTR"), which prevented CONFIG_GCOV_KERNEL from being selected with clang 12.0.0 and older because it does not support the no_profile_instrument_function attribute. As a result, CONFIG_ARM64_BTI_KERNEL gets enabled and there are more warnings of this nature.

Quoted 4 lines

I'm half asleep today, but I'm struggling to follow what GCOV_KERNEL has to do with the warnings here. Prior to 51c2ee6d121c, you could still go and turn that option off and so wouldn't the same warnings be triggered in that configuration?

Nathan, consider if any of the below can be reused to reword the commit message?

```
Prior to 51c2ee6d121c, allmodconfig with clang-11 would not select
ARM64_BTI_KERNEL because GCOV_KERNEL already had been.

After 51c2ee6d121c, allmodconfig with clang-11 does select
ARM64_BTI_KERNEL because GCOV_KERNEL cannot be selected.

clang-11 had basic support for BTI, but failed to instrument compiler
generated functions properly.  This produces the following stream of
warnings for clang-11 allmodconfig builds of ARCH=arm64:

warning: some functions compiled with BTI and some compiled without BTI
warning: not setting BTI in feature flags

This was fixed in clang-12 via https://reviews.llvm.org/D85649.
```

Perhaps its of interest to find which config in (allmodconfig minus GCOV_KERNEL) but not in (defconfig) produces these, since I don't think we see them in defconfig builds of clang-11?
https://github.com/ClangBuiltLinux/continuous-integration2/runs/3009146591?check_suite_focus=true

Quoted 12 lines

In other words, I think there are two aspects to this patch:

1. Removing the (now useless) !GCOV_KERNEL dependency
2. Requiring Clang >= version 12

but the commit message doesn't really say where these warnings are coming from or why Clang 12 gets rid of them.

Thanks,

Will

Signature · 2 lines
Thanks,
~Nick Desaulniers
#3 lore raw

PATCH v2 1/1 arm64: Restrict ARM64_BTI_KERNEL to clang 12.0.0 and newer

In reply to Nathan Chancellor

Commit 97fed779f2a6 ("arm64: bti: Provide Kconfig for kernel mode BTI") disabled CONFIG_ARM64_BTI_KERNEL when CONFIG_GCOV_KERNEL was enabled and compiling with clang because of warnings that were seen with allmodconfig because LLVM was not emitting PAC/BTI instructions for compiler generated functions:

warning: some functions compiled with BTI and some compiled without BTI
warning: not setting BTI in feature flags

This depedency was fine for avoiding the warnings with allmodconfig until commit 51c2ee6d121c ("Kconfig: Introduce ARCH_WANTS_NO_INSTR and CC_HAS_NO_PROFILE_FN_ATTR"), which prevents CONFIG_GCOV_KERNEL from being enabled with clang 12.0.0 or older because those versions do not support the no_profile_instrument_function attribute.

As a result, CONFIG_ARM64_BTI_KERNEL gets enabled with allmodconfig and there are more warnings like the ones above due to CONFIG_KASAN, which suffers from the same problem as CONFIG_GCOV_KERNEL. This was most likely not noticed at the time because allmodconfig + CONFIG_GCOV_KERNEL=n was not tested. defconfig + CONFIG_KASAN=y is enough to reproduce the same warnings as above.

The root cause of the warnings was resolved in LLVM during the 12.0.0 release so rather than play whack-a-mole with the dependencies, just update CONFIG_ARM64_BTI_KERNEL to require clang 12.0.0, which will have all of the issues ironed out.

---

v1 -> v2:

* Fix typo ("compilign" to "compiling") per Will.

* Clarify that these warnings are reproducible back at the time of 97fed779f2a6 with allmodconfig + CONFIG_GCOV_KERNEL=n but that configuration was most likely not tested.

* Clarify that the warnings come from CONFIG_KASAN, which suffers from the same issue at CONFIG_GCOV_KERNEL because there are compiler generated functions.

* Clarify that the root cause of the warnings is resolved in 12.0.0 and link to the GitHub commit rather than the Phabricator instance, in case LLVM moves to GitHub reviews in the future.

Hopefully this is adequate explanation for what is going on, thanks to Will and Nick for providing input on the commit message.

 arch/arm64/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/Kconfig +2 -1
index e07e7de9ac49..b5b13a932561 100644
@@ -1605,7 +1605,8 @@ config ARM64_BTI_KERNEL
 	depends on CC_HAS_BRANCH_PROT_PAC_RET_BTI
 	# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697
 	depends on !CC_IS_GCC || GCC_VERSION >= 100100
-	depends on !(CC_IS_CLANG && GCOV_KERNEL)
+	# https://github.com/llvm/llvm-project/commit/a88c722e687e6780dcd6a58718350dc76fcc4cc9
+	depends on !CC_IS_CLANG || CLANG_VERSION >= 120000
 	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
 	help
 	  Build the kernel with Branch Target Identification annotations

base-commit: e73f0f0ee7541171d89f2e2491130c7771ba58d3

Signature · 1 line
2.32.0.93.g670b81a890
Reviewed-by on v2 1/1
#4 lore raw
Quoted 47 lines · Nathan Chancellor

Commit 97fed779f2a6 ("arm64: bti: Provide Kconfig for kernel mode BTI") disabled CONFIG_ARM64_BTI_KERNEL when CONFIG_GCOV_KERNEL was enabled and compiling with clang because of warnings that were seen with allmodconfig because LLVM was not emitting PAC/BTI instructions for compiler generated functions:

warning: some functions compiled with BTI and some compiled without BTI
warning: not setting BTI in feature flags

This depedency was fine for avoiding the warnings with allmodconfig until commit 51c2ee6d121c ("Kconfig: Introduce ARCH_WANTS_NO_INSTR and CC_HAS_NO_PROFILE_FN_ATTR"), which prevents CONFIG_GCOV_KERNEL from being enabled with clang 12.0.0 or older because those versions do not support the no_profile_instrument_function attribute.

As a result, CONFIG_ARM64_BTI_KERNEL gets enabled with allmodconfig and there are more warnings like the ones above due to CONFIG_KASAN, which suffers from the same problem as CONFIG_GCOV_KERNEL. This was most likely not noticed at the time because allmodconfig + CONFIG_GCOV_KERNEL=n was not tested. defconfig + CONFIG_KASAN=y is enough to reproduce the same warnings as above.

The root cause of the warnings was resolved in LLVM during the 12.0.0 release so rather than play whack-a-mole with the dependencies, just update CONFIG_ARM64_BTI_KERNEL to require clang 12.0.0, which will have all of the issues ironed out.

---

v1 -> v2:

* Fix typo ("compilign" to "compiling") per Will.

* Clarify that these warnings are reproducible back at the time of 97fed779f2a6 with allmodconfig + CONFIG_GCOV_KERNEL=n but that configuration was most likely not tested.

* Clarify that the warnings come from CONFIG_KASAN, which suffers from the same issue at CONFIG_GCOV_KERNEL because there are compiler generated functions.

Quoted 6 lines

* Clarify that the root cause of the warnings is resolved in 12.0.0 and link to the GitHub commit rather than the Phabricator instance, in case LLVM moves to GitHub reviews in the future.

Hopefully this is adequate explanation for what is going on, thanks to Will and Nick for providing input on the commit message.

Perfect, sorry for being a pedant on the commit message, but this provides much more context should we need to revisit this again in the future. Never say never, (again)! Thanks very much for this patch.

Quoted 23 lines
 arch/arm64/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/Kconfig +2 -1
index e07e7de9ac49..b5b13a932561 100644
@@ -1605,7 +1605,8 @@ config ARM64_BTI_KERNEL
        depends on CC_HAS_BRANCH_PROT_PAC_RET_BTI
        # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94697
        depends on !CC_IS_GCC || GCC_VERSION >= 100100
-       depends on !(CC_IS_CLANG && GCOV_KERNEL)
+       # https://github.com/llvm/llvm-project/commit/a88c722e687e6780dcd6a58718350dc76fcc4cc9
+       depends on !CC_IS_CLANG || CLANG_VERSION >= 120000
        depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
        help
          Build the kernel with Branch Target Identification annotations

base-commit: e73f0f0ee7541171d89f2e2491130c7771ba58d3 --
2.32.0.93.g670b81a890

Signature · 2 lines
Thanks,
~Nick Desaulniers
maintainer Applied
#5 lore raw
In reply to Nathan Chancellor
Quoted 10 lines · Nathan Chancellor

Commit 97fed779f2a6 ("arm64: bti: Provide Kconfig for kernel mode BTI") disabled CONFIG_ARM64_BTI_KERNEL when CONFIG_GCOV_KERNEL was enabled and compiling with clang because of warnings that were seen with allmodconfig because LLVM was not emitting PAC/BTI instructions for compiler generated functions:

warning: some functions compiled with BTI and some compiled without BTI
warning: not setting BTI in feature flags

[...]

Thanks Nathan for turning this around so quickly. The new commit message is much better.

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm64: Restrict ARM64_BTI_KERNEL to clang 12.0.0 and newer

      https://git.kernel.org/arm64/c/8cdd23c23c3d

Cheers,

Signature · 5 lines
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev