Currently we are wrongly accessing plugin_tb->mem_helper at
translation time from plugin_gen_disable_mem_helpers, which is
called before generating a TB exit, e.g. with exit_tb.
Recall that it is only during TB finalisation, i.e. when we go over
the TB post-translation to inject or remove plugin instrumentation,
when plugin_tb->mem_helper is set. This means that we never clear
plugin_mem_cbs when calling plugin_gen_disable_mem_helpers since
mem_helper is always false. Therefore a guest instruction that uses
helpers and emits an explicit TB exit results in plugin_mem_cbs being
set upon exiting, which is caught by an assertion as reported in
the reopening of issue #1381 and replicated below.
Fix this by (1) adding an insertion point before exiting a TB
("before_exit"), and (2) deciding whether or not to emit the
clearing of plugin_mem_cbs at this newly-added insertion point
during TB finalisation.
While at it, suffix plugin_gen_disable_mem_helpers with _before_exit
to make its intent more clear.
- Before:
$ ./qemu-system-riscv32 -M spike -nographic -plugin
contrib/plugins/libexeclog.so -d plugin,in_asm,op
IN:
Priv: 3; Virt: 0
0x00001000: 00000297 auipc t0,0
# 0x1000
0x00001004: 02828613 addi a2,t0,40
0x00001008: f1402573 csrrs a0,mhartid,zero
OP:
ld_i32 tmp1,env,$0xfffffffffffffff0
brcond_i32 tmp1,$0x0,lt,$L0
---- 00001000 00000000
mov_i64 tmp2,$0x7ff9940152e0
ld_i32 tmp1,env,$0xffffffffffffef80
call plugin(0x7ff9edbcb6f0),$0x11,$0,tmp1,tmp2
mov_i32 x5/t0,$0x1000
---- 00001004 00000000
mov_i64 tmp2,$0x7ff9940153e0
ld_i32 tmp1,env,$0xffffffffffffef80
call plugin(0x7ff9edbcb6f0),$0x11,$0,tmp1,tmp2
add_i32 x12/a2,x5/t0,$0x28
---- 00001008 f1402573
mov_i64 tmp2,$0x7ff9940136c0
st_i64 tmp2,env,$0xffffffffffffef68
mov_i64 tmp2,$0x7ff994015530
ld_i32 tmp1,env,$0xffffffffffffef80
call plugin(0x7ff9edbcb6f0),$0x11,$0,tmp1,tmp2 <-- sets plugin_mem_cbs
call csrr,$0x0,$1,x10/a0,env,$0xf14 <-- helper that might access memory
mov_i32 pc,$0x100c
exit_tb $0x0 <-- exit_tb right after the helper; missing clearing of
plugin_mem_cbs
mov_i64 tmp2,$0x0
st_i64 tmp2,env,$0xffffffffffffef68 <-- after_insn clearing: dead due to
exit_tb above
set_label $L0
exit_tb $0x7ff9a4000043 <-- again, missing clearing (doesn't matter due to
$L0 label)
0, 0x1000, 0x297, "00000297 auipc t0,0
# 0x1000"
0, 0x1004, 0x2828613, "02828613 addi a2,t0,40"
**
ERROR:../accel/tcg/cpu-exec.c:983:cpu_exec_loop: assertion failed:
(cpu->plugin_mem_cbs == ((void *)0))
Bail out! ERROR:../accel/tcg/cpu-exec.c:983:cpu_exec_loop: assertion failed:
(cpu->plugin_mem_cbs == ((void *)0))
Aborted (core dumped)
- After:
$ ./qemu-system-riscv32 -M spike -nographic -plugin
contrib/plugins/libexeclog.so -d plugin,in_asm,op
(snip)
call plugin(0x7f19bc9e36f0),$0x11,$0,tmp1,tmp2 <-- sets plugin_mem_cbs
call csrr,$0x0,$1,x10/a0,env,$0xf14
mov_i32 pc,$0x100c
mov_i64 tmp2,$0x0
st_i64 tmp2,env,$0xffffffffffffef68 <-- before_exit clearing of plugin_mem_cbs
exit_tb $0x0
mov_i64 tmp2,$0x0
st_i64 tmp2,env,$0xffffffffffffef68 <-- after_insn clearing (dead)
set_label $L0
mov_i64 tmp2,$0x0
st_i64 tmp2,env,$0xffffffffffffef68 <-- before_exit clearing (doesn't matter
due to $L0)
exit_tb $0x7f38c8000043
[...]
Fixes: #1381
Signed-off-by: Emilio Cota <cota@braap.org>
---
accel/tcg/plugin-gen.c | 44 ++++++++++++++++++++-------------------
include/exec/plugin-gen.h | 4 ++--
include/qemu/plugin.h | 3 ---
tcg/tcg-op.c | 6 +++---
4 files changed, 28 insertions(+), 29 deletions(-)