Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Maksim Tiushev <mandesero@gmail.com>,
	Sergey Bronnikov <sergeyb@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH v5 luajit 1/3] Ensure full init of IR_NOP instructions.
Date: Wed, 11 Dec 2024 16:21:06 +0300	[thread overview]
Message-ID: <c6f0e186834eef000569d3911c5d8d1aedb2cebb.1733909411.git.skaplun@tarantool.org> (raw)
In-Reply-To: <cover.1733909411.git.skaplun@tarantool.org>

From: Mike Pall <mike>

(cherry picked from commit de6b1a11dd1a3349179084578c5d533be1c30234)

Before this patch, Valgrind produces tons of warnings during the VMevent
calls since the `IR_NOP` instruction isn't fully initialized. Hence, any
parsing operations for it in handlers during `jit.dump()` leads to the
"uninitialised value" error. This patch fixes the issue by the proper
init of such IRs.

Sergey Kaplun:
* added the description for the problem

Needed for tarantool/tarantool#3705
Part of tarantool/tarantool#10709
---
 src/lj_asm.c     |  2 +-
 src/lj_ir.h      |  8 ++++++++
 src/lj_opt_dce.c |  5 +----
 src/lj_opt_mem.c | 25 +++++--------------------
 4 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/src/lj_asm.c b/src/lj_asm.c
index 5137d05c..f7540165 100644
--- a/src/lj_asm.c
+++ b/src/lj_asm.c
@@ -2377,7 +2377,7 @@ void lj_asm_trace(jit_State *J, GCtrace *T)
   /* Ensure an initialized instruction beyond the last one for HIOP checks. */
   /* This also allows one RENAME to be added without reallocating curfinal. */
   as->orignins = lj_ir_nextins(J);
-  J->cur.ir[as->orignins].o = IR_NOP;
+  lj_ir_nop(&J->cur.ir[as->orignins]);
 
   /* Setup initial state. Copy some fields to reduce indirections. */
   as->J = J;
diff --git a/src/lj_ir.h b/src/lj_ir.h
index 27c66f63..46e7413e 100644
--- a/src/lj_ir.h
+++ b/src/lj_ir.h
@@ -587,4 +587,12 @@ static LJ_AINLINE int ir_sideeff(IRIns *ir)
 
 LJ_STATIC_ASSERT((int)IRT_GUARD == (int)IRM_W);
 
+/* Replace IR instruction with NOP. */
+static LJ_AINLINE void lj_ir_nop(IRIns *ir)
+{
+  ir->ot = IRT(IR_NOP, IRT_NIL);
+  ir->op1 = ir->op2 = 0;
+  ir->prev = 0;
+}
+
 #endif
diff --git a/src/lj_opt_dce.c b/src/lj_opt_dce.c
index 6948179c..41e36d99 100644
--- a/src/lj_opt_dce.c
+++ b/src/lj_opt_dce.c
@@ -46,10 +46,7 @@ static void dce_propagate(jit_State *J)
       irt_clearmark(ir->t);
     } else if (!ir_sideeff(ir)) {
       *pchain[ir->o] = ir->prev;  /* Reroute original instruction chain. */
-      ir->t.irt = IRT_NIL;
-      ir->o = IR_NOP;  /* Replace instruction with NOP. */
-      ir->op1 = ir->op2 = 0;
-      ir->prev = 0;
+      lj_ir_nop(ir);
       continue;
     }
     pchain[ir->o] = &ir->prev;
diff --git a/src/lj_opt_mem.c b/src/lj_opt_mem.c
index c9f1216c..a6e9a072 100644
--- a/src/lj_opt_mem.c
+++ b/src/lj_opt_mem.c
@@ -375,10 +375,7 @@ TRef LJ_FASTCALL lj_opt_dse_ahstore(jit_State *J)
 	    goto doemit;  /* No elimination possible. */
 	/* Remove redundant store from chain and replace with NOP. */
 	*refp = store->prev;
-	store->o = IR_NOP;
-	store->t.irt = IRT_NIL;
-	store->op1 = store->op2 = 0;
-	store->prev = 0;
+	lj_ir_nop(store);
 	/* Now emit the new store instead. */
       }
       goto doemit;
@@ -479,10 +476,7 @@ TRef LJ_FASTCALL lj_opt_dse_ustore(jit_State *J)
 	    goto doemit;  /* No elimination possible. */
 	/* Remove redundant store from chain and replace with NOP. */
 	*refp = store->prev;
-	store->o = IR_NOP;
-	store->t.irt = IRT_NIL;
-	store->op1 = store->op2 = 0;
-	store->prev = 0;
+	lj_ir_nop(store);
 	if (ref+1 < J->cur.nins &&
 	    store[1].o == IR_OBAR && store[1].op1 == xref) {
 	  IRRef1 *bp = &J->chain[IR_OBAR];
@@ -491,10 +485,7 @@ TRef LJ_FASTCALL lj_opt_dse_ustore(jit_State *J)
 	    bp = &obar->prev;
 	  /* Remove OBAR, too. */
 	  *bp = obar->prev;
-	  obar->o = IR_NOP;
-	  obar->t.irt = IRT_NIL;
-	  obar->op1 = obar->op2 = 0;
-	  obar->prev = 0;
+	  lj_ir_nop(obar);
 	}
 	/* Now emit the new store instead. */
       }
@@ -585,10 +576,7 @@ TRef LJ_FASTCALL lj_opt_dse_fstore(jit_State *J)
 	    goto doemit;  /* No elimination possible. */
 	/* Remove redundant store from chain and replace with NOP. */
 	*refp = store->prev;
-	store->o = IR_NOP;
-	store->t.irt = IRT_NIL;
-	store->op1 = store->op2 = 0;
-	store->prev = 0;
+	lj_ir_nop(store);
 	/* Now emit the new store instead. */
       }
       goto doemit;
@@ -839,10 +827,7 @@ TRef LJ_FASTCALL lj_opt_dse_xstore(jit_State *J)
 	    goto doemit;  /* No elimination possible. */
 	/* Remove redundant store from chain and replace with NOP. */
 	*refp = store->prev;
-	store->o = IR_NOP;
-	store->t.irt = IRT_NIL;
-	store->op1 = store->op2 = 0;
-	store->prev = 0;
+	lj_ir_nop(store);
 	/* Now emit the new store instead. */
       }
       goto doemit;
-- 
2.47.0


  reply	other threads:[~2024-12-11 13:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-11 13:21 [Tarantool-patches] [PATCH v5 luajit 0/3] Valgrind testing Sergey Kaplun via Tarantool-patches
2024-12-11 13:21 ` Sergey Kaplun via Tarantool-patches [this message]
2024-12-13 12:54   ` [Tarantool-patches] [PATCH v5 luajit 1/3] Ensure full init of IR_NOP instructions Sergey Bronnikov via Tarantool-patches
2024-12-16 11:24     ` Sergey Kaplun via Tarantool-patches
2024-12-17 11:08       ` Sergey Bronnikov via Tarantool-patches
2024-12-11 13:21 ` [Tarantool-patches] [PATCH v5 luajit 2/3] cmake: run tests with Valgrind Sergey Kaplun via Tarantool-patches
2024-12-13 13:18   ` Sergey Bronnikov via Tarantool-patches
2024-12-16 16:40     ` Sergey Kaplun via Tarantool-patches
2024-12-17 11:42       ` Sergey Bronnikov via Tarantool-patches
2024-12-17 12:17         ` Sergey Kaplun via Tarantool-patches
2024-12-17 19:31           ` Sergey Bronnikov via Tarantool-patches
2024-12-11 13:21 ` [Tarantool-patches] [PATCH v5 luajit 3/3] ci: add Valgrind testing workflow Sergey Kaplun via Tarantool-patches
2024-12-13 13:23   ` Sergey Bronnikov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c6f0e186834eef000569d3911c5d8d1aedb2cebb.1733909411.git.skaplun@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=mandesero@gmail.com \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v5 luajit 1/3] Ensure full init of IR_NOP instructions.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox