Messages in this thread | | | Date | Thu, 12 Oct 2023 00:35:13 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 1/3] Revert "x86/retpoline: Remove .text..__x86.return_thunk section" |
| |
On Wed, Oct 11, 2023 at 09:28:43AM -0700, Josh Poimboeuf wrote: > On Wed, Oct 11, 2023 at 09:41:42AM +0200, Peter Zijlstra wrote: > > > +++ b/tools/objtool/check.c > > > @@ -1610,6 +1610,15 @@ static int add_jump_destinations(struct objtool_file *file) > > > return -1; > > > } > > > > > > + /* > > > + * Since retpolines are in the same section as the return > > > + * thunk, they might not use a relocation when branching to it. > > > + */ > > > + if (jump_dest->sym && jump_dest->sym->return_thunk) { > > > + add_return_call(file, insn, true); > > > + continue; > > > + } > > > > *urgh*... I mean, yes, that obviously works, but should we not also have > > the retpoline thingy for consistency? That case makes less sense though > > :/ > > Consistency with what?
the reloc case; specifically, I was thinking something along these lines:
if (jump-dest->sym && jump_dest->sym->retpoline_thunk) { add_retpoline_call(file, insn); continue; }
Then both reloc and immediate versions are more or less the same.
> The extra section seems pointless but maybe I'm missing something.
By having the section things are better delineated I suppose, be it retpolines or rethunks, all references should be to inside the section (and thus have a reloc) while within the section there should never be a reference to itself.
I'm not sure it's worth much, but then we can have the above two cases issue a WARN instead of fixing it up.
I don't care too deeply, I can't make up my mind either way. But perhaps keeping the section is easier on all the backports, it's easy to forget a tiny objtool patch like this.
| |