* [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 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 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] 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