* [tip: objtool/core] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-20 0:17 [PATCH] x86/speculation, objtool: Use absolute relocations for annotations Fangrui Song
@ 2023-09-21 7:12 ` tip-bot2 for Fangrui Song
2023-09-21 7:27 ` Peter Zijlstra
2023-09-21 7:26 ` [PATCH] " Peter Zijlstra
2023-09-22 9:51 ` [tip: objtool/core] " tip-bot2 for Fangrui Song
2 siblings, 1 reply; 15+ messages in thread
From: tip-bot2 for Fangrui Song @ 2023-09-21 7:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: Fangrui Song, Ingo Molnar, Josh Poimboeuf, Peter Zijlstra, x86,
linux-kernel
The following commit has been merged into the objtool/core branch of tip:
Commit-ID: 0ca0043d89930cb162070598e7e4a9ed3fe57795
Gitweb: https://git.kernel.org/tip/0ca0043d89930cb162070598e7e4a9ed3fe57795
Author: Fangrui Song <maskray@google.com>
AuthorDate: Tue, 19 Sep 2023 17:17:28 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 21 Sep 2023 08:42:38 +02:00
x86/speculation, objtool: Use absolute relocations for annotations
.discard.retpoline_safe sections do not have the SHF_ALLOC flag. These
sections referencing text sections' STT_SECTION symbols with PC-relative
relocations like R_386_PC32 [0] is conceptually not suitable. Newer
LLD will report warnings for REL relocations even for relocatable links [1]:
ld.lld: warning: vmlinux.a(drivers/i2c/busses/i2c-i801.o):(.discard.retpoline_safe+0x120): has non-ABS relocation R_386_PC32 against symbol ''
Switch to absolute relocations instead, which indicate link-time
addresses. In a relocatable link, these addresses are also output
section offsets, used by checks in tools/objtool/check.c. When linking
vmlinux, these .discard.* sections will be discarded, therefore it is
not a problem that R_X86_64_32 cannot represent a kernel address.
Alternatively, we could set the SHF_ALLOC flag for .discard.* sections,
but I think non-SHF_ALLOC for sections to be discarded makes more sense.
Note: if we decide to never support REL architectures (e.g. arm, i386),
we can utilize R_*_NONE relocations (.reloc ., BFD_RELOC_NONE, sym),
making .discard.* sections zero-sized. That said, the section content
waste is 4 bytes per entry, much smaller than sizeof(Elf{32,64}_Rel).
[0] commit 1c0c1faf5692 ("objtool: Use relative pointers for annotations")
[1] https://github.com/ClangBuiltLinux/linux/issues/1937
Signed-off-by: Fangrui Song <maskray@google.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20230920001728.1439947-1-maskray@google.com
---
arch/x86/include/asm/alternative.h | 4 ++--
arch/x86/include/asm/nospec-branch.h | 4 ++--
include/linux/objtool.h | 10 +++++-----
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 9c4da69..65f7909 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -58,7 +58,7 @@
#define ANNOTATE_IGNORE_ALTERNATIVE \
"999:\n\t" \
".pushsection .discard.ignore_alts\n\t" \
- ".long 999b - .\n\t" \
+ ".long 999b\n\t" \
".popsection\n\t"
/*
@@ -352,7 +352,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
.macro ANNOTATE_IGNORE_ALTERNATIVE
.Lannotate_\@:
.pushsection .discard.ignore_alts
- .long .Lannotate_\@ - .
+ .long .Lannotate_\@
.popsection
.endm
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index c55cc24..4952b73 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -196,7 +196,7 @@
.macro ANNOTATE_RETPOLINE_SAFE
.Lhere_\@:
.pushsection .discard.retpoline_safe
- .long .Lhere_\@ - .
+ .long .Lhere_\@
.popsection
.endm
@@ -334,7 +334,7 @@
#define ANNOTATE_RETPOLINE_SAFE \
"999:\n\t" \
".pushsection .discard.retpoline_safe\n\t" \
- ".long 999b - .\n\t" \
+ ".long 999b\n\t" \
".popsection\n\t"
typedef u8 retpoline_thunk_t[RETPOLINE_THUNK_SIZE];
diff --git a/include/linux/objtool.h b/include/linux/objtool.h
index 03f82c2..6f6da95 100644
--- a/include/linux/objtool.h
+++ b/include/linux/objtool.h
@@ -48,13 +48,13 @@
#define ANNOTATE_NOENDBR \
"986: \n\t" \
".pushsection .discard.noendbr\n\t" \
- ".long 986b - .\n\t" \
+ ".long 986b\n\t" \
".popsection\n\t"
#define ASM_REACHABLE \
"998:\n\t" \
".pushsection .discard.reachable\n\t" \
- ".long 998b - .\n\t" \
+ ".long 998b\n\t" \
".popsection\n\t"
#else /* __ASSEMBLY__ */
@@ -66,7 +66,7 @@
#define ANNOTATE_INTRA_FUNCTION_CALL \
999: \
.pushsection .discard.intra_function_calls; \
- .long 999b - .; \
+ .long 999b; \
.popsection;
/*
@@ -118,7 +118,7 @@
.macro ANNOTATE_NOENDBR
.Lhere_\@:
.pushsection .discard.noendbr
- .long .Lhere_\@ - .
+ .long .Lhere_\@
.popsection
.endm
@@ -141,7 +141,7 @@
.macro REACHABLE
.Lhere_\@:
.pushsection .discard.reachable
- .long .Lhere_\@ - .
+ .long .Lhere_\@
.popsection
.endm
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [tip: objtool/core] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 7:12 ` [tip: objtool/core] " tip-bot2 for Fangrui Song
@ 2023-09-21 7:27 ` Peter Zijlstra
2023-09-21 8:03 ` Ingo Molnar
0 siblings, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2023-09-21 7:27 UTC (permalink / raw)
To: linux-kernel
Cc: linux-tip-commits, Fangrui Song, Ingo Molnar, Josh Poimboeuf, x86
On Thu, Sep 21, 2023 at 07:12:27AM -0000, tip-bot2 for Fangrui Song wrote:
> The following commit has been merged into the objtool/core branch of tip:
>
> Commit-ID: 0ca0043d89930cb162070598e7e4a9ed3fe57795
> Gitweb: https://git.kernel.org/tip/0ca0043d89930cb162070598e7e4a9ed3fe57795
> Author: Fangrui Song <maskray@google.com>
> AuthorDate: Tue, 19 Sep 2023 17:17:28 -07:00
> Committer: Ingo Molnar <mingo@kernel.org>
> CommitterDate: Thu, 21 Sep 2023 08:42:38 +02:00
Ingo, can we please make this patch go away until further clarified?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [tip: objtool/core] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 7:27 ` Peter Zijlstra
@ 2023-09-21 8:03 ` Ingo Molnar
0 siblings, 0 replies; 15+ messages in thread
From: Ingo Molnar @ 2023-09-21 8:03 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-tip-commits, Fangrui Song, Josh Poimboeuf,
x86
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Sep 21, 2023 at 07:12:27AM -0000, tip-bot2 for Fangrui Song wrote:
> > The following commit has been merged into the objtool/core branch of tip:
> >
> > Commit-ID: 0ca0043d89930cb162070598e7e4a9ed3fe57795
> > Gitweb: https://git.kernel.org/tip/0ca0043d89930cb162070598e7e4a9ed3fe57795
> > Author: Fangrui Song <maskray@google.com>
> > AuthorDate: Tue, 19 Sep 2023 17:17:28 -07:00
> > Committer: Ingo Molnar <mingo@kernel.org>
> > CommitterDate: Thu, 21 Sep 2023 08:42:38 +02:00
>
> Ingo, can we please make this patch go away until further clarified?
Yeah, already gone.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-20 0:17 [PATCH] x86/speculation, objtool: Use absolute relocations for annotations Fangrui Song
2023-09-21 7:12 ` [tip: objtool/core] " tip-bot2 for Fangrui Song
@ 2023-09-21 7:26 ` Peter Zijlstra
2023-09-21 7:58 ` Fangrui Song
2023-09-22 9:51 ` [tip: objtool/core] " tip-bot2 for Fangrui Song
2 siblings, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2023-09-21 7:26 UTC (permalink / raw)
To: Fangrui Song; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Tue, Sep 19, 2023 at 05:17:28PM -0700, Fangrui Song wrote:
> .discard.retpoline_safe sections do not have the SHF_ALLOC flag. These
> sections referencing text sections' STT_SECTION symbols with PC-relative
> relocations like R_386_PC32 [0] is conceptually not suitable. Newer
> LLD will report warnings for REL relocations even for relocatable links
> [1].
>
> ld.lld: warning: vmlinux.a(drivers/i2c/busses/i2c-i801.o):(.discard.retpoline_safe+0x120): has non-ABS relocation R_386_PC32 against symbol ''
What, why ?!? Please explain more.
> Switch to absolute relocations instead, which indicate link-time
> addresses. In a relocatable link, these addresses are also output
> section offsets, used by checks in tools/objtool/check.c. When linking
> vmlinux, these .discard.* sections will be discarded, therefore it is
> not a problem that R_X86_64_32 cannot represent a kernel address.
>
> Alternatively, we could set the SHF_ALLOC flag for .discard.* sections,
> but I think non-SHF_ALLOC for sections to be discarded makes more sense.
>
> Note: if we decide to never support REL architectures (e.g. arm, i386),
We have explicit support for REL (as opposed to RELA) architectures, so
I don't think we can do that.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 7:26 ` [PATCH] " Peter Zijlstra
@ 2023-09-21 7:58 ` Fangrui Song
2023-09-21 15:35 ` Peter Zijlstra
0 siblings, 1 reply; 15+ messages in thread
From: Fangrui Song @ 2023-09-21 7:58 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Thu, Sep 21, 2023 at 12:26 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Tue, Sep 19, 2023 at 05:17:28PM -0700, Fangrui Song wrote:
> > .discard.retpoline_safe sections do not have the SHF_ALLOC flag. These
> > sections referencing text sections' STT_SECTION symbols with PC-relative
> > relocations like R_386_PC32 [0] is conceptually not suitable. Newer
> > LLD will report warnings for REL relocations even for relocatable links
> > [1].
> >
> > ld.lld: warning: vmlinux.a(drivers/i2c/busses/i2c-i801.o):(.discard.retpoline_safe+0x120): has non-ABS relocation R_386_PC32 against symbol ''
>
> What, why ?!? Please explain more.
This can be read as a pedantic warning from the linker.
A location relocated by an R_386_PC32 relocation in
.discard.retpoline_safe records an offset from the current location
(non-allocable) to an text symbol.
This offset is conceptually not suitable: in the ELF object file
format's model, the non-SHF_ALLOC section is not part of the memory
image, so
we cannot say that the offset from the non-memory thing to a text
symbol is a fixed value.
> > Switch to absolute relocations instead, which indicate link-time
> > addresses. In a relocatable link, these addresses are also output
> > section offsets, used by checks in tools/objtool/check.c. When linking
> > vmlinux, these .discard.* sections will be discarded, therefore it is
> > not a problem that R_X86_64_32 cannot represent a kernel address.
> >
> > Alternatively, we could set the SHF_ALLOC flag for .discard.* sections,
> > but I think non-SHF_ALLOC for sections to be discarded makes more sense.
> >
> > Note: if we decide to never support REL architectures (e.g. arm, i386),
>
> We have explicit support for REL (as opposed to RELA) architectures, so
> I don't think we can do that.
>
--
宋方睿
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 7:58 ` Fangrui Song
@ 2023-09-21 15:35 ` Peter Zijlstra
2023-09-21 16:26 ` Fangrui Song
0 siblings, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2023-09-21 15:35 UTC (permalink / raw)
To: Fangrui Song; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Thu, Sep 21, 2023 at 12:58:13AM -0700, Fangrui Song wrote:
> On Thu, Sep 21, 2023 at 12:26 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Tue, Sep 19, 2023 at 05:17:28PM -0700, Fangrui Song wrote:
> > > .discard.retpoline_safe sections do not have the SHF_ALLOC flag. These
> > > sections referencing text sections' STT_SECTION symbols with PC-relative
> > > relocations like R_386_PC32 [0] is conceptually not suitable. Newer
> > > LLD will report warnings for REL relocations even for relocatable links
> > > [1].
> > >
> > > ld.lld: warning: vmlinux.a(drivers/i2c/busses/i2c-i801.o):(.discard.retpoline_safe+0x120): has non-ABS relocation R_386_PC32 against symbol ''
> >
> > What, why ?!? Please explain more.
>
> This can be read as a pedantic warning from the linker.
>
> A location relocated by an R_386_PC32 relocation in
> .discard.retpoline_safe records an offset from the current location
> (non-allocable) to an text symbol.
> This offset is conceptually not suitable: in the ELF object file
> format's model, the non-SHF_ALLOC section is not part of the memory
> image, so
> we cannot say that the offset from the non-memory thing to a text
> symbol is a fixed value.
Bah, so why has this worked at all then? Clearly the linkers aren't very
strict about things.
Anyway, I think what we want is to just mark the section SHF_ALLOC. The
reason is that one of the plans we have is to collapse all the different
annotations into a single section and then have something like:
struct objtoo_annotation {
s32 location;
u32 type;
}
So that we can easily extend the annotations and don't need to add
yet-another-section-reader-function to objtool.
This is just one of the things we've not gotten around to yet. But as
is, we have:
.discard.unreachable
.discard.reachable
.discard.func_stack_frame_non_standard
.discard.ignore_alts
.discard.unwind_hints
.discard.noendbr
.discard.retpoline_safe
.discard.instr_end
.discard.instr_begin
.discard.validate_unret
.discard.intra_function_calls
And with the exception of unwind_hints, they're all just trivial
location things.
The very last thing we need is yet more of that.
If we were to use absolute things, we get 12 byte entries and while that
probably wouldn't spell the end of the world, why make thing larger than
they have to be.
After all, its not like any of this actually survives the final link.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 15:35 ` Peter Zijlstra
@ 2023-09-21 16:26 ` Fangrui Song
2023-09-21 17:19 ` Peter Zijlstra
0 siblings, 1 reply; 15+ messages in thread
From: Fangrui Song @ 2023-09-21 16:26 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Thu, Sep 21, 2023 at 8:35 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Sep 21, 2023 at 12:58:13AM -0700, Fangrui Song wrote:
> > On Thu, Sep 21, 2023 at 12:26 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Tue, Sep 19, 2023 at 05:17:28PM -0700, Fangrui Song wrote:
> > > > .discard.retpoline_safe sections do not have the SHF_ALLOC flag. These
> > > > sections referencing text sections' STT_SECTION symbols with PC-relative
> > > > relocations like R_386_PC32 [0] is conceptually not suitable. Newer
> > > > LLD will report warnings for REL relocations even for relocatable links
> > > > [1].
> > > >
> > > > ld.lld: warning: vmlinux.a(drivers/i2c/busses/i2c-i801.o):(.discard.retpoline_safe+0x120): has non-ABS relocation R_386_PC32 against symbol ''
> > >
> > > What, why ?!? Please explain more.
> >
> > This can be read as a pedantic warning from the linker.
> >
> > A location relocated by an R_386_PC32 relocation in
> > .discard.retpoline_safe records an offset from the current location
> > (non-allocable) to an text symbol.
> > This offset is conceptually not suitable: in the ELF object file
> > format's model, the non-SHF_ALLOC section is not part of the memory
> > image, so
> > we cannot say that the offset from the non-memory thing to a text
> > symbol is a fixed value.
>
> Bah, so why has this worked at all then? Clearly the linkers aren't very
> strict about things.
GNU ld isn't very strict, but LLD has had a warning for
non-relocatable links for a long time
(https://github.com/llvm/llvm-project/blob/main/lld/test/ELF/non-abs-reloc.s).
LLD just did not report warnings for relocatable links.
> Anyway, I think what we want is to just mark the section SHF_ALLOC. The
> reason is that one of the plans we have is to collapse all the different
> annotations into a single section and then have something like:
>
> struct objtoo_annotation {
> s32 location;
> u32 type;
> }
>
> So that we can easily extend the annotations and don't need to add
> yet-another-section-reader-function to objtool.
>
> This is just one of the things we've not gotten around to yet. But as
> is, we have:
>
> .discard.unreachable
> .discard.reachable
> .discard.func_stack_frame_non_standard
> .discard.ignore_alts
> .discard.unwind_hints
> .discard.noendbr
> .discard.retpoline_safe
> .discard.instr_end
> .discard.instr_begin
> .discard.validate_unret
> .discard.intra_function_calls
>
> And with the exception of unwind_hints, they're all just trivial
> location things.
>
> The very last thing we need is yet more of that.
If these sections are guaranteed to be discarded (*(.discard.*) in
scripts/module.lds.S and include/asm-generic/vmlinux.lds.h),
using non-SHF_ALLOC sections isn't a bad choice.
The intention is actually clearer.
> If we were to use absolute things, we get 12 byte entries and while that
> probably wouldn't spell the end of the world, why make thing larger than
> they have to be.
>
> After all, its not like any of this actually survives the final link.
I do not see why absolute things need 12 byte entries.
We can freely use `.long .text.foo` even in ELFCLASS64 object files.
There is no risk of overflow (the ultimate link .text.foo may have an
address of 0xffffffff........) since the section will be discarded.
Referencing SHF_ALLOC sections with absolute relocations in
non-SHF_ALLOC sections has well-defined semantics, as used by .debug_*
sections.
--
宋方睿
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 16:26 ` Fangrui Song
@ 2023-09-21 17:19 ` Peter Zijlstra
2023-09-21 17:36 ` Fangrui Song
0 siblings, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2023-09-21 17:19 UTC (permalink / raw)
To: Fangrui Song; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Thu, Sep 21, 2023 at 09:26:43AM -0700, Fangrui Song wrote:
> I do not see why absolute things need 12 byte entries.
> We can freely use `.long .text.foo` even in ELFCLASS64 object files.
> There is no risk of overflow (the ultimate link .text.foo may have an
> address of 0xffffffff........) since the section will be discarded.
And you're sure no toolchain is going to be clever and tell me that the
absolute relocation will want to be 8 bytes and does not fit in the 4
bytes allotted?
Because clearly that is something some clever assembler is going to
complain about any day now.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 17:19 ` Peter Zijlstra
@ 2023-09-21 17:36 ` Fangrui Song
2023-09-21 19:22 ` Peter Zijlstra
0 siblings, 1 reply; 15+ messages in thread
From: Fangrui Song @ 2023-09-21 17:36 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Thu, Sep 21, 2023 at 10:19 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Sep 21, 2023 at 09:26:43AM -0700, Fangrui Song wrote:
>
> > I do not see why absolute things need 12 byte entries.
> > We can freely use `.long .text.foo` even in ELFCLASS64 object files.
> > There is no risk of overflow (the ultimate link .text.foo may have an
> > address of 0xffffffff........) since the section will be discarded.
>
> And you're sure no toolchain is going to be clever and tell me that the
> absolute relocation will want to be 8 bytes and does not fit in the 4
> bytes allotted?
> Because clearly that is something some clever assembler is going to
> complain about any day now.
Well, only if the clever assembler doesn't support 32-bit absolute
relocation for a 64-bit architecture.
I don't know such an architecture. In addition, as long as the
architecture intends to support DWARF32, it has to support 32-bit
absolute relocations for a 64-bit architecture.
Of course, I cannot predict new toolchains for new architectures from
doing insensible thing, but
DWARF32 support and other metadata section uses are pretty strong
arguments for them to add a 32-bit absolute relocation type.
Note: some .discard.* sections before commit 1c0c1faf5692 ("objtool:
Use relative pointers for annotations") used
absolute relocations.
--
宋方睿
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 17:36 ` Fangrui Song
@ 2023-09-21 19:22 ` Peter Zijlstra
2023-09-21 19:31 ` Peter Zijlstra
2023-09-21 20:07 ` Fangrui Song
0 siblings, 2 replies; 15+ messages in thread
From: Peter Zijlstra @ 2023-09-21 19:22 UTC (permalink / raw)
To: Fangrui Song; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Thu, Sep 21, 2023 at 10:36:27AM -0700, Fangrui Song wrote:
> Well, only if the clever assembler doesn't support 32-bit absolute
> relocation for a 64-bit architecture.
> I don't know such an architecture. In addition, as long as the
> architecture intends to support DWARF32, it has to support 32-bit
> absolute relocations for a 64-bit architecture.
Ooh... my bad. For some reason I thought that absolute meant native word
size. But you already mentioned R_X86_64_32 (and I failed to check) and
that is indeed an absolute (S+A) relocation of 32bit (dword) size.
And apparently we also have R_X64_64_16 and R_X86_64_8, which would even
allow something like:
#define OBJTOOL_ANNOTATE(type) \
"999:\n\t" \
".pushsection .discard.objtool_annotate\n\t" \
".byte 999b\n\t" \
".byte " __stringify(type) "\n\t" \
".popsection\n\t"
And since we only read the relocation and don't care for the actual
result that would actually work just fine.
Anyway, thanks for bearing with me.
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 19:22 ` Peter Zijlstra
@ 2023-09-21 19:31 ` Peter Zijlstra
2023-09-21 20:07 ` Fangrui Song
1 sibling, 0 replies; 15+ messages in thread
From: Peter Zijlstra @ 2023-09-21 19:31 UTC (permalink / raw)
To: Fangrui Song; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Thu, Sep 21, 2023 at 09:22:53PM +0200, Peter Zijlstra wrote:
> On Thu, Sep 21, 2023 at 10:36:27AM -0700, Fangrui Song wrote:
>
> > Well, only if the clever assembler doesn't support 32-bit absolute
> > relocation for a 64-bit architecture.
> > I don't know such an architecture. In addition, as long as the
> > architecture intends to support DWARF32, it has to support 32-bit
> > absolute relocations for a 64-bit architecture.
>
> Ooh... my bad. For some reason I thought that absolute meant native word
> size. But you already mentioned R_X86_64_32 (and I failed to check) and
> that is indeed an absolute (S+A) relocation of 32bit (dword) size.
>
> And apparently we also have R_X64_64_16 and R_X86_64_8, which would even
> allow something like:
Hurm, just checked PPC/PPC64 and ARM64 and they only do 16bit (and up)
absolute relocations, not the single byte form.
So if I want to keep this portable, I suppose I shouldn't go smaller.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 19:22 ` Peter Zijlstra
2023-09-21 19:31 ` Peter Zijlstra
@ 2023-09-21 20:07 ` Fangrui Song
2023-09-21 20:18 ` Peter Zijlstra
1 sibling, 1 reply; 15+ messages in thread
From: Fangrui Song @ 2023-09-21 20:07 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Thu, Sep 21, 2023 at 12:22 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Sep 21, 2023 at 10:36:27AM -0700, Fangrui Song wrote:
>
> > Well, only if the clever assembler doesn't support 32-bit absolute
> > relocation for a 64-bit architecture.
> > I don't know such an architecture. In addition, as long as the
> > architecture intends to support DWARF32, it has to support 32-bit
> > absolute relocations for a 64-bit architecture.
>
> Ooh... my bad. For some reason I thought that absolute meant native word
> size. But you already mentioned R_X86_64_32 (and I failed to check) and
> that is indeed an absolute (S+A) relocation of 32bit (dword) size.
>
> And apparently we also have R_X64_64_16 and R_X86_64_8, which would even
> allow something like:
>
> #define OBJTOOL_ANNOTATE(type) \
> "999:\n\t" \
> ".pushsection .discard.objtool_annotate\n\t" \
> ".byte 999b\n\t" \
> ".byte " __stringify(type) "\n\t" \
> ".popsection\n\t"
>
> And since we only read the relocation and don't care for the actual
> result that would actually work just fine.
>
> Anyway, thanks for bearing with me.
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Thanks! I assume that the patch can be picked up without me doing anything :)
Yes, x86 also supports 1-byte and 2-byte absolute relocation types,
which are missing from some other architectures.
If we want to optimize for RELA, we can use R_*_NONE by utilizing
`.reloc ., BFD_RELOC_NONE, .text.foo` (as mentioned in the commit
message).
(binutils 2.26 https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=740bdc67c057ee8012327420848eb134e1db4211;
I added the support to most LLVM targets in 2021).
However, this is just 4 byte per entry difference compared with the
large sizeof(Elf{32,64}_Rel), so not worth the trouble:)
--
宋方睿
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-21 20:07 ` Fangrui Song
@ 2023-09-21 20:18 ` Peter Zijlstra
0 siblings, 0 replies; 15+ messages in thread
From: Peter Zijlstra @ 2023-09-21 20:18 UTC (permalink / raw)
To: Fangrui Song; +Cc: x86, Josh Poimboeuf, linux-kernel, llvm, Nick Desaulniers
On Thu, Sep 21, 2023 at 01:07:09PM -0700, Fangrui Song wrote:
> On Thu, Sep 21, 2023 at 12:22 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > Anyway, thanks for bearing with me.
> >
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> Thanks! I assume that the patch can be picked up without me doing anything :)
Yep, we'll get it sorted. Thanks!
^ permalink raw reply [flat|nested] 15+ messages in thread
* [tip: objtool/core] x86/speculation, objtool: Use absolute relocations for annotations
2023-09-20 0:17 [PATCH] x86/speculation, objtool: Use absolute relocations for annotations Fangrui Song
2023-09-21 7:12 ` [tip: objtool/core] " tip-bot2 for Fangrui Song
2023-09-21 7:26 ` [PATCH] " Peter Zijlstra
@ 2023-09-22 9:51 ` tip-bot2 for Fangrui Song
2 siblings, 0 replies; 15+ messages in thread
From: tip-bot2 for Fangrui Song @ 2023-09-22 9:51 UTC (permalink / raw)
To: linux-tip-commits
Cc: Fangrui Song, Ingo Molnar, Peter Zijlstra (Intel), Josh Poimboeuf,
x86, linux-kernel
The following commit has been merged into the objtool/core branch of tip:
Commit-ID: b8ec60e1186cdcfce41e7db4c827cb107e459002
Gitweb: https://git.kernel.org/tip/b8ec60e1186cdcfce41e7db4c827cb107e459002
Author: Fangrui Song <maskray@google.com>
AuthorDate: Tue, 19 Sep 2023 17:17:28 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 22 Sep 2023 11:41:24 +02:00
x86/speculation, objtool: Use absolute relocations for annotations
.discard.retpoline_safe sections do not have the SHF_ALLOC flag. These
sections referencing text sections' STT_SECTION symbols with PC-relative
relocations like R_386_PC32 [0] is conceptually not suitable. Newer
LLD will report warnings for REL relocations even for relocatable links [1]:
ld.lld: warning: vmlinux.a(drivers/i2c/busses/i2c-i801.o):(.discard.retpoline_safe+0x120): has non-ABS relocation R_386_PC32 against symbol ''
Switch to absolute relocations instead, which indicate link-time
addresses. In a relocatable link, these addresses are also output
section offsets, used by checks in tools/objtool/check.c. When linking
vmlinux, these .discard.* sections will be discarded, therefore it is
not a problem that R_X86_64_32 cannot represent a kernel address.
Alternatively, we could set the SHF_ALLOC flag for .discard.* sections,
but I think non-SHF_ALLOC for sections to be discarded makes more sense.
Note: if we decide to never support REL architectures (e.g. arm, i386),
we can utilize R_*_NONE relocations (.reloc ., BFD_RELOC_NONE, sym),
making .discard.* sections zero-sized. That said, the section content
waste is 4 bytes per entry, much smaller than sizeof(Elf{32,64}_Rel).
[0] commit 1c0c1faf5692 ("objtool: Use relative pointers for annotations")
[1] https://github.com/ClangBuiltLinux/linux/issues/1937
Signed-off-by: Fangrui Song <maskray@google.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Link: https://lore.kernel.org/r/20230920001728.1439947-1-maskray@google.com
---
arch/x86/include/asm/alternative.h | 4 ++--
arch/x86/include/asm/nospec-branch.h | 4 ++--
include/linux/objtool.h | 10 +++++-----
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 9c4da69..65f7909 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -58,7 +58,7 @@
#define ANNOTATE_IGNORE_ALTERNATIVE \
"999:\n\t" \
".pushsection .discard.ignore_alts\n\t" \
- ".long 999b - .\n\t" \
+ ".long 999b\n\t" \
".popsection\n\t"
/*
@@ -352,7 +352,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
.macro ANNOTATE_IGNORE_ALTERNATIVE
.Lannotate_\@:
.pushsection .discard.ignore_alts
- .long .Lannotate_\@ - .
+ .long .Lannotate_\@
.popsection
.endm
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index c55cc24..4952b73 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -196,7 +196,7 @@
.macro ANNOTATE_RETPOLINE_SAFE
.Lhere_\@:
.pushsection .discard.retpoline_safe
- .long .Lhere_\@ - .
+ .long .Lhere_\@
.popsection
.endm
@@ -334,7 +334,7 @@
#define ANNOTATE_RETPOLINE_SAFE \
"999:\n\t" \
".pushsection .discard.retpoline_safe\n\t" \
- ".long 999b - .\n\t" \
+ ".long 999b\n\t" \
".popsection\n\t"
typedef u8 retpoline_thunk_t[RETPOLINE_THUNK_SIZE];
diff --git a/include/linux/objtool.h b/include/linux/objtool.h
index 03f82c2..6f6da95 100644
--- a/include/linux/objtool.h
+++ b/include/linux/objtool.h
@@ -48,13 +48,13 @@
#define ANNOTATE_NOENDBR \
"986: \n\t" \
".pushsection .discard.noendbr\n\t" \
- ".long 986b - .\n\t" \
+ ".long 986b\n\t" \
".popsection\n\t"
#define ASM_REACHABLE \
"998:\n\t" \
".pushsection .discard.reachable\n\t" \
- ".long 998b - .\n\t" \
+ ".long 998b\n\t" \
".popsection\n\t"
#else /* __ASSEMBLY__ */
@@ -66,7 +66,7 @@
#define ANNOTATE_INTRA_FUNCTION_CALL \
999: \
.pushsection .discard.intra_function_calls; \
- .long 999b - .; \
+ .long 999b; \
.popsection;
/*
@@ -118,7 +118,7 @@
.macro ANNOTATE_NOENDBR
.Lhere_\@:
.pushsection .discard.noendbr
- .long .Lhere_\@ - .
+ .long .Lhere_\@
.popsection
.endm
@@ -141,7 +141,7 @@
.macro REACHABLE
.Lhere_\@:
.pushsection .discard.reachable
- .long .Lhere_\@ - .
+ .long .Lhere_\@
.popsection
.endm
^ permalink raw reply related [flat|nested] 15+ messages in thread