* [Tarantool-patches] [PATCH luajit] luajit-gdb: support dualnum mode
@ 2021-07-31 13:36 Maxim Kokryashkin via Tarantool-patches
2021-08-11 8:27 ` Sergey Kaplun via Tarantool-patches
2021-08-17 9:23 ` [Tarantool-patches] [PATCH luajit] " Igor Munkin via Tarantool-patches
0 siblings, 2 replies; 13+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2021-07-31 13:36 UTC (permalink / raw)
To: tarantool-patches, imun, skaplun
For x86/x64 LJ_DUALNUM mode is disabled. But for some other arches
(arm or arm64, for example) it is enabled by default. luajit-gdb.py
displays integers in LJ_DUALNUM mode as nan-s.
As it turned out, luajit-gdb detects those integers as integers, but
there was a problem with the dumper function itself. The dumper
function produces output thinking of any input value as of a double.
However, in DUALNUM mode, integers and floats 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.
But in any mode, lua numbers are stored as doubles on the C side, so it
takes an ugly cast chain on the Python side to perform the some sort of
the `reinterpret_cast` because the gdb module doesn't have any
mechanism to get the address of a symbol.
Closes tarantool/tarantool#6224
---
Github branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-6224-support-dulanum
Issue: https://github.com/tarantool/tarantool/issues/6224
For more info, see line 273 in lj_obj.h
src/luajit-gdb.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index c50405ad..5f79c277 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -343,7 +343,11 @@ 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 itype(tv) == (0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX']):
+ integer = cast('int32_t', cast('uint64_t', cast('void*', tv['n'])) & 0xFFFFFFFF)
+ return 'number {}'.format(integer)
+ else:
+ return 'number {}'.format(cast('double', tv['n']))
def dump_lj_invalid(tv):
return 'not valid type @ {}'.format(strx64(gcval(tv['gcr'])))
--
2.32.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] luajit-gdb: support dualnum mode
2021-07-31 13:36 [Tarantool-patches] [PATCH luajit] luajit-gdb: support dualnum mode Maxim Kokryashkin via Tarantool-patches
@ 2021-08-11 8:27 ` Sergey Kaplun via Tarantool-patches
2021-08-13 14:29 ` Максим Корякшин via Tarantool-patches
2021-08-17 9:23 ` [Tarantool-patches] [PATCH luajit] " Igor Munkin via Tarantool-patches
1 sibling, 1 reply; 13+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2021-08-11 8:27 UTC (permalink / raw)
To: Maxim Kokryashkin; +Cc: tarantool-patches
Hi, Maxim!
Thanks for the patch!
Please consider my comments below.
Side note: First of all, I'm very disappointed, that there this patch
[1] isn't merged (in any form) into gdb. Those work with the expanding
of macros is very helpful...
On 31.07.21, Maxim Kokryashkin wrote:
> For x86/x64 LJ_DUALNUM mode is disabled. But for some other arches
Nit: It can be enabled by corresponding configuration options for
x86/x64, too, IINM. So I suggest to drop the first and the second
sentence.
> (arm or arm64, for example) it is enabled by default. luajit-gdb.py
> displays integers in LJ_DUALNUM mode as nan-s.
>
Nit: This paragraph may be joined to the next after suggesting changes.
> As it turned out, luajit-gdb detects those integers as integers, but
> there was a problem with the dumper function itself.
Nit: The next sentence is about the problem with the dumping function, so
I suggest to drop this opening sentence:)
Feel free to ignore.
> The dumper
> function produces output thinking of any input value as of a double.
> However, in DUALNUM mode, integers and floats are stored differently,
Typo: s/floats/doubles/
Here and below.
> 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.
>
> But in any mode, lua numbers are stored as doubles on the C side, so it
Typo: s/lua/Lua/
Do you mean LuaJIT here? Because this is not true for LuaJIT. See
<lj_obj.h> for details.
> takes an ugly cast chain on the Python side to perform the some sort of
> the `reinterpret_cast` because the gdb module doesn't have any
> mechanism to get the address of a symbol.
This sentence isn't clear to me. What symbol do you mean?
>
> Closes tarantool/tarantool#6224
Side note: You can say that it really closes the issue, because we use
luajit-gdb from this fork. OTOH, maybe one uses it as a part of
third_party from Tarantool repo.
"Closes" is good to me, but I'm not sure what is idiomatically correct.
> ---
> Github branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-6224-support-dulanum
> Issue: https://github.com/tarantool/tarantool/issues/6224
> For more info, see line 273 in lj_obj.h
>
> src/luajit-gdb.py | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
> index c50405ad..5f79c277 100644
> --- a/src/luajit-gdb.py
> +++ b/src/luajit-gdb.py
> @@ -343,7 +343,11 @@ 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 itype(tv) == (0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX']):
This is true only in LJ_DUALNUM mode. So I suggest we can add another
one global constant for this: LJ_DUALNUM. We can set it via hack with
lookuping symbol `lj_lib_checknumber` -- it is compiled only for
LJ_DUALNUM mode for now. So, if a result of lookup() isn't None we are
in LJ_DUALNUM mode.
Also, I suggest to create another global constant for LJ_TISNUM, like it
is done for PADDING constant.
I suppose we want to do something similar to tvisint() macro for this check:
| LJ_DUALNUM && itype(tv) == LJ_TISNUM
> + integer = cast('int32_t', cast('uint64_t', cast('void*', tv['n'])) & 0xFFFFFFFF)
I don't get this cast to uint64_t and mask. Why can't we just take
`(int32_t)(o)->i` value, like it is done for `intV()` macro?
> + return 'number {}'.format(integer)
Nit: I suppose, it is better to highlight the fact that TValue stores
integer here with corresponding return. It may be helpful for debugging
some issues, related to storing type, I suppose.
But I'm not so sure at this point. Wait for Igor's opinion.
> + else:
> + return 'number {}'.format(cast('double', tv['n']))
>
> def dump_lj_invalid(tv):
> return 'not valid type @ {}'.format(strx64(gcval(tv['gcr'])))
> --
> 2.32.0
>
[1]: https://sourceware.org/legacy-ml/gdb-patches/2011-08/msg00441.html
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] luajit-gdb: support dualnum mode
2021-08-11 8:27 ` Sergey Kaplun via Tarantool-patches
@ 2021-08-13 14:29 ` Максим Корякшин via Tarantool-patches
2021-08-14 11:38 ` [Tarantool-patches] [PATCH luajit v2] " Maxim Kokryashkin via Tarantool-patches
0 siblings, 1 reply; 13+ messages in thread
From: Максим Корякшин via Tarantool-patches @ 2021-08-13 14:29 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: Maxim Kokryashkin, tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 7147 bytes --]
Hi, Sergey!
Thanks for the comments, here is the new commit message:
=======================================================
luajit-gdb: support dualnum mode
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
=======================================================
And here is the diff:
=======================================================
diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index 5f79c277..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
def tvislightud(o):
if LJ_64 and not LJ_GC64:
@@ -343,9 +346,8 @@ def dump_lj_tudata(tv):
return 'userdata @ {}'.format(strx64(gcval(tv['gcr'])))
def dump_lj_tnumx(tv):
- if itype(tv) == (0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX']):
- integer = cast('int32_t', cast('uint64_t', cast('void*', tv['n'])) & 0xFFFFFFFF)
- return 'number {}'.format(integer)
+ if tvisint(tv):
+ return 'number {}'.format(cast('int32_t', tv['i']))
else:
return 'number {}'.format(cast('double', tv['n']))
@@ -687,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).
@@ -728,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')
@@ -737,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')
=======================================================
>Среда, 11 августа 2021, 11:28 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:
>
>Hi, Maxim!
>
>Thanks for the patch!
>
>Please consider my comments below.
>
>Side note: First of all, I'm very disappointed, that there this patch
>[1] isn't merged (in any form) into gdb. Those work with the expanding
>of macros is very helpful...
>
>On 31.07.21, Maxim Kokryashkin wrote:
>> For x86/x64 LJ_DUALNUM mode is disabled. But for some other arches
>
>Nit: It can be enabled by corresponding configuration options for
>x86/x64, too, IINM. So I suggest to drop the first and the second
>sentence.
>
>> (arm or arm64, for example) it is enabled by default. luajit-gdb.py
>> displays integers in LJ_DUALNUM mode as nan-s.
>>
>
>Nit: This paragraph may be joined to the next after suggesting changes.
>
>> As it turned out, luajit-gdb detects those integers as integers, but
>> there was a problem with the dumper function itself.
>
>Nit: The next sentence is about the problem with the dumping function, so
>I suggest to drop this opening sentence:)
>Feel free to ignore.
>
>> The dumper
>> function produces output thinking of any input value as of a double.
>> However, in DUALNUM mode, integers and floats are stored differently,
>
>Typo: s/floats/doubles/
>Here and below.
>
>> 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.
>>
>> But in any mode, lua numbers are stored as doubles on the C side, so it
>
>Typo: s/lua/Lua/
>
>Do you mean LuaJIT here? Because this is not true for LuaJIT. See
><lj_obj.h> for details.
>
>> takes an ugly cast chain on the Python side to perform the some sort of
>> the `reinterpret_cast` because the gdb module doesn't have any
>> mechanism to get the address of a symbol.
>
>This sentence isn't clear to me. What symbol do you mean?
>
>>
>> Closes tarantool/tarantool#6224
>
>Side note: You can say that it really closes the issue, because we use
>luajit-gdb from this fork. OTOH, maybe one uses it as a part of
>third_party from Tarantool repo.
>"Closes" is good to me, but I'm not sure what is idiomatically correct.
>
>> ---
>> Github branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-6224-support-dulanum
>> Issue: https://github.com/tarantool/tarantool/issues/6224
>> For more info, see line 273 in lj_obj.h
>>
>> src/luajit-gdb.py | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
>> index c50405ad..5f79c277 100644
>> --- a/src/luajit-gdb.py
>> +++ b/src/luajit-gdb.py
>> @@ -343,7 +343,11 @@ 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 itype(tv) == (0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX']):
>
>This is true only in LJ_DUALNUM mode. So I suggest we can add another
>one global constant for this: LJ_DUALNUM. We can set it via hack with
>lookuping symbol `lj_lib_checknumber` -- it is compiled only for
>LJ_DUALNUM mode for now. So, if a result of lookup() isn't None we are
>in LJ_DUALNUM mode.
>
>Also, I suggest to create another global constant for LJ_TISNUM, like it
>is done for PADDING constant.
>
>I suppose we want to do something similar to tvisint() macro for this check:
>| LJ_DUALNUM && itype(tv) == LJ_TISNUM
>
>> + integer = cast('int32_t', cast('uint64_t', cast('void*', tv['n'])) & 0xFFFFFFFF)
>
>I don't get this cast to uint64_t and mask. Why can't we just take
>`(int32_t)(o)->i` value, like it is done for `intV()` macro?
>
>> + return 'number {}'.format(integer)
>
>Nit: I suppose, it is better to highlight the fact that TValue stores
>integer here with corresponding return. It may be helpful for debugging
>some issues, related to storing type, I suppose.
>
>But I'm not so sure at this point. Wait for Igor's opinion.
>
>> + else:
>> + return 'number {}'.format(cast('double', tv['n']))
>>
>> def dump_lj_invalid(tv):
>> return 'not valid type @ {}'.format(strx64(gcval(tv['gcr'])))
>> --
>> 2.32.0
>>
>
>[1]: https://sourceware.org/legacy-ml/gdb-patches/2011-08/msg00441.html
>
>--
>Best regards,
>Sergey Kaplun
[-- Attachment #2: Type: text/html, Size: 8942 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode
2021-08-13 14:29 ` Максим Корякшин via Tarantool-patches
@ 2021-08-14 11:38 ` Maxim Kokryashkin via Tarantool-patches
2021-08-14 13:36 ` Максим Корякшин via Tarantool-patches
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2021-08-14 11:38 UTC (permalink / raw)
To: tarantool-patches, imun, skaplun
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
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']))
+ 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode
2021-08-14 11:38 ` [Tarantool-patches] [PATCH luajit v2] " Maxim Kokryashkin via Tarantool-patches
@ 2021-08-14 13:36 ` Максим Корякшин via Tarantool-patches
2021-08-14 13:47 ` Sergey Kaplun via Tarantool-patches
2021-08-16 8:49 ` Igor Munkin via Tarantool-patches
2 siblings, 0 replies; 13+ messages in thread
From: Максим Корякшин via Tarantool-patches @ 2021-08-14 13:36 UTC (permalink / raw)
To: Maxim Kokryashkin; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 3382 bytes --]
While I was working on another luajit-gdb enhancement,
I noticed that I forgot to add LJ_DUALNUM and LJ_TISNUM
declarations. So here is the diff that fixes it:
===========================================
diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index d4882dd7..b656a808 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -157,9 +157,10 @@ def frame_prev(framelink):
LJ_64 = None
LJ_GC64 = None
LJ_FR2 = None
+LJ_DUALNUM = None
LJ_GCVMASK = ((1 << 47) - 1)
-
+LJ_TISNUM = None
PADDING = None
# }}}
===========================================
>Суббота, 14 августа 2021, 14:39 +03:00 от Maxim Kokryashkin <max.kokryashkin@gmail.com>:
>
>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
>
> 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']))
>+ 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
[-- Attachment #2: Type: text/html, Size: 4525 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode
2021-08-14 11:38 ` [Tarantool-patches] [PATCH luajit v2] " Maxim Kokryashkin via Tarantool-patches
2021-08-14 13:36 ` Максим Корякшин via Tarantool-patches
@ 2021-08-14 13:47 ` Sergey Kaplun via Tarantool-patches
2021-08-14 17:26 ` Максим Корякшин via Tarantool-patches
2021-08-17 7:56 ` Igor Munkin via Tarantool-patches
2021-08-16 8:49 ` Igor Munkin via Tarantool-patches
2 siblings, 2 replies; 13+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2021-08-14 13:47 UTC (permalink / raw)
To: Maxim Kokryashkin; +Cc: tarantool-patches
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode
2021-08-14 13:47 ` Sergey Kaplun via Tarantool-patches
@ 2021-08-14 17:26 ` Максим Корякшин via Tarantool-patches
2021-08-17 7:56 ` Igor Munkin via Tarantool-patches
1 sibling, 0 replies; 13+ messages in thread
From: Максим Корякшин via Tarantool-patches @ 2021-08-14 17:26 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: Maxim Kokryashkin, tarantool-patches
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode
2021-08-14 11:38 ` [Tarantool-patches] [PATCH luajit v2] " Maxim Kokryashkin via Tarantool-patches
2021-08-14 13:36 ` Максим Корякшин via Tarantool-patches
2021-08-14 13:47 ` Sergey Kaplun via Tarantool-patches
@ 2021-08-16 8:49 ` Igor Munkin via Tarantool-patches
2021-08-16 10:01 ` Максим Корякшин via Tarantool-patches
2 siblings, 1 reply; 13+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-08-16 8:49 UTC (permalink / raw)
To: Maxim Kokryashkin; +Cc: tarantool-patches
Max,
Thanks for the patch! Please consider the comments below.
At first, please use 'gdb:' instead of 'luajit-gdb:' like for the others
patches. I would also use 'LJ_DUALNUM' instead of the plain 'dualnum'.
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
Minor: s/thinking of/considering/, but feel free to ignore.
> double. However, in DUALNUM mode, integers and doubles are stored
Please, be consistent in mode name: choose one from LJ_DUALNUM, DUALNUM
or dualnum and use it patch-wide.
> differently, so the `itype` of a float number must be less than
Minor: It's either double (for the storage type) or floating point.
> `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
<snipped>
> 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']))
Side note: Agree with Sergey here. It is more convenient to understand
that TValue structure for integer slot differs from double.
> + 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
Minor: Why did you split LJ_GC64 and LJ_FR2. I understand you want to
group all LJ_* entries together, so you didn't just append the new ones,
but please move the new LJ_* entries after the old ones.
>
> # 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
Ouch... Not such elegant as for other defines, but I see we have no
choice here... Anyway, this doesn't work for the following scenario:
| # gdb --args ./luajit -e 'print(1)'
| <snipped>
| (gdb) source luajit-gdb.py
| luajit-gdb.py failed to load: no debugging symbols found for libluajit
However, the next case is fine:
| # gdb --args ./luajit -e 'print(1)'
| <snipped>
| (gdb) b lj_cf_print
| Breakpoint 1 at 0x28350: file /luajit/src/lib_base.c, line 486.
| (gdb) r
| Starting program: /luajit/src/luajit -e print\(1\)
|
| Breakpoint 1, lj_cf_print (L=0x7fb7d57378) at /luajit/src/lib_base.c:486
| 486 ptrdiff_t i, nargs = L->top - L->base;
| (gdb) source luajit-gdb.py
| 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
The problem is that you use <gdb.lookup_symbol> that requires a frame,
but there is none, until the program is run. The function you need for
your case is <gdb.lookup_global_symbol>. Consider the diff below:
| diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
| index 9ccca66a..4dc8c844 100644
| --- a/src/luajit-gdb.py
| +++ b/src/luajit-gdb.py
| @@ -731,7 +731,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
| + LJ_DUALNUM = gdb.lookup_global_symbol('lj_lib_checknumber') is not None
| except:
| gdb.write('luajit-gdb.py failed to load: '
| 'no debugging symbols found for libluajit\n')
As a result, the extension loads fine even before the program is started:
| # gdb --args ./luajit -e 'print(1)'
| <snipped>
| (gdb) source luajit-gdb.py
| 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
BTW, it's worth to extend <lj-arch> output with LJ_DUALNUM value.
> except:
> gdb.write('luajit-gdb.py failed to load: '
> 'no debugging symbols found for libluajit\n')
<snipped>
> --
> 2.32.0
>
--
Best regards,
IM
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode
2021-08-16 8:49 ` Igor Munkin via Tarantool-patches
@ 2021-08-16 10:01 ` Максим Корякшин via Tarantool-patches
2021-08-16 16:23 ` Vitaliia Ioffe via Tarantool-patches
2021-08-17 7:47 ` Igor Munkin via Tarantool-patches
0 siblings, 2 replies; 13+ messages in thread
From: Максим Корякшин via Tarantool-patches @ 2021-08-16 10:01 UTC (permalink / raw)
To: Igor Munkin; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 3104 bytes --]
Hello, Igor!
Thanks for your comments!
Here is the new commit message:
=========================================================
gdb: support LJ_DUALNUM mode
luajit-gdb.py displays integers in LJ_DUALNUM mode as nan-s. The
dumper function produces output considering any input value as a
double. However, in LJ_DUALNUM mode, integers and doubles are stored
differently, so the `itype` of a double 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
=========================================================
Here is the diff:
=========================================================
diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index 9ccca66a..25428745 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -507,10 +507,14 @@ pointers respectively.
'''
def invoke(self, arg, from_tty):
- gdb.write('LJ_64: {LJ_64}, LJ_GC64: {LJ_GC64}\n'.format(
- LJ_64 = LJ_64,
- LJ_GC64 = LJ_GC64
- ))
+ gdb.write(
+ 'LJ_64: {LJ_64}, LJ_GC64: {LJ_GC64}, LJ_DUALNUM : {LJ_DUALNUM}\n'
+ .format(
+ LJ_64 = LJ_64,
+ LJ_GC64 = LJ_GC64,
+ LJ_DUALNUM = LJ_DUALNUM
+ )
+ )
class LJDumpTValue(LJBase):
'''
@@ -690,7 +694,7 @@ The command requires no args and dumps current GC stats:
))
def init(commands):
- global LJ_64, LJ_GC64, LJ_DUALNUM, LJ_TISNUM, LJ_FR2, PADDING
+ global LJ_64, LJ_GC64, LJ_FR2, LJ_DUALNUM, LJ_TISNUM, PADDING
# XXX Fragile: though connecting the callback looks like a crap but it
# respects both Python 2 and Python 3 (see #4828).
@@ -731,7 +735,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
+ LJ_DUALNUM = gdb.lookup_global_symbol('lj_lib_checknumber') is not None
except:
gdb.write('luajit-gdb.py failed to load: '
'no debugging symbols found for libluajit\n')
=========================================================
>
>>Max,
>>
>>Thanks for the patch! Please consider the comments below.
><snipped>
>>> 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']))
>>
>>Side note: Agree with Sergey here. It is more convenient to understand
>>that TValue structure for integer slot differs from double.
>I have already fixed the output format here with the fix for v2 of the patch.
><snipped>
>
>Best regards,
>Maxim Kokryashkin
>
[-- Attachment #2: Type: text/html, Size: 4639 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode
2021-08-16 10:01 ` Максим Корякшин via Tarantool-patches
@ 2021-08-16 16:23 ` Vitaliia Ioffe via Tarantool-patches
2021-08-17 7:47 ` Igor Munkin via Tarantool-patches
1 sibling, 0 replies; 13+ messages in thread
From: Vitaliia Ioffe via Tarantool-patches @ 2021-08-16 16:23 UTC (permalink / raw)
To: Максим
Корякшин
Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 3389 bytes --]
QA LGTM
--
Vitaliia Ioffe
>Понедельник, 16 августа 2021, 13:02 +03:00 от Максим Корякшин via Tarantool-patches <tarantool-patches@dev.tarantool.org>:
>
>Hello, Igor!
>Thanks for your comments!
>
>Here is the new commit message:
>=========================================================
>gdb: support LJ_DUALNUM mode
>
>luajit-gdb.py displays integers in LJ_DUALNUM mode as nan-s. The
>dumper function produces output considering any input value as a
>double. However, in LJ_DUALNUM mode, integers and doubles are stored
>differently, so the `itype` of a double 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
>=========================================================
>
>Here is the diff:
>=========================================================
>diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
>index 9ccca66a..25428745 100644
>--- a/src/luajit-gdb.py
>+++ b/src/luajit-gdb.py
>@@ -507,10 +507,14 @@ pointers respectively.
> '''
> def invoke(self, arg, from_tty):
>- gdb.write('LJ_64: {LJ_64}, LJ_GC64: {LJ_GC64}\n'.format(
>- LJ_64 = LJ_64,
>- LJ_GC64 = LJ_GC64
>- ))
>+ gdb.write(
>+ 'LJ_64: {LJ_64}, LJ_GC64: {LJ_GC64}, LJ_DUALNUM : {LJ_DUALNUM}\n'
>+ .format(
>+ LJ_64 = LJ_64,
>+ LJ_GC64 = LJ_GC64,
>+ LJ_DUALNUM = LJ_DUALNUM
>+ )
>+ )
> class LJDumpTValue(LJBase):
> '''
>@@ -690,7 +694,7 @@ The command requires no args and dumps current GC stats:
> ))
> def init(commands):
>- global LJ_64, LJ_GC64, LJ_DUALNUM, LJ_TISNUM, LJ_FR2, PADDING
>+ global LJ_64, LJ_GC64, LJ_FR2, LJ_DUALNUM, LJ_TISNUM, PADDING
> # XXX Fragile: though connecting the callback looks like a crap but it
> # respects both Python 2 and Python 3 (see #4828).
>@@ -731,7 +735,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
>+ LJ_DUALNUM = gdb.lookup_global_symbol('lj_lib_checknumber') is not None
> except:
> gdb.write('luajit-gdb.py failed to load: '
> 'no debugging symbols found for libluajit\n')
>=========================================================
>
>>
>>>Max,
>>>
>>>Thanks for the patch! Please consider the comments below.
>><snipped>
>>>> 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']))
>>>
>>>Side note: Agree with Sergey here. It is more convenient to understand
>>>that TValue structure for integer slot differs from double.
>>I have already fixed the output format here with the fix for v2 of the patch.
>><snipped>
>>
>>Best regards,
>>Maxim Kokryashkin
>>
[-- Attachment #2: Type: text/html, Size: 5333 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode
2021-08-16 10:01 ` Максим Корякшин via Tarantool-patches
2021-08-16 16:23 ` Vitaliia Ioffe via Tarantool-patches
@ 2021-08-17 7:47 ` Igor Munkin via Tarantool-patches
1 sibling, 0 replies; 13+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-08-17 7:47 UTC (permalink / raw)
To: Максим
Корякшин
Cc: tarantool-patches
Max,
Thanks for the fixes! LGTM now. BTW, I've reverted one fix proposed by
Sergey (see the corresponding thread) and fixed a typo below.
On 16.08.21, Максим Корякшин wrote:
>
> Hello, Igor!
> Thanks for your comments!
>
> Here is the new commit message:
> =========================================================
> gdb: support LJ_DUALNUM mode
>
> luajit-gdb.py displays integers in LJ_DUALNUM mode as nan-s. The
> dumper function produces output considering any input value as a
> double. However, in LJ_DUALNUM mode, integers and doubles are stored
> differently, so the `itype` of a double 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
> =========================================================
>
> Here is the diff:
> =========================================================
> diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
> index 9ccca66a..25428745 100644
> --- a/src/luajit-gdb.py
> +++ b/src/luajit-gdb.py
> @@ -507,10 +507,14 @@ pointers respectively.
> '''
> def invoke(self, arg, from_tty):
> - gdb.write('LJ_64: {LJ_64}, LJ_GC64: {LJ_GC64}\n'.format(
> - LJ_64 = LJ_64,
> - LJ_GC64 = LJ_GC64
> - ))
> + gdb.write(
> + 'LJ_64: {LJ_64}, LJ_GC64: {LJ_GC64}, LJ_DUALNUM : {LJ_DUALNUM}\n'
Side note: Excess whitespace. I've fixed this before merging the patch
into the trunk.
> + .format(
> + LJ_64 = LJ_64,
> + LJ_GC64 = LJ_GC64,
> + LJ_DUALNUM = LJ_DUALNUM
> + )
> + )
> class LJDumpTValue(LJBase):
> '''
<snipped>
> =========================================================
>
> >
<snipped>
> >Best regards,
> >Maxim Kokryashkin
> >
--
Best regards,
IM
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit v2] luajit-gdb: support dualnum mode
2021-08-14 13:47 ` Sergey Kaplun via Tarantool-patches
2021-08-14 17:26 ` Максим Корякшин via Tarantool-patches
@ 2021-08-17 7:56 ` Igor Munkin via Tarantool-patches
1 sibling, 0 replies; 13+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-08-17 7:56 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] luajit-gdb: support dualnum mode
2021-07-31 13:36 [Tarantool-patches] [PATCH luajit] luajit-gdb: support dualnum mode Maxim Kokryashkin via Tarantool-patches
2021-08-11 8:27 ` Sergey Kaplun via Tarantool-patches
@ 2021-08-17 9:23 ` Igor Munkin via Tarantool-patches
1 sibling, 0 replies; 13+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-08-17 9:23 UTC (permalink / raw)
To: Maxim Kokryashkin; +Cc: tarantool-patches
Max,
I've checked the patch into all long-term branches in tarantool/luajit
and bumped a new version in 1.10, 2.7, 2.8 and master.
On 31.07.21, Maxim Kokryashkin wrote:
> For x86/x64 LJ_DUALNUM mode is disabled. But for some other arches
> (arm or arm64, for example) it is enabled by default. luajit-gdb.py
> displays integers in LJ_DUALNUM mode as nan-s.
>
> As it turned out, luajit-gdb detects those integers as integers, but
> there was a problem with the dumper function itself. The dumper
> function produces output thinking of any input value as of a double.
> However, in DUALNUM mode, integers and floats 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.
>
> But in any mode, lua numbers are stored as doubles on the C side, so it
> takes an ugly cast chain on the Python side to perform the some sort of
> the `reinterpret_cast` because the gdb module doesn't have any
> mechanism to get the address of a symbol.
>
> Closes tarantool/tarantool#6224
> ---
> Github branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-6224-support-dulanum
> Issue: https://github.com/tarantool/tarantool/issues/6224
> For more info, see line 273 in lj_obj.h
>
> src/luajit-gdb.py | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
<snipped>
> --
> 2.32.0
>
--
Best regards,
IM
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-08-17 9:48 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-31 13:36 [Tarantool-patches] [PATCH luajit] luajit-gdb: support dualnum mode Maxim Kokryashkin via Tarantool-patches
2021-08-11 8:27 ` Sergey Kaplun via Tarantool-patches
2021-08-13 14:29 ` Максим Корякшин via Tarantool-patches
2021-08-14 11:38 ` [Tarantool-patches] [PATCH luajit v2] " Maxim Kokryashkin via Tarantool-patches
2021-08-14 13:36 ` Максим Корякшин via Tarantool-patches
2021-08-14 13:47 ` Sergey Kaplun via Tarantool-patches
2021-08-14 17:26 ` Максим Корякшин via Tarantool-patches
2021-08-17 7:56 ` Igor Munkin via Tarantool-patches
2021-08-16 8:49 ` Igor Munkin via Tarantool-patches
2021-08-16 10:01 ` Максим Корякшин via Tarantool-patches
2021-08-16 16:23 ` Vitaliia Ioffe via Tarantool-patches
2021-08-17 7:47 ` Igor Munkin via Tarantool-patches
2021-08-17 9:23 ` [Tarantool-patches] [PATCH luajit] " Igor Munkin via Tarantool-patches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox