[Tarantool-patches] [PATCH luajit] Add NaN check to IR_NEWREF.

[Tarantool-patches] [PATCH luajit] Add NaN check to IR_NEWREF.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Thanks to Peter Cawley.

(cherry-picked from commit 7f9907b4ed0870ba64342bcc4b26cff0a94540da)

When emitting IR NEWREF, there is no check for a non-NaN stored key
value. Thus, when the NaN number value is given to trace, it may be
stored as a key. This patch adds the corresponding check. If fold
optimization is enabled, this IR EQ check is dropped if it references
CONV IR from any (unsigned) integer type since NaN can be created via
conversion from an integer.

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

Part of tarantool/tarantool#9145
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1069-newref-nan-key
Tarantool PR: https://github.com/tarantool/tarantool/pull/9374
Fuzzer link: https://oss-fuzz.com/testcase-detail/5251574662037504
Relate issues:
* https://github.com/LuaJIT/LuaJIT/issues/1069
* https://github.com/tarantool/tarantool/issues/9145

 src/lj_opt_fold.c                             |   5 +-
 src/lj_record.c                               |  12 +-
 .../lj-1069-newref-nan-key.test.lua           | 151 ++++++++++++++++++
 3 files changed, 164 insertions(+), 4 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1069-newref-nan-key.test.lua

diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c
index 944a9ecc..61169397 100644
--- a/src/lj_opt_fold.c
+++ b/src/lj_opt_fold.c
@@ -1936,7 +1936,10 @@ LJFOLD(NE any any)
 LJFOLDF(comm_equal)
 {
   /* For non-numbers only: x == x ==> drop; x ~= x ==> fail */
-  if (fins->op1 == fins->op2 && !irt_isnum(fins->t))
+  if (fins->op1 == fins->op2 &&
+      (!irt_isnum(fins->t) ||
+       (fleft->o == IR_CONV &&  /* Converted integers cannot be NaN. */
+	(uint32_t)(fleft->op2 & IRCONV_SRCMASK) - (uint32_t)IRT_I8 <= (uint32_t)(IRT_U64 - IRT_U8))))
     return CONDFOLD(fins->o == IR_EQ);
   return fold_comm_swap(J);
 }
diff --git a/src/lj_record.c b/src/lj_record.c
index 3189a7c3..3e4914e1 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -1509,10 +1509,16 @@ TRef lj_record_idx(jit_State *J, RecordIndex *ix)
       lj_assertJ(!hasmm, "inconsistent metamethod handling");
       if (oldv == niltvg(J2G(J))) {  /* Need to insert a new key. */
 	TRef key = ix->key;
-	if (tref_isinteger(key))  /* NEWREF needs a TValue as a key. */
+	if (tref_isinteger(key)) {  /* NEWREF needs a TValue as a key. */
 	  key = emitir(IRTN(IR_CONV), key, IRCONV_NUM_INT);
-	else if (tref_isnumber(key) && tref_isk(key) && tvismzero(&ix->keyv))
-	  key = lj_ir_knum_zero(J);  /* Canonicalize -0.0 to +0.0. */
+	} else if (tref_isnum(key)) {
+	  if (tref_isk(key)) {
+	    if (tvismzero(&ix->keyv))
+	      key = lj_ir_knum_zero(J);  /* Canonicalize -0.0 to +0.0. */
+	  } else {
+	    emitir(IRTG(IR_EQ, IRT_NUM), key, key);  /* Check for !NaN. */
+	  }
+	}
 	xref = emitir(IRT(IR_NEWREF, IRT_PGC), ix->tab, key);
 	keybarrier = 0;  /* NEWREF already takes care of the key barrier. */
 #ifdef LUAJIT_ENABLE_TABLE_BUMP
diff --git a/test/tarantool-tests/lj-1069-newref-nan-key.test.lua b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
new file mode 100644
index 00000000..ec28b274
--- /dev/null
+++ b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
@@ -0,0 +1,151 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT misbehaviour for NEWREF IR
+-- taken NaN value.
+-- See also, https://github.com/LuaJIT/LuaJIT/issues/1069.
+
+local test = tap.test('lj-1069-newref-nan-key'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+local table_new = require('table.new')
+local ffi = require('ffi')
+
+local NaN = tonumber('nan')
+
+test:plan(4)
+
+test:test('NaN on trace in the non-constant IR', function(subtest)
+  local NKEYS = 3
+
+  -- XXX: NaN isn't stored, so the number of tests is:
+  -- (NKEYS - 1) + test status + test error message + test keys
+  -- amount.
+  subtest:plan(NKEYS + 2)
+
+  local tset_table = table_new(0, NKEYS)
+
+  local function tset(t, k)
+    -- Value doesn't matter.
+    t[k] = true
+  end
+
+  -- Compile the function.
+  jit.opt.start('hotloop=1')
+
+  -- Use number keys to emit NEWREF.
+  tset(tset_table, 0.1)
+  tset(tset_table, 0.2)
+
+  -- Insert NaN on the trace.
+  local ok, err = pcall(tset, tset_table, NaN)
+
+  subtest:ok(not ok, 'function returns an error')
+  subtest:like(err, 'table index is NaN', 'correct error message')
+
+  local nkeys = 0
+  for k in pairs(tset_table) do
+    nkeys = nkeys + 1
+    subtest:ok(k == k, ('not NaN key by number %d'):format(nkeys))
+  end
+  subtest:is(nkeys, NKEYS - 1, 'correct amount of keys')
+end)
+
+test:test('NaN on trace in the non-constant IR CONV', function(subtest)
+  local tonumber = tonumber
+  local NKEYS = 3
+
+  -- XXX: NaN isn't stored, so the number of tests is:
+  -- (NKEYS - 1) + test status + test error message + test keys
+  -- amount.
+  subtest:plan(NKEYS + 2)
+
+  local tset_table = table_new(0, NKEYS)
+
+  local function tset(t, k)
+    -- XXX: Emit CONV to number type. Value doesn't matter.
+    t[tonumber(k)] = true
+  end
+
+  -- Compile the function.
+  jit.on()
+  jit.opt.start('hotloop=1')
+
+  -- Use number keys to emit NEWREF.
+  tset(tset_table, ffi.new('float', 0.1))
+  tset(tset_table, ffi.new('float', 0.2))
+
+  -- Insert NaN on the trace.
+  local ok, err = pcall(tset, tset_table, ffi.new('float', NaN))
+
+  subtest:ok(not ok, 'function returns an error')
+  subtest:like(err, 'table index is NaN', 'correct error message')
+
+  local nkeys = 0
+  for k in pairs(tset_table) do
+    nkeys = nkeys + 1
+    subtest:ok(k == k, ('not NaN key by number %d'):format(nkeys))
+  end
+  subtest:is(nkeys, NKEYS - 1, 'correct amount of keys')
+end)
+
+-- Test the constant IR NaN value on the trace.
+
+test:test('constant NaN on the trace', function(subtest)
+  -- Test the status and the error message.
+  subtest:plan(2)
+  local function protected()
+    local counter = 0
+    -- Use a number key to emit NEWREF.
+    local key = 0.1
+
+    jit.opt.start('hotloop=1')
+
+    while counter < 2 do
+      counter = counter + 1
+      -- luacheck: ignore
+      local tab = {_ = 'unused'}
+      tab[key] = 'NaN'
+      -- XXX: Use the constant NaN value on the trace.
+      key = 0/0
+    end
+  end
+
+  local ok, err = pcall(protected)
+  subtest:ok(not ok, 'function returns an error')
+  subtest:like(err, 'table index is NaN', 'NaN index error message')
+end)
+
+test:test('constant NaN on the trace and rehash of the table', function(subtest)
+  -- A little bit different test case: after rehashing the table,
+  -- the node is lost, and a hash part of the table isn't freed.
+  -- This leads to the assertion failure, which checks memory
+  -- leaks on shutdown.
+  -- XXX: The test didn't fail even before the patch. Check the
+  -- same values as in the previous test for consistency.
+  subtest:plan(2)
+  local function protected()
+    local counter = 0
+    -- Use a number key to emit NEWREF.
+    local key = 0.1
+
+    jit.opt.start('hotloop=1')
+
+    while counter < 2 do
+      counter = counter + 1
+      -- luacheck: ignore
+      local tab = {_ = 'unused'}
+      tab[key] = 'NaN'
+      -- Rehash the table.
+      tab[counter] = 'unused'
+      -- XXX: Use the constant NaN value on the trace.
+      key = 0/0
+    end
+  end
+
+  local ok, err = pcall(protected)
+  subtest:ok(not ok, 'function returns an error')
+  subtest:like(err, 'table index is NaN', 'NaN index error message')
+end)
+
+test:done(true)
-- 
2.42.0

Re: [Tarantool-patches] [PATCH luajit] Add NaN check to IR_NEWREF.

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM, except for a single nit and a question below.
On Thu, Nov 16, 2023 at 11:49:59AM +0300, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Peter Cawley.
>
> (cherry-picked from commit 7f9907b4ed0870ba64342bcc4b26cff0a94540da)
>
> When emitting IR NEWREF, there is no check for a non-NaN stored key
> value. Thus, when the NaN number value is given to trace, it may be
> stored as a key. This patch adds the corresponding check. If fold
> optimization is enabled, this IR EQ check is dropped if it references
> CONV IR from any (unsigned) integer type since NaN can be created via
> conversion from an integer.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9145
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1069-newref-nan-key
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9374
> Fuzzer link: https://oss-fuzz.com/testcase-detail/5251574662037504
> Relate issues:
> * https://github.com/LuaJIT/LuaJIT/issues/1069
> * https://github.com/tarantool/tarantool/issues/9145
>
>  src/lj_opt_fold.c                             |   5 +-
>  src/lj_record.c                               |  12 +-
>  .../lj-1069-newref-nan-key.test.lua           | 151 ++++++++++++++++++
>  3 files changed, 164 insertions(+), 4 deletions(-)
>  create mode 100644 test/tarantool-tests/lj-1069-newref-nan-key.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-1069-newref-nan-key.test.lua b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
> new file mode 100644
> index 00000000..ec28b274
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
> @@ -0,0 +1,151 @@

> +-- Test the constant IR NaN value on the trace.
Nit: This comment seems a bit redundant, as it duplicates the test name. Feel
free to ignore.
> +test:test('constant NaN on the trace', function(subtest)
> +  -- Test the status and the error message.
> +  subtest:plan(2)
> +  local function protected()
> +    local counter = 0
> +    -- Use a number key to emit NEWREF.
> +    local key = 0.1
> +
> +    jit.opt.start('hotloop=1')
> +
> +    while counter < 2 do
> +      counter = counter + 1
> +      -- luacheck: ignore
> +      local tab = {_ = 'unused'}
> +      tab[key] = 'NaN'
> +      -- XXX: Use the constant NaN value on the trace.
> +      key = 0/0
> +    end
> +  end
> +
> +  local ok, err = pcall(protected)
> +  subtest:ok(not ok, 'function returns an error')
> +  subtest:like(err, 'table index is NaN', 'NaN index error message')
> +end)
> +
> +test:test('constant NaN on the trace and rehash of the table', function(subtest)
> +  -- A little bit different test case: after rehashing the table,
> +  -- the node is lost, and a hash part of the table isn't freed.
> +  -- This leads to the assertion failure, which checks memory
> +  -- leaks on shutdown.
> +  -- XXX: The test didn't fail even before the patch. Check the
> +  -- same values as in the previous test for consistency.
What do you mean by "didn't fail"? AFAICS, it leads to the assertion failure.
If you've meant no fails for a build with no assertions, then it worth
clarifying it in this comment.

<snipped>

Re: [Tarantool-patches] [PATCH luajit] Add NaN check to IR_NEWREF.

[Sergey Bronnikov via Tarantool-patches]

Hello, Sergey,


thanks for the patch. LGTM

See my comments below.


On 11/16/23 11:49, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Peter Cawley.
>
> (cherry-picked from commit 7f9907b4ed0870ba64342bcc4b26cff0a94540da)
>
> When emitting IR NEWREF, there is no check for a non-NaN stored key
> value. Thus, when the NaN number value is given to trace, it may be
> stored as a key. This patch adds the corresponding check. If fold
> optimization is enabled, this IR EQ check is dropped if it references
> CONV IR from any (unsigned) integer type since NaN can be created via
> conversion from an integer.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9145
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1069-newref-nan-key
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9374
> Fuzzer link: https://oss-fuzz.com/testcase-detail/5251574662037504
Updated https://github.com/tarantool/tarantool/wiki/Fuzzing
> Relate issues:
> * https://github.com/LuaJIT/LuaJIT/issues/1069
> * https://github.com/tarantool/tarantool/issues/9145
>
>   src/lj_opt_fold.c                             |   5 +-
>   src/lj_record.c                               |  12 +-
>   .../lj-1069-newref-nan-key.test.lua           | 151 ++++++++++++++++++
>   3 files changed, 164 insertions(+), 4 deletions(-)
>   create mode 100644 test/tarantool-tests/lj-1069-newref-nan-key.test.lua
>
<snipped>
> index 00000000..ec28b274
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
> @@ -0,0 +1,151 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate LuaJIT misbehaviour for NEWREF IR
> +-- taken NaN value.
> +-- See also, https://github.com/LuaJIT/LuaJIT/issues/1069.
> +
> +local test = tap.test('lj-1069-newref-nan-key'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +local table_new = require('table.new')
> +local ffi = require('ffi')
> +
> +local NaN = tonumber('nan')
> +
> +test:plan(4)
> +
> +test:test('NaN on trace in the non-constant IR', function(subtest)
> +  local NKEYS = 3
> +
> +  -- XXX: NaN isn't stored, so the number of tests is:
> +  -- (NKEYS - 1) + test status + test error message + test keys
> +  -- amount.
> +  subtest:plan(NKEYS + 2)
> +
> +  local tset_table = table_new(0, NKEYS)
> +
> +  local function tset(t, k)
> +    -- Value doesn't matter.
> +    t[k] = true
> +  end
> +
> +  -- Compile the function.
> +  jit.opt.start('hotloop=1')
> +
> +  -- Use number keys to emit NEWREF.
> +  tset(tset_table, 0.1)
> +  tset(tset_table, 0.2)
> +
> +  -- Insert NaN on the trace.
> +  local ok, err = pcall(tset, tset_table, NaN)
> +
> +  subtest:ok(not ok, 'function returns an error')
> +  subtest:like(err, 'table index is NaN', 'correct error message')
> +
> +  local nkeys = 0
> +  for k in pairs(tset_table) do
> +    nkeys = nkeys + 1
> +    subtest:ok(k == k, ('not NaN key by number %d'):format(nkeys))
> +  end
> +  subtest:is(nkeys, NKEYS - 1, 'correct amount of keys')
> +end)
> +
> +test:test('NaN on trace in the non-constant IR CONV', function(subtest)
> +  local tonumber = tonumber
What for do you need this?
> +  local NKEYS = 3
> +
> +  -- XXX: NaN isn't stored, so the number of tests is:
> +  -- (NKEYS - 1) + test status + test error message + test keys
> +  -- amount.
> +  subtest:plan(NKEYS + 2)

BTW TAP reports "bad plan" with reverted fix due to a reason described 
in the comment above:

<snipped>

not ok - bad plan
   planned: 5
   run:  6

<snipped>

Nothing to do, just an observation.


<snipped>

Re: [Tarantool-patches] [PATCH luajit] Add NaN check to IR_NEWREF.

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the review!
Fixed your comment with question and force-pushed the branch.

On 17.11.23, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a single nit and a question below.
> On Thu, Nov 16, 2023 at 11:49:59AM +0300, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Thanks to Peter Cawley.
> >
> > (cherry-picked from commit 7f9907b4ed0870ba64342bcc4b26cff0a94540da)
> >
> > When emitting IR NEWREF, there is no check for a non-NaN stored key
> > value. Thus, when the NaN number value is given to trace, it may be
> > stored as a key. This patch adds the corresponding check. If fold
> > optimization is enabled, this IR EQ check is dropped if it references
> > CONV IR from any (unsigned) integer type since NaN can be created via
> > conversion from an integer.
> >
> > Sergey Kaplun:
> > * added the description and the test for the problem
> >
> > Part of tarantool/tarantool#9145
> > ---
> >
> > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1069-newref-nan-key
> > Tarantool PR: https://github.com/tarantool/tarantool/pull/9374
> > Fuzzer link: https://oss-fuzz.com/testcase-detail/5251574662037504
> > Relate issues:
> > * https://github.com/LuaJIT/LuaJIT/issues/1069
> > * https://github.com/tarantool/tarantool/issues/9145
> >
> >  src/lj_opt_fold.c                             |   5 +-
> >  src/lj_record.c                               |  12 +-
> >  .../lj-1069-newref-nan-key.test.lua           | 151 ++++++++++++++++++
> >  3 files changed, 164 insertions(+), 4 deletions(-)
> >  create mode 100644 test/tarantool-tests/lj-1069-newref-nan-key.test.lua
> 
> <snipped>
> 
> > diff --git a/test/tarantool-tests/lj-1069-newref-nan-key.test.lua b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
> > new file mode 100644
> > index 00000000..ec28b274
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
> > @@ -0,0 +1,151 @@
> 
> > +-- Test the constant IR NaN value on the trace.
> Nit: This comment seems a bit redundant, as it duplicates the test name. Feel
> free to ignore.

It's also related to the second test. It's just like the section header
for both of the tests. I prefer to leave it as is.

> > +test:test('constant NaN on the trace', function(subtest)
> > +  -- Test the status and the error message.
> > +  subtest:plan(2)
> > +  local function protected()
> > +    local counter = 0
> > +    -- Use a number key to emit NEWREF.
> > +    local key = 0.1
> > +
> > +    jit.opt.start('hotloop=1')
> > +
> > +    while counter < 2 do
> > +      counter = counter + 1
> > +      -- luacheck: ignore
> > +      local tab = {_ = 'unused'}
> > +      tab[key] = 'NaN'
> > +      -- XXX: Use the constant NaN value on the trace.
> > +      key = 0/0
> > +    end
> > +  end
> > +
> > +  local ok, err = pcall(protected)
> > +  subtest:ok(not ok, 'function returns an error')
> > +  subtest:like(err, 'table index is NaN', 'NaN index error message')
> > +end)
> > +
> > +test:test('constant NaN on the trace and rehash of the table', function(subtest)
> > +  -- A little bit different test case: after rehashing the table,
> > +  -- the node is lost, and a hash part of the table isn't freed.
> > +  -- This leads to the assertion failure, which checks memory
> > +  -- leaks on shutdown.
> > +  -- XXX: The test didn't fail even before the patch. Check the
> > +  -- same values as in the previous test for consistency.
> What do you mean by "didn't fail"? AFAICS, it leads to the assertion failure.
> If you've meant no fails for a build with no assertions, then it worth
> clarifying it in this comment.

I mean that the checks below (`subtest:ok`, etc.) aren't fail.
I've rephrased as the following:

===================================================================
diff --git a/test/tarantool-tests/lj-1069-newref-nan-key.test.lua b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
index ec28b274..22553423 100644
--- a/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
+++ b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
@@ -121,8 +121,8 @@ test:test('constant NaN on the trace and rehash of the table', function(subtest)
   -- the node is lost, and a hash part of the table isn't freed.
   -- This leads to the assertion failure, which checks memory
   -- leaks on shutdown.
-  -- XXX: The test didn't fail even before the patch. Check the
-  -- same values as in the previous test for consistency.
+  -- XXX: The test checks didn't fail even before the patch. Check
+  -- the same values as in the previous test for consistency.
   subtest:plan(2)
   local function protected()
     local counter = 0
===================================================================

Branch is force-pushed.

> 
> <snipped>

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Add NaN check to IR_NEWREF.

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the review!
Fixed your comment and force-pushed the branch.

On 18.11.23, Sergey Bronnikov wrote:
> Hello, Sergey,
> 
> 
> thanks for the patch. LGTM
> 
> See my comments below.
> 
> 
> On 11/16/23 11:49, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Thanks to Peter Cawley.
> >
> > (cherry-picked from commit 7f9907b4ed0870ba64342bcc4b26cff0a94540da)
> >
> > When emitting IR NEWREF, there is no check for a non-NaN stored key
> > value. Thus, when the NaN number value is given to trace, it may be
> > stored as a key. This patch adds the corresponding check. If fold
> > optimization is enabled, this IR EQ check is dropped if it references
> > CONV IR from any (unsigned) integer type since NaN can be created via
> > conversion from an integer.
> >
> > Sergey Kaplun:
> > * added the description and the test for the problem
> >
> > Part of tarantool/tarantool#9145
> > ---
> >
> > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1069-newref-nan-key
> > Tarantool PR: https://github.com/tarantool/tarantool/pull/9374
> > Fuzzer link: https://oss-fuzz.com/testcase-detail/5251574662037504
> Updated https://github.com/tarantool/tarantool/wiki/Fuzzing

Thanks!

> > Relate issues:
> > * https://github.com/LuaJIT/LuaJIT/issues/1069
> > * https://github.com/tarantool/tarantool/issues/9145
> >
> >   src/lj_opt_fold.c                             |   5 +-
> >   src/lj_record.c                               |  12 +-
> >   .../lj-1069-newref-nan-key.test.lua           | 151 ++++++++++++++++++
> >   3 files changed, 164 insertions(+), 4 deletions(-)
> >   create mode 100644 test/tarantool-tests/lj-1069-newref-nan-key.test.lua
> >

<snipped>

> > +
> > +  local tset_table = table_new(0, NKEYS)
> > +
> > +  local function tset(t, k)
> > +    -- Value doesn't matter.
> > +    t[k] = true
> > +  end
> > +
> > +  -- Compile the function.
> > +  jit.opt.start('hotloop=1')
> > +
> > +  -- Use number keys to emit NEWREF.
> > +  tset(tset_table, 0.1)
> > +  tset(tset_table, 0.2)
> > +
> > +  -- Insert NaN on the trace.
> > +  local ok, err = pcall(tset, tset_table, NaN)
> > +
> > +  subtest:ok(not ok, 'function returns an error')
> > +  subtest:like(err, 'table index is NaN', 'correct error message')
> > +
> > +  local nkeys = 0
> > +  for k in pairs(tset_table) do
> > +    nkeys = nkeys + 1
> > +    subtest:ok(k == k, ('not NaN key by number %d'):format(nkeys))
> > +  end
> > +  subtest:is(nkeys, NKEYS - 1, 'correct amount of keys')
> > +end)
> > +
> > +test:test('NaN on trace in the non-constant IR CONV', function(subtest)
> > +  local tonumber = tonumber
> What for do you need this?

Added the following comment for clarification:

===================================================================
diff --git a/test/tarantool-tests/lj-1069-newref-nan-key.test.lua b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
index 22553423..67b4f5b3 100644
--- a/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
+++ b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua
@@ -52,7 +52,9 @@ test:test('NaN on trace in the non-constant IR', function(subtest)
 end)
 
 test:test('NaN on trace in the non-constant IR CONV', function(subtest)
+  -- XXX: simplify `jit.dump()` output.
   local tonumber = tonumber
+
   local NKEYS = 3
 
   -- XXX: NaN isn't stored, so the number of tests is:
===================================================================

Branch is force-pushed.

> > +  local NKEYS = 3
> > +

<snipped>

> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Add NaN check to IR_NEWREF.

[Sergey Bronnikov via Tarantool-patches]

On 11/20/23 11:51, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Fixed your comment and force-pushed the branch.
Thanks for the fixes! LGTM
>
> On 18.11.23, Sergey Bronnikov wrote:
> <snipped>

Re: [Tarantool-patches] [PATCH luajit] Add NaN check to IR_NEWREF.

[Igor Munkin via Tarantool-patches]

Sergey,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/2.11 and
release/2.10.

On 16.11.23, Sergey Kaplun via Tarantool-patches wrote:
> From: Mike Pall <mike>
> 
> Thanks to Peter Cawley.
> 
> (cherry-picked from commit 7f9907b4ed0870ba64342bcc4b26cff0a94540da)
> 
> When emitting IR NEWREF, there is no check for a non-NaN stored key
> value. Thus, when the NaN number value is given to trace, it may be
> stored as a key. This patch adds the corresponding check. If fold
> optimization is enabled, this IR EQ check is dropped if it references
> CONV IR from any (unsigned) integer type since NaN can be created via
> conversion from an integer.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#9145
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1069-newref-nan-key
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9374
> Fuzzer link: https://oss-fuzz.com/testcase-detail/5251574662037504
> Relate issues:
> * https://github.com/LuaJIT/LuaJIT/issues/1069
> * https://github.com/tarantool/tarantool/issues/9145
> 
>  src/lj_opt_fold.c                             |   5 +-
>  src/lj_record.c                               |  12 +-
>  .../lj-1069-newref-nan-key.test.lua           | 151 ++++++++++++++++++
>  3 files changed, 164 insertions(+), 4 deletions(-)
>  create mode 100644 test/tarantool-tests/lj-1069-newref-nan-key.test.lua
> 

<snipped>

> -- 
> 2.42.0
> 

-- 
Best regards,
IM