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

Josh Poimboeuf
2022-03-04T18:27:27Z

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