Re: [PATCH] usb: atm: cxacru: Zero initialize bp in cxacru_heavy_init()

[Nathan Chancellor <nathan@kernel.org>]

On Wed, Jul 16, 2025 at 07:06:50AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jul 15, 2025 at 01:33:32PM -0700, Nathan Chancellor wrote:
> > After a recent change in clang to expose uninitialized warnings from
> > const variables [1], there is a warning in cxacru_heavy_init():
> > 
> >   drivers/usb/atm/cxacru.c:1104:6: error: variable 'bp' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
> >    1104 |         if (instance->modem_type->boot_rom_patch) {
> >         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   drivers/usb/atm/cxacru.c:1113:39: note: uninitialized use occurs here
> >    1113 |         cxacru_upload_firmware(instance, fw, bp);
> >         |                                              ^~
> >   drivers/usb/atm/cxacru.c:1104:2: note: remove the 'if' if its condition is always true
> >    1104 |         if (instance->modem_type->boot_rom_patch) {
> >         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   drivers/usb/atm/cxacru.c:1095:32: note: initialize the variable 'bp' to silence this warning
> >    1095 |         const struct firmware *fw, *bp;
> >         |                                       ^
> >         |                                        = NULL
> > 
> > This warning occurs in clang's frontend before inlining occurs, so it
> > cannot notice that bp is only used within cxacru_upload_firmware() under
> > the same condition that initializes it in cxacru_heavy_init(). Just
> > initialize bp to NULL to silence the warning without functionally
> > changing the code, which is what happens with modern compilers when they
> > support '-ftrivial-auto-var-init=zero' (CONFIG_INIT_STACK_ALL_ZERO=y).
> 
> We generally do not want to paper over compiler bugs, when our code is
> correct, so why should we do that here?  Why not fix clang instead?

I would not really call this a compiler bug. It IS passed uninitialized
to this function and while the uninitialized value is not actually used,
clang has no way of knowing that at this point in its pipeline, so I
don't think warning in this case is unreasonable. This type of warning
is off for GCC because of how unreliable it was when it is done in the
middle end with optimizations. Furthermore, it is my understanding based
on [1] that just the passing of an uninitialized variable in this manner
is UB.

[1]: https://lore.kernel.org/20220614214039.GA25951@gate.crashing.org/

Cheers,
Nathan