Re: [PATCH v2] plugins: Set final instruction count in plugin_gen_tb_end

["Alex Bennée" <alex.bennee@linaro.org>]

Matt Borgerson <contact@mborgerson.com> writes:

> Thanks Alex!
>
>
> On Mon, Jul 17, 2023 at 8:34 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> Alex Bennée <alex.bennee@linaro.org> writes:
>>
>> > Matt Borgerson <contact@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
  (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?



-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro