Re: [PATCH] plugin: fix clearing of plugin_mem_cbs before TB exit

[Emilio Cota]

On Tue, Feb 28, 2023 at 10:50:26 -1000, Richard Henderson wrote:
> On 2/21/23 18:32, Emilio Cota wrote:
> > 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.
> 
> This is an improvement, but incomplete, because it does not handle the
> exception exit case, via cpu_loop_exit.

AFAICT that is already handled -- see the call to
qemu_plugin_disable_mem_helpers upon returning from the longjmp
in cpu-exec.c.

I do think that doing the clearing in C as done in your series
is a better solution. It is simpler and what I most like about
it is that it generates less code. In fact I wanted to mention
that approach as an alternative in the commit log, but forgot
to do so.

Thanks,
                Emilio