Message-ID: <an7pjo3yx4hkpvdz6di4t75c2jtpij23zumtqicqwqmt4abmhs@ajd4mlxmreq3>
Date: Mon, 17 Mar 2025 21:55:49 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Ingo Molnar <mingo@...nel.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, 
	Peter Zijlstra <peterz@...radead.org>, Brendan Jackman <jackmanb@...gle.com>, 
	Nathan Chancellor <nathan@...nel.org>, Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 13/13] objtool: Add CONFIG_OBJTOOL_WERROR

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

Powered by blists - more mailing lists