diff mbox series

plugins: fix qemu_plugin_reset

Message ID 20241014223353.900481-1-pierrick.bouvier@linaro.org
State Superseded
Headers show
Series plugins: fix qemu_plugin_reset | expand

Commit Message

Pierrick Bouvier Oct. 14, 2024, 10:33 p.m. UTC
34e5e1 refactored the plugin context initialization. After this change,
tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
one plugin at least is active.

When uninstalling the last plugin active, we stopped reinitializing
tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
This results in an error as they don't appear in a plugin op sequence as
expected.

The correct fix is to make sure we reset plugin translation variables
after current block translation ends. This way, we can catch any
potential misuse of those after a given block, in more than fixing the
current bug.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 accel/tcg/plugin-gen.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Richard Henderson Oct. 14, 2024, 11:14 p.m. UTC | #1
On 10/14/24 15:33, Pierrick Bouvier wrote:
> 34e5e1 refactored the plugin context initialization. After this change,
> tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
> one plugin at least is active.
> 
> When uninstalling the last plugin active, we stopped reinitializing
> tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
> This results in an error as they don't appear in a plugin op sequence as
> expected.
> 
> The correct fix is to make sure we reset plugin translation variables
> after current block translation ends. This way, we can catch any
> potential misuse of those after a given block, in more than fixing the
> current bug.
> 
> Fixes:https://gitlab.com/qemu-project/qemu/-/issues/2570
> Signed-off-by: Pierrick Bouvier<pierrick.bouvier@linaro.org>
> ---
>   accel/tcg/plugin-gen.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Pierrick Bouvier Oct. 15, 2024, 12:38 a.m. UTC | #2
Sent a v2 to fix a leak issue with tcg_ctx->plugin_tb.

On 10/14/24 15:33, Pierrick Bouvier wrote:
> 34e5e1 refactored the plugin context initialization. After this change,
> tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
> one plugin at least is active.
> 
> When uninstalling the last plugin active, we stopped reinitializing
> tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
> This results in an error as they don't appear in a plugin op sequence as
> expected.
> 
> The correct fix is to make sure we reset plugin translation variables
> after current block translation ends. This way, we can catch any
> potential misuse of those after a given block, in more than fixing the
> current bug.
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   accel/tcg/plugin-gen.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 2ee4c22befd..2a8c8b2ad14 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -467,4 +467,9 @@ void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
>   
>       /* inject the instrumentation at the appropriate places */
>       plugin_gen_inject(ptb);
> +
> +    /* reset plugin translation state */
> +    tcg_ctx->plugin_db = NULL;
> +    tcg_ctx->plugin_insn = NULL;
> +    tcg_ctx->plugin_tb = NULL;
>   }
diff mbox series

Patch

diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 2ee4c22befd..2a8c8b2ad14 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -467,4 +467,9 @@  void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
 
     /* inject the instrumentation at the appropriate places */
     plugin_gen_inject(ptb);
+
+    /* reset plugin translation state */
+    tcg_ctx->plugin_db = NULL;
+    tcg_ctx->plugin_insn = NULL;
+    tcg_ctx->plugin_tb = NULL;
 }