[Tarantool-patches] [PATCH luajit 1/2] Prevent recording of loops with -0 step or NaN values.
Sergey Kaplun
skaplun at tarantool.org
Fri Mar 13 13:07:30 MSK 2026
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
More information about the Tarantool-patches
mailing list