On Fri, 20 Aug 2021 at 18:12, Peter Maydell <[email protected]> wrote: > > In the SSE decode function gen_sse(), we combine a byte > 'b' and a value 'b1' which can be [0..3], and switch on them: > b |= (b1 << 8); > switch (b) { > ... > default: > unknown_op: > gen_unknown_opcode(env, s); > return; > } > > In three cases inside this switch, we were then also checking for > "if (b1 >= 2) { goto unknown_op; }". > However, this can never happen, because the 'case' values in each place > are 0x0nn or 0x1nn and the switch will have directed the b1 == (2, 3) > cases to the default already. > > Delete the dead code. > > This check was added in commit c045af25a52e9 in 2010; the added code > was unnecessary then as well. this commit amounts to a revert of > c045af25a52e9. > > Fixes: Coverity CID 1460207 > Signed-off-by: Peter Maydell <[email protected]> > --- > Somebody should double-check this, because one assumes Andi > added the code for a reason...
It occurred to me that maybe we'd be better just changing these into assert(b1 < 2); These are guarding all the places where we do a dereference of an sse_op_table*[x][b1] and the inner table only has 2 elements. So asserting would be a sensible guard. I'll send out a v2 patch that does that. -- PMM
