diff mbox series

arm64: Restrict ARM64_BTI_KERNEL to clang 12.0.0 and newer

Message ID 20210709000627.3183718-1-nathan@kernel.org (mailing list archive)
State New, archived
Headers show
Series arm64: Restrict ARM64_BTI_KERNEL to clang 12.0.0 and newer | expand

Commit Message

Nathan Chancellor July 9, 2021, 12:06 a.m. UTC
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.

Link: https://github.com/ClangBuiltLinux/linux/issues/1428
Link: https://github.com/ClangBuiltLinux/continuous-integration2/runs/3010034706?check_suite_focus=true
Link: https://github.com/ClangBuiltLinux/continuous-integration2/runs/3010035725?check_suite_focus=true
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 arch/arm64/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


base-commit: f55966571d5eb2876a11e48e798b4592fa1ffbb7

Comments

Will Deacon July 12, 2021, 12:24 p.m. UTC | #1
Hi Nathan,

On Thu, Jul 08, 2021 at 05:06:27PM -0700, Nathan Chancellor wrote:
> 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"

> 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
Nick Desaulniers July 12, 2021, 5:04 p.m. UTC | #2
On Mon, Jul 12, 2021 at 5:24 AM Will Deacon <will@kernel.org> wrote:
>
> Hi Nathan,
>
> On Thu, Jul 08, 2021 at 05:06:27PM -0700, Nathan Chancellor wrote:
> > 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"
>
> > 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?

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

>
> 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
diff mbox series

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index e07e7de9ac49..c2127595980f 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -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