* [PATCH] ARM: Thumb2: avoid __builtin_thread_pointer() on Clang
@ 2021-10-28 8:35 Ard Biesheuvel
2021-10-28 16:25 ` Nick Desaulniers
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2021-10-28 8:35 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux, arnd, ndesaulniers, nathan, keescook, Ard Biesheuvel
If available, we use the __builtin_thread_pointer() helper to get the
value of the TLS register, to help the compiler understand that it
doesn't need to reload it every time we access 'current'.
Unfortunately, Clang fails to emit the MRC system register read
directly when building for Thumb2, and instead, it issues a call to the
__aeabi_read_tp helper, which the kernel does not provide, and so this
result in link failures at build time.
So create a special case for this, and emit the MRC directly using an
asm() block, just like we do when the helper is not available to begin
with.
Link: https://github.com/ClangBuiltLinux/linux/issues/1485
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/include/asm/current.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/current.h b/arch/arm/include/asm/current.h
index 1d472fa7697b..6bf0aad672c3 100644
--- a/arch/arm/include/asm/current.h
+++ b/arch/arm/include/asm/current.h
@@ -26,12 +26,17 @@ static inline struct task_struct *get_current(void)
{
struct task_struct *cur;
-#if __has_builtin(__builtin_thread_pointer)
+#if __has_builtin(__builtin_thread_pointer) && \
+ !(defined(CONFIG_THUMB2_KERNEL) && \
+ defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 130001)
/*
* Use the __builtin helper when available - this results in better
* code, especially when using GCC in combination with the per-task
* stack protector, as the compiler will recognize that it needs to
* load the TLS register only once in every function.
+ *
+ * Clang < 13.0.1 gets this wrong for Thumb2 builds:
+ * https://github.com/ClangBuiltLinux/linux/issues/1485
*/
cur = __builtin_thread_pointer();
#else
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ARM: Thumb2: avoid __builtin_thread_pointer() on Clang
2021-10-28 8:35 [PATCH] ARM: Thumb2: avoid __builtin_thread_pointer() on Clang Ard Biesheuvel
@ 2021-10-28 16:25 ` Nick Desaulniers
2021-10-28 16:32 ` Nathan Chancellor
2021-10-28 16:52 ` Kees Cook
2 siblings, 0 replies; 4+ messages in thread
From: Nick Desaulniers @ 2021-10-28 16:25 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-arm-kernel, linux, arnd, nathan, keescook
On Thu, Oct 28, 2021 at 1:36 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> If available, we use the __builtin_thread_pointer() helper to get the
> value of the TLS register, to help the compiler understand that it
> doesn't need to reload it every time we access 'current'.
>
> Unfortunately, Clang fails to emit the MRC system register read
> directly when building for Thumb2, and instead, it issues a call to the
> __aeabi_read_tp helper, which the kernel does not provide, and so this
> result in link failures at build time.
>
> So create a special case for this, and emit the MRC directly using an
> asm() block, just like we do when the helper is not available to begin
> with.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1485
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Thanks for the patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Note that the fix will ship in clang-14; the backport Nathan has
requested in https://bugs.llvm.org/show_bug.cgi?id=52329 technically
hasn't landed yet in 13.0.1. I don't foresee any issues there getting
the backport accepted into 13.0.1 quickly, but wanted to mention since
it should be ok to apply this patch to get clang-10, -11, -12, builds
green again while there might be another couple days of -13 breakage
until the backport actually lands.
> ---
> arch/arm/include/asm/current.h | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/current.h b/arch/arm/include/asm/current.h
> index 1d472fa7697b..6bf0aad672c3 100644
> --- a/arch/arm/include/asm/current.h
> +++ b/arch/arm/include/asm/current.h
> @@ -26,12 +26,17 @@ static inline struct task_struct *get_current(void)
> {
> struct task_struct *cur;
>
> -#if __has_builtin(__builtin_thread_pointer)
> +#if __has_builtin(__builtin_thread_pointer) && \
> + !(defined(CONFIG_THUMB2_KERNEL) && \
> + defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 130001)
> /*
> * Use the __builtin helper when available - this results in better
> * code, especially when using GCC in combination with the per-task
> * stack protector, as the compiler will recognize that it needs to
> * load the TLS register only once in every function.
> + *
> + * Clang < 13.0.1 gets this wrong for Thumb2 builds:
> + * https://github.com/ClangBuiltLinux/linux/issues/1485
> */
> cur = __builtin_thread_pointer();
> #else
> --
> 2.30.2
>
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ARM: Thumb2: avoid __builtin_thread_pointer() on Clang
2021-10-28 8:35 [PATCH] ARM: Thumb2: avoid __builtin_thread_pointer() on Clang Ard Biesheuvel
2021-10-28 16:25 ` Nick Desaulniers
@ 2021-10-28 16:32 ` Nathan Chancellor
2021-10-28 16:52 ` Kees Cook
2 siblings, 0 replies; 4+ messages in thread
From: Nathan Chancellor @ 2021-10-28 16:32 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-arm-kernel, linux, arnd, ndesaulniers, keescook
On Thu, Oct 28, 2021 at 10:35:27AM +0200, Ard Biesheuvel wrote:
> If available, we use the __builtin_thread_pointer() helper to get the
> value of the TLS register, to help the compiler understand that it
> doesn't need to reload it every time we access 'current'.
>
> Unfortunately, Clang fails to emit the MRC system register read
> directly when building for Thumb2, and instead, it issues a call to the
> __aeabi_read_tp helper, which the kernel does not provide, and so this
> result in link failures at build time.
>
> So create a special case for this, and emit the MRC directly using an
> asm() block, just like we do when the helper is not available to begin
> with.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1485
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>
> ---
> arch/arm/include/asm/current.h | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/current.h b/arch/arm/include/asm/current.h
> index 1d472fa7697b..6bf0aad672c3 100644
> --- a/arch/arm/include/asm/current.h
> +++ b/arch/arm/include/asm/current.h
> @@ -26,12 +26,17 @@ static inline struct task_struct *get_current(void)
> {
> struct task_struct *cur;
>
> -#if __has_builtin(__builtin_thread_pointer)
> +#if __has_builtin(__builtin_thread_pointer) && \
> + !(defined(CONFIG_THUMB2_KERNEL) && \
> + defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 130001)
> /*
> * Use the __builtin helper when available - this results in better
> * code, especially when using GCC in combination with the per-task
> * stack protector, as the compiler will recognize that it needs to
> * load the TLS register only once in every function.
> + *
> + * Clang < 13.0.1 gets this wrong for Thumb2 builds:
> + * https://github.com/ClangBuiltLinux/linux/issues/1485
> */
> cur = __builtin_thread_pointer();
> #else
> --
> 2.30.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ARM: Thumb2: avoid __builtin_thread_pointer() on Clang
2021-10-28 8:35 [PATCH] ARM: Thumb2: avoid __builtin_thread_pointer() on Clang Ard Biesheuvel
2021-10-28 16:25 ` Nick Desaulniers
2021-10-28 16:32 ` Nathan Chancellor
@ 2021-10-28 16:52 ` Kees Cook
2 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2021-10-28 16:52 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-arm-kernel, linux, arnd, ndesaulniers, nathan
On Thu, Oct 28, 2021 at 10:35:27AM +0200, Ard Biesheuvel wrote:
> If available, we use the __builtin_thread_pointer() helper to get the
> value of the TLS register, to help the compiler understand that it
> doesn't need to reload it every time we access 'current'.
>
> Unfortunately, Clang fails to emit the MRC system register read
> directly when building for Thumb2, and instead, it issues a call to the
> __aeabi_read_tp helper, which the kernel does not provide, and so this
> result in link failures at build time.
>
> So create a special case for this, and emit the MRC directly using an
> asm() block, just like we do when the helper is not available to begin
> with.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1485
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-28 16:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-28 8:35 [PATCH] ARM: Thumb2: avoid __builtin_thread_pointer() on Clang Ard Biesheuvel
2021-10-28 16:25 ` Nick Desaulniers
2021-10-28 16:32 ` Nathan Chancellor
2021-10-28 16:52 ` Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.