On Fri, 19 Nov 2021 at 21:31, Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> > + if (insn->sec->noinstr && sym->removable_instr) {
> > if (reloc) {
> > reloc->type = R_NONE;
> > elf_write_reloc(file->elf, reloc);
>
> I'd love to have a clearer name than 'removable_instr', though I'm
> having trouble coming up with something.
>
> 'profiling_func'?
>
> Profiling isn't really accurate but maybe it gets the point across. I'm
> definitely open to other suggestions.
Well, this bit is not true for all "profiling functions" either. It's
only true for instrumentation functions that appear in 'noinstr' and
that the compiler can't remove on its own, but are valid to remove by
objtool in noinstr code, hence 'removable_instr'.
I'm really quite indifferent what we call it, so I'll leave you to
pick whatever sounds best:
-- profiling_func
-- nop_profiling_func
-- optional_profiling_func
-- noinstr_remove
-- removable_profiling_func
-- noinstr_nop_func
-- noinstr_nop
-- nop_in_noinstr
-- invalid_in_noinstr
?
> Also, the above code isn't very self-evident so there still needs to be
> a comment there, like:
>
> /*
> * Many compilers cannot disable KCOV or sanitizer calls with a
> * function attribute so they need a little help, NOP out any
> * such calls from noinstr text.
> */
>
I'll add it.
> > +{
> > + /*
> > + * Many compilers cannot disable KCOV with a function attribute so they
> > + * need a little help, NOP out any KCOV calls from noinstr text.
> > + */
> > + if (!strncmp(name, "__sanitizer_cov_", 16))
> > + return true;
>
> A comment is good here, but the NOP-ing bit seems out of place.
I'll fix that.
