Re: [PATCH] tcg: Fix the overflow in indexing tcg_ctx->temps

[Peter Maydell]

On Fri, 19 Apr 2024 at 10:37, 姜智伟 <jiangzw@tecorigin.com> wrote:
> Peter Maydell wrote:
> > I feel like this might be a bug elsewhere. Can you provide
> > a repro binary and command line?
>
> The test file has been attached with RISCV CBO instruction as the first 
> instruction to execute, with command-line arguments as
> ./build/qemu-system-riscv64 -M virt -smp 1 -nographic -bios crash_test.bin

It looks like you're building without --enable-debug. If you do
that then you'll find that we hit an assert in the debug version
of the function, which your patch doesn't touch:

#6  0x00007ffff4b90e96 in __GI___assert_fail
    (assertion=0x55555639a508 "o < sizeof(TCGTemp) *
tcg_ctx->nb_temps", file=0x5555563995d5 "../../tcg/tcg.c", line=1940,
function=0x55555639c000 <__PRETTY_FUNCTION__.60> "tcgv_i32_temp") at
./assert/assert.c:101
#7  0x000055555613104c in tcgv_i32_temp (v=0x0) at ../../tcg/tcg.c:1940
#8  0x0000555555d0882b in tcgv_i64_temp (v=0x0) at
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/tcg/tcg.h:638
#9  0x0000555555d0c17b in gen_helper_cbo_inval (arg1=0x2a8, arg2=0x0)
at ../../target/riscv/helper.h:121
#10 0x0000555555d7be65 in trans_cbo_inval (ctx=0x7fffef1c8e50,
a=0x7fffef1c8cf0) at
../../target/riscv/insn_trans/trans_rvzicbo.c.inc:48
#11 0x0000555555d41f4f in decode_insn32 (ctx=0x7fffef1c8e50,
insn=8207) at libqemu-riscv64-softmmu.fa.p/decode-insn32.c.inc:2332
#12 0x0000555555d925f1 in decode_opc (env=0x555556d53e30,
ctx=0x7fffef1c8e50, opcode=8207) at
../../target/riscv/translate.c:1165
#13 0x0000555555d92ab4 in riscv_tr_translate_insn
(dcbase=0x7fffef1c8e50, cpu=0x555556d51670) at
../../target/riscv/translate.c:1236

This happens because we've been passed in 0 as our TCGv,
which isn't valid. That in turn is because trans_cbo_inval()
does:
           gen_helper_cbo_inval(tcg_env, cpu_gpr[a->rs1]);
but a->rs1 is 0.

The comment in riscv_translate_init() says:
    /*
     * cpu_gpr[0] is a placeholder for the zero register. Do not use it.
     * Use the gen_set_gpr and get_gpr helper functions when accessing regs,
     * unless you specifically block reads/writes to reg 0.
     */

trans_cbo_inval() doesn't do either of those things, so that is
where your bug is.

thanks
-- PMM