[Tarantool-patches] [PATCH luajit 0/2] Fix corner cases of for loop recording

[Tarantool-patches] [PATCH luajit 0/2] Fix corner cases of for loop recording

[Sergey Kaplun via Tarantool-patches]

The first patch prevents recording of useless traces with always-fail
guards for NaN control vars or -0 step values. The second patch is the
follow-up to fix DUALNUM mode.

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1432-1433-bad-for-loops
Related issues:
* https://github.com/LuaJIT/LuaJIT/issues/1432
* https://github.com/LuaJIT/LuaJIT/issues/1433
* https://github.com/LuaJIT/LuaJIT/issues/1438
* https://github.com/tarantool/tarantool/issues/12134

Mike Pall (2):
  Prevent recording of loops with -0 step or NaN values.
  DUALNUM: Fix recording of loops broken by previous change.

 src/lj_record.c                               |  6 ++
 .../lj-1432-minus-zero-step.test.lua          | 57 +++++++++++++
 .../lj-1433-nan-for-control-var.test.lua      | 79 +++++++++++++++++++
 .../lj-1438-jit-for-canary.test.lua           | 22 ++++++
 4 files changed, 164 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
 create mode 100644 test/tarantool-tests/lj-1438-jit-for-canary.test.lua

-- 
2.53.0

[Tarantool-patches] [PATCH luajit 1/2] Prevent recording of loops with -0 step or NaN values.

[Sergey Kaplun via Tarantool-patches]

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({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(3)
+
+local traceinfo = require('jit.util').traceinfo
+
+local function trace_nan_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)
+
-- 
2.53.0

[Tarantool-patches] [PATCH luajit 2/2] DUALNUM: Fix recording of loops broken by previous change.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Thanks to Nicholas Davies.

(cherry picked from commit 1c3b5a4d722598ecbb9219480142eda682e87bb1)

The previous commit forbids recording for the NaN control variables
stack slots. These checks are false positives for integers in DUALNUM
mode. This prevents any for loop with integers as slots from being
recorded.

This patch fixes the checks.

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

Part of tarantool/tarantool#12134
---
 src/lj_record.c                               |  6 ++---
 .../lj-1438-jit-for-canary.test.lua           | 22 +++++++++++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1438-jit-for-canary.test.lua

diff --git a/src/lj_record.c b/src/lj_record.c
index a3a68b57..dbfc7f47 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -510,9 +510,9 @@ static LoopEvent rec_for(jit_State *J, const BCIns *fori, int isforl)
   TRef stop;
   IRType t;
   /* Avoid semantic mismatches and always failing guards. */
-  if (tvisnan(&tv[FORL_IDX]) ||
-      tvisnan(&tv[FORL_STOP]) ||
-      tvisnan(&tv[FORL_STEP]) ||
+  if ((tvisnum(&tv[FORL_IDX]) && tvisnan(&tv[FORL_IDX])) ||
+      (tvisnum(&tv[FORL_STOP]) && tvisnan(&tv[FORL_STOP])) ||
+      (tvisnum(&tv[FORL_STEP]) && tvisnan(&tv[FORL_STEP])) ||
       tvismzero(&tv[FORL_STEP]))
     lj_trace_err(J, LJ_TRERR_GFAIL);
   if (isforl) {  /* Handle FORL/JFORL opcodes. */
diff --git a/test/tarantool-tests/lj-1438-jit-for-canary.test.lua b/test/tarantool-tests/lj-1438-jit-for-canary.test.lua
new file mode 100644
index 00000000..4b67df4c
--- /dev/null
+++ b/test/tarantool-tests/lj-1438-jit-for-canary.test.lua
@@ -0,0 +1,22 @@
+local tap = require('tap')
+
+-- The test file to check the correct recording of the for loop.
+-- Used as a canary test, since we have none.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/1438.
+
+local test = tap.test('lj-1438-jit-for-canary'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+local traceinfo = require('jit.util').traceinfo
+
+jit.flush()
+jit.opt.start('hotloop=1')
+
+for _ = 1, 4 do end
+
+test:ok(traceinfo(1), 'simple for loop is recorded')
+
+test:done(true)
-- 
2.53.0

Re: [Tarantool-patches] [PATCH luajit 1/2] Prevent recording of loops with -0 step or NaN values.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 6463 bytes --]

Hi, Sergey,

thanks for the patch! LGTM with minor comments.

Sergey

On 3/12/26 18:55, Sergey Kaplun wrote:
> 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 alsohttps://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.
nit: comment can be omitted
> +  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 alsohttps://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/

the same below
> +  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
s/control/loop control/
> +-- 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)
> +

[-- Attachment #2: Type: text/html, Size: 7580 bytes --]

Re: [Tarantool-patches] [PATCH luajit 1/2] Prevent recording of loops with -0 step or NaN values.

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the review!
Fixed your comments and updated the branch.

On 13.03.26, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch! LGTM with minor comments.
> 
> Sergey
> 
> On 3/12/26 18:55, Sergey Kaplun wrote:
> > 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

<snipped>

> > 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 alsohttps://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.
> nit: comment can be omitted

I prefer not to. There may be the question: why we don't declare this
parameters once? The reason is that the hotcounters may cause collisions
and lead to the false-positive tests failures. Should I make the comment
more verbose?

> > +  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 alsohttps://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.

Renamed here and the test name.

===================================================================
diff --git a/test/tarantool-tests/lj-1433-nan-for-control-var.test.lua b/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
similarity index 96%
rename from test/tarantool-tests/lj-1433-nan-for-control-var.test.lua
rename to test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
index 1f67f0ad..fccd13c4 100644
--- a/test/tarantool-tests/lj-1433-nan-for-control-var.test.lua
+++ b/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
@@ -4,7 +4,7 @@ local tap = require('tap')
 -- variable with NaN value.
 -- See also https://github.com/LuaJIT/LuaJIT/issues/1433.
 
-local test = tap.test('lj-1433-nan-for-control-var'):skipcond({
+local test = tap.test('lj-1433-nan-for-loop-control-var'):skipcond({
   ['Test requires JIT enabled'] = not jit.status(),
 })
 
===================================================================

> 
> > +  ['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/
> 
> the same below

Renamed:

===================================================================
diff --git a/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua b/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
index 63bbddcc..b9e5ad92 100644
--- a/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
+++ b/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
@@ -12,7 +12,7 @@ test:plan(3)
 
 local traceinfo = require('jit.util').traceinfo
 
-local function trace_nan_start()
+local function trace_nan_loop_start()
   local counter = 0
   -- XXX: Use NaN as stack slot, not upvalue.
   local nan = 0 / 0
@@ -28,7 +28,7 @@ local function trace_nan_start()
   end
 end
 
-local function trace_nan_stop()
+local function trace_nan_loop_stop()
   local counter = 0
   -- XXX: Use NaN as stack slot, not upvalue.
   local nan = 0 / 0
@@ -44,7 +44,7 @@ local function trace_nan_stop()
   end
 end
 
-local function trace_nan_step()
+local function trace_nan_loop_step()
   local counter = 0
   -- XXX: Use NaN as stack slot, not upvalue.
   local nan = 0 / 0
@@ -71,9 +71,12 @@ end
 -- The NaN loop 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:ok(not test_trace_recorded(trace_nan_loop_start),
+        'no trace recorded NaN start')
+test:ok(not test_trace_recorded(trace_nan_loop_stop),
+        'no trace recorded NaN stop')
+test:ok(not test_trace_recorded(trace_nan_loop_step),
+        'no trace recorded NaN step')
 
 test:done(true)
===================================================================


> > +  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
> s/control/loop control/

Fixed.
===================================================================
diff --git a/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua b/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
index fccd13c4..63bbddcc 100644
--- a/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
+++ b/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
@@ -68,8 +68,8 @@ local function test_trace_recorded(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.
+-- The NaN loop 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')
===================================================================

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

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/2] DUALNUM: Fix recording of loops broken by previous change.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]

Hi, Sergey,

thanks for the patch! LGTM

Sergey

On 3/12/26 18:55, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Nicholas Davies.
>
> (cherry picked from commit 1c3b5a4d722598ecbb9219480142eda682e87bb1)
>
> The previous commit forbids recording for the NaN control variables
> stack slots. These checks are false positives for integers in DUALNUM
> mode. This prevents any for loop with integers as slots from being
> recorded.
>
> This patch fixes the checks.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#12134
> ---
>   src/lj_record.c                               |  6 ++---
>   .../lj-1438-jit-for-canary.test.lua           | 22 +++++++++++++++++++
>   2 files changed, 25 insertions(+), 3 deletions(-)
>   create mode 100644 test/tarantool-tests/lj-1438-jit-for-canary.test.lua
>
> diff --git a/src/lj_record.c b/src/lj_record.c
> index a3a68b57..dbfc7f47 100644
> --- a/src/lj_record.c
> +++ b/src/lj_record.c
> @@ -510,9 +510,9 @@ static LoopEvent rec_for(jit_State *J, const BCIns *fori, int isforl)
>     TRef stop;
>     IRType t;
>     /* Avoid semantic mismatches and always failing guards. */
> -  if (tvisnan(&tv[FORL_IDX]) ||
> -      tvisnan(&tv[FORL_STOP]) ||
> -      tvisnan(&tv[FORL_STEP]) ||
> +  if ((tvisnum(&tv[FORL_IDX]) && tvisnan(&tv[FORL_IDX])) ||
> +      (tvisnum(&tv[FORL_STOP]) && tvisnan(&tv[FORL_STOP])) ||
> +      (tvisnum(&tv[FORL_STEP]) && tvisnan(&tv[FORL_STEP])) ||
>         tvismzero(&tv[FORL_STEP]))
>       lj_trace_err(J, LJ_TRERR_GFAIL);
>     if (isforl) {  /* Handle FORL/JFORL opcodes. */
> diff --git a/test/tarantool-tests/lj-1438-jit-for-canary.test.lua b/test/tarantool-tests/lj-1438-jit-for-canary.test.lua
> new file mode 100644
> index 00000000..4b67df4c
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1438-jit-for-canary.test.lua
> @@ -0,0 +1,22 @@
> +local tap = require('tap')
> +
> +-- The test file to check the correct recording of the for loop.
> +-- Used as a canary test, since we have none.
> +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1438.
> +
> +local test = tap.test('lj-1438-jit-for-canary'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(1)
> +
> +local traceinfo = require('jit.util').traceinfo
> +
> +jit.flush()
> +jit.opt.start('hotloop=1')
> +
> +for _ = 1, 4 do end
> +
> +test:ok(traceinfo(1), 'simple for loop is recorded')
> +
> +test:done(true)

[-- Attachment #2: Type: text/html, Size: 2965 bytes --]