[Tarantool-patches] [PATCH luajit 1/2] Fix use-def analysis for BC_VARG.

[Sergey Kaplun]

From: Mike Pall <mike>

Reported by Ryan Lucia.

(cherry-picked from commit 2801500a26084491ae035170cad4700513790890)

Use-def analizis for BC_VARG has to strong limit for the top/maxslot, so
no slots may considered as used. This leads to addititional SLOAD on
trace with incorrect value used later. This patch disables the use-def
analisis for BC_VARG as NIY.

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

Part of tarantool/tarantool#8516
Relates to tarantool/tarantool#8718
---
 src/lj_snap.c                                 |  4 +-
 .../lj-704-bc-varg-use-def.test.lua           | 65 +++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-704-bc-varg-use-def.test.lua

diff --git a/src/lj_snap.c b/src/lj_snap.c
index a8b49fcb..5bbe8498 100644
--- a/src/lj_snap.c
+++ b/src/lj_snap.c
@@ -267,7 +267,7 @@ static BCReg snap_usedef(jit_State *J, uint8_t *udf,
        if (!(op == BC_ISTC || op == BC_ISFC)) DEF_SLOT(bc_a(ins));
        break;
     case BCMbase:
-      if (op >= BC_CALLM && op <= BC_VARG) {
+      if (op >= BC_CALLM && op <= BC_ITERN) {
 	BCReg top = (op == BC_CALLM || op == BC_CALLMT || bc_c(ins) == 0) ?
 		    maxslot : (bc_a(ins) + bc_c(ins)+LJ_FR2);
 	if (LJ_FR2) DEF_SLOT(bc_a(ins)+1);
@@ -278,6 +278,8 @@ static BCReg snap_usedef(jit_State *J, uint8_t *udf,
 	  for (s = 0; s < bc_a(ins); s++) DEF_SLOT(s);
 	  return 0;
 	}
+      } else if (op == BC_VARG) {
+	return maxslot;  /* NYI: punt. */
       } else if (op == BC_KNIL) {
 	for (s = bc_a(ins); s <= bc_d(ins); s++) DEF_SLOT(s);
       } else if (op == BC_TSETM) {
diff --git a/test/tarantool-tests/lj-704-bc-varg-use-def.test.lua b/test/tarantool-tests/lj-704-bc-varg-use-def.test.lua
new file mode 100644
index 00000000..c3ba65dd
--- /dev/null
+++ b/test/tarantool-tests/lj-704-bc-varg-use-def.test.lua
@@ -0,0 +1,65 @@
+local tap = require('tap')
+-- Test file to demonstrate LuaJIT misbehaviour in use-def
+-- snapshot analysis for BC_VARG.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/704.
+local test = tap.test('lj-704-bc-varg-use-def'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+-- XXX: we don't really need to store this builtins, but this is
+-- reduces `jitdump()` output for reader significantly.
+local fmod = math.fmod
+local pcall = pcall
+
+-- Use the 2 values for `fmod()` to produce non-zero value for
+-- the call on trace (the last one call).
+local ARG_ON_RECORDING = 6
+local ON_TRACE_VALUE = ARG_ON_RECORDING + 1
+
+-- The `jitdump()` output was like the following before the patch:
+-- 0003 >  num SLOAD  #1    T
+-- ....        SNAP   #1   [`wrap()`|---- pcall|`varg()`|----]
+-- 0004 }  tab TNEW   #3    #0
+-- 0005 >  num SLOAD  #4    T
+-- 0006    p32 FLOAD  0004  tab.array
+-- 0007    p32 AREF   0006  +1
+-- 0008 }  num ASTORE 0007  0005
+-- ....        SNAP   #2   [`wrap()`|---- pcall|math.fmod|+6 0005]
+--
+-- The first snapshot misses the 0003 IR in the last slot to be
+-- used in the `fmod()` later, so it leads to the additional
+-- 0005 SLOAD #4, and storing it in the second snapshot.
+--
+-- The correct snapshot content after the patch is the following:
+-- ....        SNAP   #1   [`wrap()`|---- pcall|`varg()`|0003]
+-- ....
+-- ....        SNAP   #2   [`wrap()`|---- pcall|math.fmod|+6 0003]
+local function varg(...)
+  -- Generate snapshot after `pcall()` with missing slot.
+  -- The snapshot is generated before each TNEW after the commit
+  -- 7505e78bd6c24cac6e93f5163675021734801b65 ("Handle on-trace
+  -- OOM errors from helper functions.")
+  local slot = ({...})[1]
+  -- Forcify stitch and usage of vararg slot.
+  return fmod(ARG_ON_RECORDING, slot)
+end
+
+jit.opt.start('hotloop=1')
+
+local _, result
+local function wrap(arg)
+  -- `pcall()` is needed to emit snapshot to handle on-trace
+  -- errors.
+  _, result = pcall(varg, arg)
+end
+-- Record trace with the 0 result.
+wrap(ARG_ON_RECORDING)
+wrap(ARG_ON_RECORDING)
+-- Record trace with the non-zero result.
+wrap(ON_TRACE_VALUE)
+
+test:ok(result ~= 0, 'use-def analysis for BC_VARG')
+
+os.exit(test:check() and 0 or 1)
-- 
2.34.1