target/i386: introduce flags writeback mechanism

ALU instructions can write to both memory and flags.  If the CC_SRC*
and CC_DST locations have been written already when a memory access
causes a fault, the value in CC_SRC* and CC_DST might be interpreted
with the wrong CC_OP (the one that is in effect before the instruction.

Besides just using the wrong result for the flags, something like
subtracting -1 can have disastrous effects if the current CC_OP is
CC_OP_EFLAGS: this is because QEMU does not expect bits outside the ALU
flags to be set in CC_SRC, and env->eflags can end up set to all-ones.
In the case of the attached testcase, this sets IOPL to 3 and would
cause an assertion failure if SUB is moved to the new decoder.

This mechanism is not really needed for BMI instructions, which can
only write to a register, but put it to use anyway for cleanliness.
In the case of BZHI, the code has to be modified slightly to ensure
that decode->cc_src is written, otherwise the new assertions trigger.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Paolo Bonzini 2023-10-11 15:26:40 +02:00
parent 4b2baf4a55
commit e7bbb7cb71
6 changed files with 101 additions and 13 deletions

View file

@ -1662,6 +1662,7 @@ static void disas_insn_new(DisasContext *s, CPUState *cpu, int b)
bool first = true;
X86DecodedInsn decode;
X86DecodeFunc decode_func = decode_root;
uint8_t cc_live;
s->has_modrm = false;
@ -1815,6 +1816,7 @@ static void disas_insn_new(DisasContext *s, CPUState *cpu, int b)
}
memset(&decode, 0, sizeof(decode));
decode.cc_op = -1;
decode.b = b;
if (!decode_insn(s, env, decode_func, &decode)) {
goto illegal_op;
@ -1953,6 +1955,38 @@ static void disas_insn_new(DisasContext *s, CPUState *cpu, int b)
decode.e.gen(s, env, &decode);
gen_writeback(s, &decode, 0, s->T0);
}
/*
* Write back flags after last memory access. Some newer ALU instructions, as
* well as SSE instructions, write flags in the gen_* function, but that can
* cause incorrect tracking of CC_OP for instructions that write to both memory
* and flags.
*/
if (decode.cc_op != -1) {
if (decode.cc_dst) {
tcg_gen_mov_tl(cpu_cc_dst, decode.cc_dst);
}
if (decode.cc_src) {
tcg_gen_mov_tl(cpu_cc_src, decode.cc_src);
}
if (decode.cc_src2) {
tcg_gen_mov_tl(cpu_cc_src2, decode.cc_src2);
}
if (decode.cc_op == CC_OP_DYNAMIC) {
tcg_gen_mov_i32(cpu_cc_op, decode.cc_op_dynamic);
}
set_cc_op(s, decode.cc_op);
cc_live = cc_op_live[decode.cc_op];
} else {
cc_live = 0;
}
if (decode.cc_op != CC_OP_DYNAMIC) {
assert(!decode.cc_op_dynamic);
assert(!!decode.cc_dst == !!(cc_live & USES_CC_DST));
assert(!!decode.cc_src == !!(cc_live & USES_CC_SRC));
assert(!!decode.cc_src2 == !!(cc_live & USES_CC_SRC2));
}
return;
gp_fault:
gen_exception_gpf(s);