Re: [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Sergey,

On 14.08.21, Sergey Kaplun wrote:
> Hi, Maxim!
> 
> Thanks for the fixes!
> 
> LGTM, except two nitpicks below.
> 
> On 14.08.21, Maxim Kokryashkin wrote:

<snipped>

> > diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
> > index c50405ad..d4882dd7 100644
> > --- a/src/luajit-gdb.py
> > +++ b/src/luajit-gdb.py
> > @@ -238,8 +238,11 @@ def jit_state(g):
> >          0x15: 'ERR',
> >      }.get(int(J(g)['state']), 'INVALID')
> >  
> > +def tvisint(o):
> > +    return LJ_DUALNUM and itype(o) == LJ_TISNUM
> > +
> >  def tvisnumber(o):
> > -    return itype(o) <= (0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX'])
> > +    return itype(o) <= LJ_TISNUM
> 
> Strictly saying, it is `<`, because `==` is staying for integer
> representation.

At first, I believed you're right, but unfortunately, you're wrong.
Consider the following:
| $ cmake . -DLUAJIT_NUMMODE=2 -DCMAKE_BUILD_TYPE=Debug
| <snipped> # Manually enable LJ_DUALNUM for x64
| $ make -j
| <snipped>
| $ cd src
| $ gdb --args ./luajit -e 'print(1)'
| <snipped>
| Reading symbols from ./luajit...
| lj-arch command initialized
| lj-tv command initialized
| lj-str command initialized
| lj-tab command initialized
| lj-stack command initialized
| lj-state command initialized
| lj-gc command initialized
| luajit-gdb.py is successfully loaded
| (gdb) b lj_cf_print
| Breakpoint 1 at 0x2960a: file /luajit/src/lib_base.c, line 485.
| (gdb) r
| Starting program: /luajit/src/luajit -e print\(1\)
| 
| Breakpoint 1, lj_cf_print (L=0x0) at /luajit/src/lib_base.c:485
| 485     {
| (gdb) n
| 486       ptrdiff_t i, nargs = L->top - L->base;
| (gdb) lj-stack L
| 0x40001a50:0x40001a70 [    ] 5 slots: Red zone
| 0x40001a48            [   M]
| 0x40001958:0x40001a40 [    ] 30 slots: Free stack slots
| 0x40001950            [  T ]
| 0x40001948            [ B  ] VALUE: not valid type @ 0x1
| 0x40001940            [    ] FRAME: [L] delta=1, fast function #29
| 0x40001938            [    ] FRAME: [V] delta=1, Lua function @ 0x40008428, 0 upvalues, "=(command line)":0
| 0x40001930            [    ] FRAME: [CP] delta=3, Lua function @ 0x40008428, 0 upvalues, "=(command line)":0
| 0x40001928            [    ] VALUE: C function @ 0x55555555c945
| 0x40001920            [    ] VALUE: light userdata @ 0x0
| 0x40001918            [    ] FRAME: [CP] delta=1, C function @ 0x55555555def8
| 0x40001910            [S   ] FRAME: dummy L

There is a "not valid type" stub, since < is used instead of <=. I
reverted this change and here is the result:
| $ git diff
| diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
| index f7347cf3..4488775c 100644
| --- a/src/luajit-gdb.py
| +++ b/src/luajit-gdb.py
| @@ -243,7 +243,7 @@ def tvisint(o):
|      return LJ_DUALNUM and itype(o) == LJ_TISNUM
|  
|  def tvisnumber(o):
| -    return itype(o) < LJ_TISNUM
| +    return itype(o) <= LJ_TISNUM
|  
|  def tvislightud(o):
|      if LJ_64 and not LJ_GC64:
| $ cmake . -DLUAJIT_NUMMODE=2 -DCMAKE_BUILD_TYPE=Debug
| <snipped> # Manually enable LJ_DUALNUM for x64
| $ make -j
| <snipped>
| $ cd src
| $ gdb --args ./luajit -e 'print(1)'
| <snipped>
| Reading symbols from ./luajit...
| lj-arch command initialized
| lj-tv command initialized
| lj-str command initialized
| lj-tab command initialized
| lj-stack command initialized
| lj-state command initialized
| lj-gc command initialized
| luajit-gdb.py is successfully loaded
| (gdb) b lj_cf_print
| Breakpoint 1 at 0x2960a: file /luajit/src/lib_base.c, line 485.
| (gdb) r
| Starting program: /luajit/src/luajit -e print\(1\)
| 
| Breakpoint 1, lj_cf_print (L=0x0) at /luajit/src/lib_base.c:485
| 485     {
| (gdb) n
| 486       ptrdiff_t i, nargs = L->top - L->base;
| (gdb) lj-stack L
| 0x40001a50:0x40001a70 [    ] 5 slots: Red zone
| 0x40001a48            [   M]
| 0x40001958:0x40001a40 [    ] 30 slots: Free stack slots
| 0x40001950            [  T ]
| 0x40001948            [ B  ] VALUE: integer 1
| 0x40001940            [    ] FRAME: [L] delta=1, fast function #29
| 0x40001938            [    ] FRAME: [V] delta=1, Lua function @ 0x40008428, 0 upvalues, "=(command line)":0
| 0x40001930            [    ] FRAME: [CP] delta=3, Lua function @ 0x40008428, 0 upvalues, "=(command line)":0
| 0x40001928            [    ] VALUE: C function @ 0x55555555c945
| 0x40001920            [    ] VALUE: light userdata @ 0x0
| 0x40001918            [    ] FRAME: [CP] delta=1, C function @ 0x55555555def8
| 0x40001910            [S   ] FRAME: dummy L

Hence, I dropped this change from the resulting patch.

> 

<snipped>

> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM