Re: [linux-next:master 7731/9053] arch/arm64/kvm/vgic/vgic-mmio.c:1094:3: warning: variable 'len' is used uninitialized whenever 'if' condition is false

[Nathan Chancellor <nathan@kernel.org>]

Hi Sascha,

On Mon, Jul 14, 2025 at 12:37:35PM +0000, Sascha Bischoff wrote:
> On Sun, 2025-07-13 at 03:29 +0800, kernel test robot wrote:
> > commit: c017e49ed1381001ba7a6521daae8f968b11cf09 [7731/9053] KVM: arm64: gic-v5: Support GICv3 compat
> > config: arm64-randconfig-004-20250713 (https://download.01.org/0day-ci/archive/20250713/202507130334.ddFcXn4N-lkp@intel.com/config)
> > compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 01c97b4953e87ae455bd4c41e3de3f0f0f29c61c)
...
> > arch/arm64/kvm/vgic/vgic-mmio.c:1094:3: warning: variable 'len' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
> >     1094 |                 BUG_ON(1);
> >          |                 ^~~~~~~~~
> >    include/asm-generic/bug.h:71:36: note: expanded from macro 'BUG_ON'
> >       71 | #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
> >          |                                    ^~~~~~~~~~~~~~~~~~~
> >    include/linux/compiler.h:47:23: note: expanded from macro 'unlikely'
> >       47 | #  define unlikely(x)   (__branch_check__(x, 0, __builtin_constant_p(x)))
> >          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
> There is no actual uninitialized use of len occurring here. It is
> explicitly initialized in both the VGIC_V2 and VGIC_V3 cases. The only
> other path is the default case, which triggers BUG_ON(1).
> 
> For arm64, BUG() itself is defined as:
> 
> #define BUG() do {					\
> 	__BUG_FLAGS(0);					\
> 	unreachable();					\
> } while (0)
> 
> Since __BUG_FLAGS(0) is followed by unreachable(), the compiler should
> ideally recognize that code paths beyond this point are dead. 
> The fact that this warning is still being emitted suggests the compiler
> isn’t correctly propagating the unreachable() information in this
> context.
> 
> Given that, this seems like an issue on the compiler side, or possibly
> a case where BUG_ON() could be restructured to more explicitly convey
> control-flow termination to the compiler.

In a normal configuration, it is easy for the compiler to see that
BUG_ON(1) is the same thing as BUG() and take into account the
__builtin_unreachable() at the end to avoid warnings.

However, the key to what is going on here is in the second note: this
configuration has CONFIG_TRACE_BRANCH_PROFILING=y, which turns
unlikely() into

#define __branch_check__(x, expect, is_constant) ({			\
			long ______r;					\
			static struct ftrace_likely_data		\
				__aligned(4)				\
				__section("_ftrace_annotated_branch")	\
				______f = {				\
				.data.func = __func__,			\
				.data.file = __FILE__,			\
				.data.line = __LINE__,			\
			};						\
			______r = __builtin_expect(!!(x), expect);	\
			ftrace_likely_update(&______f, ______r,		\
					     expect, is_constant);	\
			______r;					\
		})

Clang's semantic analysis phase happens early in the front end, so it is
not able to value track ______r yet, meaning that it thinks BUG() is
only conditionally called, leaving len uninitialized in the other
imaginary branch.

I see two ways to fix this. The most obvious is just turn this BUG_ON(1)
into BUG() directly to avoid having an unlikely() in the first place.

diff --git a/arch/arm64/kvm/vgic/vgic-mmio.c b/arch/arm64/kvm/vgic/vgic-mmio.c
index e416e433baff..a573b1f0c6cb 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio.c
@@ -1091,7 +1091,7 @@ int vgic_register_dist_iodev(struct kvm *kvm, gpa_t dist_base_address,
 		len = vgic_v3_init_dist_iodev(io_device);
 		break;
 	default:
-		BUG_ON(1);
+		BUG();
 	}
 
 	io_device->base_addr = dist_base_address;
---

The other option would be to turn the unlikely() in the generic BUG_ON()
to unlikely_notrace(), so that the '1' condition can always be
constantly evaluated. I am not sure having branch checks on BUG_ON() is
super useful since these are conditions that are intentionally hard to
hit and you will know when they are :)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 2d9f61346dab..35f3e914b068 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -74,7 +74,7 @@ struct bug_entry {
 #endif
 
 #ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
+#define BUG_ON(condition) do { if (unlikely_notrace(condition)) BUG(); } while (0)
 #endif
 
 /*
---

Both of these resolve the warning from the original configuration for
me.

Cheers,
Nathan