On 19/7/23 00:05, Alex Bennée wrote:

Matt Borgerson <cont...@mborgerson.com> writes:

Thanks Alex!


On Mon, Jul 17, 2023 at 8:34 AM Alex Bennée <alex.ben...@linaro.org> wrote:


Alex Bennée <alex.ben...@linaro.org> writes:

Matt Borgerson <cont...@mborgerson.com> writes:

Translation logic may partially decode an instruction, then abort and
remove the instruction from the TB. This can happen for example when an
instruction spans two pages. In this case, plugins may get an incorrect
result when calling qemu_plugin_tb_n_insns to query for the number of
instructions in the TB. This patch updates plugin_gen_tb_end to set the
final instruction count.

For some reason this fails to apply cleanly:

   git am 
./v2_20230714_contact_plugins_set_final_instruction_count_in_plugin_gen_tb_end.mbx
   Applying: plugins: Set final instruction count in plugin_gen_tb_end
   error: corrupt patch at line 31
   Patch failed at 0001 plugins: Set final instruction count in
   plugin_gen_tb_end

I think some newlines crept in, I was able to fix. Queued to
for-8.1/misc-fixes with the assert added.

Hmm so I ran into an issue:

   ./qemu-sh4 -plugin tests/plugin/libbb.so -d plugin 
./tests/tcg/sh4-linux-user/testthread
   ERROR:../../accel/tcg/plugin-gen.c:874:plugin_gen_tb_end: assertion failed: 
(num_insns <= ptb->n)
   Bail out! ERROR:../../accel/tcg/plugin-gen.c:874:plugin_gen_tb_end: assertion 
failed: (num_insns <= ptb->n)
   qemu: uncaught target signal 11 (Segmentation fault) - core dumped
   bb's: 9202, insns: 42264
   fish: Job 1, './qemu-sh4 -plugin tests/plugin…' terminated by signal SIGSEGV 
(Address boundary error)

Further investigation shows that gUSA sequences can cause the number of
instructions to run ahead, which also makes the setting of the ptb->n =
num_insns unsafe, running ahead of the number of instructions signalled
by plugin_gen_insn_start/plugin_gen_insn_end.

   Thread 1 hit Hardware watchpoint 5: *(int *) 0x7ffd410a2904
   Old value = 4
   New value = 1
   0x000055f148c00ea8 in decode_gusa (ctx=0x7ffd410a28f0, env=0x55f14a4106e8) 
at ../../target/sh4/translate.c:2167
   2167        ctx->base.num_insns += max_insns - 1;
   (rr) p max_insns
   $6 = 4
   (rr) p max_insns -1
   $7 = 3

Is this the 'fail' label? (You can check by using '-d unimp'
which would dump "Unrecognized gUSA sequence").

'fail' is followed by 'done' which updates ctx->base.num_insns.

   (rr) p ctx->base.num_insns
   $8 = 1

So I think we have to drop this for now until we can either fix
decode_gusa or find something else.

Richard,

Any ideas?





Reply via email to