Re: [PATCH 2/4] translator: always pair plugin_gen_insn_{start,end} calls

2023-01-10 Thread Aaron Lindsay 
On Jan 08 11:47, Emilio Cota wrote:
> Related: #1381
> 
> Signed-off-by: Emilio Cota 
> ---
>  accel/tcg/translator.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)

Tested-by: Aaron Lindsay

Re: [PATCH 2/4] translator: always pair plugin_gen_insn_{start, end} calls

2023-01-08 Thread Philippe Mathieu-Daudé 

On 8/1/23 17:47, Emilio Cota wrote:

Related: #1381

Signed-off-by: Emilio Cota 
---
  accel/tcg/translator.c | 15 ++-
  1 file changed, 10 insertions(+), 5 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé

[PATCH 2/4] translator: always pair plugin_gen_insn_{start, end} calls

2023-01-08 Thread Emilio Cota 
Related: #1381

Signed-off-by: Emilio Cota 
---
 accel/tcg/translator.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 061519691f..ef5193c67e 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -100,19 +100,24 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, 
int max_insns,
 ops->translate_insn(db, cpu);
 }
 
-/* Stop translation if translate_insn so indicated.  */
-if (db->is_jmp != DISAS_NEXT) {
-break;
-}
-
 /*
  * We can't instrument after instructions that change control
  * flow although this only really affects post-load operations.
+ *
+ * Calling plugin_gen_insn_end() before we possibly stop translation
+ * is important. Even if this ends up as dead code, plugin generation
+ * needs to see a matching plugin_gen_insn_{start,end}() pair in order
+ * to accurately track instrumented helpers that might access memory.
  */
 if (plugin_enabled) {
 plugin_gen_insn_end();
 }
 
+/* Stop translation if translate_insn so indicated.  */
+if (db->is_jmp != DISAS_NEXT) {
+break;
+}
+
 /* Stop translation if the output buffer is full,
or we have executed all of the allowed instructions.  */
 if (tcg_op_buf_full() || db->num_insns >= db->max_insns) {
-- 
2.34.1