Re: [PATCH 1/2] powerpc/64s: Work around spurious warning on old gccs with -fsanitize-coverage

[Segher Boessenkool <segher@kernel.crashing.org>]

On Thu, Feb 07, 2019 at 05:59:48PM +1100, Andrew Donnellan wrote:
> On 7/2/19 5:37 pm, Segher Boessenkool wrote:
> >On Thu, Feb 07, 2019 at 04:33:23PM +1100, Andrew Donnellan wrote:
> >>Some older gccs (<GCC 7), when invoked with -fsanitize-coverage=trace-pc,
> >>cause a spurious uninitialised variable warning in dt_cpu_ftrs.c:
> >>
> >>   arch/powerpc/kernel/dt_cpu_ftrs.c: In function 
> >>   ‘cpufeatures_process_feature’:
> >>   arch/powerpc/kernel/dt_cpu_ftrs.c:686:7: warning: ‘m’ may be used 
> >>   uninitialized in this function [-Wmaybe-uninitialized]
> >>     if (m->cpu_ftr_bit_mask)
> >
> >It seems to me the warning is correct?  If enable_unknown is false and no
> >cpu_feature is found, it will in
> >
> >	if (m->cpu_ftr_bit_mask)
> >		cur_cpu_spec->cpu_features |= m->cpu_ftr_bit_mask;
> >
> >enable random features (whatever was last in the table), or indeed access
> >via NULL if the table is length 0?  So maybe this should be
> >
> >	if (known && m->cpu_ftr_bit_mask)
> >		cur_cpu_spec->cpu_features |= m->cpu_ftr_bit_mask;
> >
> >instead?  (The code would be much clearer if all the known vs. !known
> >codepath was fully separated here).
> 
> The table is never length 0, it's a statically defined array.

Sure, and presumably that is why newer GCC doesn't warn.  But what about
the other point?  Is this code ever correct?  Enabling random features
(in cur_cpu_spec->cpu_features) when the name isn't found seems wrong.


Segher