divdu (without a dot) sometimes updates cr0, even though it shouldn't.
The reason is that gen_op_arith_divd() checks Rc(ctx->opcode), which is
not initialized. This field is initialized only for instructions that
go through decode_legacy(), and not decodetree.

There already was a similar issue fixed in commit 86e6202a57b1
("target/ppc: Make divw[u] handler method decodetree compatible.").

It's not immediately clear what else may access the uninitialized
ctx->opcode, so instead of playing whack-a-mole and changing the check
to compute_rc0, simply initialize ctx->opcode.

Cc: qemu-stable@nongnu.org
Fixes: 99082815f17f ("target/ppc: Add infrastructure for prefixed insns")
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/ppc/translate.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 71513ba9646..02c810e8848 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6426,8 +6426,6 @@ static bool decode_legacy(PowerPCCPU *cpu, DisasContext *ctx, uint32_t insn)
     opc_handler_t **table, *handler;
     uint32_t inval;
 
-    ctx->opcode = insn;
-
     LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) (%s)\n",
               insn, opc1(insn), opc2(insn), opc3(insn), opc4(insn),
               ctx->le_mode ? "little" : "big");
@@ -6561,6 +6559,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     ctx->base.pc_next = pc += 4;
 
     if (!is_prefix_insn(ctx, insn)) {
+        ctx->opcode = insn;
         ok = (decode_insn32(ctx, insn) ||
               decode_legacy(cpu, ctx, insn));
     } else if ((pc & 63) == 0) {
-- 
2.45.2

On 7/31/24 20:07, Ilya Leoshkevich wrote:
> divdu (without a dot) sometimes updates cr0, even though it shouldn't.
> The reason is that gen_op_arith_divd() checks Rc(ctx->opcode), which is
> not initialized. This field is initialized only for instructions that
> go through decode_legacy(), and not decodetree.
> 
> There already was a similar issue fixed in commit 86e6202a57b1
> ("target/ppc: Make divw[u] handler method decodetree compatible.").
> 
> It's not immediately clear what else may access the uninitialized
> ctx->opcode, so instead of playing whack-a-mole and changing the check
> to compute_rc0, simply initialize ctx->opcode.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 99082815f17f ("target/ppc: Add infrastructure for prefixed insns")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

Accessing opcode from decodetree should not happen -- divd needs fixing.

But I agree this is a good short-term workaround for the release, so:
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

On 31/7/24 12:07, Ilya Leoshkevich wrote:
> divdu (without a dot) sometimes updates cr0, even though it shouldn't.
> The reason is that gen_op_arith_divd() checks Rc(ctx->opcode), which is
> not initialized. This field is initialized only for instructions that
> go through decode_legacy(), and not decodetree.
> 
> There already was a similar issue fixed in commit 86e6202a57b1
> ("target/ppc: Make divw[u] handler method decodetree compatible.").
> 
> It's not immediately clear what else may access the uninitialized
> ctx->opcode, so instead of playing whack-a-mole and changing the check
> to compute_rc0, simply initialize ctx->opcode.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 99082815f17f ("target/ppc: Add infrastructure for prefixed insns")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/ppc/translate.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>