Re: [PATCH v2 32/62] objtool: Suppress section skipping warnings with --dryrun
On Thu, Jun 05, 2025 at 04:52:24PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 05, 2025 at 09:32:46AM +0200, Peter Zijlstra wrote:
>
> > > But also, feel free to resurrect --backup, or you can yell at me to do
> > > it as the backup code changed a bit.
> >
> > I have the patch somewhere, failed to send it out. I'll try and dig it
> > out later today.
>
> This is what I had. Wasn't sure we wanted to make -v imply --backup ?
Yeah, I suppose --verbose shouldn't be doing unrequested changes.
Regardless I want to keep the feature where print_args() modifies the
args to use the backup as input as that's very convenient. We can just
tie that (and the printing of the args itself) to --backup.
> I'm used to stealing the objtool arguments from V=1 builds. I suppose
> the print_args thing is easier, might get used to it eventually.
>
>
> diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
> index 80239843e9f0..7d8f99cf9b0b 100644
> --- a/tools/objtool/builtin-check.c
> +++ b/tools/objtool/builtin-check.c
> @@ -101,6 +101,7 @@ static const struct option check_options[] = {
> OPT_BOOLEAN(0, "stats", &opts.stats, "print statistics"),
> OPT_BOOLEAN('v', "verbose", &opts.verbose, "verbose warnings"),
> OPT_BOOLEAN(0, "Werror", &opts.werror, "return error on warnings"),
> + OPT_BOOLEAN(0, "backup", &opts.backup, "create a backup (.orig) file
> on error"),
It should also work on warnings (non-werror) as well.
Something like so?
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 80239843e9f0..d73ae71861fc 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -91,6 +91,7 @@ static const struct option check_options[] = {
OPT_GROUP("Options:"),
OPT_BOOLEAN(0, "backtrace", &opts.backtrace, "unwind on error"),
+ OPT_BOOLEAN(0, "backup", &opts.backup, "create a backup (.orig) file
on warning"),
OPT_BOOLEAN(0, "dry-run", &opts.dryrun, "don't write modifications"),
OPT_BOOLEAN(0, "link", &opts.link, "object is a linked object"),
OPT_BOOLEAN(0, "module", &opts.module, "object is part of a kernel
module"),
@@ -244,12 +245,9 @@ static void save_argv(int argc, const char **argv)
};
}
-void print_args(void)
+int make_backup(void)
{
- char *backup = NULL;
-
- if (opts.output || opts.dryrun)
- goto print;
+ char *backup;
/*
* Make a backup before kbuild deletes the file so the error
@@ -258,33 +256,32 @@ void print_args(void)
backup = malloc(strlen(objname) + strlen(ORIG_SUFFIX) + 1);
if (!backup) {
ERROR_GLIBC("malloc");
- goto print;
+ return 1;
}
strcpy(backup, objname);
strcat(backup, ORIG_SUFFIX);
- if (copy_file(objname, backup)) {
- backup = NULL;
- goto print;
- }
+ if (copy_file(objname, backup))
+ return 1;
-print:
/*
-* Print the cmdline args to make it easier to recreate. If '--output'
-* wasn't used, add it to the printed args with the backup as input.
+* Print the cmdline args to make it easier to recreate.
*/
+
fprintf(stderr, "%s", orig_argv[0]);
for (int i = 1; i < orig_argc; i++) {
char *arg = orig_argv[i];
- if (backup && !strcmp(arg, objname))
+ /* Modify the printed args to use the backup */
+ if (!opts.output && !strcmp(arg, objname))
fprintf(stderr, " %s -o %s", backup, objname);
else
fprintf(stderr, " %s", arg);
}
fprintf(stderr, "\n");
+ return 0;
}
int objtool_run(int argc, const char **argv)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 3a411064fa34..848dead666ae 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4798,9 +4798,11 @@ int check(struct objtool_file *file)
if (opts.verbose) {
if (opts.werror && warnings)
WARN("%d warning(s) upgraded to errors", warnings);
- print_args();
disas_warned_funcs(file);
}
+ if (opts.backup && make_backup())
+ return 1;
+
return ret;
}
diff --git a/tools/objtool/include/objtool/builtin.h
b/tools/objtool/include/objtool/builtin.h
index 6b08666fa69d..de6c08f8e060 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -29,6 +29,7 @@ struct opts {
/* options: */
bool backtrace;
+ bool backup;
bool dryrun;
bool link;
bool mnop;
@@ -47,6 +48,6 @@ int cmd_parse_options(int argc, const char **argv, const char
* const usage[]);
int objtool_run(int argc, const char **argv);
-void print_args(void);
+int make_backup(void);
#endif /* _BUILTIN_H
Re: [PATCH v2 32/62] objtool: Suppress section skipping warnings with --dryrun
On Thu, Jun 05, 2025 at 09:32:46AM +0200, Peter Zijlstra wrote:
> > But also, feel free to resurrect --backup, or you can yell at me to do
> > it as the backup code changed a bit.
>
> I have the patch somewhere, failed to send it out. I'll try and dig it
> out later today.
This is what I had. Wasn't sure we wanted to make -v imply --backup ?
I'm used to stealing the objtool arguments from V=1 builds. I suppose
the print_args thing is easier, might get used to it eventually.
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 80239843e9f0..7d8f99cf9b0b 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -101,6 +101,7 @@ static const struct option check_options[] = {
OPT_BOOLEAN(0, "stats", &opts.stats, "print statistics"),
OPT_BOOLEAN('v', "verbose", &opts.verbose, "verbose warnings"),
OPT_BOOLEAN(0, "Werror", &opts.werror, "return error on warnings"),
+ OPT_BOOLEAN(0, "backup", &opts.backup, "create a backup (.orig) file
on error"),
OPT_END(),
};
@@ -244,13 +245,10 @@ static void save_argv(int argc, const char **argv)
};
}
-void print_args(void)
+static void make_backup(void)
{
char *backup = NULL;
- if (opts.output || opts.dryrun)
- goto print;
-
/*
* Make a backup before kbuild deletes the file so the error
* can be recreated without recompiling or relinking.
@@ -258,17 +256,19 @@ void print_args(void)
backup = malloc(strlen(objname) + strlen(ORIG_SUFFIX) + 1);
if (!backup) {
ERROR_GLIBC("malloc");
- goto print;
+ return;
}
strcpy(backup, objname);
strcat(backup, ORIG_SUFFIX);
if (copy_file(objname, backup)) {
backup = NULL;
- goto print;
+ return;
}
+}
-print:
+void print_args(void)
+{
/*
* Print the cmdline args to make it easier to recreate. If '--output'
* wasn't used, add it to the printed args with the backup as input.
@@ -278,10 +278,7 @@ void print_args(void)
for (int i = 1; i < orig_argc; i++) {
char *arg = orig_argv[i];
- if (backup && !strcmp(arg, objname))
- fprintf(stderr, " %s -o %s", backup, objname);
- else
- fprintf(stderr, " %s", arg);
+ fprintf(stderr, " %s", arg);
}
fprintf(stderr, "\n");
@@ -324,8 +321,11 @@ int objtool_run(int argc, const char **argv)
}
ret = check(file);
- if (ret)
+ if (ret) {
+ if (opts.backup)
+ make_backup();
return ret;
+ }
if (!opts.dryrun && file->elf->changed && elf_write(file->elf))
return 1;
diff --git a/tools/objtool/include/objtool/builtin.h
b/tools/objtool/include/objtool/builtin.h
index 6b08666fa69d..97c36fb1fe9a 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -39,6 +39,7 @@ struct opts {
bool stats;
bool verbose;
bool werror;
+ bool backup;
};
extern struct opts opts;
Re: [PATCH v2 32/62] objtool: Suppress section skipping warnings with --dryrun
On Wed, Jun 04, 2025 at 05:19:51PM -0700, Josh Poimboeuf wrote: > On Wed, May 28, 2025 at 12:34:53PM +0200, Peter Zijlstra wrote: > > On Mon, May 26, 2025 at 12:52:40PM +0200, Peter Zijlstra wrote: > > > On Fri, May 09, 2025 at 01:16:56PM -0700, Josh Poimboeuf wrote: > > > > It's common to use --dryrun on binaries that have already been > > > > processed. Don't print the section skipping warnings in that case. > > > > > > Ah, I rather like this warning, it gives me an easy check to see if the > > > file has already been processed. > > > > > > I typically do a OBJTOOL_ARGS="--backup" build and run dryrun debug > > > sessions against those .orig files. > > > > Turns out, you already broke this.. :-( > > > > I'm now having a case where objtool fails on vmlinux.o and make happily > > deletes vmlinux.o and I'm left empty handed. > > > > Let me go resurrect --backup > > Yeah, as I just mentioned in that other email, --verbose should give you > what you need. It also prints the cmdline args, which is nice. > > But also, feel free to resurrect --backup, or you can yell at me to do > it as the backup code changed a bit. I have the patch somewhere, failed to send it out. I'll try and dig it out later today.
Re: [PATCH v2 32/62] objtool: Suppress section skipping warnings with --dryrun
On Mon, May 26, 2025 at 12:52:40PM +0200, Peter Zijlstra wrote: > On Fri, May 09, 2025 at 01:16:56PM -0700, Josh Poimboeuf wrote: > > It's common to use --dryrun on binaries that have already been > > processed. Don't print the section skipping warnings in that case. > > Ah, I rather like this warning, it gives me an easy check to see if the > file has already been processed. > > I typically do a OBJTOOL_ARGS="--backup" build and run dryrun debug > sessions against those .orig files. Ok. Though, note that as of a few months ago, --backup no longer exists. A backup is now automatically created with --verbose. But we can revive it if you want. -- Josh
Re: [PATCH v2 32/62] objtool: Suppress section skipping warnings with --dryrun
On Wed, May 28, 2025 at 12:34:53PM +0200, Peter Zijlstra wrote: > On Mon, May 26, 2025 at 12:52:40PM +0200, Peter Zijlstra wrote: > > On Fri, May 09, 2025 at 01:16:56PM -0700, Josh Poimboeuf wrote: > > > It's common to use --dryrun on binaries that have already been > > > processed. Don't print the section skipping warnings in that case. > > > > Ah, I rather like this warning, it gives me an easy check to see if the > > file has already been processed. > > > > I typically do a OBJTOOL_ARGS="--backup" build and run dryrun debug > > sessions against those .orig files. > > Turns out, you already broke this.. :-( > > I'm now having a case where objtool fails on vmlinux.o and make happily > deletes vmlinux.o and I'm left empty handed. > > Let me go resurrect --backup Yeah, as I just mentioned in that other email, --verbose should give you what you need. It also prints the cmdline args, which is nice. But also, feel free to resurrect --backup, or you can yell at me to do it as the backup code changed a bit. -- Josh
Re: [PATCH v2 32/62] objtool: Suppress section skipping warnings with --dryrun
On Mon, May 26, 2025 at 12:52:40PM +0200, Peter Zijlstra wrote: > On Fri, May 09, 2025 at 01:16:56PM -0700, Josh Poimboeuf wrote: > > It's common to use --dryrun on binaries that have already been > > processed. Don't print the section skipping warnings in that case. > > Ah, I rather like this warning, it gives me an easy check to see if the > file has already been processed. > > I typically do a OBJTOOL_ARGS="--backup" build and run dryrun debug > sessions against those .orig files. Turns out, you already broke this.. :-( I'm now having a case where objtool fails on vmlinux.o and make happily deletes vmlinux.o and I'm left empty handed. Let me go resurrect --backup
Re: [PATCH v2 32/62] objtool: Suppress section skipping warnings with --dryrun
On Fri, May 09, 2025 at 01:16:56PM -0700, Josh Poimboeuf wrote: > It's common to use --dryrun on binaries that have already been > processed. Don't print the section skipping warnings in that case. Ah, I rather like this warning, it gives me an easy check to see if the file has already been processed. I typically do a OBJTOOL_ARGS="--backup" build and run dryrun debug sessions against those .orig files.
[PATCH v2 32/62] objtool: Suppress section skipping warnings with --dryrun
It's common to use --dryrun on binaries that have already been
processed. Don't print the section skipping warnings in that case.
Signed-off-by: Josh Poimboeuf
---
tools/objtool/check.c | 28 +---
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 3b9443b98fd5..66cbeebd16ea 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -637,7 +637,9 @@ static int create_static_call_sections(struct objtool_file
*file)
sec = find_section_by_name(file->elf, ".static_call_sites");
if (sec) {
- WARN("file already has .static_call_sites section, skipping");
+ if (!opts.dryrun)
+ WARN("file already has .static_call_sites section,
skipping");
+
return 0;
}
@@ -719,7 +721,9 @@ static int create_retpoline_sites_sections(struct
objtool_file *file)
sec = find_section_by_name(file->elf, ".retpoline_sites");
if (sec) {
- WARN("file already has .retpoline_sites, skipping");
+ if (!opts.dryrun)
+ WARN("file already has .retpoline_sites, skipping");
+
return 0;
}
@@ -757,7 +761,9 @@ static int create_return_sites_sections(struct objtool_file
*file)
sec = find_section_by_name(file->elf, ".return_sites");
if (sec) {
- WARN("file already has .return_sites, skipping");
+ if (!opts.dryrun)
+ WARN("file already has .return_sites, skipping");
+
return 0;
}
@@ -795,7 +801,9 @@ static int create_ibt_endbr_seal_sections(struct
objtool_file *file)
sec = find_section_by_name(file->elf, ".ibt_endbr_seal");
if (sec) {
- WARN("file already has .ibt_endbr_seal, skipping");
+ if (!opts.dryrun)
+ WARN("file already has .ibt_endbr_seal, skipping");
+
return 0;
}
@@ -852,7 +860,9 @@ static int create_cfi_sections(struct objtool_file *file)
sec = find_section_by_name(file->elf, ".cfi_sites");
if (sec) {
- WARN("file already has .cfi_sites section, skipping");
+ if (!opts.dryrun)
+ WARN("file already has .cfi_sites section, skipping");
+
return 0;
}
@@ -900,7 +910,9 @@ static int create_mcount_loc_sections(struct objtool_file
*file)
sec = find_section_by_name(file->elf, "__mcount_loc");
if (sec) {
- WARN("file already has __mcount_loc section, skipping");
+ if (!opts.dryrun)
+ WARN("file already has __mcount_loc section, skipping");
+
return 0;
}
@@ -944,7 +956,9 @@ static int create_direct_call_sections(struct objtool_file
*file)
sec = find_section_by_name(file->elf, ".call_sites");
if (sec) {
- WARN("file already has .call_sites section, skipping");
+ if (!opts.dryrun)
+ WARN("file already has .call_sites section, skipping");
+
return 0;
}
--
2.49.0
