Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk

[Fangrui Song]

On 2021-01-11, Nick Desaulniers wrote:
> Arnd found a randconfig that produces the warning:
>
> arch/x86/entry/thunk_64.o: warning: objtool: missing symbol for insn at
> offset 0x3e
>
> when building with LLVM_IAS=1 (use Clang's integrated assembler). Josh
> notes:
>
>  With the LLVM assembler stripping the .text section symbol, objtool
>  has no way to reference this code when it generates ORC unwinder
>  entries, because this code is outside of any ELF function.
>
> Fangrui notes that this optimization is helpful for reducing images size
> when compiling with -ffunction-sections and -fdata-sections. I have
> observerd on the order of tens of thousands of symbols for the kernel
> images built with those flags. A patch has been authored against GNU
> binutils to match this behavior, with a new flag
> --generate-unused-section-symbols=[yes|no].

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d1bcae833b32f1408485ce69f844dcd7ded093a8
has been committed. The patch should be included in binutils 2.37.
The maintainers are welcome to the idea, but fixing all the arch-specific tests is tricky.

H.J. fixed the x86 tests and enabled this for x86. When binutils 2.37
come out, some other architectures may follow as well.

> We can omit the .L prefix on a label to emit an entry into the symbol
> table for the label, with STB_LOCAL binding.  This enables objtool to
> generate proper unwind info here with LLVM_IAS=1.

Josh, I think objtool orc generate needs to synthesize STT_SECTION
symbols even if they do not exist in object files.

rg 'SYM_CODE.*\.L' reveals a few other .S files which may have similar problems.

> Cc: Fangrui Song <maskray@xxxxxxxxxx>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1209
> Link: https://reviews.llvm.org/D93783
> Link: https://sourceware.org/binutils/docs/as/Symbol-Names.html
> Link: https://sourceware.org/pipermail/binutils/2020-December/114671.html
> Reported-by: Arnd Bergmann <arnd@xxxxxxxx>
> Suggested-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Signed-off-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> ---
> Changes v2 -> v3:
> * rework to use STB_LOCAL rather than STB_GLOBAL by dropping .L prefix,
>  as per Josh.
> * rename oneline to drop STB_GLOBAL in commit message.
> * add link to GAS docs on .L prefix.
> * drop Josh's ack since patch changed.
>
> Changes v1 -> v2:
> * Pick up Josh's Ack.
> * Add commit message info about -ffunction-sections/-fdata-sections, and
>  link to binutils patch.
>
>
> arch/x86/entry/thunk_64.S | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S
> index ccd32877a3c4..c9a9fbf1655f 100644
> --- a/arch/x86/entry/thunk_64.S
> +++ b/arch/x86/entry/thunk_64.S
> @@ -31,7 +31,7 @@ SYM_FUNC_START_NOALIGN(\name)
> 	.endif
>
> 	call \func
> -	jmp  .L_restore
> +	jmp  __thunk_restore
> SYM_FUNC_END(\name)
> 	_ASM_NOKPROBE(\name)
> 	.endm
> @@ -44,7 +44,7 @@ SYM_FUNC_END(\name)
> #endif
>
> #ifdef CONFIG_PREEMPTION
> -SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
> +SYM_CODE_START_LOCAL_NOALIGN(__thunk_restore)
> 	popq %r11
> 	popq %r10
> 	popq %r9
> @@ -56,6 +56,6 @@ SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
> 	popq %rdi
> 	popq %rbp
> 	ret
> -	_ASM_NOKPROBE(.L_restore)
> -SYM_CODE_END(.L_restore)
> +	_ASM_NOKPROBE(__thunk_restore)
> +SYM_CODE_END(__thunk_restore)
> #endif
> --
> 2.30.0.284.gd98b1dd5eaa7-goog