2023-04-18 21:29:01

by Josh Poimboeuf

Subject: [PATCH v3 0/8] objtool: warning improvements

v3:
- dropped merged patches and rebased on tip/objtool/core
- moved list of noreturn functions to separate file
- improved WARN_INSN() to do single evaluation of insn arg

Josh Poimboeuf (8):
objtool: Limit unreachable warnings to once per function
objtool: Add verbose option for disassembling affected functions
objtool: Include backtrace in verbose mode
objtool: Detect missing __noreturn annotations
objtool: Ignore exc_double_fault() __noreturn warnings
objtool: Remove superfluous global_noreturns entries
tools/lib/subcmd: Replace NORETURN usage with __noreturn
objtool: Move noreturn function list to separate file

tools/lib/subcmd/parse-options.h | 8 +-
tools/lib/subcmd/subcmd-util.h | 5 +-
tools/objtool/Documentation/objtool.txt | 10 ++
tools/objtool/builtin-check.c | 5 +
tools/objtool/check.c | 195 ++++++++++++++++--------
tools/objtool/include/objtool/builtin.h | 1 +
tools/objtool/include/objtool/elf.h | 1 +
tools/objtool/include/objtool/warn.h | 21 ++-
tools/objtool/noreturns.h | 45 ++++++
9 files changed, 214 insertions(+), 77 deletions(-)
create mode 100644 tools/objtool/noreturns.h

--
2.39.2

2023-04-18 21:29:04

by Josh Poimboeuf

Subject: [PATCH v3 3/8] objtool: Include backtrace in verbose mode

Include backtrace in verbose mode. This makes it easy to gather all the
information needed for diagnosing objtool warnings.

Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/objtool/Documentation/objtool.txt | 4 ++--
tools/objtool/check.c | 26 ++++++++++---------------
tools/objtool/include/objtool/warn.h | 14 +++++++------
3 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/tools/objtool/Documentation/objtool.txt b/tools/objtool/Documentation/objtool.txt
index 4d6c5acde7a3..5a69c207a10e 100644
--- a/tools/objtool/Documentation/objtool.txt
+++ b/tools/objtool/Documentation/objtool.txt
@@ -246,8 +246,8 @@ Objtool warnings

NOTE: When requesting help with an objtool warning, please recreate with
OBJTOOL_VERBOSE=1 (e.g., "make OBJTOOL_VERBOSE=1") and send the full
-output, including any disassembly below the warning, to the objtool
-maintainers.
+output, including any disassembly or backtrace below the warning, to the
+objtool maintainers.

For asm files, if you're getting an error which doesn't make sense,
first make sure that the affected code follows the above rules.
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c63e0867daca..aa083296af1e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3657,8 +3657,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,

ret = validate_branch(file, func, alt->insn, state);
if (ret) {
- if (opts.backtrace)
- BT_FUNC("(alt)", insn);
+ BT_INSN(insn, "(alt)");
return ret;
}
}
@@ -3703,8 +3702,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
ret = validate_branch(file, func,
insn->jump_dest, state);
if (ret) {
- if (opts.backtrace)
- BT_FUNC("(branch)", insn);
+ BT_INSN(insn, "(branch)");
return ret;
}
}
@@ -3802,8 +3800,8 @@ static int validate_unwind_hint(struct objtool_file *file,
{
if (insn->hint && !insn->visited && !insn->ignore) {
int ret = validate_branch(file, insn_func(insn), insn, *state);
- if (ret && opts.backtrace)
- BT_FUNC("<=== (hint)", insn);
+ if (ret)
+ BT_INSN(insn, "<=== (hint)");
return ret;
}

@@ -3861,8 +3859,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)

ret = validate_unret(file, alt->insn);
if (ret) {
- if (opts.backtrace)
- BT_FUNC("(alt)", insn);
+ BT_INSN(insn, "(alt)");
return ret;
}
}
@@ -3888,10 +3885,8 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
}
ret = validate_unret(file, insn->jump_dest);
if (ret) {
- if (opts.backtrace) {
- BT_FUNC("(branch%s)", insn,
- insn->type == INSN_JUMP_CONDITIONAL ? "-cond" : "");
- }
+ BT_INSN(insn, "(branch%s)",
+ insn->type == INSN_JUMP_CONDITIONAL ? "-cond" : "");
return ret;
}

@@ -3913,8 +3908,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)

ret = validate_unret(file, dest);
if (ret) {
- if (opts.backtrace)
- BT_FUNC("(call)", insn);
+ BT_INSN(insn, "(call)");
return ret;
}
/*
@@ -4216,8 +4210,8 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
state->uaccess = sym->uaccess_safe;

ret = validate_branch(file, insn_func(insn), insn, *state);
- if (ret && opts.backtrace)
- BT_FUNC("<=== (sym)", insn);
+ if (ret)
+ BT_INSN(insn, "<=== (sym)");
return ret;
}

diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index f195deab456e..ac04d3fe4dd9 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -63,12 +63,14 @@ static inline char *offstr(struct section *sec, unsigned long offset)
_insn->sym->warned = 1; \
})

-#define BT_FUNC(format, insn, ...) \
-({ \
- struct instruction *_insn = (insn); \
- char *_str = offstr(_insn->sec, _insn->offset); \
- WARN(" %s: " format, _str, ##__VA_ARGS__); \
- free(_str); \
+#define BT_INSN(insn, format, ...) \
+({ \
+ if (opts.verbose || opts.backtrace) { \
+ struct instruction *_insn = (insn); \
+ char *_str = offstr(_insn->sec, _insn->offset); \
+ WARN(" %s: " format, _str, ##__VA_ARGS__); \
+ free(_str); \
+ } \
})

#define WARN_ELF(format, ...) \
--
2.39.2

2023-04-18 21:29:31

by Josh Poimboeuf

Subject: [PATCH v3 7/8] tools/lib/subcmd: Replace NORETURN usage with __noreturn

NORETURN is redundant with __noreturn, just use the latter.

Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/lib/subcmd/parse-options.h | 8 ++------
tools/lib/subcmd/subcmd-util.h | 5 ++---
2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/tools/lib/subcmd/parse-options.h b/tools/lib/subcmd/parse-options.h
index 41b9b942504d..8e9147358a28 100644
--- a/tools/lib/subcmd/parse-options.h
+++ b/tools/lib/subcmd/parse-options.h
@@ -6,10 +6,6 @@
#include <stdbool.h>
#include <stdint.h>

-#ifndef NORETURN
-#define NORETURN __attribute__((__noreturn__))
-#endif
-
enum parse_opt_type {
/* special types */
OPTION_END,
@@ -183,9 +179,9 @@ extern int parse_options_subcommand(int argc, const char **argv,
const char *const subcommands[],
const char *usagestr[], int flags);

-extern NORETURN void usage_with_options(const char * const *usagestr,
+extern __noreturn void usage_with_options(const char * const *usagestr,
const struct option *options);
-extern NORETURN __attribute__((format(printf,3,4)))
+extern __noreturn __attribute__((format(printf,3,4)))
void usage_with_options_msg(const char * const *usagestr,
const struct option *options,
const char *fmt, ...);
diff --git a/tools/lib/subcmd/subcmd-util.h b/tools/lib/subcmd/subcmd-util.h
index b2aec04fce8f..dfac76e35ac7 100644
--- a/tools/lib/subcmd/subcmd-util.h
+++ b/tools/lib/subcmd/subcmd-util.h
@@ -5,8 +5,7 @@
#include <stdarg.h>
#include <stdlib.h>
#include <stdio.h>
-
-#define NORETURN __attribute__((__noreturn__))
+#include <linux/compiler.h>

static inline void report(const char *prefix, const char *err, va_list params)
{
@@ -15,7 +14,7 @@ static inline void report(const char *prefix, const char *err, va_list params)
fprintf(stderr, " %s%s\n", prefix, msg);
}

-static NORETURN inline void die(const char *err, ...)
+static __noreturn inline void die(const char *err, ...)
{
va_list params;

--
2.39.2

2023-04-18 21:38:18

by Josh Poimboeuf

Subject: [PATCH v3 8/8] objtool: Move noreturn function list to separate file

This makes it a little cleaner and easier to maintain.

Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/objtool/Documentation/objtool.txt | 5 ++-
tools/objtool/check.c | 44 ++----------------------
tools/objtool/noreturns.h | 45 +++++++++++++++++++++++++
3 files changed, 50 insertions(+), 44 deletions(-)
create mode 100644 tools/objtool/noreturns.h

diff --git a/tools/objtool/Documentation/objtool.txt b/tools/objtool/Documentation/objtool.txt
index 2cd1fa16ed08..00f0a7e385ec 100644
--- a/tools/objtool/Documentation/objtool.txt
+++ b/tools/objtool/Documentation/objtool.txt
@@ -306,9 +306,8 @@ the objtool maintainers.
3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation

The call from foo() to bar() doesn't return, but bar() is missing the
- __noreturn annotation. NOTE: In addition to adding the __noreturn
- annotation, the function name also needs to be added to
- 'global_noreturns' in tools/objtool/check.c.
+ __noreturn annotation. NOTE: In addition to annotating the function
+ with __noreturn, please also add it to tools/objtool/noreturns.h.

4. file.o: warning: objtool: func(): can't find starting instruction
or
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2047a6d5339b..69794c25f857 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -192,49 +192,11 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
struct instruction *insn;
bool empty = true;

- /*
- * Unfortunately these have to be hard coded because the noreturn
- * attribute isn't provided in ELF data. Keep 'em sorted.
- */
+#define NORETURN(func) __stringify(func),
static const char * const global_noreturns[] = {
- "__invalid_creds",
- "__module_put_and_kthread_exit",
- "__reiserfs_panic",
- "__stack_chk_fail",
- "__ubsan_handle_builtin_unreachable",
- "arch_call_rest_init",
- "arch_cpu_idle_dead",
- "btrfs_assertfail",
- "cpu_bringup_and_idle",
- "cpu_startup_entry",
- "do_exit",
- "do_group_exit",
- "do_task_dead",
- "ex_handler_msr_mce",
- "fortify_panic",
- "hlt_play_dead",
- "hv_ghcb_terminate",
- "kthread_complete_and_exit",
- "kthread_exit",
- "kunit_try_catch_throw",
- "machine_real_restart",
- "make_task_dead",
- "mpt_halt_firmware",
- "nmi_panic_self_stop",
- "panic",
- "panic_smp_self_stop",
- "rest_init",
- "rewind_stack_and_make_dead",
- "sev_es_terminate",
- "snp_abort",
- "start_kernel",
- "stop_this_cpu",
- "usercopy_abort",
- "x86_64_start_kernel",
- "x86_64_start_reservations",
- "xen_cpu_bringup_again",
- "xen_start_kernel",
+#include "noreturns.h"
};
+#undef NORETURN

if (!func)
return false;
diff --git a/tools/objtool/noreturns.h b/tools/objtool/noreturns.h
new file mode 100644
index 000000000000..cede6068ddf6
--- /dev/null
+++ b/tools/objtool/noreturns.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This is a (sorted!) list of all known __noreturn functions in the kernel.
+ * It's needed for objtool to properly reverse-engineer the control flow graph.
+ *
+ * Yes, this is unfortunate. A better solution is in the works.
+ */
+NORETURN(__invalid_creds)
+NORETURN(__module_put_and_kthread_exit)
+NORETURN(__reiserfs_panic)
+NORETURN(__stack_chk_fail)
+NORETURN(__ubsan_handle_builtin_unreachable)
+NORETURN(arch_call_rest_init)
+NORETURN(arch_cpu_idle_dead)
+NORETURN(btrfs_assertfail)
+NORETURN(cpu_bringup_and_idle)
+NORETURN(cpu_startup_entry)
+NORETURN(do_exit)
+NORETURN(do_group_exit)
+NORETURN(do_task_dead)
+NORETURN(ex_handler_msr_mce)
+NORETURN(fortify_panic)
+NORETURN(hlt_play_dead)
+NORETURN(hv_ghcb_terminate)
+NORETURN(kthread_complete_and_exit)
+NORETURN(kthread_exit)
+NORETURN(kunit_try_catch_throw)
+NORETURN(machine_real_restart)
+NORETURN(make_task_dead)
+NORETURN(mpt_halt_firmware)
+NORETURN(nmi_panic_self_stop)
+NORETURN(panic)
+NORETURN(panic_smp_self_stop)
+NORETURN(rest_init)
+NORETURN(rewind_stack_and_make_dead)
+NORETURN(sev_es_terminate)
+NORETURN(snp_abort)
+NORETURN(start_kernel)
+NORETURN(stop_this_cpu)
+NORETURN(usercopy_abort)
+NORETURN(x86_64_start_kernel)
+NORETURN(x86_64_start_reservations)
+NORETURN(xen_cpu_bringup_again)
+NORETURN(xen_start_kernel)
--
2.39.2

2023-04-18 21:45:25

by Josh Poimboeuf

Subject: [PATCH v3 6/8] objtool: Remove superfluous global_noreturns entries

lbug_with_loc() no longer exists, and resume_play_dead() is static
(objtool only checks globals and weaks).

Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/objtool/check.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 1cace399d32a..2047a6d5339b 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -217,7 +217,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
"kthread_complete_and_exit",
"kthread_exit",
"kunit_try_catch_throw",
- "lbug_with_loc",
"machine_real_restart",
"make_task_dead",
"mpt_halt_firmware",
@@ -225,7 +224,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
"panic",
"panic_smp_self_stop",
"rest_init",
- "resume_play_dead",
"rewind_stack_and_make_dead",
"sev_es_terminate",
"snp_abort",
--
2.39.2

2023-04-18 21:45:34

by Josh Poimboeuf

Subject: [PATCH v3 5/8] objtool: Ignore exc_double_fault() __noreturn warnings

This is a hack, but it works for now.

Problem is, exc_double_fault() may or may not return, depending on
whether CONFIG_X86_ESPFIX64 is set. But objtool has no visibility to
the kernel config.

"Fix" it by silencing the exc_double_fault() __noreturn warning.

This removes the following warning:

vmlinux.o: warning: objtool: xenpv_exc_double_fault+0xd: exc_double_fault() is missing a __noreturn annotation

Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/objtool/check.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index dd10128ab013..1cace399d32a 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4505,6 +4505,35 @@ static int validate_sls(struct objtool_file *file)
return warnings;
}

+static bool ignore_noreturn_call(struct instruction *insn)
+{
+ struct symbol *call_dest = insn_call_dest(insn);
+
+ /*
+ * FIXME: hack, we need a real noreturn solution
+ *
+ * Problem is, exc_double_fault() may or may not return, depending on
+ * whether CONFIG_X86_ESPFIX64 is set. But objtool has no visibility
+ * to the kernel config.
+ *
+ * Other potential ways to fix it:
+ *
+ * - have compiler communicate __noreturn functions somehow
+ * - remove CONFIG_X86_ESPFIX64
+ * - read the .config file
+ * - add a cmdline option
+ * - create a generic objtool annotation format (vs a bunch of custom
+ * formats) and annotate it
+ */
+ if (!strcmp(call_dest->name, "exc_double_fault")) {
+ /* prevent further unreachable warnings for the caller */
+ insn->sym->warned = 1;
+ return true;
+ }
+
+ return false;
+}
+
static int validate_reachable_instructions(struct objtool_file *file)
{
struct instruction *insn, *prev_insn;
@@ -4521,7 +4550,7 @@ static int validate_reachable_instructions(struct objtool_file *file)
prev_insn = prev_insn_same_sec(file, insn);
if (prev_insn && prev_insn->dead_end) {
call_dest = insn_call_dest(prev_insn);
- if (call_dest) {
+ if (call_dest && !ignore_noreturn_call(prev_insn)) {
WARN_INSN(insn, "%s() is missing a __noreturn annotation",
call_dest->name);
warnings++;
--
2.39.2

2023-04-19 15:04:48

by Miroslav Benes

Subject: Re: [PATCH v3 0/8] objtool: warning improvements

On Tue, 18 Apr 2023, Josh Poimboeuf wrote:

> v3:
> - dropped merged patches and rebased on tip/objtool/core
> - moved list of noreturn functions to separate file
> - improved WARN_INSN() to do single evaluation of insn arg
>
> Josh Poimboeuf (8):
> objtool: Limit unreachable warnings to once per function
> objtool: Add verbose option for disassembling affected functions
> objtool: Include backtrace in verbose mode
> objtool: Detect missing __noreturn annotations
> objtool: Ignore exc_double_fault() __noreturn warnings
> objtool: Remove superfluous global_noreturns entries
> tools/lib/subcmd: Replace NORETURN usage with __noreturn
> objtool: Move noreturn function list to separate file
>
> tools/lib/subcmd/parse-options.h | 8 +-
> tools/lib/subcmd/subcmd-util.h | 5 +-
> tools/objtool/Documentation/objtool.txt | 10 ++
> tools/objtool/builtin-check.c | 5 +
> tools/objtool/check.c | 195 ++++++++++++++++--------
> tools/objtool/include/objtool/builtin.h | 1 +
> tools/objtool/include/objtool/elf.h | 1 +
> tools/objtool/include/objtool/warn.h | 21 ++-
> tools/objtool/noreturns.h | 45 ++++++
> 9 files changed, 214 insertions(+), 77 deletions(-)
> create mode 100644 tools/objtool/noreturns.h

Reviewed-by: Miroslav Benes <[email protected]>

M

2023-05-18 11:22:05

by tip-bot2 for Alexey Makhalov

Subject: [tip: objtool/core] objtool: Remove superfluous global_noreturns entries

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: 34245659debd194cbd4148d2ee5176306bdf8899
Gitweb: https://git.kernel.org/tip/34245659debd194cbd4148d2ee5176306bdf8899
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Tue, 18 Apr 2023 14:27:52 -07:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Tue, 16 May 2023 06:31:54 -07:00

objtool: Remove superfluous global_noreturns entries

lbug_with_loc() no longer exists, and resume_play_dead() is static
(objtool only checks globals and weaks).

Reviewed-by: Miroslav Benes <[email protected]>
Link: https://lore.kernel.org/r/2725d7f2ccc2361c6903de9ebaa2b5bb304f7ac2.1681853186.git.jpoimboe@kernel.org
Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/objtool/check.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8dac1e3..8c2762d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -217,7 +217,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
"kthread_complete_and_exit",
"kthread_exit",
"kunit_try_catch_throw",
- "lbug_with_loc",
"machine_real_restart",
"make_task_dead",
"mpt_halt_firmware",
@@ -225,7 +224,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
"panic",
"panic_smp_self_stop",
"rest_init",
- "resume_play_dead",
"rewind_stack_and_make_dead",
"sev_es_terminate",
"snp_abort",

2023-05-18 11:24:10

by tip-bot2 for Alexey Makhalov

Subject: [tip: objtool/core] objtool: Ignore exc_double_fault() __noreturn warnings

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: 55eeab2a8a11b71586ef0ad3adf532ca5f97d4be
Gitweb: https://git.kernel.org/tip/55eeab2a8a11b71586ef0ad3adf532ca5f97d4be
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Tue, 18 Apr 2023 14:27:51 -07:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Tue, 16 May 2023 06:31:53 -07:00

objtool: Ignore exc_double_fault() __noreturn warnings

This is a hack, but it works for now.

Problem is, exc_double_fault() may or may not return, depending on
whether CONFIG_X86_ESPFIX64 is set. But objtool has no visibility to
the kernel config.

"Fix" it by silencing the exc_double_fault() __noreturn warning.

This removes the following warning:

vmlinux.o: warning: objtool: xenpv_exc_double_fault+0xd: exc_double_fault() is missing a __noreturn annotation

Reviewed-by: Miroslav Benes <[email protected]>
Link: https://lore.kernel.org/r/a45b085071d3a7d049a20f9e78754452336ecbe8.1681853186.git.jpoimboe@kernel.org
Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/objtool/check.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8d1b422..8dac1e3 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4505,6 +4505,35 @@ static int validate_sls(struct objtool_file *file)
return warnings;
}

+static bool ignore_noreturn_call(struct instruction *insn)
+{
+ struct symbol *call_dest = insn_call_dest(insn);
+
+ /*
+ * FIXME: hack, we need a real noreturn solution
+ *
+ * Problem is, exc_double_fault() may or may not return, depending on
+ * whether CONFIG_X86_ESPFIX64 is set. But objtool has no visibility
+ * to the kernel config.
+ *
+ * Other potential ways to fix it:
+ *
+ * - have compiler communicate __noreturn functions somehow
+ * - remove CONFIG_X86_ESPFIX64
+ * - read the .config file
+ * - add a cmdline option
+ * - create a generic objtool annotation format (vs a bunch of custom
+ * formats) and annotate it
+ */
+ if (!strcmp(call_dest->name, "exc_double_fault")) {
+ /* prevent further unreachable warnings for the caller */
+ insn->sym->warned = 1;
+ return true;
+ }
+
+ return false;
+}
+
static int validate_reachable_instructions(struct objtool_file *file)
{
struct instruction *insn, *prev_insn;
@@ -4521,7 +4550,7 @@ static int validate_reachable_instructions(struct objtool_file *file)
prev_insn = prev_insn_same_sec(file, insn);
if (prev_insn && prev_insn->dead_end) {
call_dest = insn_call_dest(prev_insn);
- if (call_dest) {
+ if (call_dest && !ignore_noreturn_call(prev_insn)) {
WARN_INSN(insn, "%s() is missing a __noreturn annotation",
call_dest->name);
warnings++;

2023-05-18 11:28:03

by tip-bot2 for Alexey Makhalov

Subject: [tip: objtool/core] objtool: Include backtrace in verbose mode

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: ced23d2e3762ecfb859ae65d3a351218edff7205
Gitweb: https://git.kernel.org/tip/ced23d2e3762ecfb859ae65d3a351218edff7205
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Tue, 18 Apr 2023 14:27:49 -07:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Tue, 16 May 2023 06:31:52 -07:00

objtool: Include backtrace in verbose mode

Include backtrace in verbose mode. This makes it easy to gather all the
information needed for diagnosing objtool warnings.

Reviewed-by: Miroslav Benes <[email protected]>
Link: https://lore.kernel.org/r/c255224fabcf7e64bac232fec1c77c9fc2d7d7ab.1681853186.git.jpoimboe@kernel.org
Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/objtool/Documentation/objtool.txt | 4 ++--
tools/objtool/check.c | 26 +++++++++---------------
tools/objtool/include/objtool/warn.h | 14 +++++++------
3 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/tools/objtool/Documentation/objtool.txt b/tools/objtool/Documentation/objtool.txt
index 8db1f29..9ec8cbf 100644
--- a/tools/objtool/Documentation/objtool.txt
+++ b/tools/objtool/Documentation/objtool.txt
@@ -246,8 +246,8 @@ Objtool warnings

NOTE: When requesting help with an objtool warning, please recreate with
OBJTOOL_VERBOSE=1 (e.g., "make OBJTOOL_VERBOSE=1") and send the full
-output, including any disassembly below the warning, to the objtool
-maintainers.
+output, including any disassembly or backtrace below the warning, to the
+objtool maintainers.

For asm files, if you're getting an error which doesn't make sense,
first make sure that the affected code follows the above rules.
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0bd0ca4..71985f3 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3657,8 +3657,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,

ret = validate_branch(file, func, alt->insn, state);
if (ret) {
- if (opts.backtrace)
- BT_FUNC("(alt)", insn);
+ BT_INSN(insn, "(alt)");
return ret;
}
}
@@ -3703,8 +3702,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
ret = validate_branch(file, func,
insn->jump_dest, state);
if (ret) {
- if (opts.backtrace)
- BT_FUNC("(branch)", insn);
+ BT_INSN(insn, "(branch)");
return ret;
}
}
@@ -3802,8 +3800,8 @@ static int validate_unwind_hint(struct objtool_file *file,
{
if (insn->hint && !insn->visited && !insn->ignore) {
int ret = validate_branch(file, insn_func(insn), insn, *state);
- if (ret && opts.backtrace)
- BT_FUNC("<=== (hint)", insn);
+ if (ret)
+ BT_INSN(insn, "<=== (hint)");
return ret;
}

@@ -3861,8 +3859,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)

ret = validate_unret(file, alt->insn);
if (ret) {
- if (opts.backtrace)
- BT_FUNC("(alt)", insn);
+ BT_INSN(insn, "(alt)");
return ret;
}
}
@@ -3888,10 +3885,8 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
}
ret = validate_unret(file, insn->jump_dest);
if (ret) {
- if (opts.backtrace) {
- BT_FUNC("(branch%s)", insn,
- insn->type == INSN_JUMP_CONDITIONAL ? "-cond" : "");
- }
+ BT_INSN(insn, "(branch%s)",
+ insn->type == INSN_JUMP_CONDITIONAL ? "-cond" : "");
return ret;
}

@@ -3913,8 +3908,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)

ret = validate_unret(file, dest);
if (ret) {
- if (opts.backtrace)
- BT_FUNC("(call)", insn);
+ BT_INSN(insn, "(call)");
return ret;
}
/*
@@ -4216,8 +4210,8 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
state->uaccess = sym->uaccess_safe;

ret = validate_branch(file, insn_func(insn), insn, *state);
- if (ret && opts.backtrace)
- BT_FUNC("<=== (sym)", insn);
+ if (ret)
+ BT_INSN(insn, "<=== (sym)");
return ret;
}

diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index f195dea..ac04d3f 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -63,12 +63,14 @@ static inline char *offstr(struct section *sec, unsigned long offset)
_insn->sym->warned = 1; \
})

-#define BT_FUNC(format, insn, ...) \
-({ \
- struct instruction *_insn = (insn); \
- char *_str = offstr(_insn->sec, _insn->offset); \
- WARN(" %s: " format, _str, ##__VA_ARGS__); \
- free(_str); \
+#define BT_INSN(insn, format, ...) \
+({ \
+ if (opts.verbose || opts.backtrace) { \
+ struct instruction *_insn = (insn); \
+ char *_str = offstr(_insn->sec, _insn->offset); \
+ WARN(" %s: " format, _str, ##__VA_ARGS__); \
+ free(_str); \
+ } \
})

#define WARN_ELF(format, ...) \

2023-05-18 11:29:06

by tip-bot2 for Alexey Makhalov

Subject: [tip: objtool/core] tools/lib/subcmd: Replace NORETURN usage with __noreturn

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: d59fec29b131f30b27343d54bdf1071ee98eda8e
Gitweb: https://git.kernel.org/tip/d59fec29b131f30b27343d54bdf1071ee98eda8e
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Tue, 18 Apr 2023 14:27:53 -07:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Tue, 16 May 2023 06:31:54 -07:00

tools/lib/subcmd: Replace NORETURN usage with __noreturn

NORETURN is redundant with __noreturn, just use the latter.

Reviewed-by: Miroslav Benes <[email protected]>
Link: https://lore.kernel.org/r/c7c83d1e6b3d2b0c3e65dd3790c22c772d3b2527.1681853186.git.jpoimboe@kernel.org
Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/lib/subcmd/parse-options.h | 8 ++------
tools/lib/subcmd/subcmd-util.h | 5 ++---
2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/tools/lib/subcmd/parse-options.h b/tools/lib/subcmd/parse-options.h
index 41b9b94..8e91473 100644
--- a/tools/lib/subcmd/parse-options.h
+++ b/tools/lib/subcmd/parse-options.h
@@ -6,10 +6,6 @@
#include <stdbool.h>
#include <stdint.h>

-#ifndef NORETURN
-#define NORETURN __attribute__((__noreturn__))
-#endif
-
enum parse_opt_type {
/* special types */
OPTION_END,
@@ -183,9 +179,9 @@ extern int parse_options_subcommand(int argc, const char **argv,
const char *const subcommands[],
const char *usagestr[], int flags);

-extern NORETURN void usage_with_options(const char * const *usagestr,
+extern __noreturn void usage_with_options(const char * const *usagestr,
const struct option *options);
-extern NORETURN __attribute__((format(printf,3,4)))
+extern __noreturn __attribute__((format(printf,3,4)))
void usage_with_options_msg(const char * const *usagestr,
const struct option *options,
const char *fmt, ...);
diff --git a/tools/lib/subcmd/subcmd-util.h b/tools/lib/subcmd/subcmd-util.h
index b2aec04..dfac76e 100644
--- a/tools/lib/subcmd/subcmd-util.h
+++ b/tools/lib/subcmd/subcmd-util.h
@@ -5,8 +5,7 @@
#include <stdarg.h>
#include <stdlib.h>
#include <stdio.h>
-
-#define NORETURN __attribute__((__noreturn__))
+#include <linux/compiler.h>

static inline void report(const char *prefix, const char *err, va_list params)
{
@@ -15,7 +14,7 @@ static inline void report(const char *prefix, const char *err, va_list params)
fprintf(stderr, " %s%s\n", prefix, msg);
}

-static NORETURN inline void die(const char *err, ...)
+static __noreturn inline void die(const char *err, ...)
{
va_list params;

2023-05-18 11:29:08

by tip-bot2 for Alexey Makhalov

Subject: [tip: objtool/core] objtool: Move noreturn function list to separate file

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: 6245ce4ab670166efcdae843c35c14e4c0811aa3
Gitweb: https://git.kernel.org/tip/6245ce4ab670166efcdae843c35c14e4c0811aa3
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Tue, 18 Apr 2023 14:27:54 -07:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Tue, 16 May 2023 06:31:55 -07:00

objtool: Move noreturn function list to separate file

This makes it a little cleaner and easier to maintain.

Suggested-by: Peter Zijlstra <[email protected]>
Reviewed-by: Miroslav Benes <[email protected]>
Link: https://lore.kernel.org/r/cecacf07a69a244c74474c18b7652627de67a528.1681853186.git.jpoimboe@kernel.org
Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/objtool/Documentation/objtool.txt | 5 +--
tools/objtool/check.c | 44 +----------------------
tools/objtool/noreturns.h | 45 ++++++++++++++++++++++++-
3 files changed, 50 insertions(+), 44 deletions(-)
create mode 100644 tools/objtool/noreturns.h

diff --git a/tools/objtool/Documentation/objtool.txt b/tools/objtool/Documentation/objtool.txt
index f9345e0..fe39c2a 100644
--- a/tools/objtool/Documentation/objtool.txt
+++ b/tools/objtool/Documentation/objtool.txt
@@ -306,9 +306,8 @@ the objtool maintainers.
3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation

The call from foo() to bar() doesn't return, but bar() is missing the
- __noreturn annotation. NOTE: In addition to adding the __noreturn
- annotation, the function name also needs to be added to
- 'global_noreturns' in tools/objtool/check.c.
+ __noreturn annotation. NOTE: In addition to annotating the function
+ with __noreturn, please also add it to tools/objtool/noreturns.h.

4. file.o: warning: objtool: func(): can't find starting instruction
or
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8c2762d..a13c257 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -192,49 +192,11 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
struct instruction *insn;
bool empty = true;

- /*
- * Unfortunately these have to be hard coded because the noreturn
- * attribute isn't provided in ELF data. Keep 'em sorted.
- */
+#define NORETURN(func) __stringify(func),
static const char * const global_noreturns[] = {
- "__invalid_creds",
- "__module_put_and_kthread_exit",
- "__reiserfs_panic",
- "__stack_chk_fail",
- "__ubsan_handle_builtin_unreachable",
- "arch_call_rest_init",
- "arch_cpu_idle_dead",
- "btrfs_assertfail",
- "cpu_bringup_and_idle",
- "cpu_startup_entry",
- "do_exit",
- "do_group_exit",
- "do_task_dead",
- "ex_handler_msr_mce",
- "fortify_panic",
- "hlt_play_dead",
- "hv_ghcb_terminate",
- "kthread_complete_and_exit",
- "kthread_exit",
- "kunit_try_catch_throw",
- "machine_real_restart",
- "make_task_dead",
- "mpt_halt_firmware",
- "nmi_panic_self_stop",
- "panic",
- "panic_smp_self_stop",
- "rest_init",
- "rewind_stack_and_make_dead",
- "sev_es_terminate",
- "snp_abort",
- "start_kernel",
- "stop_this_cpu",
- "usercopy_abort",
- "x86_64_start_kernel",
- "x86_64_start_reservations",
- "xen_cpu_bringup_again",
- "xen_start_kernel",
+#include "noreturns.h"
};
+#undef NORETURN

if (!func)
return false;
diff --git a/tools/objtool/noreturns.h b/tools/objtool/noreturns.h
new file mode 100644
index 0000000..cede606
--- /dev/null
+++ b/tools/objtool/noreturns.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This is a (sorted!) list of all known __noreturn functions in the kernel.
+ * It's needed for objtool to properly reverse-engineer the control flow graph.
+ *
+ * Yes, this is unfortunate. A better solution is in the works.
+ */
+NORETURN(__invalid_creds)
+NORETURN(__module_put_and_kthread_exit)
+NORETURN(__reiserfs_panic)
+NORETURN(__stack_chk_fail)
+NORETURN(__ubsan_handle_builtin_unreachable)
+NORETURN(arch_call_rest_init)
+NORETURN(arch_cpu_idle_dead)
+NORETURN(btrfs_assertfail)
+NORETURN(cpu_bringup_and_idle)
+NORETURN(cpu_startup_entry)
+NORETURN(do_exit)
+NORETURN(do_group_exit)
+NORETURN(do_task_dead)
+NORETURN(ex_handler_msr_mce)
+NORETURN(fortify_panic)
+NORETURN(hlt_play_dead)
+NORETURN(hv_ghcb_terminate)
+NORETURN(kthread_complete_and_exit)
+NORETURN(kthread_exit)
+NORETURN(kunit_try_catch_throw)
+NORETURN(machine_real_restart)
+NORETURN(make_task_dead)
+NORETURN(mpt_halt_firmware)
+NORETURN(nmi_panic_self_stop)
+NORETURN(panic)
+NORETURN(panic_smp_self_stop)
+NORETURN(rest_init)
+NORETURN(rewind_stack_and_make_dead)
+NORETURN(sev_es_terminate)
+NORETURN(snp_abort)
+NORETURN(start_kernel)
+NORETURN(stop_this_cpu)
+NORETURN(usercopy_abort)
+NORETURN(x86_64_start_kernel)
+NORETURN(x86_64_start_reservations)
+NORETURN(xen_cpu_bringup_again)
+NORETURN(xen_start_kernel)