Re: [PATCH v3 37/39] objtool: Optionally WARN about unused ANNOTATE_NOENDBR

[Josh Poimboeuf]

On Thu, Mar 03, 2022 at 12:23:58PM +0100, Peter Zijlstra wrote:
>  bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats,
>       lto, vmlinux, mcount, noinstr, backup, sls, dryrun,
> -     ibt;
> +     ibt, ibt_warn;
>  
>  static const char * const check_usage[] = {
>  	"objtool check [<options>] file.o",
> @@ -49,6 +49,7 @@ const struct option check_options[] = {
>  	OPT_BOOLEAN('S', "sls", &sls, "validate straight-line-speculation"),
>  	OPT_BOOLEAN(0, "dry-run", &dryrun, "don't write the modifications"),
>  	OPT_BOOLEAN(0, "ibt", &ibt, "validate ENDBR placement"),
> +	OPT_BOOLEAN(0, "ibt-warn", &ibt_warn, "warn about unused ANNOTATE_ENDBR"),

"ibt-warn" doesn't really describe it.  Most options warn.

"ibt-unused-annotation"?

>  	OPT_END(),
>  };
>  
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1814,6 +1814,19 @@ static void set_func_state(struct cfi_st
>  	state->stack_size = initial_func_cfi.cfa.offset;
>  }
>  
> +static bool insn_is_endbr(struct instruction *insn)
> +{
> +	if (insn->type == INSN_ENDBR)
> +		return true;
> +
> +	if (insn->noendbr) {
> +		insn->noendbr_hit = 1;
> +		return true;
> +	}
> +
> +	return false;
> +}

Ew...

> @@ -3695,6 +3707,31 @@ static int validate_ibt(struct objtool_f
>  		}
>  	}
>  
> +	if (ibt_warn) {
> +		struct symbol *hypercall_page = find_symbol_by_name(file->elf, "hypercall_page");
> +		struct instruction *insn;
> +
> +		for_each_insn(file, insn) {
> +			if (!insn->noendbr || insn->noendbr_hit)
> +				continue;
> +
> +			if (hypercall_page) {
> +				/*
> +				 * The Xen hypercall page contains many
> +				 * hypercalls (and unused slots) that are never
> +				 * indirectly called. Still every slot has an
> +				 * annotation. Suppress complaints.
> +				 */
> +				if (insn->sec == hypercall_page->sec &&
> +				    insn->offset >= hypercall_page->offset &&
> +				    insn->offset <  hypercall_page->offset + hypercall_page->len)
> +					continue;
> +			}
> +
> +			WARN_FUNC("unused ANNOTATE_NOENDBR", insn->sec, insn->offset);
> +		}
> +	}

Ewww... :-/

I can see how this option was useful for development.  But I'm never
going to use it.  Are you?  I'd much rather just drop it.

-- 
Josh