FW: New Defects reported by Coverity Scan for QEMU

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* FW: New Defects reported by Coverity Scan for QEMU
       [not found] <61844bb6ced54_21aa5f2b09742af98856497@prd-scan-dashboard-0.mail>
@ 2021-11-04 22:32 ` Taylor Simpson
  2021-11-05 15:31   ` Peter Maydell
  0 siblings, 1 reply; 4+ messages in thread
From: Taylor Simpson @ 2021-11-04 22:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson, Philippe Mathieu-Daudé

Coverity is getting confused here.  The index can never actually overflow.  Does Coverity have a pragma or something to tell it it's OK?

The loop nest in question is (the index must be < 128)
    for (int offset = 1; offset < 128; offset <<= 1) {
        for (int k = 0; k < 128; k++) {
            if (!(k & offset)) {
                swap(vector1.ub[k], vector0.ub[k + offset]);
            }
        }
    }
Basically, it's looking for elements to swap, and the "if (!(k & offset))" prevents "k + offset" from overflowing.

Thanks,
Taylor

-----Original Message-----
From: scan-admin@coverity.com <scan-admin@coverity.com> 
Sent: Thursday, November 4, 2021 4:08 PM
To: Taylor Simpson <tsimpson@quicinc.com>
Subject: New Defects reported by Coverity Scan for QEMU

WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.

Hi,

Please find the latest report on new defect(s) introduced to QEMU found with Coverity Scan.

4 new defect(s) introduced to QEMU found with Coverity Scan.


New defect(s) Reported-by: Coverity Scan Showing 4 of 4 defect(s)


** CID 1465283:    (OVERRUN)
/target/hexagon/helper_funcs_generated.c.inc: 12539 in helper_V6_vshuffvdd()
/target/hexagon/helper_funcs_generated.c.inc: 12539 in helper_V6_vshuffvdd()


________________________________________________________________________________________________________
*** CID 1465283:    (OVERRUN)
/target/hexagon/helper_funcs_generated.c.inc: 12539 in helper_V6_vshuffvdd()
12533     void HELPER(V6_vshuffvdd)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void, int32_t RtV)
12534     {
12535         uint32_t slot __attribute__((unused)) = 4;
12536         /* VddV is *(MMVectorPair *))VddV_void) */
12537         /* VuV is *(MMVector *)(VuV_void) */
12538         /* VvV is *(MMVector *)(VvV_void) */
>>>     CID 1465283:    (OVERRUN)
>>>     Overrunning array "((MMVectorPair *)VddV_void)->v[0].ub" of 128 bytes at byte offset 128 using index "k + offset" (which evaluates to 128).
12539         { fHIDE(int offset;) VddV.v[0] = VvV; VddV.v[1] = VuV; for (offset=1; offset<fVBYTES(); offset<<=1) { if ( RtV & offset) { fHIDE(int k;) fVFOREACH(8, k) { if (!( k & offset)) { fSWAPB(VddV.v[1].ub[k], VddV.v[0].ub[k+offset]); } } } } }
12540     }
12541
12542     void HELPER(V6_vdeal)(CPUHexagonState *env, void *VyV_void, void *VxV_void, int32_t RtV)
12543     {
12544         uint32_t slot __attribute__((unused)) = 4;
/target/hexagon/helper_funcs_generated.c.inc: 12539 in helper_V6_vshuffvdd()
12533     void HELPER(V6_vshuffvdd)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void, int32_t RtV)
12534     {
12535         uint32_t slot __attribute__((unused)) = 4;
12536         /* VddV is *(MMVectorPair *))VddV_void) */
12537         /* VuV is *(MMVector *)(VuV_void) */
12538         /* VvV is *(MMVector *)(VvV_void) */
>>>     CID 1465283:    (OVERRUN)
>>>     Overrunning array "((MMVectorPair *)VddV_void)->v[0].ub" of 128 bytes at byte offset 128 using index "k + offset" (which evaluates to 128).
12539         { fHIDE(int offset;) VddV.v[0] = VvV; VddV.v[1] = VuV; for (offset=1; offset<fVBYTES(); offset<<=1) { if ( RtV & offset) { fHIDE(int k;) fVFOREACH(8, k) { if (!( k & offset)) { fSWAPB(VddV.v[1].ub[k], VddV.v[0].ub[k+offset]); } } } } }
12540     }
12541
12542     void HELPER(V6_vdeal)(CPUHexagonState *env, void *VyV_void, void *VxV_void, int32_t RtV)
12543     {
12544         uint32_t slot __attribute__((unused)) = 4;

** CID 1465282:    (OVERRUN)
/target/hexagon/helper_funcs_generated.c.inc: 12547 in helper_V6_vdeal()
/target/hexagon/helper_funcs_generated.c.inc: 12547 in helper_V6_vdeal()


________________________________________________________________________________________________________
*** CID 1465282:    (OVERRUN)
/target/hexagon/helper_funcs_generated.c.inc: 12547 in helper_V6_vdeal()
12541
12542     void HELPER(V6_vdeal)(CPUHexagonState *env, void *VyV_void, void *VxV_void, int32_t RtV)
12543     {
12544         uint32_t slot __attribute__((unused)) = 4;
12545         /* VyV is *(MMVector *)(VyV_void) */
12546         /* VxV is *(MMVector *)(VxV_void) */
>>>     CID 1465282:    (OVERRUN)
>>>     Overrunning array "((MMVector *)VxV_void)->ub" of 128 bytes at byte offset 191 using index "k + offset" (which evaluates to 191).
12547         { fHIDE(int offset;) for (offset=fVBYTES()>>1; offset>0; offset>>=1) { if ( RtV & offset) { fHIDE(int k;) fVFOREACH(8, k) { if (!( k & offset)) { fSWAPB(VyV.ub[k], VxV.ub[k+offset]); } } } } }
12548     }
12549
12550     void HELPER(V6_vdealvdd)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void, int32_t RtV)
12551     {
12552         uint32_t slot __attribute__((unused)) = 4;
/target/hexagon/helper_funcs_generated.c.inc: 12547 in helper_V6_vdeal()
12541
12542     void HELPER(V6_vdeal)(CPUHexagonState *env, void *VyV_void, void *VxV_void, int32_t RtV)
12543     {
12544         uint32_t slot __attribute__((unused)) = 4;
12545         /* VyV is *(MMVector *)(VyV_void) */
12546         /* VxV is *(MMVector *)(VxV_void) */
>>>     CID 1465282:    (OVERRUN)
>>>     Overrunning array "((MMVector *)VxV_void)->ub" of 128 bytes at byte offset 191 using index "k + offset" (which evaluates to 191).
12547         { fHIDE(int offset;) for (offset=fVBYTES()>>1; offset>0; offset>>=1) { if ( RtV & offset) { fHIDE(int k;) fVFOREACH(8, k) { if (!( k & offset)) { fSWAPB(VyV.ub[k], VxV.ub[k+offset]); } } } } }
12548     }
12549
12550     void HELPER(V6_vdealvdd)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void, int32_t RtV)
12551     {
12552         uint32_t slot __attribute__((unused)) = 4;

** CID 1465281:    (OVERRUN)
/target/hexagon/helper_funcs_generated.c.inc: 12556 in helper_V6_vdealvdd()
/target/hexagon/helper_funcs_generated.c.inc: 12556 in helper_V6_vdealvdd()


________________________________________________________________________________________________________
*** CID 1465281:    (OVERRUN)
/target/hexagon/helper_funcs_generated.c.inc: 12556 in helper_V6_vdealvdd()
12550     void HELPER(V6_vdealvdd)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void, int32_t RtV)
12551     {
12552         uint32_t slot __attribute__((unused)) = 4;
12553         /* VddV is *(MMVectorPair *))VddV_void) */
12554         /* VuV is *(MMVector *)(VuV_void) */
12555         /* VvV is *(MMVector *)(VvV_void) */
>>>     CID 1465281:    (OVERRUN)
>>>     Overrunning array "((MMVectorPair *)VddV_void)->v[0].ub" of 128 bytes at byte offset 191 using index "k + offset" (which evaluates to 191).
12556         { fHIDE(int offset;) VddV.v[0] = VvV; VddV.v[1] = VuV; for (offset=fVBYTES()>>1; offset>0; offset>>=1) { if ( RtV & offset) { fHIDE(int k;) fVFOREACH(8, k) { if (!( k & offset)) { fSWAPB(VddV.v[1].ub[k], VddV.v[0].ub[k+offset]); } } } } }
12557     }
12558
12559     void HELPER(V6_vshufoeh)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void)
12560     {
12561         uint32_t slot __attribute__((unused)) = 4;
/target/hexagon/helper_funcs_generated.c.inc: 12556 in helper_V6_vdealvdd()
12550     void HELPER(V6_vdealvdd)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void, int32_t RtV)
12551     {
12552         uint32_t slot __attribute__((unused)) = 4;
12553         /* VddV is *(MMVectorPair *))VddV_void) */
12554         /* VuV is *(MMVector *)(VuV_void) */
12555         /* VvV is *(MMVector *)(VvV_void) */
>>>     CID 1465281:    (OVERRUN)
>>>     Overrunning array "((MMVectorPair *)VddV_void)->v[0].ub" of 128 bytes at byte offset 191 using index "k + offset" (which evaluates to 191).
12556         { fHIDE(int offset;) VddV.v[0] = VvV; VddV.v[1] = VuV; for (offset=fVBYTES()>>1; offset>0; offset>>=1) { if ( RtV & offset) { fHIDE(int k;) fVFOREACH(8, k) { if (!( k & offset)) { fSWAPB(VddV.v[1].ub[k], VddV.v[0].ub[k+offset]); } } } } }
12557     }
12558
12559     void HELPER(V6_vshufoeh)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void)
12560     {
12561         uint32_t slot __attribute__((unused)) = 4;

** CID 1465280:    (OVERRUN)
/target/hexagon/helper_funcs_generated.c.inc: 12530 in helper_V6_vshuff()
/target/hexagon/helper_funcs_generated.c.inc: 12530 in helper_V6_vshuff()


________________________________________________________________________________________________________
*** CID 1465280:    (OVERRUN)
/target/hexagon/helper_funcs_generated.c.inc: 12530 in helper_V6_vshuff()
12524
12525     void HELPER(V6_vshuff)(CPUHexagonState *env, void *VyV_void, void *VxV_void, int32_t RtV)
12526     {
12527         uint32_t slot __attribute__((unused)) = 4;
12528         /* VyV is *(MMVector *)(VyV_void) */
12529         /* VxV is *(MMVector *)(VxV_void) */
>>>     CID 1465280:    (OVERRUN)
>>>     Overrunning array "((MMVector *)VxV_void)->ub" of 128 bytes at byte offset 128 using index "k + offset" (which evaluates to 128).
12530         { fHIDE(int offset;) for (offset=1; offset<fVBYTES(); offset<<=1) { if ( RtV & offset) { fHIDE(int k;) fVFOREACH(8, k) { if (!( k & offset)) { fSWAPB(VyV.ub[k], VxV.ub[k+offset]); } } } } }
12531     }
12532
12533     void HELPER(V6_vshuffvdd)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void, int32_t RtV)
12534     {
12535         uint32_t slot __attribute__((unused)) = 4;
/target/hexagon/helper_funcs_generated.c.inc: 12530 in helper_V6_vshuff()
12524
12525     void HELPER(V6_vshuff)(CPUHexagonState *env, void *VyV_void, void *VxV_void, int32_t RtV)
12526     {
12527         uint32_t slot __attribute__((unused)) = 4;
12528         /* VyV is *(MMVector *)(VyV_void) */
12529         /* VxV is *(MMVector *)(VxV_void) */
>>>     CID 1465280:    (OVERRUN)
>>>     Overrunning array "((MMVector *)VxV_void)->ub" of 128 bytes at byte offset 128 using index "k + offset" (which evaluates to 128).
12530         { fHIDE(int offset;) for (offset=1; offset<fVBYTES(); offset<<=1) { if ( RtV & offset) { fHIDE(int k;) fVFOREACH(8, k) { if (!( k & offset)) { fSWAPB(VyV.ub[k], VxV.ub[k+offset]); } } } } }
12531     }
12532
12533     void HELPER(V6_vshuffvdd)(CPUHexagonState *env, void *VddV_void, void *VuV_void, void *VvV_void, int32_t RtV)
12534     {
12535         uint32_t slot __attribute__((unused)) = 4;


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yrzEQNXe51mg-2FlKoEnRoarMq5nOxxfhqLUuo8HvG2S4Ew-3D-3DWlA7_8inUyGh-2BW1HT32W32IvBHxb7aFSEdSRWGNQjOj6Pu5XN1yTi5xRqR-2FauzTzQ8TTCMynbzlKK38dd4vnBgBOISaYuCt2P2K4Nr-2BQrwIRxT6vP6cSps-2FKfJeDwWOYjfHKEOE-2B37NVXcSeHy1g85Xu96TxRZSAMYgPybtVUE7VaHktMnSyFARcHi5Smh-2BOt7C17iCCDe0sIJueOT9wnUDuhhw-3D-3D

  To manage Coverity Scan email notifications for "tsimpson@quicinc.com", click https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXxhog3V-2Bya4sYLKnckU-2FWJNYoLyk5CFNN4-2FzXAhh2yQgYkIiZFhMvotFhFxZUytWoxPeX55igX3u7EjcDuJbLuFws0HNjzT5-2FUbfBZiYi6yb8-3DVX4C_8inUyGh-2BW1HT32W32IvBHxb7aFSEdSRWGNQjOj6Pu5XN1yTi5xRqR-2FauzTzQ8TTCw-2FEqvaCcduK-2Fu0844eqQecio6W8FxY0JZVtlm0PPIP0AD11nL4sJi3gMc9nHHjGJSV9SIW-2B2UfY06pXWi-2BYos8wCxnEOUkZNMcTI-2BiUeK3HF1W2QdnwXTO58IHvsG3MEAqdq9-2BESzEdEojR94qp3ZA-3D-3D



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: FW: New Defects reported by Coverity Scan for QEMU
  2021-11-04 22:32 ` FW: New Defects reported by Coverity Scan for QEMU Taylor Simpson
@ 2021-11-05 15:31   ` Peter Maydell
  2021-11-05 15:41     ` Taylor Simpson
  2021-11-05 15:59     ` Paolo Bonzini
  0 siblings, 2 replies; 4+ messages in thread
From: Peter Maydell @ 2021-11-05 15:31 UTC (permalink / raw)
  To: Taylor Simpson; +Cc: Richard Henderson, qemu-devel, Philippe Mathieu-Daudé

On Thu, 4 Nov 2021 at 22:34, Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> Coverity is getting confused here.  The index can never actually overflow.
>  Does Coverity have a pragma or something to tell it it's OK?
>
> The loop nest in question is (the index must be < 128)
>     for (int offset = 1; offset < 128; offset <<= 1) {
>         for (int k = 0; k < 128; k++) {
>             if (!(k & offset)) {
>                 swap(vector1.ub[k], vector0.ub[k + offset]);
>             }
>         }
>     }
> Basically, it's looking for elements to swap, and the
> "if (!(k & offset))" prevents "k + offset" from overflowing.

Yes, I agree this is a false positive. I've marked it as an FP
in the Coverity web UI.

I suspect that changing "k + offset" to "k | offset" would
probably stop Coverity complaining, but we should only do that
if you think it's more readable that way. (As I read the code
we are doing bit operations here rather than addition, so
it seems slightly better to be using the OR; but there's not
a lot in it.)

thanks
-- PMM


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: FW: New Defects reported by Coverity Scan for QEMU
  2021-11-05 15:31   ` Peter Maydell
@ 2021-11-05 15:41     ` Taylor Simpson
  2021-11-05 15:59     ` Paolo Bonzini
  1 sibling, 0 replies; 4+ messages in thread
From: Taylor Simpson @ 2021-11-05 15:41 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Richard Henderson, qemu-devel, Philippe Mathieu-Daudé

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Friday, November 5, 2021 10:31 AM
> To: Taylor Simpson <tsimpson@quicinc.com>
> Cc: qemu-devel@nongnu.org; Richard Henderson
> <richard.henderson@linaro.org>; Philippe Mathieu-Daudé
> <f4bug@amsat.org>
> Subject: Re: FW: New Defects reported by Coverity Scan for QEMU
> 
> On Thu, 4 Nov 2021 at 22:34, Taylor Simpson <tsimpson@quicinc.com> wrote:
> >
> > Coverity is getting confused here.  The index can never actually overflow.
> >  Does Coverity have a pragma or something to tell it it's OK?
> >
> > The loop nest in question is (the index must be < 128)
> >     for (int offset = 1; offset < 128; offset <<= 1) {
> >         for (int k = 0; k < 128; k++) {
> >             if (!(k & offset)) {
> >                 swap(vector1.ub[k], vector0.ub[k + offset]);
> >             }
> >         }
> >     }
> > Basically, it's looking for elements to swap, and the "if (!(k &
> > offset))" prevents "k + offset" from overflowing.
> 
> Yes, I agree this is a false positive. I've marked it as an FP in the Coverity web
> UI.
> 
> I suspect that changing "k + offset" to "k | offset" would probably stop
> Coverity complaining, but we should only do that if you think it's more
> readable that way. (As I read the code we are doing bit operations here
> rather than addition, so it seems slightly better to be using the OR; but
> there's not a lot in it.)

Thanks Peter!

I prefer to leave the code as-is.

Taylor


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: FW: New Defects reported by Coverity Scan for QEMU
  2021-11-05 15:31   ` Peter Maydell
  2021-11-05 15:41     ` Taylor Simpson
@ 2021-11-05 15:59     ` Paolo Bonzini
  1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2021-11-05 15:59 UTC (permalink / raw)
  To: Peter Maydell, Taylor Simpson
  Cc: Richard Henderson, qemu-devel, Philippe Mathieu-Daudé

On 11/5/21 16:31, Peter Maydell wrote:
> The loop nest in question is (the index must be < 128)
>      for (int offset = 1; offset < 128; offset <<= 1) {
>          for (int k = 0; k < 128; k++) {
>              if (!(k & offset)) {
>                  swap(vector1.ub[k], vector0.ub[k + offset]);
>              }
>          }
>      }
> Basically, it's looking for elements to swap, and the
> "if (!(k & offset))" prevents "k + offset" from overflowing.

It would still be slightly more efficient however to change the loop to 
k < 128 - offset.

Another possibility is to change the inner loop to

for (int k = offset; k < 128; k = (k + 1) | offset)
     swap(vector1.ub[k-offset], vector0.ub[k]);

Paolo



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-05 16:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <61844bb6ced54_21aa5f2b09742af98856497@prd-scan-dashboard-0.mail>
2021-11-04 22:32 ` FW: New Defects reported by Coverity Scan for QEMU Taylor Simpson
2021-11-05 15:31   ` Peter Maydell
2021-11-05 15:41     ` Taylor Simpson
2021-11-05 15:59     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).