Re: [PATCH] x86/speculation, objtool: Use absolute relocations for annotations

[Fangrui Song]

On Thu, Sep 21, 2023 at 8:35 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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.


-- 
宋方睿