On Fri, Nov 15, 2024 at 10:38:28AM -0800, Josh Poimboeuf wrote:
> On Mon, Nov 11, 2024 at 12:59:36PM +0100, Peter Zijlstra wrote:
> > +#define ASM_ANNOTATE(x) \
> > + "911:\n\t" \
> > + ".pushsection .discard.annotate,\"M\",@progbits,8\n\t" \
> > + ".long 911b - .\n\t" \
> > + ".long " __stringify(x) "\n\t" \
> > + ".popsection\n\t"
>
> Why mergeable and progbits?
In order to get sh_entsize ?
> > +static int read_annotate(struct objtool_file *file, void (*func)(int type, struct instruction *insn))
> > +{
> > + struct section *rsec, *sec;
> > + struct instruction *insn;
> > + struct reloc *reloc;
> > + int type;
> > +
> > + rsec = find_section_by_name(file->elf, ".rela.discard.annotate");
> > + if (!rsec)
> > + return 0;
> > +
> > + sec = find_section_by_name(file->elf, ".discard.annotate");
> > + if (!sec)
> > + return 0;
>
> Instead of looking for .rela.discard.annotate you can just get it from
> sec->rsec.
Oh, indeed.
> > +
> > + if (sec->sh.sh_entsize != 8) {
> > + static bool warn = false;
>
> "warned" ?
Sure.
> > + if (!warn) {
> > + WARN("%s: dodgy linker, sh_entsize != 8", sec->name);
> > + warn = true;
> > + }
>
> Any reason not to make this a fatal error?
lld is currently suffering from this, it would get us build failures on
llvm builds. Once that's fixed, then yes, this should become fatal.
https://github.com/ClangBuiltLinux/linux/issues/2057
> > + sec->sh.sh_entsize = 8;
> > + }
> > +
> > + for_each_reloc(rsec, reloc) {
> > + insn = find_insn(file, reloc->sym->sec,
> > + reloc->sym->offset + reloc_addend(reloc));
> > + if (!insn) {
> > + WARN("bad .discard.annotate entry: %d", reloc_idx(reloc));
> > + return -1;
> > + }
>
> Would be nice to print the type here as well.
Sure.
> > @@ -2670,6 +2714,8 @@ static int decode_sections(struct objtoo
> > if (ret)
> > return ret;
> >
> > + ret = read_annotate(file, __annotate_nop);
> > +
>
> 'ret' is ignored here (not that it matters much as this goes away in the
> next patch)
Right..
