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

[Peter Zijlstra <peterz@infradead.org>]

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.