| Message ID | 20250113-objtool-strict-v3-1-8b51f94957fb@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | objtool: Add option to fail build on vmlinux warnings | expand |
On Mon, Jan 13, 2025 at 02:05:15PM +0000, Brendan Jackman wrote: > At present objtool only prints to the terminal when observing "fatal > warnings". This option lets you have it produce an error instead. > > The use case for this is noinstr validation; so far I've never seen any > false warnings here, but it quite often detects real bugs. It would > be useful for the build to fail when I have those bugs. > > Signed-off-by: Brendan Jackman <jackmanb@google.com> > --- > tools/objtool/builtin-check.c | 6 ++++++ > tools/objtool/check.c | 7 ++----- > tools/objtool/include/objtool/builtin.h | 1 + > 3 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c > index 387d56a7f5fb8da8435d0a3f5c05eeee66932c9b..0b28082df90710ff7127327deb857c0548f378c7 100644 > --- a/tools/objtool/builtin-check.c > +++ b/tools/objtool/builtin-check.c > @@ -94,6 +94,12 @@ static const struct option check_options[] = { > OPT_BOOLEAN(0, "sec-address", &opts.sec_address, "print section addresses in warnings"), > OPT_BOOLEAN(0, "stats", &opts.stats, "print statistics"), > OPT_BOOLEAN('v', "verbose", &opts.verbose, "verbose warnings"), > + /* > + * For now, don't fail the kernel build on fatal warnings by default. > + * These errors are still fairly common due to the growing matrix of > + * supported toolchains and their recent pace of change. > + */ > + OPT_BOOLEAN(0, "Werror", &opts.werror, "fail on fatal warnings"), > > OPT_END(), > }; > diff --git a/tools/objtool/check.c b/tools/objtool/check.c > index 76060da755b5c51cda3a669d8245d7d004e25f22..e44135293eb45f908e00359d84d954cfeddd266f 100644 > --- a/tools/objtool/check.c > +++ b/tools/objtool/check.c > @@ -4944,10 +4944,7 @@ int check(struct objtool_file *file) > } > > out: > - /* > - * For now, don't fail the kernel build on fatal warnings. These > - * errors are still fairly common due to the growing matrix of > - * supported toolchains and their recent pace of change. > - */ > + if (opts.werror && warnings) It might be a good idea to print a message here about why the build is failing, in lieu of turning all "objtool: warning:" messages into "objtool: error:" messages ala -Werror for C compilers, which does not seem simple on quick glance. Otherwise, I am not entirely sure it will be obvious to people why a build like allmodconfig may start failing if this configuration gets turned on. https://lore.kernel.org/Z4X8j%2FqJj7ib0vkh@rli9-mobl/ > + return 1; > return 0; > } > diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h > index fcca6662c8b4b5e0048e54fada8694cc2e6ebc34..97d668010efadfa05bb6e25e1967a7d72bf77815 100644 > --- a/tools/objtool/include/objtool/builtin.h > +++ b/tools/objtool/include/objtool/builtin.h > @@ -38,6 +38,7 @@ struct opts { > bool sec_address; > bool stats; > bool verbose; > + bool werror; > }; > > extern struct opts opts; > > -- > 2.47.1.613.gc27f4b7a9f-goog >
On Tue, Jan 14, 2025 at 10:24:29AM -0700, Nathan Chancellor wrote: > > It might be a good idea to print a message here about why the build is > failing, in lieu of turning all "objtool: warning:" messages into > "objtool: error:" messages ala -Werror for C compilers, which does not > seem simple on quick glance. Otherwise, I am not entirely sure it will > be obvious to people why a build like allmodconfig may start failing if > this configuration gets turned on. > > https://lore.kernel.org/Z4X8j%2FqJj7ib0vkh@rli9-mobl/ Yeah, makes sense. I'll add this: From: Josh Poimboeuf <jpoimboe@kernel.org> Subject: [PATCH] objtool: Change "warning:" to "error:" for CONFIG_OBJTOOL_WERROR This matches GCC's behavior and makes it more obvious why the build failed. Suggested-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- tools/objtool/include/objtool/warn.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h index ac04d3fe4dd9..78f602b5daed 100644 --- a/tools/objtool/include/objtool/warn.h +++ b/tools/objtool/include/objtool/warn.h @@ -43,8 +43,10 @@ static inline char *offstr(struct section *sec, unsigned long offset) #define WARN(format, ...) \ fprintf(stderr, \ - "%s: warning: objtool: " format "\n", \ - objname, ##__VA_ARGS__) + "%s: %s: objtool: " format "\n", \ + objname, \ + opts.werror ? "error" : "warning", \ + ##__VA_ARGS__) #define WARN_FUNC(format, sec, offset, ...) \ ({ \
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index 387d56a7f5fb8da8435d0a3f5c05eeee66932c9b..0b28082df90710ff7127327deb857c0548f378c7 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -94,6 +94,12 @@ static const struct option check_options[] = { OPT_BOOLEAN(0, "sec-address", &opts.sec_address, "print section addresses in warnings"), OPT_BOOLEAN(0, "stats", &opts.stats, "print statistics"), OPT_BOOLEAN('v', "verbose", &opts.verbose, "verbose warnings"), + /* + * For now, don't fail the kernel build on fatal warnings by default. + * These errors are still fairly common due to the growing matrix of + * supported toolchains and their recent pace of change. + */ + OPT_BOOLEAN(0, "Werror", &opts.werror, "fail on fatal warnings"), OPT_END(), }; diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 76060da755b5c51cda3a669d8245d7d004e25f22..e44135293eb45f908e00359d84d954cfeddd266f 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -4944,10 +4944,7 @@ int check(struct objtool_file *file) } out: - /* - * For now, don't fail the kernel build on fatal warnings. These - * errors are still fairly common due to the growing matrix of - * supported toolchains and their recent pace of change. - */ + if (opts.werror && warnings) + return 1; return 0; } diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h index fcca6662c8b4b5e0048e54fada8694cc2e6ebc34..97d668010efadfa05bb6e25e1967a7d72bf77815 100644 --- a/tools/objtool/include/objtool/builtin.h +++ b/tools/objtool/include/objtool/builtin.h @@ -38,6 +38,7 @@ struct opts { bool sec_address; bool stats; bool verbose; + bool werror; }; extern struct opts opts;
At present objtool only prints to the terminal when observing "fatal warnings". This option lets you have it produce an error instead. The use case for this is noinstr validation; so far I've never seen any false warnings here, but it quite often detects real bugs. It would be useful for the build to fail when I have those bugs. Signed-off-by: Brendan Jackman <jackmanb@google.com> --- tools/objtool/builtin-check.c | 6 ++++++ tools/objtool/check.c | 7 ++----- tools/objtool/include/objtool/builtin.h | 1 + 3 files changed, 9 insertions(+), 5 deletions(-)