On Sun, Oct 03, 2021 at 12:10:38PM -0700, Linus Torvalds wrote:
> Replying to myself just to add more proper people to the cc.
>
> I'm also wondering how I could possibly be the only person who saw the warning.
>
> I don't think I am, and I think that people who signed off on commit
> 24ff65257375 ("objtool: Teach get_alt_entry() about more relocation
> types") and claimed to have "tested" it, clearly didn't actually do
> so.
>
> PeterZ/Josh/Nathan: see the thread at
>
> https://lore.kernel.org/lkml/CAHk-=wiZwq-0LknKhXN4M+T8jbxn_2i9mcKpO+OaBSSq_Eh7tg@xxxxxxxxxxxxxx/
>
> if you need more context, but I suspect you can figure it out just
> from this email too.
Sorry about that. I think Peter and I failed to run this through
regression testing. We can work on tightening up our process.
Definitely *not* Nathan's fault. His 'Tested-by' only means that this
fixed his particular issue.
> > > Looking at the kvm code, that kvm_fastop_exception thing is some funky sh*t.
> > >
> > > I _think_ the problem is that 'kvm_fastop_exception' is done with bare
> > > asm at the top-level and that triggers some odd interaction with other
> > > section data, but I really don't know.
> >
> > No, it's the fact that it is marked as a global function (why?) that
> > it then causes problems.
> >
> > Now, I don't actually see why it would cause problems (the same way I
> > don't see why it's marked global). But removing that
> >
> > ".global kvm_fastop_exception \n"
> >
> > works.
> >
> > I suspect it makes the linker do the relocation for us before objtool
> > runs, because now that it's a local name, there is no worry about
> > multiply defined symbols of the same name or anything like that.
> >
> > I also suspect that the reason for the warning is that the symbol type
> > has never been declared, so it's not marked as a STT_FUNC in the
> > relocation information.
Right, basically objtool's complaining that it doesn't know how to
handle the NOTYPE symbol. But really, any non-object symbol should be
straightforward. I may just remove these warnings altogether in favor
of something much simpler (something like the patch below).
> > In the meantime, I think the exception handling for kvm
> > divide/multiply emulation is badly broken right now. Hmm?
The warning is harmless, so it doesn't necessarily mean anything's
broken. That said, I have no idea what's going in that code or why
kvm_fastop_exception() is clearing %esi.
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index f58ecc50fb10..0217ac3fa7ff 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -58,22 +58,11 @@ void __weak arch_handle_alternative(unsigned short feature, struct special_alt *
{
}
-static bool reloc2sec_off(struct reloc *reloc, struct section **sec, unsigned long *off)
+static void reloc_to_sec_off(struct reloc *reloc, struct section **sec,
+ unsigned long *off)
{
- switch (reloc->sym->type) {
- case STT_FUNC:
- *sec = reloc->sym->sec;
- *off = reloc->sym->offset + reloc->addend;
- return true;
-
- case STT_SECTION:
- *sec = reloc->sym->sec;
- *off = reloc->addend;
- return true;
-
- default:
- return false;
- }
+ *sec = reloc->sym->sec;
+ *off = reloc->sym->offset + reloc->addend;
}
static int get_alt_entry(struct elf *elf, struct special_entry *entry,
@@ -109,11 +98,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
WARN_FUNC("can't find orig reloc", sec, offset + entry->orig);
return -1;
}
- if (!reloc2sec_off(orig_reloc, &alt->orig_sec, &alt->orig_off)) {
- WARN_FUNC("don't know how to handle reloc symbol type: %s",
- sec, offset + entry->orig, orig_reloc->sym->name);
- return -1;
- }
+ reloc_to_sec_off(orig_reloc, &alt->orig_sec, &alt->orig_off);
if (!entry->group || alt->new_len) {
new_reloc = find_reloc_by_dest(elf, sec, offset + entry->new);
@@ -131,11 +116,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
if (arch_is_retpoline(new_reloc->sym))
return 1;
- if (!reloc2sec_off(new_reloc, &alt->new_sec, &alt->new_off)) {
- WARN_FUNC("don't know how to handle reloc symbol type: %s",
- sec, offset + entry->new, new_reloc->sym->name);
- return -1;
- }
+ reloc_to_sec_off(new_reloc, &alt->new_sec, &alt->new_off);
/* _ASM_EXTABLE_EX hack */
if (alt->new_off >= 0x7ffffff0)
