All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: x86@kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org, Miroslav Benes <mbenes@suse.cz>
Subject: [PATCH 17/18] objtool: Remove --lto and --vmlinux
Date: Wed, 13 Apr 2022 16:19:52 -0700	[thread overview]
Message-ID: <b64b57896eaedf69f53d54c7d9ea373834aef069.1649891421.git.jpoimboe@redhat.com> (raw)
In-Reply-To: <cover.1649891421.git.jpoimboe@redhat.com>

The '--lto' option is a confusing way of telling objtool to do stack
validation despite it being a linked object.  That's no longer needed
now that an explicit '--stackval' option exists.

The '--vmlinux' option can also be made redundant by adding detection of
a multi-object archive.

Remove both options.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 scripts/Makefile.build                  |  2 +-
 scripts/link-vmlinux.sh                 |  8 ++---
 tools/objtool/builtin-check.c           | 23 ++++++++++++--
 tools/objtool/check.c                   | 40 +++++++++----------------
 tools/objtool/elf.c                     |  3 ++
 tools/objtool/include/objtool/builtin.h |  2 --
 tools/objtool/include/objtool/elf.h     |  8 ++++-
 7 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3b53de3dec67..f4b44d77e8a0 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -228,7 +228,7 @@ objtool := $(objtree)/tools/objtool/objtool
 
 objtool_args =								\
 	$(if $(CONFIG_HAVE_TOOLCHAIN_HACKS), --hacks)			\
-	$(if $(CONFIG_X86_KERNEL_IBT), --lto --ibt)			\
+	$(if $(CONFIG_X86_KERNEL_IBT), --ibt)				\
 	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)		\
 	$(if $(CONFIG_UNWINDER_ORC), --orc)				\
 	$(if $(CONFIG_RETPOLINE), --retpoline)				\
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 96dbaaeaecd1..0a79f80bd789 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -114,8 +114,8 @@ objtool_link()
 
 	if is_enabled CONFIG_LTO_CLANG || is_enabled CONFIG_X86_KERNEL_IBT; then
 
-		# Don't perform vmlinux validation unless explicitly requested,
-		# but run objtool on vmlinux.o now that we have an object file.
+		# For LTO and IBT, objtool doesn't run on individual
+		# translation units.  Run everything on vmlinux instead.
 
 		if is_enabled CONFIG_HAVE_TOOLCHAIN_HACKS; then
 			objtoolopt="${objtoolopt} --hacks"
@@ -152,8 +152,6 @@ objtool_link()
 		if is_enabled CONFIG_X86_SMAP; then
 			objtoolopt="${objtoolopt} --uaccess"
 		fi
-
-		objtoolopt="${objtoolopt} --lto"
 	fi
 
 	if is_enabled CONFIG_NOINSTR_VALIDATION; then
@@ -166,8 +164,6 @@ objtool_link()
 			objtoolopt="${objtoolopt} --no-unreachable"
 		fi
 
-		objtoolopt="${objtoolopt} --vmlinux"
-
 		info OBJTOOL ${1}
 		tools/objtool/objtool ${objtoolopt} ${1}
 	fi
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 13e1c46f155a..80fc0d1c0a53 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -54,11 +54,9 @@ const struct option check_options[] = {
 	OPT_BOOLEAN(0, "backtrace", &opts.backtrace, "unwind on error"),
 	OPT_BOOLEAN(0, "backup", &opts.backup, "create .orig files before modification"),
 	OPT_BOOLEAN(0, "dry-run", &opts.dryrun, "don't write modifications"),
-	OPT_BOOLEAN(0, "lto", &opts.lto, "whole-archive like runs"),
 	OPT_BOOLEAN(0, "module", &opts.module, "object is part of a kernel module"),
 	OPT_BOOLEAN(0, "no-unreachable", &opts.no_unreachable, "skip 'unreachable instruction' warnings"),
 	OPT_BOOLEAN(0, "stats", &opts.stats, "print statistics"),
-	OPT_BOOLEAN(0, "vmlinux", &opts.vmlinux, "vmlinux.o validation"),
 
 	OPT_END(),
 };
@@ -117,6 +115,24 @@ static bool opts_valid(void)
 	return false;
 }
 
+static bool link_opts_valid(struct objtool_file *file)
+{
+	if (is_linked_object(file->elf))
+		return true;
+
+	if (opts.noinstr) {
+		fprintf(stderr, "--noinstr requires linked object\n");
+		return false;
+	}
+
+	if (opts.ibt) {
+		fprintf(stderr, "--ibt requires linked object\n");
+		return false;
+	}
+
+	return true;
+}
+
 int objtool_run(int argc, const char **argv)
 {
 	const char *objname;
@@ -136,6 +152,9 @@ int objtool_run(int argc, const char **argv)
 	if (!file)
 		return 1;
 
+	if (!link_opts_valid(file))
+		return 1;
+
 	ret = check(file);
 	if (ret)
 		return ret;
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 273ba6840ed2..2d18f23a895b 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -264,7 +264,8 @@ static void init_cfi_state(struct cfi_state *cfi)
 	cfi->drap_offset = -1;
 }
 
-static void init_insn_state(struct insn_state *state, struct section *sec)
+static void init_insn_state(struct objtool_file *file, struct insn_state *state,
+			    struct section *sec)
 {
 	memset(state, 0, sizeof(*state));
 	init_cfi_state(&state->cfi);
@@ -274,7 +275,7 @@ static void init_insn_state(struct insn_state *state, struct section *sec)
 	 * not correctly determine insn->call_dest->sec (external symbols do
 	 * not have a section).
 	 */
-	if (opts.vmlinux && opts.noinstr && sec)
+	if (is_linked_object(file->elf) && opts.noinstr && sec)
 		state->noinstr = sec->noinstr;
 }
 
@@ -3418,7 +3419,7 @@ static int validate_unwind_hints(struct objtool_file *file, struct section *sec)
 	if (!file->hints)
 		return 0;
 
-	init_insn_state(&state, sec);
+	init_insn_state(file, &state, sec);
 
 	if (sec) {
 		insn = find_insn(file, sec, 0);
@@ -3504,14 +3505,14 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 		return true;
 
 	/*
-	 * Whole archive runs might encounder dead code from weak symbols.
+	 * Whole archive runs might encounter dead code from weak symbols.
 	 * This is where the linker will have dropped the weak symbol in
 	 * favour of a regular symbol, but leaves the code in place.
 	 *
 	 * In this case we'll find a piece of code (whole function) that is not
 	 * covered by a !section symbol. Ignore them.
 	 */
-	if (!insn->func && opts.lto) {
+	if (!insn->func && is_linked_object(file->elf)) {
 		int size = find_symbol_hole_containing(insn->sec, insn->offset);
 		unsigned long end = insn->offset + size;
 
@@ -3633,7 +3634,7 @@ static int validate_section(struct objtool_file *file, struct section *sec)
 		if (func->type != STT_FUNC)
 			continue;
 
-		init_insn_state(&state, sec);
+		init_insn_state(file, &state, sec);
 		set_func_state(&state.cfi);
 
 		warnings += validate_symbol(file, sec, func, &state);
@@ -3642,7 +3643,7 @@ static int validate_section(struct objtool_file *file, struct section *sec)
 	return warnings;
 }
 
-static int validate_vmlinux_functions(struct objtool_file *file)
+static int validate_noinstr_sections(struct objtool_file *file)
 {
 	struct section *sec;
 	int warnings = 0;
@@ -3841,16 +3842,6 @@ int check(struct objtool_file *file)
 {
 	int ret, warnings = 0;
 
-	if (opts.lto && !(opts.vmlinux || opts.module)) {
-		fprintf(stderr, "--lto requires: --vmlinux or --module\n");
-		return 1;
-	}
-
-	if (opts.ibt && !opts.lto) {
-		fprintf(stderr, "--ibt requires: --lto\n");
-		return 1;
-	}
-
 	arch_initial_func_cfi_state(&initial_func_cfi);
 	init_cfi_state(&init_cfi);
 	init_cfi_state(&func_cfi);
@@ -3871,15 +3862,6 @@ int check(struct objtool_file *file)
 	if (list_empty(&file->insn_list))
 		goto out;
 
-	if (opts.vmlinux && !opts.lto) {
-		ret = validate_vmlinux_functions(file);
-		if (ret < 0)
-			goto out;
-
-		warnings += ret;
-		goto out;
-	}
-
 	if (opts.retpoline) {
 		ret = validate_retpoline(file);
 		if (ret < 0)
@@ -3904,6 +3886,12 @@ int check(struct objtool_file *file)
 				goto out;
 			warnings += ret;
 		}
+
+	} else if (opts.noinstr) {
+		ret = validate_noinstr_sections(file);
+		if (ret < 0)
+			goto out;
+		warnings += ret;
 	}
 
 	if (opts.ibt) {
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index f7b2ad27bb1c..41fea838aeba 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -377,6 +377,9 @@ static void elf_add_symbol(struct elf *elf, struct symbol *sym)
 	sym->type = GELF_ST_TYPE(sym->sym.st_info);
 	sym->bind = GELF_ST_BIND(sym->sym.st_info);
 
+	if (sym->type == STT_FILE)
+		elf->num_files++;
+
 	sym->offset = sym->sym.st_value;
 	sym->len = sym->sym.st_size;
 
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 7bc76edb0a85..5c698fb33eca 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -32,11 +32,9 @@ struct opts {
 	bool backtrace;
 	bool backup;
 	bool dryrun;
-	bool lto;
 	bool module;
 	bool no_unreachable;
 	bool stats;
-	bool vmlinux;
 };
 
 extern struct opts opts;
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 22ba7e2b816e..1b03a270c58c 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -86,7 +86,7 @@ struct elf {
 	int fd;
 	bool changed;
 	char *name;
-	unsigned int text_size;
+	unsigned int text_size, num_files;
 	struct list_head sections;
 
 	int symbol_bits;
@@ -131,6 +131,12 @@ static inline u32 reloc_hash(struct reloc *reloc)
 	return sec_offset_hash(reloc->sec, reloc->offset);
 }
 
+/* is it a whole archive (vmlinux.o or module)? */
+static inline bool is_linked_object(struct elf *elf)
+{
+	return elf->num_files > 1;
+}
+
 struct elf *elf_open_read(const char *name, int flags);
 struct section *elf_create_section(struct elf *elf, const char *name, unsigned int sh_flags, size_t entsize, int nr);
 
-- 
2.34.1


  parent reply	other threads:[~2022-04-13 23:21 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-13 23:19 [PATCH 00/18] objtool: Interface overhaul Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 01/18] objtool: Enable unreachable warnings for CLANG LTO Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 02/18] objtool: Support data symbol printing Josh Poimboeuf
2022-04-14  7:05   ` Peter Zijlstra
2022-04-14 15:21     ` Josh Poimboeuf
2022-04-14 15:31       ` Peter Zijlstra
2022-04-14 15:38         ` Josh Poimboeuf
2022-04-14 16:36           ` Peter Zijlstra
2022-04-14 17:01             ` Josh Poimboeuf
2022-04-14 17:21               ` Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 03/18] objtool: Add sec+offset to warnings Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 04/18] objtool: Print data address for "!ENDBR" data warnings Josh Poimboeuf
2022-04-14  7:36   ` Peter Zijlstra
2022-04-13 23:19 ` [PATCH 05/18] objtool: Use offstr() to print address of missing ENDBR Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 06/18] libsubcmd: Fix OPTION_GROUP sorting Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 07/18] objtool: Reorganize cmdline options Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 08/18] objtool: Ditch subcommands Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 09/18] objtool: Add stack validation cmdline option Josh Poimboeuf
2022-04-14  8:43   ` Peter Zijlstra
2022-04-14 15:52     ` Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 10/18] objtool: Extricate ibt from stack validation Josh Poimboeuf
2022-04-14  7:53   ` Peter Zijlstra
2022-04-14 15:44     ` Josh Poimboeuf
2022-04-14 16:38       ` Peter Zijlstra
2022-04-14 17:05         ` Josh Poimboeuf
2022-04-14 18:25           ` Josh Poimboeuf
2022-04-14 19:01             ` Peter Zijlstra
2022-04-14 19:07               ` Josh Poimboeuf
2022-04-14 18:49           ` Peter Zijlstra
2022-04-13 23:19 ` [PATCH 11/18] objtool: Add CONFIG_OBJTOOL Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 12/18] objtool: Make stack validation frame-pointer-specific Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 13/18] objtool: Add static call cmdline option Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 14/18] objtool: Add toolchain hacks " Josh Poimboeuf
2022-04-14  8:09   ` Peter Zijlstra
2022-04-14 15:49     ` Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 15/18] objtool: Rename "VMLINUX_VALIDATION" -> "NOINSTR_VALIDATION" Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 16/18] objtool: Add HAVE_NOINSTR_VALIDATION Josh Poimboeuf
2022-04-13 23:19 ` Josh Poimboeuf [this message]
2022-04-14  8:13   ` [PATCH 17/18] objtool: Remove --lto and --vmlinux Peter Zijlstra
2022-04-15  2:18     ` Josh Poimboeuf
2022-04-13 23:19 ` [PATCH 18/18] objtool: Update documentation Josh Poimboeuf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b64b57896eaedf69f53d54c7d9ea373834aef069.1649891421.git.jpoimboe@redhat.com \
    --to=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=peterz@infradead.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.