Re: [PATCH v2 23/23] objtool, kcsan: Remove memory barrier instrumentation from noinstr

Josh Poimboeuf
2021-11-19T20:31:46Z

On Thu, Nov 18, 2021 at 09:10:27AM +0100, Marco Elver wrote:
> @@ -1071,12 +1071,7 @@ static void annotate_call_site(struct objtool_file *file,
> return;
> }
>
> - /*
> - * Many compilers cannot disable KCOV with a function attribute
> - * so they need a little help, NOP out any KCOV calls from noinstr
> - * text.
> - */
> - if (insn->sec->noinstr && sym->kcov) {
> + 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.

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.
*/

> @@ -1991,6 +1986,32 @@ static int read_intra_function_calls(struct objtool_file *file)
> return 0;
> }
>
> +static bool is_removable_instr(const char *name)

> +{
> + /*
> + * 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.

--
Josh