diff mbox series

ARM: Change arch/arm/lib/mem*.S to use WEAK instead of .weak

Message ID 20201105181542.854788-1-maskray@google.com (mailing list archive)
State New, archived
Headers show
Series ARM: Change arch/arm/lib/mem*.S to use WEAK instead of .weak | expand

Commit Message

Fangrui Song Nov. 5, 2020, 6:15 p.m. UTC
Commit d6d51a96c7d6 ("ARM: 9014/2: Replace string mem* functions for
KASan") add .weak directives to memcpy/memmove/memset to avoid collision
with KASAN interceptors.

This does not work with LLVM's integrated assembler (the assembly snippet
`.weak memcpy ... .globl memcpy` produces a STB_GLOBAL memcpy while GNU as
produces a STB_WEAK memcpy). LLVM 12 (since https://reviews.llvm.org/D90108)
will error on such an overridden symbol binding.

Use the appropriate WEAK macro instead.

Fixes: d6d51a96c7d6 ("ARM: 9014/2: Replace string mem* functions for KASan")
Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Fangrui Song <maskray@google.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1190
---
 arch/arm/lib/memcpy.S  | 3 +--
 arch/arm/lib/memmove.S | 3 +--
 arch/arm/lib/memset.S  | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

Comments

Nick Desaulniers Nov. 5, 2020, 9:55 p.m. UTC | #1
On Thu, Nov 5, 2020 at 10:15 AM Fangrui Song <maskray@google.com> wrote:
>
> Commit d6d51a96c7d6 ("ARM: 9014/2: Replace string mem* functions for
> KASan") add .weak directives to memcpy/memmove/memset to avoid collision
> with KASAN interceptors.
>
> This does not work with LLVM's integrated assembler (the assembly snippet
> `.weak memcpy ... .globl memcpy` produces a STB_GLOBAL memcpy while GNU as
> produces a STB_WEAK memcpy). LLVM 12 (since https://reviews.llvm.org/D90108)
> will error on such an overridden symbol binding.
>
> Use the appropriate WEAK macro instead.
>
> Fixes: d6d51a96c7d6 ("ARM: 9014/2: Replace string mem* functions for KASan")
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1190

Thank you, this fixes a recent regression for me using LLVM_IAS=1 with ToT LLVM.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  arch/arm/lib/memcpy.S  | 3 +--
>  arch/arm/lib/memmove.S | 3 +--
>  arch/arm/lib/memset.S  | 3 +--
>  3 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/lib/memcpy.S b/arch/arm/lib/memcpy.S
> index ad4625d16e11..e4caf48c089f 100644
> --- a/arch/arm/lib/memcpy.S
> +++ b/arch/arm/lib/memcpy.S
> @@ -58,10 +58,9 @@
>
>  /* Prototype: void *memcpy(void *dest, const void *src, size_t n); */
>
> -.weak memcpy
>  ENTRY(__memcpy)
>  ENTRY(mmiocpy)
> -ENTRY(memcpy)
> +WEAK(memcpy)
>
>  #include "copy_template.S"
>
> diff --git a/arch/arm/lib/memmove.S b/arch/arm/lib/memmove.S
> index fd123ea5a5a4..6fecc12a1f51 100644
> --- a/arch/arm/lib/memmove.S
> +++ b/arch/arm/lib/memmove.S
> @@ -24,9 +24,8 @@
>   * occurring in the opposite direction.
>   */
>
> -.weak memmove
>  ENTRY(__memmove)
> -ENTRY(memmove)
> +WEAK(memmove)
>         UNWIND( .fnstart                        )
>
>                 subs    ip, r0, r1
> diff --git a/arch/arm/lib/memset.S b/arch/arm/lib/memset.S
> index 0e7ff0423f50..9817cb258c1a 100644
> --- a/arch/arm/lib/memset.S
> +++ b/arch/arm/lib/memset.S
> @@ -13,10 +13,9 @@
>         .text
>         .align  5
>
> -.weak memset
>  ENTRY(__memset)
>  ENTRY(mmioset)
> -ENTRY(memset)
> +WEAK(memset)
>  UNWIND( .fnstart         )
>         ands    r3, r0, #3              @ 1 unaligned?
>         mov     ip, r0                  @ preserve r0 as return value
> --
> 2.29.1.341.ge80a0c044ae-goog
>
Linus Walleij Nov. 10, 2020, 2:05 p.m. UTC | #2
On Thu, Nov 5, 2020 at 7:15 PM Fangrui Song <maskray@google.com> wrote:

> Commit d6d51a96c7d6 ("ARM: 9014/2: Replace string mem* functions for
> KASan") add .weak directives to memcpy/memmove/memset to avoid collision
> with KASAN interceptors.
>
> This does not work with LLVM's integrated assembler (the assembly snippet
> `.weak memcpy ... .globl memcpy` produces a STB_GLOBAL memcpy while GNU as
> produces a STB_WEAK memcpy). LLVM 12 (since https://reviews.llvm.org/D90108)
> will error on such an overridden symbol binding.
>
> Use the appropriate WEAK macro instead.
>
> Fixes: d6d51a96c7d6 ("ARM: 9014/2: Replace string mem* functions for KASan")
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1190

Acked-by: Linus Walleij <linus.walleij@linaro.org>

When you have consensus that this is working, please put the patch
into Russell's patch tracker.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/arch/arm/lib/memcpy.S b/arch/arm/lib/memcpy.S
index ad4625d16e11..e4caf48c089f 100644
--- a/arch/arm/lib/memcpy.S
+++ b/arch/arm/lib/memcpy.S
@@ -58,10 +58,9 @@ 
 
 /* Prototype: void *memcpy(void *dest, const void *src, size_t n); */
 
-.weak memcpy
 ENTRY(__memcpy)
 ENTRY(mmiocpy)
-ENTRY(memcpy)
+WEAK(memcpy)
 
 #include "copy_template.S"
 
diff --git a/arch/arm/lib/memmove.S b/arch/arm/lib/memmove.S
index fd123ea5a5a4..6fecc12a1f51 100644
--- a/arch/arm/lib/memmove.S
+++ b/arch/arm/lib/memmove.S
@@ -24,9 +24,8 @@ 
  * occurring in the opposite direction.
  */
 
-.weak memmove
 ENTRY(__memmove)
-ENTRY(memmove)
+WEAK(memmove)
 	UNWIND(	.fnstart			)
 
 		subs	ip, r0, r1
diff --git a/arch/arm/lib/memset.S b/arch/arm/lib/memset.S
index 0e7ff0423f50..9817cb258c1a 100644
--- a/arch/arm/lib/memset.S
+++ b/arch/arm/lib/memset.S
@@ -13,10 +13,9 @@ 
 	.text
 	.align	5
 
-.weak memset
 ENTRY(__memset)
 ENTRY(mmioset)
-ENTRY(memset)
+WEAK(memset)
 UNWIND( .fnstart         )
 	ands	r3, r0, #3		@ 1 unaligned?
 	mov	ip, r0			@ preserve r0 as return value