Re: [RFC PATCH 11/20] objtool: Detect code coverage instrumentation in noinstr code

[Peter Zijlstra]

On Wed, Oct 28, 2026 at 07:59:35AM -0400, Sasha Levin wrote:
> gcov and Clang's source-based code coverage update their counters
> inline, rather than by calling into a runtime like KCOV does, so
> coverage instrumentation in noinstr code goes unnoticed by the noinstr
> validation of call destinations.
> 
> Flag instructions in noinstr code, outside of instrumentation_begin()
> and instrumentation_end() sections, that reference coverage counters:
> GCC's __gcov<N>.* counters (including the MC/DC counters), Clang's gcov
> __llvm_gcov_ctr* counters, and the __llvm_prf_cnts and __llvm_prf_bits
> sections of Clang's source-based code coverage.

Nice!

> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
>  tools/objtool/check.c | 53 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 464f6c9d9ff0b..6382d2a44ef2b 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #define _GNU_SOURCE /* memmem() */
> +#include <ctype.h>
>  #include <fnmatch.h>
>  #include <string.h>
>  #include <stdlib.h>
> @@ -3507,6 +3508,52 @@ static inline bool noinstr_call_dest(struct objtool_file *file,
>  	return false;
>  }
>  
> +/*
> + * Code coverage instrumentation (gcov, Clang's source-based coverage) updates
> + * its counters inline rather than through calls, so it can't be caught by
> + * checking the call destinations of noinstr code.
> + */
> +static bool is_coverage_counter(struct symbol *sym)
> +{
> +	if (!strcmp(sym->sec->name, "__llvm_prf_cnts") ||
> +	    !strcmp(sym->sec->name, "__llvm_prf_bits"))
> +		return true;
> +
> +	return (strstarts(sym->name, "__gcov") && isdigit((unsigned char)sym->name[6])) ||
> +	       strstarts(sym->name, "__llvm_gcov_ctr");
> +}
> +
> +static bool insn_updates_coverage(struct objtool_file *file,
> +				  struct instruction *insn)
> +{
> +	struct symbol *sym;
> +	struct reloc *reloc;
> +
> +	for (reloc = insn_reloc(file, insn);
> +	     reloc;
> +	     reloc = find_reloc_by_dest_range(file->elf, insn->sec,
> +					      reloc_offset(reloc) + 1,
> +					      insn->offset + insn->len -
> +					      (reloc_offset(reloc) + 1))) {
> +
> +		sym = reloc->sym;
> +		if (!sym->sec)
> +			continue;
> +
> +		if (is_sec_sym(sym)) {
> +			sym = find_symbol_containing(sym->sec,
> +						     arch_insn_adjusted_addend(insn, reloc));
> +			if (!sym)
> +				sym = reloc->sym;
> +		}
> +
> +		if (is_coverage_counter(sym))
> +			return true;
> +	}
> +
> +	return false;
> +}

Since you can have many relocations to the same symbol; does it make
sense to cache the results of those many strcmp()s in symbol::coverage
or something? We have plenty bits left in struct symbol.

> +
>  static int validate_call(struct objtool_file *file,
>  			 struct instruction *insn,
>  			 struct insn_state *state)
> @@ -3680,6 +3727,12 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
>  	if (statep->noinstr)
>  		statep->instr += insn->instr;
>  
> +	if (statep->noinstr && statep->instr <= 0 &&
> +	    insn_updates_coverage(file, insn)) {
> +		WARN_INSN(insn, "coverage counter update in .noinstr.text section");
> +		return 1;
> +	}

Perhaps merge with the previous conditional?