SYS-OSS/FRET-qemu
4
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

target/i386: Use assert() to sanity-check b1 in SSE decode

Browse Source 
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
This commit is contained in:
Peter Maydell 2021-09-01 15:10:08 +01:00
parent 0bdce4861f
commit e0e875a68a
1 changed files with 3 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
target/i386/tcg/translate.c
View File

@ -3519,9 +3519,6 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
case 0x171: /* shift xmm, im */ case 0x171: /* shift xmm, im */
case 0x172: case 0x172:
case 0x173: case 0x173:
if (b1 >= 2) {
goto unknown_op;
}
val = x86_ldub_code(env, s); val = x86_ldub_code(env, s);
if (is_xmm) { if (is_xmm) {
tcg_gen_movi_tl(s->T0, val); tcg_gen_movi_tl(s->T0, val);
@ -3540,6 +3537,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
offsetof(CPUX86State, mmx_t0.MMX_L(1))); offsetof(CPUX86State, mmx_t0.MMX_L(1)));
op1_offset = offsetof(CPUX86State,mmx_t0); op1_offset = offsetof(CPUX86State,mmx_t0);
} }
assert(b1 < 2);
sse_fn_epp = sse_op_table2[((b - 1) & 3) * 8 + sse_fn_epp = sse_op_table2[((b - 1) & 3) * 8 +
(((modrm >> 3)) & 7)][b1]; (((modrm >> 3)) & 7)][b1];
if (!sse_fn_epp) { if (!sse_fn_epp) {
@ -3770,10 +3768,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
rm = modrm & 7; rm = modrm & 7;
reg = ((modrm >> 3) & 7) | REX_R(s); reg = ((modrm >> 3) & 7) | REX_R(s);
mod = (modrm >> 6) & 3; mod = (modrm >> 6) & 3;
if (b1 >= 2) {
goto unknown_op;
}
assert(b1 < 2);
sse_fn_epp = sse_op_table6[b].op[b1]; sse_fn_epp = sse_op_table6[b].op[b1];
if (!sse_fn_epp) { if (!sse_fn_epp) {
goto unknown_op; goto unknown_op;
@ -4200,10 +4196,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
rm = modrm & 7; rm = modrm & 7;
reg = ((modrm >> 3) & 7) | REX_R(s); reg = ((modrm >> 3) & 7) | REX_R(s);
mod = (modrm >> 6) & 3; mod = (modrm >> 6) & 3;
if (b1 >= 2) {
goto unknown_op;
}
assert(b1 < 2);
sse_fn_eppi = sse_op_table7[b].op[b1]; sse_fn_eppi = sse_op_table7[b].op[b1];
if (!sse_fn_eppi) { if (!sse_fn_eppi) {
goto unknown_op; goto unknown_op;