svn commit: r365445 - head/sys/cam/mmc

[Mark Johnston]

On Tue, Sep 08, 2020 at 02:24:59PM +0200, Michal Meloun wrote:
> On 08.09.2020 9:10, Andriy Gapon wrote:
> > On 08/09/2020 08:46, Andriy Gapon wrote:
> >> Author: avg
> >> Date: Tue Sep  8 05:46:10 2020
> >> New Revision: 365445
> >> URL: https://svnweb.freebsd.org/changeset/base/365445
> >>
> >> Log:
> >>   mmc_da: make sure that part_index is not used uninitialized in sddastart
> > [snip]
> >> Modified: head/sys/cam/mmc/mmc_da.c
> >> ==============================================================================
> >> --- head/sys/cam/mmc/mmc_da.c	Tue Sep  8 04:44:37 2020	(r365444)
> >> +++ head/sys/cam/mmc/mmc_da.c	Tue Sep  8 05:46:10 2020	(r365445)
> >> @@ -1808,6 +1808,7 @@ sddastart(struct cam_periph *periph, union ccb *start_
> >>  	}
> >>  
> >>  	/* Find partition that has outstanding commands.  Prefer current partition. */
> >> +	part_index = softc->part_curr;
> >>  	part = softc->part[softc->part_curr];
> >>  	bp = bioq_first(&part->bio_queue);
> >>  	if (bp == NULL) {
> >>
> > 
> > One thing that concerns me is that it was obvious (to a human) that part_index
> > could be used uninitialized if bp was not NULL.
> > Yet, there was no warning or error from the compiler when I built that code for
> > armv7.
> > 
> > I wonder if we disable some relevant warnings for that architecture.
> > Or if the compiler (clang 11) could not figure that out.
> > 
> Hmm, for this in kernel code :
> int foo(void);
> int foo(void)
> {
>  int rv;
> 
>  return (rv);
> }
> 
> warning is reported for both armv7 and arm64 for native or cross compile.
> 
> It seems that clang11 doesn't emit warnings only for more complicated
> cases...
> 
> I writing this because i just found another usage of uninitialized
> variable, in this case in much more complicated abort_handler() function
> in arm/trap-v6.c again without warning emitted.

I observed the same thing recently as well: the compiler catches
uninitialized variables only in simple cases.  In my case, any uses of
goto within the function seemed to silence the warning, even if they
appeared after the uninitialized reference.