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