[Tarantool-patches] [PATCH luajit] FFI: Add tonumber() specialization for failed conversions.
Sergey Kaplun
skaplun at tarantool.org
Wed Sep 28 10:37:24 MSK 2022
Hi, Sergos!
On 26.09.22, sergos wrote:
> Since we fallback to interpreter it looks safe to keep the test for GC64.
> Still you’d better mention it in commit message.
Strictly saying, this problem still exists for GC64 mode if we get the
cdata argument to record via loading from a table, for example.
I didn't modify the recorded function `check()` to keep the test case clear.
I just added the following comment behind.
===================================================================
diff --git a/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua b/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua
index 1c175de1..bf9e8e46 100644
--- a/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua
+++ b/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua
@@ -14,6 +14,15 @@ local NULL = ffi.cast('void *', 0)
test:plan(4)
+-- This test won't fail for GC64 on x86_64. This happens due to
+-- wrong instruction emitting for SLOAD IR -- we always exit by
+-- the assertion guard on the argument type check. See also
+-- https://github.com/LuaJIT/LuaJIT/pull/350.
+-- The test fails without fix in the current commit, if the
+-- following commit is backported:
+-- https://github.com/LuaJIT/LuaJIT/commit/05fbdf56
+-- Feel free to remove this comment after backporting of the
+-- aforementioned commit.
local function check(x)
-- Don't use a tail call to avoid "leaving loop in root trace"
-- error, so the trace will be compiled.
===================================================================
The branch is force-pushed.
>
> LGTM with above.
>
> Sergos
>
>
> > On 22 Sep 2022, at 14:28, Sergey Kaplun <skaplun at tarantool.org> wrote:
> >
> > Hi, Sergos!
> >
> > On 21.09.22, sergos wrote:
> >> Hi!
> >>
> >> I see four ‘ok’s as a result of run.
> >> See the full output for the run with dump() below:
> >>
> >>
> >> s.ostanevich at s-ostanevich2:~/workspaces/t.sergos/third_party/luajit/test/tarantool-tests % ../../../../build-debug/src/tarantool -e 'require"jit.dump".start("ib")' lj-408-tonumber-cdata-record.test.lua
> >>
> >> TAP version 13
> >> 1..4
> >> ---- TRACE 1 start lj-408-tonumber-cdata-record.test.lua:17
> >> 0001 GGET 1 0 ; "tonumber"
> >> 0002 MOV 3 0
> >> 0003 CALL 1 2 2
> >> 0000 . FUNCC ; tonumber
> >> 0004 RET1 1 2
> >> ---- TRACE 1 IR
> >> 0001 fun SLOAD #0 R
> >> 0002 tab FLOAD 0001 func.env
> >> 0003 int FLOAD 0002 tab.hmask
> >> 0004 > int EQ 0003 +63
> >> 0005 p64 FLOAD 0002 tab.node
> >> 0006 > p64 HREFK 0005 "tonumber" @8
> >> 0007 > fun HLOAD 0006
> >> 0008 > cdt SLOAD #2 T
> >> 0009 > fun EQ 0007 tonumber
> >> ---- TRACE 1 stop -> return
> >
> > Gotcha!
> > It's a funny thing:
> > This is happening due to GC64 mode on x86_x64.
> >
> > There is an invalid type check for SLOAD IR with the following emitted
> > mcode:
> >
> > | 55557f6bffc9 mov rdi, [rdx+0x4]
> > | 55557f6bffcd sar rdi, 0x2f
> > | 55557f6bffd1 cmp edi, -0x0b
> > | 55557f6bffd4 jnz 0x55557f6b0010 ->0
> >
> > This 0x4 addiction is crucial:
> > We got the invalid irtype value to compare (due wrong addressing) -- so
> > the assertion guard is always failed and we always exit from the trace.
> > This is why we fallback into the interpreter and the test is passed.
> >
> > In LuaJIT upstream this issue fixed via the following commit:
> > https://github.com/LuaJIT/LuaJIT/commit/05fbdf56 <https://github.com/LuaJIT/LuaJIT/commit/05fbdf56>
> >
> > See also https://github.com/LuaJIT/LuaJIT/pull/350 <https://github.com/LuaJIT/LuaJIT/pull/350>
> >
> > <snipped>
> >
> >>
> >
> > --
> > Best regards,
> > Sergey Kaplun
>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list