Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: sergos <sergos@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit] FFI: Add tonumber() specialization for failed conversions.
Date: Thu, 22 Sep 2022 14:28:11 +0300	[thread overview]
Message-ID: <YyxGy4TDF3BzoBo9@root> (raw)
In-Reply-To: <7378A34A-6A2D-4634-B7A0-C9AA33267042@tarantool.org>

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@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

See also https://github.com/LuaJIT/LuaJIT/pull/350

<snipped>

> 

-- 
Best regards,
Sergey Kaplun

  reply	other threads:[~2022-09-22 11:30 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-12  8:01 Sergey Kaplun via Tarantool-patches
2022-09-19  7:50 ` Maxim Kokryashkin via Tarantool-patches
2022-09-20  8:53   ` sergos via Tarantool-patches
2022-09-20  9:58     ` Maxim Kokryashkin via Tarantool-patches
2022-09-20 10:10       ` sergos via Tarantool-patches
2022-09-21 12:03         ` Sergey Kaplun via Tarantool-patches
2022-09-21 12:19           ` sergos via Tarantool-patches
2022-09-22 11:28             ` Sergey Kaplun via Tarantool-patches [this message]
2022-09-25 21:37               ` sergos via Tarantool-patches
2022-09-28  7:37                 ` Sergey Kaplun via Tarantool-patches
2022-09-24 14:49   ` Sergey Kaplun via Tarantool-patches
2022-11-11  8:54 ` Igor Munkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YyxGy4TDF3BzoBo9@root \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=sergos@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit] FFI: Add tonumber() specialization for failed conversions.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox