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

["Максим Корякшин via Tarantool-patches" <tarantool-patches@dev.tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 3815 bytes --]


Hi, Sergey!
Thanks for the comments!
 
Here is the fix:
=========================================
diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index b656a808..9ccca66a 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:
@@ -348,7 +348,7 @@ def dump_lj_tudata(tv):
 def dump_lj_tnumx(tv):
     if tvisint(tv):
-        return 'number {}'.format(cast('int32_t', tv['i']))
+        return 'integer {}'.format(cast('int32_t', tv['i']))
     else:
         return 'number {}'.format(cast('double', tv['n']))
=========================================
  
>Суббота, 14 августа 2021, 16:48 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:
> 
>Hi, Maxim!
>
>Thanks for the fixes!
>
>LGTM, except two nitpicks below.
>
>On 14.08.21, Maxim Kokryashkin wrote:
>> luajit-gdb.py displays integers in LJ_DUALNUM mode as nan-s. The
>> dumper function produces output thinking of any input value as of a
>> double. However, in DUALNUM mode, integers and doubles are stored
>> differently, so the `itype` of a float number must be less than
>> `LJ_TISNUM`, and the `itype` of an integer must be `LJ_TISNUM`. With
>> this fact in mind, we can easily differentiate one from another.
>>
>> Closes tarantool/tarantool#6224
>> ---
>> Changes in v2:
>> - Fixed comments as per review by Sergey
>>
>> src/luajit-gdb.py | 14 +++++++++++---
>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> 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.
>>
>> def tvislightud(o):
>> if LJ_64 and not LJ_GC64:
>> @@ -343,7 +346,10 @@ def dump_lj_tudata(tv):
>> return 'userdata @ {}'.format(strx64(gcval(tv['gcr'])))
>>
>> def dump_lj_tnumx(tv):
>> - return 'number {}'.format(cast('double', tv['n']))
>> + if tvisint(tv):
>> + return 'number {}'.format(cast('int32_t', tv['i']))
>
>Nit: Lets change it to 'integer {}', as far as it is an integer value in
>the internal LuaJIT representation.
>
>> + else:
>> + return 'number {}'.format(cast('double', tv['n']))
>>
>> def dump_lj_invalid(tv):
>> return 'not valid type @ {}'.format(strx64(gcval(tv['gcr'])))
>> @@ -683,7 +689,7 @@ The command requires no args and dumps current GC stats:
>> ))
>>
>> def init(commands):
>> - global LJ_64, LJ_GC64, LJ_FR2, PADDING
>> + global LJ_64, LJ_GC64, LJ_DUALNUM, LJ_TISNUM, LJ_FR2, PADDING
>>
>> # XXX Fragile: though connecting the callback looks like a crap but it
>> # respects both Python 2 and Python 3 (see #4828).
>> @@ -724,6 +730,7 @@ def init(commands):
>> try:
>> LJ_64 = str(gdb.parse_and_eval('IRT_PTR')) == 'IRT_P64'
>> LJ_FR2 = LJ_GC64 = str(gdb.parse_and_eval('IRT_PGC')) == 'IRT_P64'
>> + LJ_DUALNUM = lookup('lj_lib_checknumber') is not None
>> except:
>> gdb.write('luajit-gdb.py failed to load: '
>> 'no debugging symbols found for libluajit\n')
>> @@ -733,6 +740,7 @@ def init(commands):
>> command(name)
>>
>> PADDING = ' ' * len(':' + hex((1 << (47 if LJ_GC64 else 32)) - 1))
>> + LJ_TISNUM = 0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX']
>>
>> gdb.write('luajit-gdb.py is successfully loaded\n')
>>
>> --
>> 2.32.0
>>
>
>--
>Best regards,
>Sergey Kaplun
 

[-- Attachment #2: Type: text/html, Size: 4788 bytes --]