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

Typo: s/analizis/analysis/

no slots may considered as used. This leads to addititional SLOAD on

Typo: s/may/may be/

Typo: s/to additional/to an additional/

trace with incorrect value used later. This patch disables the use-def

Typo: s/trace with/the trace with an/

analisis for BC_VARG as NIY.

Typo: s/analisis/analysis/

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

Typo: s/this/these/

+-- 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

Why are they exactly 6 and 7? Please drop a comment.

+
+-- 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)

Are there any reasons behind the `fmod` choice? If so, please drop a comment.

+end
+
+jit.opt.start('hotloop=1')
+
+local _, result
+local function wrap(arg)
+ -- `pcall()` is needed to emit snapshot to handle on-trace
+ -- errors.

Maybe it is worth mentioning Mike’s original comment[1] here.

Feel free to ignore.

+ _, 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

[1]: https://github.com/LuaJIT/LuaJIT/issues/704

--
Best regards,

Maxim Kokryashkin