Re: [PATCH] printf: mark errptr() noinline

[David Laight]

On Sun, 05 Apr 2026 13:31:50 -0400
Tamir Duberstein <tamird@xxxxxxxxxx> wrote:

> Old GCC can miscompile printf_kunit's errptr() test when branch
> profiling is enabled. BUILD_BUG_ON(IS_ERR(PTR)) is a constant false
> expression, but CONFIG_TRACE_BRANCH_PROFILING and
> CONFIG_PROFILE_ALL_BRANCHES make the IS_ERR() path side-effectful.
> GCC's IPA splitter can then outline the cold assert arm into
> errptr.part.* and leave that clone with an unconditional
> __compiletime_assert_*() call, causing a false build failure.
> 
> This started showing up after test_hashed() became a macro and moved its
> local buffer into errptr(), which changed GCC's inlining and splitting
> decisions enough to expose the compiler bug.
> 
> Mark errptr() noinline to keep it out of that buggy IPA path while
> preserving the BUILD_BUG_ON(IS_ERR(PTR)) check and the macro-based
> printf argument checking.

Why not convert that check to a run-time one?
It isn't as though IS_ERR() is usually used with constants.

(There are a lot of other tests which would be better as compile-time
ones; since the run-time code just checks the compiler generated
the correct constant. So a compile failure saves you having to run
the tests; but that isn't true here.)

I'd also test -4095 and -4096...

> 
> Fixes: 9bfa52dac27a ("printf: convert test_hashed into macro")
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Closes: https://lore.kernel.org/oe-kbuild-all/202604030636.NqjaJvYp-lkp@xxxxxxxxx/
> Signed-off-by: Tamir Duberstein <tamird@xxxxxxxxxx>
> ---
>  lib/tests/printf_kunit.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
> index f6f21b445ece..a8087e8ac826 100644
> --- a/lib/tests/printf_kunit.c
> +++ b/lib/tests/printf_kunit.c
> @@ -749,7 +749,23 @@ static void fourcc_pointer(struct kunit *kunittest)
>  	fourcc_pointer_test(kunittest, try_cb, ARRAY_SIZE(try_cb), "%p4cb");
>  }
>  
> -static void
> +/*
> + * GCC < 12.1 can miscompile this test when branch profiling is enabled.
> + *
> + * BUILD_BUG_ON(IS_ERR(PTR)) is a constant false expression, but old GCC can
> + * still trip over it after CONFIG_TRACE_BRANCH_PROFILING and
> + * CONFIG_PROFILE_ALL_BRANCHES rewrite the IS_ERR() unlikely() path into
> + * side-effectful branch counter updates. IPA splitting then outlines the cold
> + * assert arm into errptr.part.* and leaves that clone with an unconditional
> + * __compiletime_assert_*() call, so the build fails even though PTR is not an
> + * ERR_PTR.
> + *
> + * Keep this test out of that buggy IPA path so the BUILD_BUG_ON() can stay in
> + * place without open-coding IS_ERR(). This can be removed once the minimum GCC
> + * includes commit 76fe49423047 ("Fix tree-optimization/101941: IPA splitting
> + * out function with error attribute"), which first shipped in GCC 12.1.
> + */
> +static noinline void

While that might make a difference, I'm not sure that it has to.
You aren't changing anything directly related to the failing expansion.

	David

>  errptr(struct kunit *kunittest)
>  {
>  	test("-1234", "%pe", ERR_PTR(-1234));
> 
> ---
> base-commit: d8a9a4b11a137909e306e50346148fc5c3b63f9d
> change-id: 20260405-printf-test-old-gcc-f13fecda6524
> 
> Best regards,
> --  
> Tamir Duberstein <tamird@xxxxxxxxxx>
> 
>