[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/i386: Remove unnecessary checks of b2 in SSE decode
From: |
Peter Maydell |
Subject: |
Re: [PATCH] target/i386: Remove unnecessary checks of b2 in SSE decode |
Date: |
Mon, 23 Aug 2021 11:27:18 +0100 |
On Fri, 20 Aug 2021 at 18:12, Peter Maydell <peter.maydell@linaro.org> 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 <peter.maydell@linaro.org>
> ---
> 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