Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lj_snap_replay is slightly too lax about operand modes #1132

Closed
corsix opened this issue Dec 11, 2023 · 1 comment
Closed

lj_snap_replay is slightly too lax about operand modes #1132

corsix opened this issue Dec 11, 2023 · 1 comment

Comments

@corsix
Copy link

corsix commented Dec 11, 2023

The following creates a trace containing the IR instruction {sink} tab TNEW #32762 #0:

local tnew = require"table.new"
local t2
for i = 1, 36, 0.5029296875 do
  local t = tnew(0x7ff9, 0)
  if i >= 30 then t2 = t end
end

When lj_snap_replay sees this, the #32762 will fall afoul of ir->op1 >= T->nk, causing #32762 to be treated as an IRRef, and then turned into an IRIns* and passed to snap_replay_const. Due to carefully chosen magic numbers in the example, that IRIns* will in fact be a double* pointing at 0.5029296875, which is very much not an IRIns, but has been carefully chosen to have ir->o == IR_KGC when treated as an IRIns. Bad things happen after this.

One quick fix is:

diff --git a/src/lj_snap.c b/src/lj_snap.c
index 93eb8a2919..57553ff31f 100644
--- a/src/lj_snap.c
+++ b/src/lj_snap.c
@@ -562,7 +570,7 @@ void lj_snap_replay(jit_State *J, GCtrace *T)
 	lj_assertJ(ir->o == IR_TNEW || ir->o == IR_TDUP ||
 		   ir->o == IR_CNEW || ir->o == IR_CNEWI,
 		   "sunk parent IR %04d has bad op %d", refp - REF_BIAS, ir->o);
-	if (ir->op1 >= T->nk) snap_pref(J, T, map, nent, seen, ir->op1);
+	if (ir->o != IR_TNEW) snap_pref(J, T, map, nent, seen, ir->op1);
 	if (ir->op2 >= T->nk) snap_pref(J, T, map, nent, seen, ir->op2);
 	if (LJ_HASFFI && ir->o == IR_CNEWI) {
 	  if (LJ_32 && refp+1 < T->nins && (ir+1)->o == IR_HIOP)
@@ -596,7 +604,7 @@ void lj_snap_replay(jit_State *J, GCtrace *T)
 	  continue;
 	}
 	op1 = ir->op1;
-	if (op1 >= T->nk) op1 = snap_pref(J, T, map, nent, seen, op1);
+	if (ir->o != IR_TNEW) op1 = snap_pref(J, T, map, nent, seen, op1);
 	op2 = ir->op2;
 	if (op2 >= T->nk) op2 = snap_pref(J, T, map, nent, seen, op2);
 	if (LJ_HASFFI && ir->o == IR_CNEWI) {

Or a more robust fix:

diff --git a/src/lj_snap.c b/src/lj_snap.c
index 93eb8a2919..322f12883b 100644
--- a/src/lj_snap.c
+++ b/src/lj_snap.c
@@ -557,13 +565,15 @@ void lj_snap_replay(jit_State *J, GCtrace *T)
       IRRef refp = snap_ref(sn);
       IRIns *ir = &T->ir[refp];
       if (regsp_reg(ir->r) == RID_SUNK) {
+	uint8_t m;
 	if (J->slot[snap_slot(sn)] != snap_slot(sn)) continue;
 	pass23 = 1;
 	lj_assertJ(ir->o == IR_TNEW || ir->o == IR_TDUP ||
 		   ir->o == IR_CNEW || ir->o == IR_CNEWI,
 		   "sunk parent IR %04d has bad op %d", refp - REF_BIAS, ir->o);
-	if (ir->op1 >= T->nk) snap_pref(J, T, map, nent, seen, ir->op1);
-	if (ir->op2 >= T->nk) snap_pref(J, T, map, nent, seen, ir->op2);
+	m = lj_ir_mode[ir->o];
+	if (irm_op1(m) == IRMref) snap_pref(J, T, map, nent, seen, ir->op1);
+	if (irm_op2(m) == IRMref) snap_pref(J, T, map, nent, seen, ir->op2);
 	if (LJ_HASFFI && ir->o == IR_CNEWI) {
 	  if (LJ_32 && refp+1 < T->nins && (ir+1)->o == IR_HIOP)
 	    snap_pref(J, T, map, nent, seen, (ir+1)->op2);
@@ -590,15 +600,17 @@ void lj_snap_replay(jit_State *J, GCtrace *T)
       IRRef refp = snap_ref(sn);
       IRIns *ir = &T->ir[refp];
       if (regsp_reg(ir->r) == RID_SUNK) {
+	uint8_t m;
 	TRef op1, op2;
 	if (J->slot[snap_slot(sn)] != snap_slot(sn)) {  /* De-dup allocs. */
 	  J->slot[snap_slot(sn)] = J->slot[J->slot[snap_slot(sn)]];
 	  continue;
 	}
+	m = lj_ir_mode[ir->o];
 	op1 = ir->op1;
-	if (op1 >= T->nk) op1 = snap_pref(J, T, map, nent, seen, op1);
+	if (irm_op1(m) == IRMref) op1 = snap_pref(J, T, map, nent, seen, op1);
 	op2 = ir->op2;
-	if (op2 >= T->nk) op2 = snap_pref(J, T, map, nent, seen, op2);
+	if (irm_op2(m) == IRMref) op2 = snap_pref(J, T, map, nent, seen, op2);
 	if (LJ_HASFFI && ir->o == IR_CNEWI) {
 	  if (LJ_32 && refp+1 < T->nins && (ir+1)->o == IR_HIOP) {
 	    lj_needsplit(J);  /* Emit joining HIOP. */
MikePall pushed a commit that referenced this issue Dec 11, 2023
@MikePall
Copy link
Member

Fixed. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants