Re: [linux-next:master 12681/13861] drivers/i2c/i2c-core-base.o: warning: objtool: __i2c_transfer+0x120: stack state mismatch: reg1[24]=-1+0 reg2[24]=-2-24

[Josh Poimboeuf <jpoimboe@kernel.org>]

On Mon, Apr 07, 2025 at 06:52:10PM +0800, Tiezhu Yang wrote:
> There is a potential execution path with only using s0 and ra
> (without using s1, s2, s3, etc): 2d58-->2d70-->2f88-->2e78-->2e84

[...]

> From this point of view, it seems that there is no problem for the
> generated instructions of the current code, it is not a runtime bug,
> just a GCC optimization.

I don't see how this is responsive to my email.

I described a code path which revealed a GCC bug, specifically with asm
goto (unless I got something wrong).  Then you responded with a
*completely different* code path.

How does that prove my original code path isn't possible?

To summarize, the path I found was

  2d58 ... 2d9c -> 2da8 .. 2dc4 -> 2ebc .. 2ec0 (runtime patched static branch) -> 2e78 .. 2e84 (ret)

> (2) Analysis
> 
> In fact, the generated objtool warning is because the break instruction
> (2ee8) which is before the restoring s1 instruction (2eec) is annotated
> as dead end.

Actually, it's the opposite.  Objtool would normally consider BREAK to
be a dead end.  But it's annotated as "reachable", aka "non dead end".

> This issue is introduced by the following changes:
> 
>  #define __WARN_FLAGS(flags)					\
>  do {								\
>  	instrumentation_begin();				\
> -	__BUG_FLAGS(BUGFLAG_WARNING|(flags), ANNOTATE_REACHABLE(10001b));\
> +	if (!KUNIT_IS_SUPPRESSED_WARNING(__func__))			\
> +		__BUG_FLAGS(BUGFLAG_WARNING|(flags), ANNOTATE_REACHABLE(10001b));\
>  	instrumentation_end();					\
>  } while (0)
> 
> of commit e61a8b4b0d83 ("loongarch: add support for suppressing warning
> backtraces") in the linux-next.git.

Putting that annotation behind a conditional should not break anything.

> (4) Solution 1
> One way is to annotate __BUG_ENTRY() as reachable whether
> KUNIT_IS_SUPPRESSED_WARNING() is true or false, like this:
> 
> ---8<---
> diff --git a/arch/loongarch/include/asm/bug.h
> b/arch/loongarch/include/asm/bug.h
> index b79ff6696ce6..e41ebeaba204 100644
> --- a/arch/loongarch/include/asm/bug.h
> +++ b/arch/loongarch/include/asm/bug.h
> @@ -60,8 +60,9 @@
>  #define __WARN_FLAGS(flags)                                    \
>  do {                                                           \
>         instrumentation_begin();                                \
> -       if (!KUNIT_IS_SUPPRESSED_WARNING(__func__))                     \
> -               __BUG_FLAGS(BUGFLAG_WARNING|(flags),
> ANNOTATE_REACHABLE(10001b));\
> +       if (!KUNIT_IS_SUPPRESSED_WARNING(__func__))             \
> +               __BUG_FLAGS(BUGFLAG_WARNING|(flags), "");       \
> +       __BUG_FLAGS(0, ANNOTATE_REACHABLE(10001b));             \
>         instrumentation_end();                                  \
>  } while (0)

Huh?  That's basically:

	if (!suppress_warning)
		WARN();
	BUG();

So it upgrades a conditional WARN to an unconditional BUG???

Not to mention the reachable annotations are backwards: the WARN() is
annotated as dead end while the BUG() is annotated reachable.

Even if that silences objtool somehow, it will most definitely have the
wrong runtime behavior.

> (5) Solution 2
> The other way is to use "-fno-shrink-wrap" to aovid such issue under
> CONFIG_OBJTOOL at compile-time, like this:

As far as I can tell, that would be a workaround to get objtool to stop
warning about a legitimate compiler bug.

-- 
Josh