2022-03-30 13:17:11

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] x86/extable: prefer local labels in .set directives

On Tue, Mar 29, 2022 at 01:21:45PM -0700, Nick Desaulniers wrote:
> Bernardo reported an error that Nathan bisected down to
> (x86_64) defconfig+LTO_CLANG_FULL+X86_PMEM_LEGACY.
>
> LTO vmlinux.o
> ld.lld: error: <instantiation>:1:13: redefinition of 'found'
> .set found, 0
> ^
>
> <inline asm>:29:1: while in macro instantiation
> extable_type_reg reg=%eax, type=(17 | ((0) << 16))
> ^
>
> This appears to be another LTO specific issue similar to what was folded
> into commit 4b5305decc84 ("x86/extable: Extend extable functionality"),
> where the `.set found, 0` in DEFINE_EXTABLE_TYPE_REG in
> arch/x86/include/asm/asm.h conflicts with the symbol for the static
> function `found` in arch/x86/kernel/pmem.c.
>
> Assembler .set directive declare symbols with global visibility, so the
> assembler may not rename such symbols in the event of a conflict. LTO
> could rename static functions if there was a conflict in C sources, but
> it cannot see into symbols defined in inline asm.
>
> The symbols are also retained in the symbol table, regardless of LTO.
>
> Give the symbols .L prefixes making them locally visible, so that they
> may be renamed for LTO to avoid conflicts, and to drop them from the
> symbol table regardless of LTO.
>
> Fixes: 4b5305decc84 ("x86/extable: Extend extable functionality")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1612
> Link: https://sourceware.org/binutils/docs/as/Symbol-Names.html#Local-Symbol-Names
> Reported-by: Bernardo Meurer Costa <[email protected]>
> Debugged-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>

This resolves the build error for me and it passes a smoke test boot in
QEMU.

Reviewed-by: Nathan Chancellor <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>

Should this have an explicit "Cc: [email protected]"? The build
error was reported against Linux 5.17.

> ---
> arch/x86/include/asm/asm.h | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
> index c878fed3056f..fbcfec4dc4cc 100644
> --- a/arch/x86/include/asm/asm.h
> +++ b/arch/x86/include/asm/asm.h
> @@ -154,24 +154,24 @@
>
> # define DEFINE_EXTABLE_TYPE_REG \
> ".macro extable_type_reg type:req reg:req\n" \
> - ".set found, 0\n" \
> - ".set regnr, 0\n" \
> + ".set .Lfound, 0\n" \
> + ".set .Lregnr, 0\n" \
> ".irp rs,rax,rcx,rdx,rbx,rsp,rbp,rsi,rdi,r8,r9,r10,r11,r12,r13,r14,r15\n" \
> ".ifc \\reg, %%\\rs\n" \
> - ".set found, found+1\n" \
> - ".long \\type + (regnr << 8)\n" \
> + ".set .Lfound, .Lfound+1\n" \
> + ".long \\type + (.Lregnr << 8)\n" \
> ".endif\n" \
> - ".set regnr, regnr+1\n" \
> + ".set .Lregnr, .Lregnr+1\n" \
> ".endr\n" \
> - ".set regnr, 0\n" \
> + ".set .Lregnr, 0\n" \
> ".irp rs,eax,ecx,edx,ebx,esp,ebp,esi,edi,r8d,r9d,r10d,r11d,r12d,r13d,r14d,r15d\n" \
> ".ifc \\reg, %%\\rs\n" \
> - ".set found, found+1\n" \
> - ".long \\type + (regnr << 8)\n" \
> + ".set .Lfound, .Lfound+1\n" \
> + ".long \\type + (.Lregnr << 8)\n" \
> ".endif\n" \
> - ".set regnr, regnr+1\n" \
> + ".set .Lregnr, .Lregnr+1\n" \
> ".endr\n" \
> - ".if (found != 1)\n" \
> + ".if (.Lfound != 1)\n" \
> ".error \"extable_type_reg: bad register argument\"\n" \
> ".endif\n" \
> ".endm\n"
> --
> 2.35.1.1021.g381101b075-goog
>