Re: [PATCH v2] target/i386: Use assert() to sanity-check b1 in SSE decode

[Richard Henderson <richard.henderson@linaro.org>]

On 9/1/21 4:10 PM, Peter Maydell 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.
> 
> This check was added in commit c045af25a52e9 in 2010; the added code
> was unnecessary then as well, and was apparently intended only to
> ensure that we never accidentally ended up indexing off the end
> of an sse_op_table with only 2 entries as a result of future bugs
> in the decode logic.
> 
> Change the checks to assert() instead, and make sure they're always
> immediately before the array access they are protecting.
> 
> Fixes: Coverity CID 1460207
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> v1->v2: use assert() rather than just deleting the if()s

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~