[PATCH 08/18] objtool: Ditch subcommands

From: Josh Poimboeuf
Date: Wed Apr 13 2022 - 19:20:46 EST


Objtool has a fairly singular focus. It runs on object files and does
validations and transformations which can be combined in various ways.
The subcommand model has never been a good fit, making it awkward to
combine and remove options.

Remove the "check" and "orc" subcommands in favor of a more traditional
cmdline option model. This makes it much more flexible to use, and
easier to port individual features to other arches.

Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
---
scripts/Makefile.build | 2 +-
scripts/link-vmlinux.sh | 13 ++--
tools/objtool/Build | 12 ++-
tools/objtool/Makefile | 8 +-
tools/objtool/builtin-check.c | 61 +++++++++++++---
tools/objtool/builtin-orc.c | 73 -------------------
tools/objtool/check.c | 8 ++
tools/objtool/include/objtool/builtin.h | 10 ++-
tools/objtool/objtool.c | 97 +------------------------
tools/objtool/weak.c | 9 +--
10 files changed, 82 insertions(+), 211 deletions(-)
delete mode 100644 tools/objtool/builtin-orc.c

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index dd9d582808d6..116c7272b41c 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -227,9 +227,9 @@ ifdef CONFIG_STACK_VALIDATION
objtool := $(objtree)/tools/objtool/objtool

objtool_args = \
- $(if $(CONFIG_UNWINDER_ORC),orc generate,check) \
$(if $(CONFIG_X86_KERNEL_IBT), --lto --ibt) \
$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount) \
+ $(if $(CONFIG_UNWINDER_ORC), --orc) \
$(if $(CONFIG_RETPOLINE), --retpoline) \
$(if $(CONFIG_SLS), --sls) \
$(if $(CONFIG_X86_SMAP), --uaccess) \
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index c6e9fef61b11..f6db79b11573 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -113,9 +113,6 @@ objtool_link()

# Don't perform vmlinux validation unless explicitly requested,
# but run objtool on vmlinux.o now that we have an object file.
- if is_enabled CONFIG_UNWINDER_ORC; then
- objtoolcmd="orc generate"
- fi

if is_enabled CONFIG_X86_KERNEL_IBT; then
objtoolopt="${objtoolopt} --ibt"
@@ -125,6 +122,10 @@ objtool_link()
objtoolopt="${objtoolopt} --mcount"
fi

+ if is_enabled CONFIG_UNWINDER_ORC; then
+ objtoolopt="${objtoolopt} --orc"
+ fi
+
objtoolopt="${objtoolopt} --lto"
fi

@@ -134,10 +135,6 @@ objtool_link()

if [ -n "${objtoolopt}" ]; then

- if [ -z "${objtoolcmd}" ]; then
- objtoolcmd="check"
- fi
-
if is_enabled CONFIG_RETPOLINE; then
objtoolopt="${objtoolopt} --retpoline"
fi
@@ -161,7 +158,7 @@ objtool_link()
objtoolopt="${objtoolopt} --vmlinux"

info OBJTOOL ${1}
- tools/objtool/objtool ${objtoolcmd} ${objtoolopt} ${1}
+ tools/objtool/objtool ${objtoolopt} ${1}
fi
}

diff --git a/tools/objtool/Build b/tools/objtool/Build
index b7222d5cc7bc..33f2ee5a46d3 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -2,17 +2,15 @@ objtool-y += arch/$(SRCARCH)/

objtool-y += weak.o

-objtool-$(SUBCMD_CHECK) += check.o
-objtool-$(SUBCMD_CHECK) += special.o
-objtool-$(SUBCMD_ORC) += check.o
-objtool-$(SUBCMD_ORC) += orc_gen.o
-objtool-$(SUBCMD_ORC) += orc_dump.o
-
+objtool-y += check.o
+objtool-y += special.o
objtool-y += builtin-check.o
-objtool-y += builtin-orc.o
objtool-y += elf.o
objtool-y += objtool.o

+objtool-$(BUILD_ORC) += orc_gen.o
+objtool-$(BUILD_ORC) += orc_dump.o
+
objtool-y += libstring.o
objtool-y += libctype.o
objtool-y += str_error_r.o
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 0dbd397f319d..061cf1cd42c4 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -39,15 +39,13 @@ CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)

AWK = awk

-SUBCMD_CHECK := n
-SUBCMD_ORC := n
+BUILD_ORC := n

ifeq ($(SRCARCH),x86)
- SUBCMD_CHECK := y
- SUBCMD_ORC := y
+ BUILD_ORC := y
endif

-export SUBCMD_CHECK SUBCMD_ORC
+export BUILD_ORC
export srctree OUTPUT CFLAGS SRCARCH AWK
include $(srctree)/tools/build/Makefile.include

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 4f4a087720b3..3df46e9b4b03 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -3,16 +3,6 @@
* Copyright (C) 2015-2017 Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
*/

-/*
- * objtool check:
- *
- * This command analyzes every .o file and ensures the validity of its stack
- * trace metadata. It enforces a set of rules on asm code and C inline
- * assembly code so that stack traces can be reliable.
- *
- * For more information, see tools/objtool/Documentation/stack-validation.txt.
- */
-
#include <subcmd/parse-options.h>
#include <string.h>
#include <stdlib.h>
@@ -22,7 +12,7 @@
struct opts opts;

static const char * const check_usage[] = {
- "objtool check <actions> [<options>] file.o",
+ "objtool <actions> [<options>] file.o",
NULL,
};

@@ -31,14 +21,31 @@ static const char * const env_usage[] = {
NULL,
};

+static int parse_dumpstr(const struct option *opt, const char *str, int unset)
+{
+ if (unset || !str) {
+ opts.dump = DUMP_NONE;
+ return 0;
+ }
+
+ if (!strcmp(str, "orc")) {
+ opts.dump = DUMP_ORC;
+ return 0;
+ }
+
+ return -1;
+}
+
const struct option check_options[] = {
OPT_GROUP("Actions:"),
OPT_BOOLEAN('i', "ibt", &opts.ibt, "validate and annotate IBT"),
OPT_BOOLEAN('m', "mcount", &opts.mcount, "annotate mcount/fentry calls for ftrace"),
OPT_BOOLEAN('n', "noinstr", &opts.noinstr, "validate noinstr rules"),
+ OPT_BOOLEAN('o', "orc", &opts.orc, "generate ORC metadata"),
OPT_BOOLEAN('r', "retpoline", &opts.retpoline, "validate and annotate retpoline usage"),
OPT_BOOLEAN('S', "sls", &opts.sls, "validate straight-line-speculation mitigations"),
OPT_BOOLEAN('u', "uaccess", &opts.uaccess, "validate uaccess rules for SMAP"),
+ OPT_CALLBACK(0, "dump", NULL, "orc", "dump object data", parse_dumpstr),

OPT_GROUP("Options:"),
OPT_BOOLEAN(0, "backtrace", &opts.backtrace, "unwind on error"),
@@ -81,7 +88,31 @@ int cmd_parse_options(int argc, const char **argv, const char * const usage[])
return argc;
}

-int cmd_check(int argc, const char **argv)
+static bool opts_valid(void)
+{
+ if (opts.ibt ||
+ opts.mcount ||
+ opts.noinstr ||
+ opts.orc ||
+ opts.retpoline ||
+ opts.sls ||
+ opts.uaccess) {
+ if (opts.dump) {
+ fprintf(stderr, "--dump can't be combined with other options\n");
+ return false;
+ }
+
+ return true;
+ }
+
+ if (opts.dump)
+ return true;
+
+ fprintf(stderr, "At least one command required\n");
+ return false;
+}
+
+int objtool_run(int argc, const char **argv)
{
const char *objname;
struct objtool_file *file;
@@ -90,6 +121,12 @@ int cmd_check(int argc, const char **argv)
argc = cmd_parse_options(argc, argv, check_usage);
objname = argv[0];

+ if (!opts_valid())
+ return 1;
+
+ if (opts.dump == DUMP_ORC)
+ return orc_dump(objname);
+
file = objtool_open_read(objname);
if (!file)
return 1;
diff --git a/tools/objtool/builtin-orc.c b/tools/objtool/builtin-orc.c
deleted file mode 100644
index 17f8b9307738..000000000000
--- a/tools/objtool/builtin-orc.c
+++ /dev/null
@@ -1,73 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Copyright (C) 2017 Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
- */
-
-/*
- * objtool orc:
- *
- * This command analyzes a .o file and adds .orc_unwind and .orc_unwind_ip
- * sections to it, which is used by the in-kernel ORC unwinder.
- *
- * This command is a superset of "objtool check".
- */
-
-#include <string.h>
-#include <objtool/builtin.h>
-#include <objtool/objtool.h>
-
-static const char *orc_usage[] = {
- "objtool orc generate [<options>] file.o",
- "objtool orc dump file.o",
- NULL,
-};
-
-int cmd_orc(int argc, const char **argv)
-{
- const char *objname;
-
- argc--; argv++;
- if (argc <= 0)
- usage_with_options(orc_usage, check_options);
-
- if (!strncmp(argv[0], "gen", 3)) {
- struct objtool_file *file;
- int ret;
-
- argc = cmd_parse_options(argc, argv, orc_usage);
- objname = argv[0];
-
- file = objtool_open_read(objname);
- if (!file)
- return 1;
-
- ret = check(file);
- if (ret)
- return ret;
-
- if (list_empty(&file->insn_list))
- return 0;
-
- ret = orc_create(file);
- if (ret)
- return ret;
-
- if (!file->elf->changed)
- return 0;
-
- return elf_write(file->elf);
- }
-
- if (!strcmp(argv[0], "dump")) {
- if (argc != 2)
- usage_with_options(orc_usage, check_options);
-
- objname = argv[1];
-
- return orc_dump(objname);
- }
-
- usage_with_options(orc_usage, check_options);
-
- return 0;
-}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 5059c097c563..490ed3560d99 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3950,6 +3950,14 @@ int check(struct objtool_file *file)
warnings += ret;
}

+ if (opts.orc && !list_empty(&file->insn_list)) {
+ ret = orc_create(file);
+ if (ret < 0)
+ goto out;
+ warnings += ret;
+ }
+
+
if (opts.stats) {
printf("nr_insns_visited: %ld\n", nr_insns_visited);
printf("nr_cfi: %ld\n", nr_cfi);
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 87c1a7351e3c..0cac9bd6a97f 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -9,11 +9,18 @@

extern const struct option check_options[];

+enum dump {
+ DUMP_NONE,
+ DUMP_ORC,
+};
+
struct opts {
/* actions: */
+ enum dump dump;
bool ibt;
bool mcount;
bool noinstr;
+ bool orc;
bool retpoline;
bool sls;
bool uaccess;
@@ -34,7 +41,6 @@ extern struct opts opts;

extern int cmd_parse_options(int argc, const char **argv, const char * const usage[]);

-extern int cmd_check(int argc, const char **argv);
-extern int cmd_orc(int argc, const char **argv);
+extern int objtool_run(int argc, const char **argv);

#endif /* _BUILTIN_H */
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index a0f3d3c9558d..512669ce064c 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -3,16 +3,6 @@
* Copyright (C) 2015 Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
*/

-/*
- * objtool:
- *
- * The 'check' subcmd analyzes every .o file and ensures the validity of its
- * stack trace metadata. It enforces a set of rules on asm code and C inline
- * assembly code so that stack traces can be reliable.
- *
- * For more information, see tools/objtool/Documentation/stack-validation.txt.
- */
-
#include <stdio.h>
#include <stdbool.h>
#include <string.h>
@@ -26,20 +16,6 @@
#include <objtool/objtool.h>
#include <objtool/warn.h>

-struct cmd_struct {
- const char *name;
- int (*fn)(int, const char **);
- const char *help;
-};
-
-static const char objtool_usage_string[] =
- "objtool COMMAND [ARGS]";
-
-static struct cmd_struct objtool_cmds[] = {
- {"check", cmd_check, "Perform stack metadata validation on an object file" },
- {"orc", cmd_orc, "Generate in-place ORC unwind tables for an object file" },
-};
-
bool help;

const char *objname;
@@ -161,70 +137,6 @@ void objtool_pv_add(struct objtool_file *f, int idx, struct symbol *func)
f->pv_ops[idx].clean = false;
}

-static void cmd_usage(void)
-{
- unsigned int i, longest = 0;
-
- printf("\n usage: %s\n\n", objtool_usage_string);
-
- for (i = 0; i < ARRAY_SIZE(objtool_cmds); i++) {
- if (longest < strlen(objtool_cmds[i].name))
- longest = strlen(objtool_cmds[i].name);
- }
-
- puts(" Commands:");
- for (i = 0; i < ARRAY_SIZE(objtool_cmds); i++) {
- printf(" %-*s ", longest, objtool_cmds[i].name);
- puts(objtool_cmds[i].help);
- }
-
- printf("\n");
-
- if (!help)
- exit(129);
- exit(0);
-}
-
-static void handle_options(int *argc, const char ***argv)
-{
- while (*argc > 0) {
- const char *cmd = (*argv)[0];
-
- if (cmd[0] != '-')
- break;
-
- if (!strcmp(cmd, "--help") || !strcmp(cmd, "-h")) {
- help = true;
- break;
- } else {
- fprintf(stderr, "Unknown option: %s\n", cmd);
- cmd_usage();
- }
-
- (*argv)++;
- (*argc)--;
- }
-}
-
-static void handle_internal_command(int argc, const char **argv)
-{
- const char *cmd = argv[0];
- unsigned int i, ret;
-
- for (i = 0; i < ARRAY_SIZE(objtool_cmds); i++) {
- struct cmd_struct *p = objtool_cmds+i;
-
- if (strcmp(p->name, cmd))
- continue;
-
- ret = p->fn(argc, argv);
-
- exit(ret);
- }
-
- cmd_usage();
-}
-
int main(int argc, const char **argv)
{
static const char *UNUSED = "OBJTOOL_NOT_IMPLEMENTED";
@@ -233,14 +145,7 @@ int main(int argc, const char **argv)
exec_cmd_init("objtool", UNUSED, UNUSED, UNUSED);
pager_init(UNUSED);

- argv++;
- argc--;
- handle_options(&argc, &argv);
-
- if (!argc || help)
- cmd_usage();
-
- handle_internal_command(argc, argv);
+ objtool_run(argc, argv);

return 0;
}
diff --git a/tools/objtool/weak.c b/tools/objtool/weak.c
index 8314e824db4a..d83f607733b0 100644
--- a/tools/objtool/weak.c
+++ b/tools/objtool/weak.c
@@ -15,17 +15,12 @@
return ENOSYS; \
})

-int __weak check(struct objtool_file *file)
-{
- UNSUPPORTED("check subcommand");
-}
-
int __weak orc_dump(const char *_objname)
{
- UNSUPPORTED("orc");
+ UNSUPPORTED("ORC");
}

int __weak orc_create(struct objtool_file *file)
{
- UNSUPPORTED("orc");
+ UNSUPPORTED("ORC");
}
--
2.34.1