Hi, Sergey,

thanks for the patch! LGTM with minor comments.

Sergey

From: Mike Pall <mike>

Thanks to Sergey Kaplun.

(cherry picked from commit 54cce2e1719a15fc33e40c57dbc3d62e9c104b03)

The -0 step and NaN control variable values may lead to the traces
with always failed guards.

This patch forbids recording of such traces since these traces are not
very useful. Unfortunately, this breaks for loop recording in DUALNUM
mode. This will be fixed in the next commit.

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

Part of tarantool/tarantool#12134
---
 src/lj_record.c                               |  6 ++
 .../lj-1432-minus-zero-step.test.lua          | 57 +++++++++++++
 .../lj-1433-nan-for-control-var.test.lua      | 79 +++++++++++++++++++
 3 files changed, 142 insertions(+)
 create mode 100644 test/tarantool-tests/lj-1432-minus-zero-step.test.lua
 create mode 100644 test/tarantool-tests/lj-1433-nan-for-control-var.test.lua

diff --git a/src/lj_record.c b/src/lj_record.c
index 81da43f5..a3a68b57 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -509,6 +509,12 @@ static LoopEvent rec_for(jit_State *J, const BCIns *fori, int isforl)
   LoopEvent ev;
   TRef stop;
   IRType t;
+  /* Avoid semantic mismatches and always failing guards. */
+  if (tvisnan(&tv[FORL_IDX]) ||
+      tvisnan(&tv[FORL_STOP]) ||
+      tvisnan(&tv[FORL_STEP]) ||
+      tvismzero(&tv[FORL_STEP]))
+    lj_trace_err(J, LJ_TRERR_GFAIL);
   if (isforl) {  /* Handle FORL/JFORL opcodes. */
     TRef idx = tr[FORL_IDX];
     if (mref(J->scev.pc, const BCIns) == fori && tref_ref(idx) == J->scev.idx) {
diff --git a/test/tarantool-tests/lj-1432-minus-zero-step.test.lua b/test/tarantool-tests/lj-1432-minus-zero-step.test.lua
new file mode 100644
index 00000000..112153dc
--- /dev/null
+++ b/test/tarantool-tests/lj-1432-minus-zero-step.test.lua
@@ -0,0 +1,57 @@
+local tap = require('tap')
+
+-- Test file to check the correct recording of -0 step for value.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/1432.
+
+local test = tap.test('lj-1432-minus-zero-step'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(2)
+
+local traceinfo = require('jit.util').traceinfo
+
+local function trace_slot()
+  local counter = 0
+  local slot = -0
+  -- Run the inner trace several times. Before the patch, it leads
+  -- to several child traces due to the always failed guards.
+  while true do
+    if counter > 5 then break end
+    counter = counter + 1;
+    -- luacheck: ignore
+    for _ = 1, 1, slot do
+      break
+    end
+  end
+end
+
+local function trace_const()
+  local counter = 0
+  -- Run the inner trace several times. Before the patch, it leads
+  -- to several child traces due to the always failed guards.
+  while true do
+    if counter > 5 then break end
+    counter = counter + 1;
+    -- luacheck: ignore
+    for _ = 1, 1, -0 do
+      break
+    end
+  end
+end
+
+local function test_trace_recorded(test_payload)
+  jit.flush()
+  -- Reset hotcounters.

+  jit.opt.start('hotloop=1', 'hotexit=1')
+  test_payload()
+  return traceinfo(1)
+end
+
+-- The -0 step leads to the always failed guard, so such traces
+-- are now aborted and not recorded.
+
+test:ok(not test_trace_recorded(trace_slot), 'no trace recorded -0 as slot')
+test:ok(not test_trace_recorded(trace_const), 'no trace recorded -0 as const')
+
+test:done(true)
diff --git a/test/tarantool-tests/lj-1433-nan-for-control-var.test.lua b/test/tarantool-tests/lj-1433-nan-for-control-var.test.lua
new file mode 100644
index 00000000..1f67f0ad
--- /dev/null
+++ b/test/tarantool-tests/lj-1433-nan-for-control-var.test.lua
@@ -0,0 +1,79 @@
+local tap = require('tap')
+
+-- Test file to check the correct recording of for control
+-- variable with NaN value.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/1433.
+
+local test = tap.test('lj-1433-nan-for-control-var'):skipcond({

I would rename: s/lj-1433-nan-for-control-var/lj-1433-nan-for-loop-control-var/

Feel free to ignore.

+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(3)
+
+local traceinfo = require('jit.util').traceinfo
+
+local function trace_nan_start()

nit: s/trace_nan_start/trace_nan_loop_start/

+  local counter = 0
+  -- XXX: Use NaN as stack slot, not upvalue.
+  local nan = 0 / 0
+  -- Run the inner trace several times. Before the patch, it leads
+  -- to the trace with always fail guard.
+  while true do
+    if counter > 5 then break end
+    counter = counter + 1;
+    -- luacheck: ignore
+    for _ = nan, 1, 1 do
+      break
+    end
+  end
+end
+
+local function trace_nan_stop()
+  local counter = 0
+  -- XXX: Use NaN as stack slot, not upvalue.
+  local nan = 0 / 0
+  -- Run the inner trace several times. Before the patch, it leads
+  -- to the trace with always fail guard.
+  while true do
+    if counter > 5 then break end
+    counter = counter + 1;
+    -- luacheck: ignore
+    for _ = 1, nan, 1 do
+      break
+    end
+  end
+end
+
+local function trace_nan_step()
+  local counter = 0
+  -- XXX: Use NaN as stack slot, not upvalue.
+  local nan = 0 / 0
+  -- Run the inner trace several times. Before the patch, it leads
+  -- to several child traces due to the always failed guards.
+  while true do
+    if counter > 5 then break end
+    counter = counter + 1;
+    -- luacheck: ignore
+    for _ = 1, 1, nan do
+      break
+    end
+  end
+end
+
+local function test_trace_recorded(test_payload)
+  jit.flush()
+  -- Reset hotcounters.
+  jit.opt.start('hotloop=1', 'hotexit=1')
+  test_payload()
+  return traceinfo(1)
+end
+
+-- The NaN control vars leads to the always failed guard, so such

+-- traces are now aborted and not recorded.
+
+test:ok(not test_trace_recorded(trace_nan_start), 'no trace recorded NaN start')
+test:ok(not test_trace_recorded(trace_nan_stop), 'no trace recorded NaN stop')
+test:ok(not test_trace_recorded(trace_nan_step), 'no trace recorded NaN step')
+
+test:done(true)
+