<HTML><BODY><div>Hi, Sergey!</div><div>Thanks for the comments, here is the new commit message:</div><div>=======================================================</div><div><div>luajit-gdb: support dualnum mode</div><div> </div><div>luajit-gdb.py displays integers in LJ_DUALNUM mode as nan-s. The</div><div>dumper function produces output thinking of any input value as of a</div><div>double. However, in DUALNUM mode, integers and doubles are stored<br>differently, so the `itype` of a float number must be less than<br>`LJ_TISNUM`, and the `itype` of an integer must be `LJ_TISNUM`. With<br>this fact in mind, we can easily differentiate one from another.</div><div> </div><div>Closes tarantool/tarantool#6224<br>=======================================================</div></div><div> </div><div>And here is the diff:</div><div>=======================================================</div><div><div>diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py<br>index 5f79c277..d4882dd7 100644<br>--- a/src/luajit-gdb.py<br>+++ b/src/luajit-gdb.py<br>@@ -238,8 +238,11 @@ def jit_state(g):<br> 0x15: 'ERR',<br> }.get(int(J(g)['state']), 'INVALID')</div><div>+def tvisint(o):<br>+ return LJ_DUALNUM and itype(o) == LJ_TISNUM<br>+<br> def tvisnumber(o):<br>- return itype(o) <= (0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX'])<br>+ return itype(o) <= LJ_TISNUM</div><div> def tvislightud(o):<br> if LJ_64 and not LJ_GC64:<br>@@ -343,9 +346,8 @@ def dump_lj_tudata(tv):<br> return 'userdata @ {}'.format(strx64(gcval(tv['gcr'])))</div><div> def dump_lj_tnumx(tv):<br>- if itype(tv) == (0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX']):<br>- integer = cast('int32_t', cast('uint64_t', cast('void*', tv['n'])) & 0xFFFFFFFF)<br>- return 'number {}'.format(integer)<br>+ if tvisint(tv):<br>+ return 'number {}'.format(cast('int32_t', tv['i']))<br> else:<br> return 'number {}'.format(cast('double', tv['n']))</div><div>@@ -687,7 +689,7 @@ The command requires no args and dumps current GC stats:<br> ))</div><div> def init(commands):<br>- global LJ_64, LJ_GC64, LJ_FR2, PADDING<br>+ global LJ_64, LJ_GC64, LJ_DUALNUM, LJ_TISNUM, LJ_FR2, PADDING</div><div> # XXX Fragile: though connecting the callback looks like a crap but it<br> # respects both Python 2 and Python 3 (see #4828).<br>@@ -728,6 +730,7 @@ def init(commands):<br> try:<br> LJ_64 = str(gdb.parse_and_eval('IRT_PTR')) == 'IRT_P64'<br> LJ_FR2 = LJ_GC64 = str(gdb.parse_and_eval('IRT_PGC')) == 'IRT_P64'<br>+ LJ_DUALNUM = lookup('lj_lib_checknumber') is not None<br> except:<br> gdb.write('luajit-gdb.py failed to load: '<br> 'no debugging symbols found for libluajit\n')<br>@@ -737,6 +740,7 @@ def init(commands):<br> command(name)</div><div> PADDING = ' ' * len(':' + hex((1 << (47 if LJ_GC64 else 32)) - 1))<br>+ LJ_TISNUM = 0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX']</div><div> gdb.write('luajit-gdb.py is successfully loaded\n')<br>=======================================================</div></div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Среда, 11 августа 2021, 11:28 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16286705191272972240_BODY">Hi, Maxim!<br><br>Thanks for the patch!<br><br>Please consider my comments below.<br><br>Side note: First of all, I'm very disappointed, that there this patch<br>[1] isn't merged (in any form) into gdb. Those work with the expanding<br>of macros is very helpful...<br><br>On 31.07.21, Maxim Kokryashkin wrote:<br>> For x86/x64 LJ_DUALNUM mode is disabled. But for some other arches<br><br>Nit: It can be enabled by corresponding configuration options for<br>x86/x64, too, IINM. So I suggest to drop the first and the second<br>sentence.<br><br>> (arm or arm64, for example) it is enabled by default. luajit-gdb.py<br>> displays integers in LJ_DUALNUM mode as nan-s.<br>><br><br>Nit: This paragraph may be joined to the next after suggesting changes.<br><br>> As it turned out, luajit-gdb detects those integers as integers, but<br>> there was a problem with the dumper function itself.<br><br>Nit: The next sentence is about the problem with the dumping function, so<br>I suggest to drop this opening sentence:)<br>Feel free to ignore.<br><br>> The dumper<br>> function produces output thinking of any input value as of a double.<br>> However, in DUALNUM mode, integers and floats are stored differently,<br><br>Typo: s/floats/doubles/<br>Here and below.<br><br>> so the `itype` of a float number must be less than `LJ_TISNUM`, and the<br>> `itype` of an integer must be `LJ_TISNUM`. With this fact in mind, we<br>> can easily differentiate one from another.<br>><br>> But in any mode, lua numbers are stored as doubles on the C side, so it<br><br>Typo: s/lua/Lua/<br><br>Do you mean LuaJIT here? Because this is not true for LuaJIT. See<br><lj_obj.h> for details.<br><br>> takes an ugly cast chain on the Python side to perform the some sort of<br>> the `reinterpret_cast` because the gdb module doesn't have any<br>> mechanism to get the address of a symbol.<br><br>This sentence isn't clear to me. What symbol do you mean?<br><br>><br>> Closes tarantool/tarantool#6224<br><br>Side note: You can say that it really closes the issue, because we use<br>luajit-gdb from this fork. OTOH, maybe one uses it as a part of<br>third_party from Tarantool repo.<br>"Closes" is good to me, but I'm not sure what is idiomatically correct.<br><br>> ---<br>> Github branch: <a href="https://github.com/tarantool/luajit/tree/fckxorg/gh-6224-support-dulanum" target="_blank">https://github.com/tarantool/luajit/tree/fckxorg/gh-6224-support-dulanum</a><br>> Issue: <a href="https://github.com/tarantool/tarantool/issues/6224" target="_blank">https://github.com/tarantool/tarantool/issues/6224</a><br>> For more info, see line 273 in lj_obj.h<br>><br>> src/luajit-gdb.py | 6 +++++-<br>> 1 file changed, 5 insertions(+), 1 deletion(-)<br>><br>> diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py<br>> index c50405ad..5f79c277 100644<br>> --- a/src/luajit-gdb.py<br>> +++ b/src/luajit-gdb.py<br>> @@ -343,7 +343,11 @@ def dump_lj_tudata(tv):<br>> return 'userdata @ {}'.format(strx64(gcval(tv['gcr'])))<br>><br>> def dump_lj_tnumx(tv):<br>> - return 'number {}'.format(cast('double', tv['n']))<br>> + if itype(tv) == (0xfffeffff if LJ_64 and not LJ_GC64 else LJ_T['NUMX']):<br><br>This is true only in LJ_DUALNUM mode. So I suggest we can add another<br>one global constant for this: LJ_DUALNUM. We can set it via hack with<br>lookuping symbol `lj_lib_checknumber` -- it is compiled only for<br>LJ_DUALNUM mode for now. So, if a result of lookup() isn't None we are<br>in LJ_DUALNUM mode.<br><br>Also, I suggest to create another global constant for LJ_TISNUM, like it<br>is done for PADDING constant.<br><br>I suppose we want to do something similar to tvisint() macro for this check:<br>| LJ_DUALNUM && itype(tv) == LJ_TISNUM<br><br>> + integer = cast('int32_t', cast('uint64_t', cast('void*', tv['n'])) & 0xFFFFFFFF)<br><br>I don't get this cast to uint64_t and mask. Why can't we just take<br>`(int32_t)(o)->i` value, like it is done for `intV()` macro?<br><br>> + return 'number {}'.format(integer)<br><br>Nit: I suppose, it is better to highlight the fact that TValue stores<br>integer here with corresponding return. It may be helpful for debugging<br>some issues, related to storing type, I suppose.<br><br>But I'm not so sure at this point. Wait for Igor's opinion.<br><br>> + else:<br>> + return 'number {}'.format(cast('double', tv['n']))<br>><br>> def dump_lj_invalid(tv):<br>> return 'not valid type @ {}'.format(strx64(gcval(tv['gcr'])))<br>> --<br>> 2.32.0<br>><br><br>[1]: <a href="https://sourceware.org/legacy-ml/gdb-patches/2011-08/msg00441.html" target="_blank">https://sourceware.org/legacy-ml/gdb-patches/2011-08/msg00441.html</a><br><br>--<br>Best regards,<br>Sergey Kaplun</div></div></div></div></blockquote><div> </div></BODY></HTML>