* [PATCH] x86/entry: use STB_GLOBAL for register restoring thunk @ 2020-12-23 23:21 Nick Desaulniers 2020-12-24 4:55 ` Josh Poimboeuf 0 siblings, 1 reply; 30+ messages in thread From: Nick Desaulniers @ 2020-12-23 23:21 UTC (permalink / raw) To: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov Cc: Nick Desaulniers, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, x86, H. Peter Anvin, Nathan Chancellor, linux-kernel, clang-built-linux 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 <maskray@google.com> Reported-by: Arnd Bergmann <arnd@arndb.de> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Link: https://github.com/ClangBuiltLinux/linux/issues/1209 Link: https://reviews.llvm.org/D93783 Signed-off-by: Nick Desaulniers <ndesaulniers@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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] x86/entry: use STB_GLOBAL for register restoring thunk 2020-12-23 23:21 [PATCH] x86/entry: use STB_GLOBAL for register restoring thunk Nick Desaulniers @ 2020-12-24 4:55 ` Josh Poimboeuf 2021-01-06 0:43 ` [PATCH v2] " Nick Desaulniers 0 siblings, 1 reply; 30+ messages in thread From: Josh Poimboeuf @ 2020-12-24 4:55 UTC (permalink / raw) To: Nick Desaulniers Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, x86, H. Peter Anvin, Nathan Chancellor, linux-kernel, clang-built-linux 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 <maskray@google.com> > Reported-by: Arnd Bergmann <arnd@arndb.de> > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > Link: https://github.com/ClangBuiltLinux/linux/issues/1209 > Link: https://reviews.llvm.org/D93783 > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Code looks familiar ;-) Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> -- Josh ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2] x86/entry: use STB_GLOBAL for register restoring thunk 2020-12-24 4:55 ` Josh Poimboeuf @ 2021-01-06 0:43 ` Nick Desaulniers 2021-01-06 1:58 ` Josh Poimboeuf 0 siblings, 1 reply; 30+ messages in thread From: Nick Desaulniers @ 2021-01-06 0:43 UTC (permalink / raw) To: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov Cc: Nick Desaulniers, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, x86, H. Peter Anvin, Nathan Chancellor, linux-kernel, clang-built-linux 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 <maskray@google.com> Link: https://github.com/ClangBuiltLinux/linux/issues/1209 Link: https://reviews.llvm.org/D93783 Link: https://sourceware.org/pipermail/binutils/2020-December/114671.html Reported-by: Arnd Bergmann <arnd@arndb.de> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- 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..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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2] x86/entry: use STB_GLOBAL for register restoring thunk 2021-01-06 0:43 ` [PATCH v2] " Nick Desaulniers @ 2021-01-06 1:58 ` Josh Poimboeuf 2021-01-11 20:38 ` [PATCH v3] x86/entry: emit a symbol " Nick Desaulniers 0 siblings, 1 reply; 30+ messages in thread From: Josh Poimboeuf @ 2021-01-06 1:58 UTC (permalink / raw) To: Nick Desaulniers Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, x86, H. Peter Anvin, Nathan Chancellor, linux-kernel, clang-built-linux 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) 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-06 1:58 ` Josh Poimboeuf @ 2021-01-11 20:38 ` Nick Desaulniers 2021-01-11 20:58 ` Fangrui Song ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: Nick Desaulniers @ 2021-01-11 20:38 UTC (permalink / raw) To: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov Cc: Nick Desaulniers, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, x86, H. Peter Anvin, Nathan Chancellor, linux-kernel, clang-built-linux 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. Cc: Fangrui Song <maskray@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 <arnd@arndb.de> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Nick Desaulniers <ndesaulniers@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(-) 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-11 20:38 ` [PATCH v3] x86/entry: emit a symbol " Nick Desaulniers @ 2021-01-11 20:58 ` Fangrui Song 2021-01-11 22:09 ` Josh Poimboeuf 2021-01-11 22:12 ` Josh Poimboeuf 2021-01-12 0:38 ` Borislav Petkov 2 siblings, 1 reply; 30+ messages in thread From: Fangrui Song @ 2021-01-11 20:58 UTC (permalink / raw) To: Nick Desaulniers, Josh Poimboeuf Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Arnd Bergmann, x86, H. Peter Anvin, Nathan Chancellor, linux-kernel, clang-built-linux 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@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 <arnd@arndb.de> >Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> >Signed-off-by: Nick Desaulniers <ndesaulniers@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(-) > >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 > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-11 20:58 ` Fangrui Song @ 2021-01-11 22:09 ` Josh Poimboeuf 2021-01-11 22:16 ` Fāng-ruì Sòng 0 siblings, 1 reply; 30+ messages in thread From: Josh Poimboeuf @ 2021-01-11 22:09 UTC (permalink / raw) To: Fangrui Song Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Arnd Bergmann, x86, H. Peter Anvin, Nathan Chancellor, linux-kernel, clang-built-linux On Mon, Jan 11, 2021 at 12:58:14PM -0800, Fangrui Song wrote: > 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. 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-11 22:09 ` Josh Poimboeuf @ 2021-01-11 22:16 ` Fāng-ruì Sòng 0 siblings, 0 replies; 30+ messages in thread From: Fāng-ruì Sòng @ 2021-01-11 22:16 UTC (permalink / raw) To: Josh Poimboeuf Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Arnd Bergmann, X86 ML, H. Peter Anvin, Nathan Chancellor, LKML, clang-built-linux On Mon, Jan 11, 2021 at 2:09 PM Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Mon, Jan 11, 2021 at 12:58:14PM -0800, Fangrui Song wrote: > > 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. > > 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) 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 <maskray@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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-11 20:38 ` [PATCH v3] x86/entry: emit a symbol " Nick Desaulniers 2021-01-11 20:58 ` Fangrui Song @ 2021-01-11 22:12 ` Josh Poimboeuf 2021-01-12 0:38 ` Borislav Petkov 2 siblings, 0 replies; 30+ messages in thread From: Josh Poimboeuf @ 2021-01-11 22:12 UTC (permalink / raw) To: Nick Desaulniers Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, x86, H. Peter Anvin, Nathan Chancellor, linux-kernel, clang-built-linux 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 <maskray@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 <arnd@arndb.de> > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@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. Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> -- Josh ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-11 20:38 ` [PATCH v3] x86/entry: emit a symbol " Nick Desaulniers 2021-01-11 20:58 ` Fangrui Song 2021-01-11 22:12 ` Josh Poimboeuf @ 2021-01-12 0:38 ` Borislav Petkov 2021-01-12 0:41 ` Fāng-ruì Sòng 2 siblings, 1 reply; 30+ messages in thread From: Borislav Petkov @ 2021-01-12 0:38 UTC (permalink / raw) To: Nick Desaulniers Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, x86, H. Peter Anvin, Nathan Chancellor, linux-kernel, clang-built-linux 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 > 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. > > Cc: Fangrui Song <maskray@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 <arnd@arndb.de> > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Changes v2 -> v3: > * rework to use STB_LOCAL rather than STB_GLOBAL by dropping .L prefix, > as per Josh. 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-12 0:38 ` Borislav Petkov @ 2021-01-12 0:41 ` Fāng-ruì Sòng 2021-01-12 1:00 ` Borislav Petkov 0 siblings, 1 reply; 30+ messages in thread From: Fāng-ruì Sòng @ 2021-01-12 0:41 UTC (permalink / raw) To: Borislav Petkov Cc: 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 4:38 PM Borislav Petkov <bp@alien8.de> wrote: > > 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 > > 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. > > > > Cc: Fangrui Song <maskray@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 <arnd@arndb.de> > > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > --- > > Changes v2 -> v3: > > * rework to use STB_LOCAL rather than STB_GLOBAL by dropping .L prefix, > > as per Josh. > > 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?! 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. > :-\ > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-12 0:41 ` Fāng-ruì Sòng @ 2021-01-12 1:00 ` Borislav Petkov 2021-01-12 1:13 ` Nick Desaulniers 0 siblings, 1 reply; 30+ messages in thread From: Borislav Petkov @ 2021-01-12 1:00 UTC (permalink / raw) To: Fāng-ruì Sòng Cc: 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? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-12 1:00 ` Borislav Petkov @ 2021-01-12 1:13 ` Nick Desaulniers 2021-01-12 1:59 ` Josh Poimboeuf 2021-01-12 11:47 ` [PATCH v3] x86/entry: emit " Borislav Petkov 0 siblings, 2 replies; 30+ messages in thread From: Nick Desaulniers @ 2021-01-12 1:13 UTC (permalink / raw) To: Borislav Petkov Cc: 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 <bp@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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-12 1:13 ` Nick Desaulniers @ 2021-01-12 1:59 ` Josh Poimboeuf 2021-01-12 11:54 ` Borislav Petkov 2021-01-12 11:47 ` [PATCH v3] x86/entry: emit " Borislav Petkov 1 sibling, 1 reply; 30+ messages in thread From: Josh Poimboeuf @ 2021-01-12 1:59 UTC (permalink / raw) To: Nick Desaulniers Cc: 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 <bp@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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-12 1:59 ` Josh Poimboeuf @ 2021-01-12 11:54 ` Borislav Petkov 2021-01-12 19:46 ` [PATCH v4] " Nick Desaulniers 0 siblings, 1 reply; 30+ messages in thread From: Borislav Petkov @ 2021-01-12 11:54 UTC (permalink / raw) To: Josh Poimboeuf Cc: 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. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4] x86/entry: emit a symbol for register restoring thunk 2021-01-12 11:54 ` Borislav Petkov @ 2021-01-12 19:46 ` Nick Desaulniers 2021-01-12 21:01 ` Mark Brown ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: Nick Desaulniers @ 2021-01-12 19:46 UTC (permalink / raw) To: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov Cc: Nick Desaulniers, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, Jonathan Corbet, x86, H. Peter Anvin, Nathan Chancellor, Mark Brown, Miguel Ojeda, Jiri Slaby, Joe Perches, linux-doc, linux-kernel, clang-built-linux 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+. Cc: Fangrui Song <maskray@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/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1408485ce69f844dcd7ded093a8 Reported-by: Arnd Bergmann <arnd@arndb.de> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Suggested-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Nick Desaulniers <ndesaulniers@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/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 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk 2021-01-12 19:46 ` [PATCH v4] " Nick Desaulniers @ 2021-01-12 21:01 ` Mark Brown 2021-01-13 16:59 ` Josh Poimboeuf 2021-01-13 11:52 ` [tip: x86/entry] x86/entry: Emit " tip-bot2 for Nick Desaulniers 2021-01-19 10:12 ` tip-bot2 for Nick Desaulniers 2 siblings, 1 reply; 30+ messages in thread From: Mark Brown @ 2021-01-12 21:01 UTC (permalink / raw) To: Nick Desaulniers Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, Josh Poimboeuf, Jonathan Corbet, x86, H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, linux-doc, linux-kernel, clang-built-linux [-- Attachment #1: Type: text/plain, Size: 1353 bytes --] 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. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk 2021-01-12 21:01 ` Mark Brown @ 2021-01-13 16:59 ` Josh Poimboeuf 2021-01-13 17:46 ` [PATCH] Documentation: asm-annotation: clarify .L local symbol names Nick Desaulniers 2021-01-13 17:56 ` [PATCH v4] x86/entry: emit a symbol for register restoring thunk Nick Desaulniers 0 siblings, 2 replies; 30+ messages in thread From: Josh Poimboeuf @ 2021-01-13 16:59 UTC (permalink / raw) To: Mark Brown Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Fangrui Song, Arnd Bergmann, Jonathan Corbet, x86, H. Peter Anvin, Nathan Chancellor, Miguel Ojeda, Jiri Slaby, Joe Perches, linux-doc, linux-kernel, clang-built-linux On Tue, Jan 12, 2021 at 09:01:54PM +0000, Mark Brown wrote: > 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. 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] Documentation: asm-annotation: clarify .L local symbol names 2021-01-13 16:59 ` Josh Poimboeuf @ 2021-01-13 17:46 ` Nick Desaulniers 2021-01-13 19:56 ` Mark Brown 2021-01-13 17:56 ` [PATCH v4] x86/entry: emit a symbol for register restoring thunk Nick Desaulniers 1 sibling, 1 reply; 30+ messages in thread From: Nick Desaulniers @ 2021-01-13 17:46 UTC (permalink / raw) To: Borislav Petkov Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Nick Desaulniers, Mark Brown, Josh Poimboeuf, Jonathan Corbet, linux-doc, linux-kernel Use more precise language and move the text to a region in the docs to show that this constraint is not just for SYM_CODE_START*. Suggested-by: Mark Brown <broonie@kernel.org> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Documentation/asm-annotations.rst | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/Documentation/asm-annotations.rst b/Documentation/asm-annotations.rst index e711ff98102a..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 @@ -153,15 +158,6 @@ 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:: -- 2.30.0.284.gd98b1dd5eaa7-goog ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] Documentation: asm-annotation: clarify .L local symbol names 2021-01-13 17:46 ` [PATCH] Documentation: asm-annotation: clarify .L local symbol names Nick Desaulniers @ 2021-01-13 19:56 ` Mark Brown 0 siblings, 0 replies; 30+ messages in thread From: Mark Brown @ 2021-01-13 19:56 UTC (permalink / raw) To: Nick Desaulniers Cc: Borislav Petkov, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Josh Poimboeuf, Jonathan Corbet, linux-doc, linux-kernel [-- Attachment #1: Type: text/plain, Size: 246 bytes --] On Wed, Jan 13, 2021 at 09:46:20AM -0800, Nick Desaulniers wrote: > Use more precise language and move the text to a region in the docs to > show that this constraint is not just for SYM_CODE_START*. Reviewed-by: Mark Brown <broonie@kernel.org> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk 2021-01-13 16:59 ` Josh Poimboeuf 2021-01-13 17:46 ` [PATCH] Documentation: asm-annotation: clarify .L local symbol names Nick Desaulniers @ 2021-01-13 17:56 ` Nick Desaulniers 2021-01-14 10:39 ` Borislav Petkov 1 sibling, 1 reply; 30+ messages in thread From: Nick Desaulniers @ 2021-01-13 17:56 UTC (permalink / raw) To: Josh Poimboeuf, Mark Brown Cc: 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 On Wed, Jan 13, 2021 at 8:59 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Tue, Jan 12, 2021 at 09:01:54PM +0000, Mark Brown wrote: > > 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. > > 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. Apologies, that was not my intention. I've sent a follow up in https://lore.kernel.org/lkml/20210113174620.958429-1-ndesaulniers@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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk 2021-01-13 17:56 ` [PATCH v4] x86/entry: emit a symbol for register restoring thunk Nick Desaulniers @ 2021-01-14 10:39 ` Borislav Petkov 2021-01-14 11:36 ` Peter Zijlstra 2021-01-14 15:14 ` [PATCH v4] x86/entry: emit a symbol for register restoring thunk Josh Poimboeuf 0 siblings, 2 replies; 30+ messages in thread From: Borislav Petkov @ 2021-01-14 10:39 UTC (permalink / raw) To: Nick Desaulniers Cc: 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.958429-1-ndesaulniers@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 <ndesaulniers@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 <arnd@arndb.de> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Suggested-by: Borislav Petkov <bp@alien8.de> Suggested-by: Mark Brown <broonie@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Borislav Petkov <bp@suse.de> Link: https://lkml.kernel.org/r/20210112194625.4181814-1-ndesaulniers@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/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 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 diff --git a/include/linux/linkage.h b/include/linux/linkage.h 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 -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk 2021-01-14 10:39 ` Borislav Petkov @ 2021-01-14 11:36 ` Peter Zijlstra 2021-01-14 13:28 ` [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument Borislav Petkov 2021-01-19 10:12 ` [tip: x86/entry] " tip-bot2 for Borislav Petkov 2021-01-14 15:14 ` [PATCH v4] x86/entry: emit a symbol for register restoring thunk Josh Poimboeuf 1 sibling, 2 replies; 30+ messages in thread From: Peter Zijlstra @ 2021-01-14 11:36 UTC (permalink / raw) To: Borislav Petkov Cc: 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 11:39:28AM +0100, Borislav Petkov wrote: > From: Nick Desaulniers <ndesaulniers@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 <arnd@arndb.de> > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> > Suggested-by: Borislav Petkov <bp@alien8.de> > Suggested-by: Mark Brown <broonie@kernel.org> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Borislav Petkov <bp@suse.de> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> And while looking, I suppose we can delete the put_ret_addr_in_rdi crud, but that's another patch. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument 2021-01-14 11:36 ` Peter Zijlstra @ 2021-01-14 13:28 ` Borislav Petkov 2021-01-14 15:53 ` Peter Zijlstra 2021-01-19 10:12 ` [tip: x86/entry] " tip-bot2 for Borislav Petkov 1 sibling, 1 reply; 30+ messages in thread From: Borislav Petkov @ 2021-01-14 13:28 UTC (permalink / raw) To: Peter Zijlstra Cc: 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 <bp@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 <peterz@infradead.org> Signed-off-by: Borislav Petkov <bp@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) -- 2.29.2 -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument 2021-01-14 13:28 ` [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument Borislav Petkov @ 2021-01-14 15:53 ` Peter Zijlstra 0 siblings, 0 replies; 30+ messages in thread From: Peter Zijlstra @ 2021-01-14 15:53 UTC (permalink / raw) To: Borislav Petkov Cc: 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 <bp@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 <peterz@infradead.org> > Signed-off-by: Borislav Petkov <bp@suse.de> Acked-by; Peter Zijlstra (Intel) <peterz@infradead.org> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [tip: x86/entry] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument 2021-01-14 11:36 ` Peter Zijlstra 2021-01-14 13:28 ` [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument Borislav Petkov @ 2021-01-19 10:12 ` tip-bot2 for Borislav Petkov 1 sibling, 0 replies; 30+ messages in thread From: tip-bot2 for Borislav Petkov @ 2021-01-19 10:12 UTC (permalink / raw) To: linux-tip-commits Cc: Peter Zijlstra (Intel), Borislav Petkov, x86, linux-kernel The following commit has been merged into the x86/entry branch of tip: Commit-ID: 0bab9cb2d980d7c075cffb9216155f7835237f98 Gitweb: https://git.kernel.org/tip/0bab9cb2d980d7c075cffb9216155f7835237f98 Author: Borislav Petkov <bp@suse.de> AuthorDate: Thu, 14 Jan 2021 14:25:35 +01:00 Committer: Borislav Petkov <bp@suse.de> CommitterDate: Tue, 19 Jan 2021 11:06:14 +01:00 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 (Intel) <peterz@infradead.org> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Signed-off-by: Borislav Petkov <bp@suse.de> Link: https://lkml.kernel.org/r/YAAszZJ2GcIYZmB5@hirez.programming.kicks-ass.net --- 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 c9a9fbf..496b11e 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) ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk 2021-01-14 10:39 ` Borislav Petkov 2021-01-14 11:36 ` Peter Zijlstra @ 2021-01-14 15:14 ` Josh Poimboeuf 1 sibling, 0 replies; 30+ messages in thread From: Josh Poimboeuf @ 2021-01-14 15:14 UTC (permalink / raw) To: Borislav Petkov Cc: 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: > 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.958429-1-ndesaulniers@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 <ndesaulniers@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. ] Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> -- Josh ^ permalink raw reply [flat|nested] 30+ messages in thread
* [tip: x86/entry] x86/entry: Emit a symbol for register restoring thunk 2021-01-12 19:46 ` [PATCH v4] " Nick Desaulniers 2021-01-12 21:01 ` Mark Brown @ 2021-01-13 11:52 ` tip-bot2 for Nick Desaulniers 2021-01-19 10:12 ` tip-bot2 for Nick Desaulniers 2 siblings, 0 replies; 30+ messages in thread From: tip-bot2 for Nick Desaulniers @ 2021-01-13 11:52 UTC (permalink / raw) To: linux-tip-commits Cc: Arnd Bergmann, Josh Poimboeuf, Borislav Petkov, Nick Desaulniers, Borislav Petkov, x86, linux-kernel The following commit has been merged into the x86/entry branch of tip: Commit-ID: bde718b7e154afc99e1956b18a848401ce8e1f8e Gitweb: https://git.kernel.org/tip/bde718b7e154afc99e1956b18a848401ce8e1f8e Author: Nick Desaulniers <ndesaulniers@google.com> AuthorDate: Tue, 12 Jan 2021 11:46:24 -08:00 Committer: Borislav Petkov <bp@suse.de> CommitterDate: Wed, 13 Jan 2021 12:23:57 +01:00 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 <arnd@arndb.de> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Suggested-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Borislav Petkov <bp@suse.de> Link: https://lkml.kernel.org/r/20210112194625.4181814-1-ndesaulniers@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/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1408485ce69f844dcd7ded093a8 [1] --- 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 32ea574..e711ff9 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/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S index ccd3287..c9a9fbf 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 diff --git a/include/linux/linkage.h b/include/linux/linkage.h index 5bcfbd9..11537ba 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) ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [tip: x86/entry] x86/entry: Emit a symbol for register restoring thunk 2021-01-12 19:46 ` [PATCH v4] " Nick Desaulniers 2021-01-12 21:01 ` Mark Brown 2021-01-13 11:52 ` [tip: x86/entry] x86/entry: Emit " tip-bot2 for Nick Desaulniers @ 2021-01-19 10:12 ` tip-bot2 for Nick Desaulniers 2 siblings, 0 replies; 30+ messages in thread From: tip-bot2 for Nick Desaulniers @ 2021-01-19 10:12 UTC (permalink / raw) To: linux-tip-commits Cc: Arnd Bergmann, Josh Poimboeuf, Borislav Petkov, Mark Brown, Nick Desaulniers, Borislav Petkov, Peter Zijlstra (Intel), x86, linux-kernel The following commit has been merged into the x86/entry branch of tip: Commit-ID: 5e6dca82bcaa49348f9e5fcb48df4881f6d6c4ae Gitweb: https://git.kernel.org/tip/5e6dca82bcaa49348f9e5fcb48df4881f6d6c4ae Author: Nick Desaulniers <ndesaulniers@google.com> AuthorDate: Tue, 12 Jan 2021 11:46:24 -08:00 Committer: Borislav Petkov <bp@suse.de> CommitterDate: Thu, 14 Jan 2021 17:18:25 +01:00 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 <arnd@arndb.de> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Suggested-by: Borislav Petkov <bp@alien8.de> Suggested-by: Mark Brown <broonie@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Borislav Petkov <bp@suse.de> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> Link: https://lkml.kernel.org/r/20210112194625.4181814-1-ndesaulniers@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/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 32ea574..76424e0 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 diff --git a/arch/x86/entry/thunk_64.S b/arch/x86/entry/thunk_64.S index ccd3287..c9a9fbf 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 diff --git a/include/linux/linkage.h b/include/linux/linkage.h index 5bcfbd9..dbf8506 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 */ ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v3] x86/entry: emit a symbol for register restoring thunk 2021-01-12 1:13 ` Nick Desaulniers 2021-01-12 1:59 ` Josh Poimboeuf @ 2021-01-12 11:47 ` Borislav Petkov 1 sibling, 0 replies; 30+ messages in thread From: Borislav Petkov @ 2021-01-12 11:47 UTC (permalink / raw) To: Nick Desaulniers Cc: 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. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2021-01-19 10:39 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-23 23:21 [PATCH] x86/entry: use STB_GLOBAL for register restoring thunk Nick Desaulniers 2020-12-24 4:55 ` Josh Poimboeuf 2021-01-06 0:43 ` [PATCH v2] " Nick Desaulniers 2021-01-06 1:58 ` Josh Poimboeuf 2021-01-11 20:38 ` [PATCH v3] x86/entry: emit a symbol " Nick Desaulniers 2021-01-11 20:58 ` Fangrui Song 2021-01-11 22:09 ` Josh Poimboeuf 2021-01-11 22:16 ` Fāng-ruì Sòng 2021-01-11 22:12 ` Josh Poimboeuf 2021-01-12 0:38 ` Borislav Petkov 2021-01-12 0:41 ` Fāng-ruì Sòng 2021-01-12 1:00 ` Borislav Petkov 2021-01-12 1:13 ` Nick Desaulniers 2021-01-12 1:59 ` Josh Poimboeuf 2021-01-12 11:54 ` Borislav Petkov 2021-01-12 19:46 ` [PATCH v4] " Nick Desaulniers 2021-01-12 21:01 ` Mark Brown 2021-01-13 16:59 ` Josh Poimboeuf 2021-01-13 17:46 ` [PATCH] Documentation: asm-annotation: clarify .L local symbol names Nick Desaulniers 2021-01-13 19:56 ` Mark Brown 2021-01-13 17:56 ` [PATCH v4] x86/entry: emit a symbol for register restoring thunk Nick Desaulniers 2021-01-14 10:39 ` Borislav Petkov 2021-01-14 11:36 ` Peter Zijlstra 2021-01-14 13:28 ` [PATCH] x86/entry: Remove put_ret_addr_in_rdi THUNK macro argument Borislav Petkov 2021-01-14 15:53 ` Peter Zijlstra 2021-01-19 10:12 ` [tip: x86/entry] " tip-bot2 for Borislav Petkov 2021-01-14 15:14 ` [PATCH v4] x86/entry: emit a symbol for register restoring thunk Josh Poimboeuf 2021-01-13 11:52 ` [tip: x86/entry] x86/entry: Emit " tip-bot2 for Nick Desaulniers 2021-01-19 10:12 ` tip-bot2 for Nick Desaulniers 2021-01-12 11:47 ` [PATCH v3] x86/entry: emit " Borislav Petkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).