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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Jul 15, 2025 at 10:24:50PM -0700, Nathan Chancellor wrote:
> 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/

Ah, I see now what you are referring to, sorry.  I'll go queue this up
now, thanks.

greg k-h