On Tue, Mar 28, 2023 at 01:19:21PM -0700, Josh Poimboeuf wrote:
> On Tue, Mar 28, 2023 at 10:07:43AM +0200, Peter Zijlstra wrote:
> > On Mon, Mar 27, 2023 at 09:00:45AM -0700, Josh Poimboeuf wrote:
> > > Get rid of the '--backtrace' option, instead including that
> > > functionality in '--verbose'. This makes it easy to gather all the
> > > information needed for diagnosing objtool warnings.
> >
> > Hurmm.. can't we have verbose imply backtrace but keep the separate
> > option? I'm not sure if I always want the objdump thing -- esp with
> > multiple warnings on vmlinux that's going to be really slow -- better to
> > dump the whole of vmlinux.o once at the end.
>
> That's a good point, vmlinux would be unbearable for multiple warnings.
>
> We could accumulate a list of affected functions and then supply that to
> objdump-func.sh at the end and dump them all at the same time.
>
> objdump-func.sh would need to be changed to look for multiple funcs at
> once.
>
> If I do that, do you still want the separate backtrace option?
For now, lets keep the orthogonal options -- we can always remove it
later if we find they go unused. This automagic stuff is going to take a
bit of getting used to I recon ;-)
@@ -246,8 +246,8 @@ Objtool warnings
NOTE: When requesting help with an objtool warning, please re-run the
kernel build with `OBJTOOL_ARGS="--verbose" make <whatever>` and send
-the full warning output (including any function disassembly below the
-warning) to the objtool maintainers.
+the full warning output (including any function disassembly or objtool
+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.
@@ -84,7 +84,6 @@ static const struct option check_options[] = {
OPT_CALLBACK_OPTARG(0, "dump", NULL, NULL, "orc", "dump metadata", parse_dump),
OPT_GROUP("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, "link", &opts.link, "object is a linked object"),
@@ -3708,8 +3708,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_FUNC("(alt)", insn);
return ret;
}
}
@@ -3755,8 +3754,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_FUNC("(branch)", insn);
return ret;
}
}
@@ -3855,7 +3853,7 @@ 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)
+ if (ret)
BT_FUNC("<=== (hint)", insn);
return ret;
}
@@ -3914,8 +3912,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_FUNC("(alt)", insn);
return ret;
}
}
@@ -3942,10 +3939,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_FUNC("(branch%s)", insn,
+ insn->type == INSN_JUMP_CONDITIONAL ? "-cond" : "");
return ret;
}
@@ -3967,8 +3962,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_FUNC("(call)", insn);
return ret;
}
/*
@@ -4254,7 +4248,7 @@ 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)
+ if (ret)
BT_FUNC("<=== (sym)", insn);
return ret;
}
@@ -28,7 +28,6 @@ struct opts {
bool cfi;
/* options: */
- bool backtrace;
bool backup;
bool dryrun;
bool link;
@@ -91,12 +91,14 @@ static inline void objdump_func(struct section *sec, unsigned long offset)
objdump_func(sec, offset); \
})
-#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_FUNC(format, insn, ...) \
+({ \
+ if (opts.verbose) { \
+ struct instruction *_insn = (insn); \
+ char *_str = offstr(_insn->sec, _insn->offset); \
+ WARN(" %s: " format, _str, ##__VA_ARGS__); \
+ free(_str); \
+ } \
})
#define WARN_ELF(format, ...) \