[PATCH] x86/entry: use STB_GLOBAL for register restoring thunk

78 views
Skip to first unread message

Nick Desaulniers

unread,
Dec 23, 2020, 6:21:36 PM12/23/20
to Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Nick Desaulniers, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
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.

This behavior was implemented as an optimization in LLVM 5 years ago,
but it's not the first time this has caused issues for objtool. A patch
has been authored against LLVM to revert the behavior, which may or may
not be accepted. Until then use a global symbol for the thunk that way
objtool can generate proper unwind info here with LLVM_IAS=1.

Cc: Fangrui Song <mas...@google.com>
Reported-by: Arnd Bergmann <ar...@arndb.de>
Suggested-by: Josh Poimboeuf <jpoi...@redhat.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1209
Link: https://reviews.llvm.org/D93783
Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---
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..878816034a73 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_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.29.2.729.g45daf8777d-goog

Josh Poimboeuf

unread,
Dec 23, 2020, 11:55:14 PM12/23/20
to Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
On Wed, Dec 23, 2020 at 03:21:26PM -0800, 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.
>
> This behavior was implemented as an optimization in LLVM 5 years ago,
> but it's not the first time this has caused issues for objtool. A patch
> has been authored against LLVM to revert the behavior, which may or may
> not be accepted. Until then use a global symbol for the thunk that way
> objtool can generate proper unwind info here with LLVM_IAS=1.

As Fangrui pointed out, the section symbol stripping is useful for when
there are a ton of sections like '-ffunction-sections' and
'-fdata-sections'. Maybe add that justification to the patch
description.

We can try to support it, though I suspect other tools may also end up
getting surprised.

> Cc: Fangrui Song <mas...@google.com>
> Reported-by: Arnd Bergmann <ar...@arndb.de>
> Suggested-by: Josh Poimboeuf <jpoi...@redhat.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1209
> Link: https://reviews.llvm.org/D93783
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>

Code looks familiar ;-)

Acked-by: Josh Poimboeuf <jpoi...@redhat.com>

--
Josh

Nick Desaulniers

unread,
Jan 5, 2021, 7:44:03 PM1/5/21
to Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Nick Desaulniers, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
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 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].

Use a global symbol for the thunk that way
objtool can generate proper unwind info here with LLVM_IAS=1.

Cc: Fangrui Song <mas...@google.com>
Link: https://sourceware.org/pipermail/binutils/2020-December/114671.html
Reported-by: Arnd Bergmann <ar...@arndb.de>
Suggested-by: Josh Poimboeuf <jpoi...@redhat.com>
Acked-by: Josh Poimboeuf <jpoi...@redhat.com>
Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---
Changes v1 -> v2:
* Pick up Josh's Ack.
* Add commit message info about -ffunction-sections/-fdata-sections, and
link to binutils patch.

Josh Poimboeuf

unread,
Jan 5, 2021, 8:58:18 PM1/5/21
to Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
On Tue, Jan 05, 2021 at 04:43:51PM -0800, 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 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].
>
> Use a global symbol for the thunk that way
> objtool can generate proper unwind info here with LLVM_IAS=1.

On second thought, there's no need to make the symbol global. Just
getting rid of the '.L' local label symbol prefix should be enough to
make an ELF symbol:

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)

Nick Desaulniers

unread,
Jan 11, 2021, 3:38:14 PM1/11/21
to Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Nick Desaulniers, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
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].

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.

Link: https://sourceware.org/binutils/docs/as/Symbol-Names.html
Link: https://sourceware.org/pipermail/binutils/2020-December/114671.html
Reported-by: Arnd Bergmann <ar...@arndb.de>
Suggested-by: Josh Poimboeuf <jpoi...@redhat.com>
Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---
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(-)

--
2.30.0.284.gd98b1dd5eaa7-goog

Fangrui Song

unread,
Jan 11, 2021, 3:58:19 PM1/11/21
to Nick Desaulniers, Josh Poimboeuf, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Arnd Bergmann, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, linux-...@vger.kernel.org, clang-bu...@googlegroups.com

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.

Josh Poimboeuf

unread,
Jan 11, 2021, 5:09:20 PM1/11/21
to Fangrui Song, Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Arnd Bergmann, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
I'm guessing you don't mean re-adding *all* missing STT_SECTIONs, as
that would just be undoing these new assembler features.

We could re-add STT_SECTION only when there's no other corresponding
symbol associated with the code, but then objtool would have to start
updating the symbol table (which right now it manages to completely
avoid). But that would only be for the niche cases, like
'SYM_CODE.*\.L' as you mentioned.

I'd rather avoid making doing something so pervasive for such a small
number of edge cases. It's hopefully easier and more robust to just say
"all code must be associated with a symbol". I suspect we're already
~99.99% there anyway.

$ git grep -e 'SYM_CODE.*\.L'
arch/x86/entry/entry_64.S:SYM_CODE_START_LOCAL_NOALIGN(.Lbad_gs)
arch/x86/entry/entry_64.S:SYM_CODE_END(.Lbad_gs)
arch/x86/entry/thunk_64.S:SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
arch/x86/entry/thunk_64.S:SYM_CODE_END(.L_restore)
arch/x86/lib/copy_user_64.S:SYM_CODE_START_LOCAL(.Lcopy_user_handle_tail)
arch/x86/lib/copy_user_64.S:SYM_CODE_END(.Lcopy_user_handle_tail)
arch/x86/lib/getuser.S:SYM_CODE_START_LOCAL(.Lbad_get_user_clac)
arch/x86/lib/getuser.S:SYM_CODE_END(.Lbad_get_user_clac)
arch/x86/lib/getuser.S:SYM_CODE_START_LOCAL(.Lbad_get_user_8_clac)
arch/x86/lib/getuser.S:SYM_CODE_END(.Lbad_get_user_8_clac)
arch/x86/lib/putuser.S:SYM_CODE_START_LOCAL(.Lbad_put_user_clac)
arch/x86/lib/putuser.S:SYM_CODE_END(.Lbad_put_user_clac)

Alternatively, the assemblers could add an option to only strip
-ffunction-sections and -fdata-sections STT_SECTION symbols, e.g. leave
".text" and friends alone.

--
Josh

Josh Poimboeuf

unread,
Jan 11, 2021, 5:12:17 PM1/11/21
to Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
On Mon, Jan 11, 2021 at 12:38:06PM -0800, 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

"image"

> when compiling with -ffunction-sections and -fdata-sections. I have
> observerd on the order of tens of thousands of symbols for the kernel

"observed"

> 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].
>
> 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.
>
> Cc: Fangrui Song <mas...@google.com>
> 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 <ar...@arndb.de>
> Suggested-by: Josh Poimboeuf <jpoi...@redhat.com>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> ---
> 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.

Fāng-ruì Sòng

unread,
Jan 11, 2021, 5:17:04 PM1/11/21
to Josh Poimboeuf, Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Arnd Bergmann, X86 ML, H. Peter Anvin, Nathan Chancellor, LKML, clang-built-linux
I'd prefer that the assembly can continue using .L and does not know
the objtool limitation.
Assemblers normally drop .L symbols. These symbols are otherwise not useful.

However, if as you said, teaching objtool about synthesizing
STT_SECTION from section header table is difficult,
this patch looks fine to me.

Reviewed-by: Fangrui Song <mas...@google.com>

> Alternatively, the assemblers could add an option to only strip
> -ffunction-sections and -fdata-sections STT_SECTION symbols, e.g. leave
> ".text" and friends alone.

I forgot to mention that --generate-unused-section-symbols=[yes|no] is
not added to GNU as.
Making the assembler behavior dependent on -ffunction-sections is not
an option in both LLVM integrated assembler and GNU as.

Borislav Petkov

unread,
Jan 11, 2021, 7:38:44 PM1/11/21
to Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
On Mon, Jan 11, 2021 at 12:38:06PM -0800, Nick Desaulniers wrote:
Ok so I read a bit around those links above...

Are you trying to tell me here that we can't use .L-prefixed local
labels anymore because, well, clang's assembler is way too overzealous
when stripping symbols to save whopping KiBs of memory?!

Btw Josh made sense to me when asking for a flag or so to keep .text.

And I see --generate-unused-section-symbols=[yes|no] for binutils.

So why isn't there a patch using that switch on clang too instead of the
kernel having to dance yet again for some tool?

:-\

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Fāng-ruì Sòng

unread,
Jan 11, 2021, 7:42:05 PM1/11/21
to Borislav Petkov, Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Arnd Bergmann, Josh Poimboeuf, X86 ML, H. Peter Anvin, Nathan Chancellor, LKML, clang-built-linux
To be fair: we cannot use .L-prefixed local because of the objtool limitation.
The LLVM integrated assembler behavior is a good one and binutils
global maintainers have agreed so H.J. went ahead and implemented it
for GNU as x86.

> Btw Josh made sense to me when asking for a flag or so to keep .text.
>
> And I see --generate-unused-section-symbols=[yes|no] for binutils.
>
> So why isn't there a patch using that switch on clang too instead of the
> kernel having to dance yet again for some tool?

--generate-unused-section-symbols=[yes|no] as an assembler option has
been rejected.

Borislav Petkov

unread,
Jan 11, 2021, 8:00:17 PM1/11/21
to Fāng-ruì Sòng, Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Arnd Bergmann, Josh Poimboeuf, X86 ML, H. Peter Anvin, Nathan Chancellor, LKML, clang-built-linux
On Mon, Jan 11, 2021 at 04:41:52PM -0800, Fāng-ruì Sòng wrote:
> To be fair: we cannot use

Who's "we"?

> .L-prefixed local because of the objtool limitation.

What objtool limitation? I thought clang's assembler removes .text which
objtool uses. It worked fine with GNU as so far.

> The LLVM integrated assembler behavior is a good one

Please explain what "good one" means in that particular context.

> and binutils global maintainers have agreed so H.J. went ahead and
> implemented it for GNU as x86.

But they don't break old behavior, do they? Or are they removing .text
unconditionally now too?

> --generate-unused-section-symbols=[yes|no] as an assembler option has
> been rejected.

Meaning what exactly? There's no way for clang's integrated assembler to
even get a cmdline option to not strip .text?

Nick Desaulniers

unread,
Jan 11, 2021, 8:13:29 PM1/11/21
to Borislav Petkov, Fāng-ruì Sòng, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Arnd Bergmann, Josh Poimboeuf, X86 ML, H. Peter Anvin, Nathan Chancellor, LKML, clang-built-linux
On Mon, Jan 11, 2021 at 5:00 PM Borislav Petkov <b...@alien8.de> wrote:
>
> On Mon, Jan 11, 2021 at 04:41:52PM -0800, Fāng-ruì Sòng wrote:
> > To be fair: we cannot use
>
> Who's "we"?
>
> > .L-prefixed local because of the objtool limitation.
>
> What objtool limitation? I thought clang's assembler removes .text which
> objtool uses. It worked fine with GNU as so far.

I don't think we need to completely stop using .L prefixes in the
kernel, just this one location since tracking the control flow seems a
little tricky for objtool. Maybe Josh can clarify more if needed?

>
> > The LLVM integrated assembler behavior is a good one
>
> Please explain what "good one" means in that particular context.
>
> > and binutils global maintainers have agreed so H.J. went ahead and
> > implemented it for GNU as x86.
>
> But they don't break old behavior, do they? Or are they removing .text
> unconditionally now too?

Unconditionally. See
https://sourceware.org/pipermail/binutils/2021-January/114700.html
where that flag was rejected and the optimization was adopted as the
optimization was obvious to GNU binutils developers. So I suspect this
will become a problem for GNU binutils users as well after the latest
release that contains
https://sourceware.org/pipermail/binutils/attachments/20210105/75dd4a9d/attachment-0001.bin.

> > --generate-unused-section-symbols=[yes|no] as an assembler option has
> > been rejected.
>
> Meaning what exactly? There's no way for clang's integrated assembler to
> even get a cmdline option to not strip .text?

I can clean that up in v5; The section symbols were not generated then
stripped; they were simply never generated.
--
Thanks,
~Nick Desaulniers

Josh Poimboeuf

unread,
Jan 11, 2021, 9:00:03 PM1/11/21
to Nick Desaulniers, Borislav Petkov, Fāng-ruì Sòng, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Arnd Bergmann, X86 ML, H. Peter Anvin, Nathan Chancellor, LKML, clang-built-linux
On Mon, Jan 11, 2021 at 05:13:16PM -0800, Nick Desaulniers wrote:
> On Mon, Jan 11, 2021 at 5:00 PM Borislav Petkov <b...@alien8.de> wrote:
> >
> > On Mon, Jan 11, 2021 at 04:41:52PM -0800, Fāng-ruì Sòng wrote:
> > > To be fair: we cannot use
> >
> > Who's "we"?
> >
> > > .L-prefixed local because of the objtool limitation.
> >
> > What objtool limitation? I thought clang's assembler removes .text which
> > objtool uses. It worked fine with GNU as so far.
>
> I don't think we need to completely stop using .L prefixes in the
> kernel, just this one location since tracking the control flow seems a
> little tricky for objtool. Maybe Josh can clarify more if needed?

Right. In the vast majority of cases, .L symbols are totally fine.

The limitation now being imposed by objtool (due to these assembler
changes) is that all code must be contained in an ELF symbol. And .L
symbols don't create such symbols.

So basically, you can use an .L symbol *inside* a function or a code
segment, you just can't use the .L symbol to contain the code using a
SYM_*_START/END annotation pair.

It only affects a tiny fraction of all .L usage. Just a handful of code
sites I think.

--
Josh

Borislav Petkov

unread,
Jan 12, 2021, 6:47:35 AM1/12/21
to Nick Desaulniers, Fāng-ruì Sòng, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Arnd Bergmann, Josh Poimboeuf, X86 ML, H. Peter Anvin, Nathan Chancellor, LKML, clang-built-linux
On Mon, Jan 11, 2021 at 05:13:16PM -0800, Nick Desaulniers wrote:
> Unconditionally. See
> https://sourceware.org/pipermail/binutils/2021-January/114700.html
> where that flag was rejected and the optimization was adopted as the
> optimization was obvious to GNU binutils developers. So I suspect this
> will become a problem for GNU binutils users as well after the latest
> release that contains
> https://sourceware.org/pipermail/binutils/attachments/20210105/75dd4a9d/attachment-0001.bin.

Aha, thanks for this.

> I can clean that up in v5; The section symbols were not generated then
> stripped; they were simply never generated.

I'd appreciate a more verbose writeup explaining why this is being done,
but written for outsiders who are not necessarily toolchain developers.
So that it is clear months/years from now why this was done. Something
structured like this maybe:

Problem is A.

It happens because of B.

Fix it by doing C.

(Potentially do D).

Thx.

Borislav Petkov

unread,
Jan 12, 2021, 6:54:23 AM1/12/21
to Josh Poimboeuf, Nick Desaulniers, Fāng-ruì Sòng, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Arnd Bergmann, X86 ML, H. Peter Anvin, Nathan Chancellor, LKML, clang-built-linux
On Mon, Jan 11, 2021 at 07:59:52PM -0600, Josh Poimboeuf wrote:
> Right. In the vast majority of cases, .L symbols are totally fine.
>
> The limitation now being imposed by objtool (due to these assembler
> changes) is that all code must be contained in an ELF symbol. And .L
> symbols don't create such symbols.
>
> So basically, you can use an .L symbol *inside* a function or a code
> segment, you just can't use the .L symbol to contain the code using a
> SYM_*_START/END annotation pair.
>
> It only affects a tiny fraction of all .L usage. Just a handful of code
> sites I think.

@Nick, this belongs into the commit message too pls.

Also,

Documentation/asm-annotations.rst
include/linux/linkage.h

would need some of that blurb added explaining to users *why* they
should not use .L local symbols as SYM_* macro arguments.

Thx.

Nick Desaulniers

unread,
Jan 12, 2021, 2:46:53 PM1/12/21
to Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Nick Desaulniers, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, Jonathan Corbet, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, Mark Brown, Miguel Ojeda, Jiri Slaby, Joe Perches, linu...@vger.kernel.org, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
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 (Clang's integrated assembler). Josh
notes:

With the LLVM assembler not generating section symbols, objtool has no
way to reference this code when it generates ORC unwinder entries,
because this code is outside of any ELF function.

The limitation now being imposed by objtool is that all code must be
contained in an ELF symbol. And .L symbols don't create such symbols.

So basically, you can use an .L symbol *inside* a function or a code
segment, you just can't use the .L symbol to contain the code using a
SYM_*_START/END annotation pair.

Fangrui notes that this optimization is helpful for reducing image size
when compiling with -ffunction-sections and -fdata-sections. I have
observed 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,
so this will also become a problem for users of GNU binutils once they
upgrade to 2.36.

We can omit the .L prefix on a label so that the assembler will 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 or
GNU binutils 2.36+.
Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1408485ce69f844dcd7ded093a8
Reported-by: Arnd Bergmann <ar...@arndb.de>
Suggested-by: Josh Poimboeuf <jpoi...@redhat.com>
Suggested-by: Borislav Petkov <b...@alien8.de>
Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---
Changes v3 -> v4:
* Add changes to Documentation/ and include/ as per Boris.
* Fix typos as per Josh.
* Replace link and note in commit message about
--generate-unused-section-symbols=[yes|no] which was dropped from GNU
binutils with link to actual commit in binutils-gdb.
* Add additional notes from Josh in commit message.
* Slightly reword commit message to indicate that section symbols are
not emitted, rather than stripped.

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.

Documentation/asm-annotations.rst | 9 +++++++++
arch/x86/entry/thunk_64.S | 8 ++++----
include/linux/linkage.h | 5 ++++-
3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/asm-annotations.rst b/Documentation/asm-annotations.rst
index 32ea57483378..e711ff98102a 100644
--- a/Documentation/asm-annotations.rst
+++ b/Documentation/asm-annotations.rst
@@ -153,6 +153,15 @@ This section covers ``SYM_FUNC_*`` and ``SYM_CODE_*`` enumerated above.
To some extent, this category corresponds to deprecated ``ENTRY`` and
``END``. Except ``END`` had several other meanings too.

+ Developers should avoid using local symbol names that are prefixed with
+ ``.L``, as this has special meaning for the assembler; a symbol entry will
+ not be emitted into the symbol table. This can prevent ``objtool`` from
+ generating correct unwind info. Symbols with STB_LOCAL binding may still be
+ used, and ``.L`` prefixed local symbol names are still generally useable
+ within a function, but ``.L`` prefixed local symbol names should not be used
+ to denote the beginning or end of code regions via
+ ``SYM_CODE_START_LOCAL``/``SYM_CODE_END``.
+
* ``SYM_INNER_LABEL*`` is used to denote a label inside some
``SYM_{CODE,FUNC}_START`` and ``SYM_{CODE,FUNC}_END``. They are very similar
to C labels, except they can be made global. An example of use::
diff --git a/include/linux/linkage.h b/include/linux/linkage.h
index 5bcfbd972e97..11537ba9f512 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -270,7 +270,10 @@
SYM_END(name, SYM_T_FUNC)
#endif

-/* SYM_CODE_START -- use for non-C (special) functions */
+/*
+ * SYM_CODE_START -- use for non-C (special) functions, avoid .L prefixed local
+ * symbol names which may not emit a symbol table entry.
+ */
#ifndef SYM_CODE_START
#define SYM_CODE_START(name) \
SYM_START(name, SYM_L_GLOBAL, SYM_A_ALIGN)
--
2.30.0.284.gd98b1dd5eaa7-goog

Mark Brown

unread,
Jan 12, 2021, 4:02:30 PM1/12/21
to Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, Jonathan Corbet, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, linu...@vger.kernel.org, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
On Tue, Jan 12, 2021 at 11:46:24AM -0800, Nick Desaulniers wrote:

This:

> when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
> notes:

> So basically, you can use an .L symbol *inside* a function or a code
> segment, you just can't use the .L symbol to contain the code using a
> SYM_*_START/END annotation pair.

is a stronger statement than this:

> + Developers should avoid using local symbol names that are prefixed with
> + ``.L``, as this has special meaning for the assembler; a symbol entry will
> + not be emitted into the symbol table. This can prevent ``objtool`` from
> + generating correct unwind info. Symbols with STB_LOCAL binding may still be
> + used, and ``.L`` prefixed local symbol names are still generally useable
> + within a function, but ``.L`` prefixed local symbol names should not be used
> + to denote the beginning or end of code regions via
> + ``SYM_CODE_START_LOCAL``/``SYM_CODE_END``.

and seems more what I'd expect - SYM_FUNC* is also affected for example.
Even though other usages are probably not very likely it seems better to
keep the stronger statement in case someone comes up with one, and to
stop anyone spending time wondering why only SYM_CODE_START_LOCAL is
affected.

This also looks like a good candiate for a checkpatch rule, but that can
be done separately of course.
signature.asc

Josh Poimboeuf

unread,
Jan 13, 2021, 11:59:34 AM1/13/21
to Mark Brown, Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, Jonathan Corbet, x...@kernel.org, H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, linu...@vger.kernel.org, linux-...@vger.kernel.org, clang-bu...@googlegroups.com
Agreed, I think the comment is misleading/wrong/unclear in multiple
ways. In most cases the use of .L symbols is still fine. What's no
longer fine is when they're used to contain code in any kind of
START/END pair.

> This also looks like a good candiate for a checkpatch rule, but that can
> be done separately of course.

I like the idea of a checkpatch rule.

--
Josh

Nick Desaulniers

unread,
Jan 13, 2021, 12:56:16 PM1/13/21
to Josh Poimboeuf, Mark Brown, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, Jonathan Corbet, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, Linux Doc Mailing List, LKML, clang-built-linux
Apologies, that was not my intention. I've sent a follow up in
https://lore.kernel.org/lkml/20210113174620.958...@google.com/T/#u
since BP picked up v3 in tip x86/entry:
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/entry&id=bde718b7e154afc99e1956b18a848401ce8e1f8e

--
Thanks,
~Nick Desaulniers

Borislav Petkov

unread,
Jan 14, 2021, 5:39:34 AM1/14/21
to Nick Desaulniers, Josh Poimboeuf, Mark Brown, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Fangrui Song, Arnd Bergmann, Jonathan Corbet, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, Linux Doc Mailing List, LKML, clang-built-linux
On Wed, Jan 13, 2021 at 09:56:04AM -0800, Nick Desaulniers wrote:
> Apologies, that was not my intention. I've sent a follow up in
> https://lore.kernel.org/lkml/20210113174620.958...@google.com/T/#u
> since BP picked up v3 in tip x86/entry:
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/entry&id=bde718b7e154afc99e1956b18a848401ce8e1f8e

It is the topmost patch so I can rebase...

Also, I replicated that text into linkage.h and removed the change over
SYM_CODE_START and I've got the below.

Further complaints?

---
From: Nick Desaulniers <ndesau...@google.com>
Date: Tue, 12 Jan 2021 11:46:24 -0800
Subject: [PATCH] x86/entry: Emit a symbol for register restoring thunk

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 (Clang's integrated assembler). Josh
notes:

With the LLVM assembler not generating section symbols, objtool has no
way to reference this code when it generates ORC unwinder entries,
because this code is outside of any ELF function.

The limitation now being imposed by objtool is that all code must be
contained in an ELF symbol. And .L symbols don't create such symbols.

So basically, you can use an .L symbol *inside* a function or a code
segment, you just can't use the .L symbol to contain the code using a
SYM_*_START/END annotation pair.

Fangrui notes that this optimization is helpful for reducing image size
when compiling with -ffunction-sections and -fdata-sections. I have
observed 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
of not generating unused section symbols ([1]), so this will
also become a problem for users of GNU binutils once they upgrade to 2.36.

Omit the .L prefix on a label so that the assembler will 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 or
GNU binutils 2.36+.

[ bp: Massage commit message. ]

Reported-by: Arnd Bergmann <ar...@arndb.de>
Suggested-by: Josh Poimboeuf <jpoi...@redhat.com>
Suggested-by: Borislav Petkov <b...@alien8.de>
Suggested-by: Mark Brown <bro...@kernel.org>
Signed-off-by: Nick Desaulniers <ndesau...@google.com>
Signed-off-by: Borislav Petkov <b...@suse.de>
Link: https://lkml.kernel.org/r/20210112194625.418...@google.com
Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1408485ce69f844dcd7ded093a8 [1]
---
Documentation/asm-annotations.rst | 5 +++++
arch/x86/entry/thunk_64.S | 8 ++++----
include/linux/linkage.h | 5 +++++
3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/Documentation/asm-annotations.rst b/Documentation/asm-annotations.rst
index 32ea57483378..76424e0431f4 100644
--- a/Documentation/asm-annotations.rst
+++ b/Documentation/asm-annotations.rst
@@ -100,6 +100,11 @@ Instruction Macros
~~~~~~~~~~~~~~~~~~
This section covers ``SYM_FUNC_*`` and ``SYM_CODE_*`` enumerated above.

+``objtool`` requires that all code must be contained in an ELF symbol. Symbol
+names that have a ``.L`` prefix do not emit symbol table entries. ``.L``
+prefixed symbols can be used within a code region, but should be avoided for
+denoting a range of code via ``SYM_*_START/END`` annotations.
+
* ``SYM_FUNC_START`` and ``SYM_FUNC_START_LOCAL`` are supposed to be **the
most frequent markings**. They are used for functions with standard calling
conventions -- global and local. Like in C, they both align the functions to
index 5bcfbd972e97..dbf8506decca 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -178,6 +178,11 @@
* Objtool generates debug info for both FUNC & CODE, but needs special
* annotations for each CODE's start (to describe the actual stack frame).
*
+ * Objtool requires that all code must be contained in an ELF symbol. Symbol
+ * names that have a .L prefix do not emit symbol table entries. .L
+ * prefixed symbols can be used within a code region, but should be avoided for
+ * denoting a range of code via ``SYM_*_START/END`` annotations.
+ *
* ALIAS -- does not generate debug info -- the aliased function will
*/

--
2.29.2

Peter Zijlstra

unread,
Jan 14, 2021, 6:37:01 AM1/14/21
to Borislav Petkov, Nick Desaulniers, Josh Poimboeuf, Mark Brown, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Fangrui Song, Arnd Bergmann, Jonathan Corbet, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, Linux Doc Mailing List, LKML, clang-built-linux
Acked-by: Peter Zijlstra (Intel) <pet...@infradead.org>

And while looking, I suppose we can delete the put_ret_addr_in_rdi crud,
but that's another patch.

Borislav Petkov

unread,
Jan 14, 2021, 8:28:17 AM1/14/21
to Peter Zijlstra, Nick Desaulniers, Josh Poimboeuf, Mark Brown, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Fangrui Song, Arnd Bergmann, Jonathan Corbet, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, Linux Doc Mailing List, LKML, clang-built-linux
On Thu, Jan 14, 2021 at 12:36:45PM +0100, Peter Zijlstra wrote:
> And while looking, I suppose we can delete the put_ret_addr_in_rdi crud,
> but that's another patch.

---
From: Borislav Petkov <b...@suse.de>
Date: Thu, 14 Jan 2021 14:25:35 +0100
Subject: [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument

That logic is unused since

320100a5ffe5 ("x86/entry: Remove the TRACE_IRQS cruft")

Remove it.

Suggested-by: Peter Zijlstra <pet...@infradead.org>
Signed-off-by: Borislav Petkov <b...@suse.de>
---
arch/x86/entry/thunk_64.S | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S
index c9a9fbf1655f..496b11ec469d 100644
--- a/arch/x86/entry/thunk_64.S
+++ b/arch/x86/entry/thunk_64.S
@@ -10,7 +10,7 @@
#include <asm/export.h>

/* rdi: arg1 ... normal C conventions. rax is saved/restored. */
- .macro THUNK name, func, put_ret_addr_in_rdi=0
+ .macro THUNK name, func
SYM_FUNC_START_NOALIGN(\name)
pushq %rbp
movq %rsp, %rbp
@@ -25,11 +25,6 @@ SYM_FUNC_START_NOALIGN(\name)
pushq %r10
pushq %r11

- .if \put_ret_addr_in_rdi
- /* 8(%rbp) is return addr on stack */
- movq 8(%rbp), %rdi
- .endif
-
call \func
jmp __thunk_restore
SYM_FUNC_END(\name)

Josh Poimboeuf

unread,
Jan 14, 2021, 10:14:50 AM1/14/21
to Borislav Petkov, Nick Desaulniers, Mark Brown, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Fangrui Song, Arnd Bergmann, Jonathan Corbet, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, Linux Doc Mailing List, LKML, clang-built-linux
On Thu, Jan 14, 2021 at 11:39:28AM +0100, Borislav Petkov wrote:

Peter Zijlstra

unread,
Jan 14, 2021, 10:54:18 AM1/14/21
to Borislav Petkov, Nick Desaulniers, Josh Poimboeuf, Mark Brown, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Fangrui Song, Arnd Bergmann, Jonathan Corbet, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, Linux Doc Mailing List, LKML, clang-built-linux
On Thu, Jan 14, 2021 at 02:28:09PM +0100, Borislav Petkov wrote:
> On Thu, Jan 14, 2021 at 12:36:45PM +0100, Peter Zijlstra wrote:
> > And while looking, I suppose we can delete the put_ret_addr_in_rdi crud,
> > but that's another patch.
>
> ---
> From: Borislav Petkov <b...@suse.de>
> Date: Thu, 14 Jan 2021 14:25:35 +0100
> Subject: [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument
>
> That logic is unused since
>
> 320100a5ffe5 ("x86/entry: Remove the TRACE_IRQS cruft")
>
> Remove it.
>
> Suggested-by: Peter Zijlstra <pet...@infradead.org>
> Signed-off-by: Borislav Petkov <b...@suse.de>

Reply all
Reply to author
Forward
0 new messages