Re: Fwd: New Defects reported by Coverity Scan for QEMU

[Matheus K. Ferst]

On 10/11/2021 05:18, Cédric Le Goater wrote:
> Hello Luis,
>
> Coverity found a couple of issues which seem related to the DFP patchset.
> Could you please take a look ?
>
> Thanks,
>
> C.
>
>
> -------- Forwarded Message --------
> Subject: New Defects reported by Coverity Scan for QEMU
> Date: Tue, 9 Nov 2021 22:09:40 +0000
> From: scan-admin@coverity.com
> To: clg@kaod.org
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to QEMU found 
> with Coverity Scan.
>
> 16 new defect(s) introduced to QEMU found with Coverity Scan.
> 19 defect(s), reported by Coverity Scan earlier, were marked fixed in 
> the recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 16 of 16 defect(s)
>
>
> ** CID 1465791:  Uninitialized variables  (UNINIT)
>
>
> ________________________________________________________________________________________________________ 
>
> *** CID 1465791:  Uninitialized variables  (UNINIT)
> /qemu/target/ppc/dfp_helper.c: 1202 in helper_DENBCD()
> 1196         
> }                                                                        \
> 1197         
> dfp_finalize_decimal##size(&dfp);                                        \
> 1198         
> dfp_set_FPRF_from_FRT(&dfp);                                             \
> 1199         set_dfp##size(t, 
> &dfp.vt);                                               \
> 1200     }
> 1201
> > > >     CID 1465791:  Uninitialized variables  (UNINIT)
> > > >     Using uninitialized element of array "dfp.vt" when calling 
> > > > "set_dfp64".
> 1202     DFP_HELPER_ENBCD(DENBCD, 64)
> 1203     DFP_HELPER_ENBCD(DENBCDQ, 128)

Hi Cédric,

The only change was the helper name that is now uppercase, so nothing 
new here. The underlying cause is that dfp_finalize_decimal64 only sets 
dfp->vt.VsrD(1) and set_dfp64 receives a pointer to the complete struct.

But since set_dfp64 also only access VsrD(1), it shouldn't be a real 
problem AFAICT. The same applies to CID 1465776~1465786 and 1465788~1465790.

> ** CID 1465787:    (BAD_SHIFT)
> /qemu/target/ppc/int_helper.c: 369 in helper_CFUGED()
> /qemu/target/ppc/int_helper.c: 370 in helper_CFUGED()
> /qemu/target/ppc/int_helper.c: 356 in helper_CFUGED()
> /qemu/target/ppc/int_helper.c: 356 in helper_CFUGED()
> /qemu/target/ppc/int_helper.c: 356 in helper_CFUGED()
> /qemu/target/ppc/int_helper.c: 369 in helper_CFUGED()
> /qemu/target/ppc/int_helper.c: 370 in helper_CFUGED()
> /qemu/target/ppc/int_helper.c: 370 in helper_CFUGED()
> /qemu/target/ppc/int_helper.c: 369 in helper_CFUGED()
>
>
> ________________________________________________________________________________________________________ 
>
> *** CID 1465787:    (BAD_SHIFT)
> /qemu/target/ppc/int_helper.c: 369 in helper_CFUGED()
> 363             /*
> 364              * Discards the processed bits from 'src' and 'mask'. 
> Note that we are
> 365              * removing 'n' trailing zeros from 'mask', but the 
> logical shift will
> 366              * add 'n' leading zeros back, so the population count 
> of 'mask' is kept
> 367              * the same.
> 368              */
> > > >     CID 1465787:    (BAD_SHIFT)
> > > >     In expression "src >>= n", right shifting by more than 63 bits 
> > > > has undefined behavior.  The shift amount, "n", is as much as 64.

Similar case here, the helper was just renamed. The value of "n" comes 
from ctz64(mask) and mask == 0 is a trivial case handled before anything 
else.

Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>