Objtool warnings can be indicative of crashes, broken live patching, or
even boot failures.  Ignoring them is not recommended.

Add CONFIG_OBJTOOL_WERROR to upgrade objtool warnings to errors by
enabling the objtool --Werror option.  Also set --backtrace to print the
branches leading up to the warning, which can help considerably when
debugging certain warnings.

To avoid breaking bots too badly for now, make it the default for real
world builds only (!COMPILE_TEST).

Co-developed-by: Brendan Jackman <jackmanb@google.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 lib/Kconfig.debug    | 12 ++++++++++++
 scripts/Makefile.lib |  1 +
 2 files changed, 13 insertions(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 35796c290ca3..bbfb9d575f97 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -545,6 +545,18 @@ config FRAME_POINTER
 config OBJTOOL
 	bool
 
+config OBJTOOL_WERROR
+	bool "Upgrade objtool warnings to errors"
+	default y
+	depends on OBJTOOL && !COMPILE_TEST
+	help
+	  Fail the build on objtool warnings.
+
+	  Objtool warnings can indicate kernel instability, including boot
+	  failures.  This option is highly recommended.
+
+	  If unsure, say Y.
+
 config STACK_VALIDATION
 	bool "Compile-time stack metadata validation"
 	depends on HAVE_STACK_VALIDATION && UNWINDER_FRAME_POINTER
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index cad20f0e66ee..99e281966ba3 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -277,6 +277,7 @@ objtool-args-$(CONFIG_HAVE_STATIC_CALL_INLINE)		+= --static-call
 objtool-args-$(CONFIG_HAVE_UACCESS_VALIDATION)		+= --uaccess
 objtool-args-$(CONFIG_GCOV_KERNEL)			+= --no-unreachable
 objtool-args-$(CONFIG_PREFIX_SYMBOLS)			+= --prefix=$(CONFIG_FUNCTION_PADDING_BYTES)
+objtool-args-$(CONFIG_OBJTOOL_WERROR)			+= --Werror --backtrace
 
 objtool-args = $(objtool-args-y)					\
 	$(if $(delay-objtool), --link)					\
-- 
2.48.1

On Fri, 14 Mar 2025, Josh Poimboeuf wrote:

> Objtool warnings can be indicative of crashes, broken live patching, or
> even boot failures.  Ignoring them is not recommended.
> 
> Add CONFIG_OBJTOOL_WERROR to upgrade objtool warnings to errors by
> enabling the objtool --Werror option.  Also set --backtrace to print the
> branches leading up to the warning, which can help considerably when
> debugging certain warnings.
> 
> To avoid breaking bots too badly for now, make it the default for real
> world builds only (!COMPILE_TEST).
> 
> Co-developed-by: Brendan Jackman <jackmanb@google.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Brendan's SoB missing.

With 'default y' being present or not

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

and fingers crossed.

M

On Mon, Mar 17, 2025 at 03:14:25PM +0100, Miroslav Benes wrote:
> On Fri, 14 Mar 2025, Josh Poimboeuf wrote:
> 
> > Objtool warnings can be indicative of crashes, broken live patching, or
> > even boot failures.  Ignoring them is not recommended.
> > 
> > Add CONFIG_OBJTOOL_WERROR to upgrade objtool warnings to errors by
> > enabling the objtool --Werror option.  Also set --backtrace to print the
> > branches leading up to the warning, which can help considerably when
> > debugging certain warnings.
> > 
> > To avoid breaking bots too badly for now, make it the default for real
> > world builds only (!COMPILE_TEST).
> > 
> > Co-developed-by: Brendan Jackman <jackmanb@google.com>
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> Brendan's SoB missing.
> 
> With 'default y' being present or not
> 
> Reviewed-by: Miroslav Benes <mbenes@suse.cz>

Miroslav and Brendan, thanks for the reviews.

This was merged with a quickness, so I wasn't able to post a v2 to
update the commit logs.  However I will be posting some patches soon to
address the other comments, along with a bunch of fixes.

-- 
Josh

* Josh Poimboeuf <jpoimboe@kernel.org> wrote:

> Objtool warnings can be indicative of crashes, broken live patching, or
> even boot failures.  Ignoring them is not recommended.
> 
> Add CONFIG_OBJTOOL_WERROR to upgrade objtool warnings to errors by
> enabling the objtool --Werror option.  Also set --backtrace to print the
> branches leading up to the warning, which can help considerably when
> debugging certain warnings.
> 
> To avoid breaking bots too badly for now, make it the default for real
> world builds only (!COMPILE_TEST).
> 
> Co-developed-by: Brendan Jackman <jackmanb@google.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
>  lib/Kconfig.debug    | 12 ++++++++++++
>  scripts/Makefile.lib |  1 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 35796c290ca3..bbfb9d575f97 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -545,6 +545,18 @@ config FRAME_POINTER
>  config OBJTOOL
>  	bool
>  
> +config OBJTOOL_WERROR
> +	bool "Upgrade objtool warnings to errors"
> +	default y
> +	depends on OBJTOOL && !COMPILE_TEST
> +	help
> +	  Fail the build on objtool warnings.

This is *way* too aggressive: objtool false positives are still common, 
and an 'allmodconfig' should not fail the build IMO.

Thanks,

	Ingo

On Sun, Mar 16, 2025 at 01:41:43AM +0100, Ingo Molnar wrote:
> > +config OBJTOOL_WERROR
> > +	bool "Upgrade objtool warnings to errors"
> > +	default y
> > +	depends on OBJTOOL && !COMPILE_TEST
> > +	help
> > +	  Fail the build on objtool warnings.
> 
> This is *way* too aggressive: objtool false positives are still common, 

I'm not sure what false positives you'd be referring to, these days the
vast majority of warnings I see are actual bugs.

> and an 'allmodconfig' should not fail the build IMO.

In fact it doesn't: allmodconfig sets COMPILE_TEST which prevents
OBJTOOL_WERROR.

I've had complaints from people who spent days debugging a broken kernel
only to discover it was related to an ignored objtool warning.

For non-bot builds which actually get run on a system, many of the
warnings should most definitely not be ignored.  Especially since
objtool is directly involved in some critical kernel infrastructure like
static calls/jumps and CPU mitigations.

All that said, I really don't know whether this default is too
aggressive.  But I do wonder if running it through a linux-next cycle
after the next merge window might help answer that question.

-- 
Josh

* Josh Poimboeuf <jpoimboe@kernel.org> wrote:

> On Sun, Mar 16, 2025 at 01:41:43AM +0100, Ingo Molnar wrote:
> > > +config OBJTOOL_WERROR
> > > +	bool "Upgrade objtool warnings to errors"
> > > +	default y
> > > +	depends on OBJTOOL && !COMPILE_TEST
> > > +	help
> > > +	  Fail the build on objtool warnings.
> > 
> > This is *way* too aggressive: objtool false positives are still common, 
> 
> I'm not sure what false positives you'd be referring to, these days the
> vast majority of warnings I see are actual bugs.

I'm not sure we are reading the same kernel log :-)

A simple 'git log arch/x86/' followed by a search for objtool pointed 
to the following recent commits:


  #
  # Included a fix for a false positive:
  #
  4e32645cd8f9 x86/smp: Fix mwait_play_dead() and acpi_processor_ffh_play_dead() noreturn behavior

  #
  # objtool poinpointed a problem that has no runtime effects,
  # ie. it's a functional false positive and breaking the build
  # for *that* would have been excessive:
  #
  73e8079be9e7 x86/ibt: Make cfi_bhi a constant for FINEIBT_BHI=n


  #
  # Commit works around an objtool false positive found during development:
  #
  b815f6877d80 x86/bhi: Add BHI stubs


  #
  # Commit works around what appears to be a objtool false positive
  # about too aggressive code generation in function prologues:
  # (An issue that does not seem to trigger in practice.)
  #
  4087e16b0331 x86/locking: Use ALT_OUTPUT_SP() for percpu_{,try_}cmpxchg{64,128}_op()

I literally tried to find the first *actual* bug that objtool prevented 
and the first 4 appear to be struggles with objtool over false 
positives or non-runtime-bugs.

At least in x86 architecture code a significant percentage of objtool 
warnings isn't bugs - and to be fair that's maybe in part due to the 
lockdep effect: developers notice warnings and prevent them, so only 
traces of false positives trickle into the kernel. But lockdep too 
tries to be rather benign and doesn't crash the kernel, it reports an 
issue and turns itself off.

> > and an 'allmodconfig' should not fail the build IMO.
> 
> In fact it doesn't: allmodconfig sets COMPILE_TEST which prevents
> OBJTOOL_WERROR.

Okay, I missed that bit. But the patch still has a 'default y' that 
causes people who didn't have it to enable it by default, right?

> I've had complaints from people who spent days debugging a broken 
> kernel only to discover it was related to an ignored objtool warning.

Same could be said about an ignored lockdep warnings, right?

As long as OBJTOOL_WERROR doesn't get turned on in the primary 8 build 
modes:

  all{mod|yes|no|def}config

and we don't turn it on via default-y I'm a happy camper.

Thanks,

	Ingo

On Sun, Mar 16, 2025 at 12:56:02PM +0100, Ingo Molnar wrote:
>   # Included a fix for a false positive:
>   #
>   4e32645cd8f9 x86/smp: Fix mwait_play_dead() and acpi_processor_ffh_play_dead() noreturn behavior

A noreturn warning is (at least) a minor bug.  It means objtool doesn't
fully grok the CFG, which compromises the ORC generation.

Even if it's only a minor bug, and only due to objtool's confusion, it
still affects runtime.

Also, while unlikely, it could be hiding other warnings for frame
pointers, noinstr, uaccess, CPU mitigations.

>   # objtool poinpointed a problem that has no runtime effects,
>   # ie. it's a functional false positive and breaking the build
>   # for *that* would have been excessive:
>   #
>   73e8079be9e7 x86/ibt: Make cfi_bhi a constant for FINEIBT_BHI=n

I hadn't seen that one.  But as PeterZ knows, making objtool happy is a
normal part of developing such "special" non-standard code.  Those
noinstr rules exist for a very good reason.

>   # Commit works around an objtool false positive found during development:
>   #
>   b815f6877d80 x86/bhi: Add BHI stubs

I don't know what false positive that was.  This is probably another
example of the "special"-ness of FineIBT+BHI.

>   # Commit works around what appears to be a objtool false positive
>   # about too aggressive code generation in function prologues:
>   # (An issue that does not seem to trigger in practice.)
>   #
>   4087e16b0331 x86/locking: Use ALT_OUTPUT_SP() for percpu_{,try_}cmpxchg{64,128}_op()

If objtool saw it, this was a real frame pointer bug, not a theoretical
one.

> I literally tried to find the first *actual* bug that objtool prevented 
> and the first 4 appear to be struggles with objtool over false 
> positives or non-runtime-bugs.

Well, at least two of those are actual runtime-affecting bugs.  Maybe
nothing earth shattering, but they're not false positives either.  And
the BHI stuff is "special".

> At least in x86 architecture code a significant percentage of objtool 
> warnings isn't bugs - and to be fair that's maybe in part due to the 
> lockdep effect: developers notice warnings and prevent them, so only 
> traces of false positives trickle into the kernel.

That's definitely a big factor.  Objtool is very good at finding
compiler bugs, uaccess bugs, noinstr, IBT, retpoline, unintended UB,
etc.  Many of those are found surprisingly often, and tend to get fixed
during development if the user sees the warning and understands it.

Also there have been a lot of those noreturn warnings lately.  But again
I don't consider those false positives.  I do have some ideas on getting
rid of those altogether.

> But lockdep too tries to be rather benign and doesn't crash the
> kernel, it reports an issue and turns itself off.

But there's a key difference: objtool warnings happen at build time,
when something can be done to fix them, rather than runtime when it's
too late.

If there were a way to detect lockdep warnings at build time, that would
absolutely justify a build failure IMO.

Anyway, despite all that, I don't have any strong objection to disabling
it by default.  I was waffling on the default anyway.  Just having the
option is already a big improvement.

Though there are some "fatal" errors which are likely to cause boot
failures and other calamaties.  At some point those should be classified
as errors which *always* fail the build regardless of OBJTOOL_WERROR.
Similar to a compiler error.  I think that still needs some cleanup
though.

-- 
Josh