* 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-14 11:49 ` [Linux-kernel-mentees] Fwd: " Lukas Bulwahn
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, 2 replies; 33+ 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] 33+ 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
2021-01-14 11:49 ` [Linux-kernel-mentees] Fwd: " Lukas Bulwahn
1 sibling, 2 replies; 33+ 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] 33+ 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; 33+ 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] 33+ 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; 33+ 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] 33+ 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; 33+ 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] 33+ 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; 33+ 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] 33+ 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; 33+ 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] 33+ 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; 33+ 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] 33+ 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; 33+ 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] 33+ 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; 33+ 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] 33+ 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; 33+ 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] 33+ messages in thread
* [Linux-kernel-mentees] Fwd: [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-14 11:49 ` Lukas Bulwahn
2021-01-14 14:21 ` Aditya
1 sibling, 1 reply; 33+ messages in thread
From: Lukas Bulwahn @ 2021-01-14 11:49 UTC (permalink / raw)
To: Dwaipayan Ray, Aditya Srivastava; +Cc: Joe Perches, linux-kernel-mentees
[-- Attachment #1: Type: text/plain, Size: 2612 bytes --]
Dwaipayan and Aditya, here is another candidate for a checkpatch rule.
Lukas
---------- Forwarded message ---------
From: Mark Brown <broonie@kernel.org>
Date: Tue, Jan 12, 2021 at 10:02 PM
Subject: Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>, Thomas Gleixner
<tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov
<bp@alien8.de>, Fangrui Song <maskray@google.com>, Arnd Bergmann
<arnd@arndb.de>, Josh Poimboeuf <jpoimboe@redhat.com>, Jonathan Corbet
<corbet@lwn.net>, <x86@kernel.org>, H. Peter Anvin <hpa@zytor.com>,
Nathan Chancellor <natechancellor@gmail.com>, Miguel Ojeda
<ojeda@kernel.org>, Jiri Slaby <jirislaby@kernel.org>, Joe Perches
<joe@perches.com>, <linux-doc@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <clang-built-linux@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.
--
You received this message because you are subscribed to the Google
Groups "Clang Built Linux" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to clang-built-linux+unsubscribe@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/clang-built-linux/20210112210154.GI4646%40sirena.org.uk.
[-- Attachment #2: signature.asc --]
[-- Type: text/plain, Size: 499 bytes --]
-----BEGIN PGP SIGNATURE-----
iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl/+DkEACgkQJNaLcl1U
h9Cq7wf/T25FXiQQZ1vqQuvuSs9y7UidITjNn00or2An/9AZvjd7fNSIGitE6JZF
ulLa3Dnm6DnJ8IHxCom/f9HA0Bhqduottun00gPpRE4yYnc6I6rs4+jD3D3yaM+e
P48KYy2zaNF6Xgud3wdMdBWrO6wHbpk/FtrGu9myxKXbPoaWXCc/2JO+lQGHy2Ld
stRoPLzuNHGsqiiQyQVFUCJcva6y2q5UTYqWG21loFirQ0khEI1aHwVuifeddpjE
JamhRqQhmlTD8qRrnf8c4iVj6oexQsKzjOKkaKX2qpyYhK8bxdgvG8r0kwJfnuZM
LObeotMsynR86MF2K/fVFTw7r4fXMw==
=H9dS
-----END PGP SIGNATURE-----
[-- Attachment #3: Type: text/plain, Size: 201 bytes --]
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Linux-kernel-mentees] Fwd: [PATCH v4] x86/entry: emit a symbol for register restoring thunk
2021-01-14 11:49 ` [Linux-kernel-mentees] Fwd: " Lukas Bulwahn
@ 2021-01-14 14:21 ` Aditya
2021-01-14 14:35 ` Lukas Bulwahn
0 siblings, 1 reply; 33+ messages in thread
From: Aditya @ 2021-01-14 14:21 UTC (permalink / raw)
To: Lukas Bulwahn, Dwaipayan Ray; +Cc: Joe Perches, linux-kernel-mentees
On 14/1/21 5:19 pm, Lukas Bulwahn wrote:
> Dwaipayan and Aditya, here is another candidate for a checkpatch rule.
>
> Lukas
>
> ---------- Forwarded message ---------
> From: Mark Brown <broonie@kernel.org>
> Date: Tue, Jan 12, 2021 at 10:02 PM
> Subject: Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk
> To: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Andy Lutomirski <luto@kernel.org>, Thomas Gleixner
> <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov
> <bp@alien8.de>, Fangrui Song <maskray@google.com>, Arnd Bergmann
> <arnd@arndb.de>, Josh Poimboeuf <jpoimboe@redhat.com>, Jonathan Corbet
> <corbet@lwn.net>, <x86@kernel.org>, H. Peter Anvin <hpa@zytor.com>,
> Nathan Chancellor <natechancellor@gmail.com>, Miguel Ojeda
> <ojeda@kernel.org>, Jiri Slaby <jirislaby@kernel.org>, Joe Perches
> <joe@perches.com>, <linux-doc@vger.kernel.org>,
> <linux-kernel@vger.kernel.org>, <clang-built-linux@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.
>
Thanks Lukas!
I'd like to take a shot at it. This is what I have understood from the
discussion:
Introduce a new warning rule for .S files, where ".L" prefixes are
used in the code, if it is occurring between any kind of START/END pair.
This is what I am planning to do:
- Check if the file is ".S" file.
- If the line contains ".L" prefixed symbol, give user a
warning/check, so that they can ensure that the line is not inside
START/END block. (As we may not be able to make sure about the same,
if the START/END line is not in the patch; otherwise we could run a
while loop)
What do you think?
Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Linux-kernel-mentees] Fwd: [PATCH v4] x86/entry: emit a symbol for register restoring thunk
2021-01-14 14:21 ` Aditya
@ 2021-01-14 14:35 ` Lukas Bulwahn
0 siblings, 0 replies; 33+ messages in thread
From: Lukas Bulwahn @ 2021-01-14 14:35 UTC (permalink / raw)
To: Aditya; +Cc: Joe Perches, Dwaipayan Ray, linux-kernel-mentees
Sounds good to me.
On Thu, Jan 14, 2021 at 3:21 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 14/1/21 5:19 pm, Lukas Bulwahn wrote:
> > Dwaipayan and Aditya, here is another candidate for a checkpatch rule.
> >
> > Lukas
> >
> > ---------- Forwarded message ---------
> > From: Mark Brown <broonie@kernel.org>
> > Date: Tue, Jan 12, 2021 at 10:02 PM
> > Subject: Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk
> > To: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: Andy Lutomirski <luto@kernel.org>, Thomas Gleixner
> > <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov
> > <bp@alien8.de>, Fangrui Song <maskray@google.com>, Arnd Bergmann
> > <arnd@arndb.de>, Josh Poimboeuf <jpoimboe@redhat.com>, Jonathan Corbet
> > <corbet@lwn.net>, <x86@kernel.org>, H. Peter Anvin <hpa@zytor.com>,
> > Nathan Chancellor <natechancellor@gmail.com>, Miguel Ojeda
> > <ojeda@kernel.org>, Jiri Slaby <jirislaby@kernel.org>, Joe Perches
> > <joe@perches.com>, <linux-doc@vger.kernel.org>,
> > <linux-kernel@vger.kernel.org>, <clang-built-linux@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.
> >
>
> Thanks Lukas!
> I'd like to take a shot at it. This is what I have understood from the
> discussion:
> Introduce a new warning rule for .S files, where ".L" prefixes are
> used in the code, if it is occurring between any kind of START/END pair.
> This is what I am planning to do:
> - Check if the file is ".S" file.
> - If the line contains ".L" prefixed symbol, give user a
> warning/check, so that they can ensure that the line is not inside
> START/END block. (As we may not be able to make sure about the same,
> if the START/END line is not in the patch; otherwise we could run a
> while loop)
>
> What do you think?
>
> Thanks
> Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 33+ 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; 33+ 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] 33+ 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; 33+ 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] 33+ messages in thread