[Tarantool-patches] [PATCH luajit] luajit-gdb: support full-range 64-bit lightud
sergos
sergos at tarantool.org
Thu Oct 6 12:02:00 MSK 2022
Hi, Max!
Thanks for the patch!
0. Please, add Mikhail<m.elhimov at vk.team> to review.
> On 5 Oct 2022, at 17:43, Maxim Kokryashkin <max.kokryashkin at gmail.com> wrote:
>
> Following up the introduction of full-range 64-bit lightuserdata
> support in commit 2cacfa8 ("Add support for full-range 64 bit
> lightuserdata."), this patch modifies the corresponding dumper
> behavior for LJ_64 platforms in the luajit-gdb extension.
>
> Resolves tarantool/tarantool#6481
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-6481-luajit-gdb-light-ud
> Issue: https://github.com/tarantool/tarantool/issues/6481
> src/luajit-gdb.py | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
> index 6480d014..2993f2c1 100644
> --- a/src/luajit-gdb.py
> +++ b/src/luajit-gdb.py
> @@ -166,6 +166,9 @@ LJ_GCVMASK = ((1 << 47) - 1)
> LJ_TISNUM = None
> PADDING = None
>
> +LJ_LIGHTUD_BITS_SEG = 8
> +LJ_LIGHTUD_BITS_LO = 47 - LJ_LIGHTUD_BITS_SEG
1. Is this ’47’ literal has the same meaning as the one above in the ‘LJ_GCVMASK’
definition and the one below in the ‘itype()’ function?
Shall we introduce a constant to avoid running across the code to fix all ’47’s
later on?
> +
> # }}}
>
> def itype(o):
> @@ -315,6 +318,12 @@ def frames(L):
> break
> framelink = frame_prev(framelink)
>
> +def lightudseg(u):
> + return (u >> LJ_LIGHTUD_BITS_LO) & ((1 << LJ_LIGHTUD_BITS_SEG) - 1)
> +
> +deflightudlo(u):
> + return u & ((1 << LJ_LIGHTUD_BITS_LO) - 1)
> +
2a. Did you expect to reuse these two above somewhere else later?
Also, the '((1 << LJ_LIGHTUD_BITS_LO) - 1)’ is a constant, can be named
‘LJ_LIGHTUD_BITS_MASK’. Then the whole ‘lightudlo’ is just an ‘&’ with
this constant. The same can be done for _SEG_MASK and we can proceed to 2b.
> # Dumpers {{{
>
> def dump_lj_tnil(tv):
> @@ -327,7 +336,14 @@ def dump_lj_ttrue(tv):
> return 'true'
>
> def dump_lj_tlightud(tv):
> - return 'light userdata @ {}'.format(strx64(gcval(tv['gcr'])))
> + if LJ_64:
> + u = int(tv['u64'])
> + seg = lightudseg(u)
> + segmap = mref('uint32_t *', G(L(None))['gc']['lightudseg'])
> + addr = (int(segmap[seg]) << 32) | lightudlo(u)
2b. Otherwise why not to put the whole iff clause above into a separate function?
> + else:
> + addr = gcval(tv['gcr'])
> + return 'light userdata @ {}'.format(strx64(addr))
>
> def dump_lj_tstr(tv):
> return 'string {body} @ {address}'.format(
> --
> 2.36.1
>
More information about the Tarantool-patches
mailing list