Emit sunk IR_NEWREF only once per key on snapshot replay.

Thanks to Sergey Kaplun and Peter Cawley.

(cherry-picked from commit 1761fd2)

Assume we have the parent trace with the following IRs:

| 0001 }  tab TNEW   #0    #0
| 0002 }  p32 NEWREF 0001  "key"
| 0003 }  fal HSTORE 0002  false
| ....        SNAP   #1   [ ---- ---- 0001 ---- ]
| 0004 >  num SLOAD  #1    T
| ....        SNAP   #2   [ ---- ---- 0001 ]
| 0005 >  num EQ     0004  0004
| 0006 }  tru HSTORE 0002  true
| ....        SNAP   #3   [ ---- ---- 0001 true ]

The side trace for the third snapshot emits the following IRs:

| 0001    tab TNEW   #0    #0
| 0002    p32 NEWREF 0001  "key"
| 0003    fal HSTORE 0002  false
| 0004    p32 NEWREF 0001  "key"
| 0005    tru HSTORE 0004  true

As we can see, `NEWREF` is emitted twice. This is a violation of its
semantics, so the second store isn't noticeable.

This patch prevents the second emitting of IR NEWREF by checking the last
one emitted NEWREF IR. There is no need to check NEWREFs beyond since
it guarantees the snapshot is taken after it, because it may cause table
rehashing, so all prior results are invalidated.

Sergey Kaplun:
* added the description and the test for the problem

Resolves tarantool/tarantool#7937
Part of tarantool/tarantool#9145

Reviewed-by: Maxim Kokryashkin <[email protected]>
Reviewed-by: Sergey Bronnikov <[email protected]>
Signed-off-by: Igor Munkin <[email protected]>
(cherry picked from commit 005e8ce)

Author: Mike Pall

src/lj_snap.c

@@ -583,9 +583,25 @@ void lj_snap_replay(jit_State *J, GCtrace *T)
 		if (irr->o == IR_HREFK || irr->o == IR_AREF) {
 		  IRIns *irf = &T->ir[irr->op1];
 		  tmp = emitir(irf->ot, tmp, irf->op2);
+		} else if (irr->o == IR_NEWREF) {
+		  IRRef allocref = tref_ref(tr);
+		  IRRef keyref = tref_ref(key);
+		  IRRef newref_ref = J->chain[IR_NEWREF];
+		  IRIns *newref = &J->cur.ir[newref_ref];
+		  lj_assertJ(irref_isk(keyref),
+			     "sunk store for parent IR %04d with bad key %04d",
+			     refp - REF_BIAS, keyref - REF_BIAS);
+		  if (newref_ref > allocref && newref->op2 == keyref) {
+		    lj_assertJ(newref->op1 == allocref,
+			       "sunk store for parent IR %04d with bad tab %04d",
+			       refp - REF_BIAS, allocref - REF_BIAS);
+		    tmp = newref_ref;
+		    goto skip_newref;
+		  }
 		}
 	      }
 	      tmp = emitir(irr->ot, tmp, key);
+	    skip_newref:
 	      val = snap_pref(J, T, map, nent, seen, irs->op2);
 	      if (val == 0) {
 		IRIns *irc = &T->ir[irs->op2];

test/tarantool-tests/lj-1128-double-ir-newref-on-restore-sunk.test.lua

@@ -0,0 +1,84 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT incorrect restoring of sunk
+-- tables with double usage of IR_NEWREF.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1128.
+
+local test = tap.test('lj-1128-double-ir-newref-on-restore-sunk'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(3)
+
+local take_side
+
+local function trace_base(num)
+  local tab = {}
+  tab.key = false
+  -- This check can't be folded since `num` can be NaN.
+  tab.key = num == num
+  -- luacheck: ignore
+  -- This side trace emits the following IRs:
+  -- 0001    tab TNEW   #0    #0
+  -- 0002    p64 NEWREF 0001  "key"
+  -- 0003    fal HSTORE 0002  false
+  -- 0004    p64 NEWREF 0001  "key"
+  -- 0005    tru HSTORE 0004  true
+  -- As we can see, `NEWREF` is emitted twice. This is a violation
+  -- of its semantics, so the second store isn't noticeable.
+  if take_side then end
+  return tab.key
+end
+
+-- Uncompiled function to end up side trace here.
+local function trace_base_wp(num)
+  return trace_base(num)
+end
+jit.off(trace_base_wp)
+
+-- Same function as above, but with two IRs NEWREF emitted.
+-- The last NEWREF references another key.
+local function trace_2newref(num)
+  local tab = {}
+  tab.key1 = false
+  -- This + op can't be folded since `num` can be -0.
+  tab.key1 = num + 0
+  tab.key2 = false
+  -- This check can't be folded since `num` can be NaN.
+  tab.key2 = num == num
+  -- luacheck: ignore
+  if take_side then end
+  return tab.key1, tab.key2
+end
+
+-- Uncompiled function to end up side trace here.
+local function trace_2newref_wp(num)
+  return trace_2newref(num)
+end
+jit.off(trace_2newref_wp)
+
+jit.opt.start('hotloop=1', 'hotexit=1', 'tryside=1')
+
+-- Compile parent traces.
+trace_base_wp(0)
+trace_base_wp(0)
+trace_2newref_wp(0)
+trace_2newref_wp(0)
+
+-- Compile side traces.
+take_side = true
+trace_base_wp(0)
+trace_base_wp(0)
+trace_2newref_wp(0)
+trace_2newref_wp(0)
+
+test:is(trace_base(0), true, 'sunk value restored correctly')
+
+local arg = 0
+local r1, r2 = trace_2newref(arg)
+-- These tests didn't fail before the patch.
+-- They check the patch's correctness.
+test:is(r1, arg, 'sunk value restored correctly with 2 keys, first key')
+test:is(r2, true, 'sunk value restored correctly with 2 keys, second key')
+
+test:done(true)