Re: [PATCH] target/mips/mxu: Rewrite D16MIN / D16MAX opcodes

[Richard Henderson]

On 3/16/21 4:51 AM, Peter Maydell wrote:
> On Mon, 15 Mar 2021 at 22:39, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > Coverity reported (CID 1450831) an array overrun in
> > gen_mxu_D16MAX_D16MIN():
> >
> >    1103     } else if (unlikely((XRb == 0) || (XRa == 0))) {
> >    ....
> >    1112         if (opc == OPC_MXU_D16MAX) {
> >    1113             tcg_gen_smax_i32(mxu_gpr[XRa - 1], t0, t1);
> >    1114         } else {
> >    1115             tcg_gen_smin_i32(mxu_gpr[XRa - 1], t0, t1);
> >    1116         }
> >
> > > > > Overrunning array "mxu_gpr" of 15 8-byte elements at element
> >      index 4294967295 (byte offset 34359738367) using index "XRa - 1U"
> >      (which evaluates to 4294967295).
> >
> > Because we check if 'XRa == 0' then access 'XRa - 1' in array.
> >
> > I figured it could be easier to rewrite this function to something
> > simpler rather than trying to understand it.
>
> This seems to drop half of the optimised cases the old
> code had.

Which is a good thing, I think.  They're all never-happen cases that nothing 
but an isa testsuite will ever issue.


r~