Hi, Sergey,

fixed and force-pushed.

Sergey

On 10.12.2024 12:32, Sergey Kaplun wrote:
Hi, Sergey!
Thanks for the fixes!
LGTM, after fixing a bunch of comments below.

diff --git a/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua b/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua
new file mode 100644
index 00000000..dc01307e
--- /dev/null
+++ b/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua
@@ -0,0 +1,74 @@
+local tap = require('tap')
+-- A test file to demonstrate an incorrect recording of
+-- `getmetatable()` for I/O handlers.
+-- https://github.com/LuaJIT/LuaJIT/issues/1279
+local test = tap.test('lj-1279-incorrect-recording-getmetatable'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(6)
+
+jit.opt.start('hotloop=1')
+
+local ud_io_file = io.stdout
+local getmetatable = getmetatable
+
+local function rec_getmetatable(obj)
+  local res
+  for _ = 1, 4 do
+    res = getmetatable(obj)
+  end
+  return res
+end
+
+-- The testcase to demonstrate a problem by comparing metatable
Typo: s/metatable/the metatable/
--- a/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua
+++ b/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua
@@ -21,8 +21,9 @@ local function rec_getmetatable(obj)
   return res
 end
 
--- The testcase to demonstrate a problem by comparing metatable
--- returned by two versions of `getmetatable()`: compiled and not.
+-- The testcase to demonstrate a problem by comparing the
+-- metatable returned by two versions of `getmetatable()`:
+-- compiled and not.
 
 local mt_orig = debug.getmetatable(ud_io_file)
 assert(type(mt_orig) == 'table')

+-- returned by two versions of `getmetatable()`: compiled and not.
+
+local mt_orig = debug.getmetatable(ud_io_file)
+assert(type(mt_orig) == 'table')
+
+local mt_rec = {}
+for i = 1, 4 do
+  mt_rec[i] = getmetatable(ud_io_file)
+end
+mt_rec[5] = mt_orig
+
+test:ok(true, 'getmetatable() recording is correct')
+test:samevalues(mt_rec, 'metatables are the same')
+
+-- Restore metatable.
+debug.setmetatable(ud_io_file, mt_orig)
+assert(type(mt_orig) == 'table')
This restoring is excess since we don't change the metatable yet.

Ok, updated:

@@ -36,17 +37,13 @@ mt_rec[5] = mt_orig
 test:ok(true, 'getmetatable() recording is correct')
 test:samevalues(mt_rec, 'metatables are the same')
 
--- Restore metatable.
-debug.setmetatable(ud_io_file, mt_orig)
-assert(type(mt_orig) == 'table')
-


+
+-- The testcase to demonstrate a problem by setting metatable for
Typo: s/metatable/the metatable/

Fixed:


@@ -40,8 +41,8 @@ test:samevalues(mt_rec, 'metatables are the same')
 debug.setmetatable(ud_io_file, mt_orig)
 assert(type(mt_orig) == 'table')
 
--- The testcase to demonstrate a problem by setting metatable for
--- `io.stdout` to a string.
+-- The testcase to demonstrate a problem by setting the metatable
+-- for `io.stdout` to a string.
 
 -- Compile `getmetatable()`, it is expected metatable has
 -- a `table` type.

+-- `io.stdout` to a string.
+
+-- Compile `getmetatable()`, it is expected metatable has
+-- a `table` type.
+rec_getmetatable(ud_io_file)
+-- Set a custom metatable to a string.
Minor: I would rephrase this like the following:
| Set IO metatable to a string.
"Custom" isn't clear for me.

Fixed:

 -- Compile `getmetatable()`, it is expected metatable has
 -- a `table` type.
 rec_getmetatable(ud_io_file)
--- Set a custom metatable to a string.
+-- Set IO metatable to a string.
 local mt = 'IO metatable'
 getmetatable(ud_io_file).__metatable = mt
 test:is(getmetatable(ud_io_file), mt, 'getmetatable() is correct')


      
+local mt = 'IO metatable'
+getmetatable(ud_io_file).__metatable = mt
+test:is(getmetatable(ud_io_file), mt, 'getmetatable() is correct')
+test:is(rec_getmetatable(ud_io_file), mt, 'compiled getmetatable() is correct')
+
+-- Restore metatable.
+debug.setmetatable(ud_io_file, mt_orig)
+assert(type(mt_orig) == 'table')
It is better also to restore the JIT state here, ie. call `jit.flush()`
and reset hotcounters via `jit.opt.start('hotloop=1')` to be in sync
with the code (we want to compile a trace again).

Added:


@@ -55,9 +56,11 @@ test:is(rec_getmetatable(ud_io_file), mt, 'compiled getmetatable() is correct')
 -- Restore metatable.
 debug.setmetatable(ud_io_file, mt_orig)
 assert(type(mt_orig) == 'table')
+jit.flush()
+jit.opt.start('hotloop=1')
 


      
+
+-- The testcase to demonstrate a problem by removing metatable for
Typo: s/metatable/the metatable/

Fixed:


@@ -56,8 +57,8 @@ test:is(rec_getmetatable(ud_io_file), mt, 'compiled getmetatable() is correct')
 debug.setmetatable(ud_io_file, mt_orig)
 assert(type(mt_orig) == 'table')
 
--- The testcase to demonstrate a problem by removing metatable for
--- `io.stdout` and calling garbage collector.
+-- The testcase to demonstrate a problem by removing the metatable
+-- for `io.stdout` and calling garbage collector.
 
 -- Compile `getmetatable()`, it is expected metatable has
 -- a `table` type.

      
+-- `io.stdout` and calling garbage collector.
Typo: s/garbage collector/the garbage collector/

Fixed:


@@ -56,8 +57,8 @@ test:is(rec_getmetatable(ud_io_file), mt, 'compiled getmetatable() is correct')
 debug.setmetatable(ud_io_file, mt_orig)
 assert(type(mt_orig) == 'table')
 
--- The testcase to demonstrate a problem by removing metatable for
--- `io.stdout` and calling garbage collector.
+-- The testcase to demonstrate a problem by removing the metatable
+-- for `io.stdout` and calling the garbage collector.
 
 -- Compile `getmetatable()`, it is expected metatable has
 -- a `table` type.

+
+-- Compile `getmetatable()`, it is expected metatable has
+-- a `table` type.
+rec_getmetatable(ud_io_file)
+-- Delete metatable.
+debug.setmetatable(ud_io_file, nil)
+collectgarbage()
+test:is(getmetatable(ud_io_file), nil, 'getmetatable() is correct')
+test:is(rec_getmetatable(ud_io_file), nil, 'compiled getmetatable() is correct')
+
+-- Restore metatable.
+debug.setmetatable(ud_io_file, mt_orig)
+
+test:done(true)
On 22.11.24, Sergey Bronnikov wrote:
Hi, Sergey,

please see below.

Updated version has been force-pushed.

Sergey

<snipped>


          
[1]:https://github.com/LuaJIT/LuaJIT/issues/1279#issuecomment-2382392847