Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Evgeniy Temirgaleev <e.temirgaleev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 2/3] dbg: introduce lj-ctype command, extend cdata dump
Date: Mon, 29 Jun 2026 22:20:28 +0300	[thread overview]
Message-ID: <akLFfMswD-uECeuZ@root> (raw)
In-Reply-To: <1782741349.288346286@f466.i.mail.ru>

Hi, Evgeniy!
Thanks for the review!
Fixed your comments and force-pushed the branch.

On 29.06.26, Evgeniy Temirgaleev wrote:
> Hi, Sergey!
> 
> Thanks for the patch!
> Please, see the commends below.
> 
> --
> Best regards,
> Evgeniy Temirgaleev
> 
> > 
> > From: Sergey Kaplun <skaplun@tarantool.org>
> > To: Sergey Bronnikov <sergeyb@tarantool.org>, Evgeniy Temirgaleev <e.temirgaleev@tarantool.org
> > >
> > Cc: tarantool-patches@dev.tarantool.org, Sergey Kaplun <skaplun@tarantool.org
> > >
> > Date: Thursday, June 25, 2026 11:29 PM +03:00
> > This patch extends dumped information for the given cdata object. Now
> > it resolves the given `CType` and prints it in the format similar to the
> > `__tostring` metamethod. The `lj-ctype` command is introduced to dump
> > this information where there is only the `CType` pointer but no cdata
> > associated with it.
> > 
> > `__or__` and `__ror__` metamethods are monkey-patched for the LLDB value
> > object. In `__sub__` metamethod for LLDB pointers `GetPointeeType()` is
> > used to get the pointee type instead of the incorrectly used
> > `GetDereferencedType()` which always returns the same type with size 8.
> > Casting from negative values to the unsigned values is supported to
> > check `CTF_UCHAR`.
> > 
> > Part of tarantool/tarantool#4808
> > ---
> > src/luajit_dbg.py | 333 +++++++++++++++++-
> > .../debug-extension-tests.py | 208 ++++++++++-
> > 2 files changed, 535 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/luajit_dbg.py b/src/luajit_dbg.py
> > index fd6ca8a5..62cd65d5 100644
> > --- a/src/luajit_dbg.py
> > +++ b/src/luajit_dbg.py
> > @@ -386,6 +386,8 @@ class _LLDBDebugger(Debugger):
> > pack_flag = '<q'
> > else:
> > pack_flag = '<Q'
> > + # Cast to unsigned.
> > 
> 
> Is /unsigned/uint64_t/ clearly?

Rephrased as you suggested. Also, lowercase the value to be consistent
with other hexademical values.

===================================================================
diff --git a/src/luajit_dbg.py b/src/luajit_dbg.py
index 6b0827d9..0769f2ee 100644
--- a/src/luajit_dbg.py
+++ b/src/luajit_dbg.py
@@ -386,8 +386,8 @@ class _LLDBDebugger(Debugger):
             pack_flag = '<q'
         else:
             pack_flag = '<Q'
-            # Cast to unsigned.
-            raw_value &= 0xFFFFFFFFFFFFFFFF
+            # Cast to 64-bit unsigned value in Python.
+            raw_value &= 0xffffffffffffffff
         raw_data = struct.pack(pack_flag, raw_value)
         sbdata = lldb.SBData()
         sbdata.SetData(
===================================================================

> 
> > 
> > + raw_value &= 0xFFFFFFFFFFFFFFFF
> > raw_data = struct.pack(pack_flag, raw_value)
> > sbdata = lldb.SBData()
> > sbdata.SetData(

<snipped>

> > +
> > +
> > +def ctinfo(ct, flags):
> > 
> 
> May we name this function ‘CTINFO’ as in ‘lj_ctype.h’? Or leave a comment with an original name for quick grep.

Added a comment since the upper case is used for constants.

===================================================================
diff --git a/src/luajit_dbg.py b/src/luajit_dbg.py
index 0769f2ee..de83a2b5 100644
--- a/src/luajit_dbg.py
+++ b/src/luajit_dbg.py
@@ -1427,6 +1427,7 @@ def ctype_attrib(info):
     return (info >> CTSHIFT_ATTRIB) & CTMASK_ATTRIB
 
 
+# Implementation of the `CTINFO()` macro.
 def ctinfo(ct, flags):
     return (tou32(ct) << CTSHIFT_NUM) + flags
 
===================================================================


> 
> > 
> > + return (tou32(ct) << CTSHIFT_NUM) + flags
> > +
> > +
> > +def ctype_isptr(info):
> > + return ctype_type(info) == CT_PTR
> > +
> > +
> > +def ctype_iscomplex(info):
> > + return (info & (CTMASK_NUM | CTF_COMPLEX)) == ctinfo(CT_ARRAY,
> > CTF_COMPLEX)
> > +
> > +
> > +def ctype_isinteger(info):
> > + return (info & (CTMASK_NUM | CTF_BOOL | CTF_FP)) == ctinfo(CT_NUM, 0)
> > +
> > +
> > +def ctype_isrefarray(info):
> > + return (info & (CTMASK_NUM | CTF_VECTOR | CTF_COMPLEX)) == \
> > + ctinfo(CT_ARRAY, 0)
> > +
> > +
> > +def ctype_cid(info):
> > 
> 
> Let’s put these function definitions in the ‘lj_ctype.h’ order?
> May we group the definitions by corresponding C files also? # lj_ctype.h … # lj_cdata.h … # lj_xxx.c …


Sorted as you suggested. The sorting is the following:
* lj_ctype.h
* lj_cdata.h -- `cdata_getptr()`
* lj_obj.h -- `cdataptr()`
===================================================================
diff --git a/src/luajit_dbg.py b/src/luajit_dbg.py
index de83a2b5..183dda3b 100644
--- a/src/luajit_dbg.py
+++ b/src/luajit_dbg.py
@@ -1362,14 +1362,6 @@ def lightudV(tv):
 # FFI.
 
 
-def ctype_ctsG(g):
-    return mref('CTState *', g['ctype_state'])
-
-
-def ctype_get(cts, id):
-    return dbg.address(cts['tab'][id])
-
-
 # Externally visible types.
 CT_NUM = 0  # Integer or floating-point numbers.
 CT_STRUCT = 1  # Struct or union.
@@ -1419,46 +1411,55 @@ DWORDSZ = 4
 QWORDSZ = 8
 
 
+# Implementation of the `CTINFO()` macro.
+def ctinfo(ct, flags):
+    return (tou32(ct) << CTSHIFT_NUM) + flags
+
+
 def ctype_type(info):
     return info >> CTSHIFT_NUM
 
 
-def ctype_attrib(info):
-    return (info >> CTSHIFT_ATTRIB) & CTMASK_ATTRIB
+def ctype_cid(info):
+    return info & CTMASK_CID
 
 
-# Implementation of the `CTINFO()` macro.
-def ctinfo(ct, flags):
-    return (tou32(ct) << CTSHIFT_NUM) + flags
+def ctype_attrib(info):
+    return (info >> CTSHIFT_ATTRIB) & CTMASK_ATTRIB
 
 
 def ctype_isptr(info):
     return ctype_type(info) == CT_PTR
 
 
-def ctype_iscomplex(info):
-    return (info & (CTMASK_NUM | CTF_COMPLEX)) == ctinfo(CT_ARRAY, CTF_COMPLEX)
-
-
 def ctype_isinteger(info):
     return (info & (CTMASK_NUM | CTF_BOOL | CTF_FP)) == ctinfo(CT_NUM, 0)
 
 
+def ctype_iscomplex(info):
+    return (info & (CTMASK_NUM | CTF_COMPLEX)) == ctinfo(CT_ARRAY, CTF_COMPLEX)
+
+
 def ctype_isrefarray(info):
     return (info & (CTMASK_NUM | CTF_VECTOR | CTF_COMPLEX)) == \
            ctinfo(CT_ARRAY, 0)
 
 
-def ctype_cid(info):
-    return info & CTMASK_CID
-
-
 def ctype_child(cts, ctype):
     return ctype_get(cts, ctype_cid(ctype['info']))
 
 
-def cdataptr(cd):
-    return dbg.cast('void *', (cd + 1))
+def ctype_ctsG(g):
+    return mref('CTState *', g['ctype_state'])
+
+
+def ctype_get(cts, id):
+    return dbg.address(cts['tab'][id])
+
+
+# Get C type ID for a C type.
+def ctype_typeid(cts, ct):
+    return ct - cts['tab']
 
 
 def cdata_getptr(p, size):
@@ -1468,9 +1469,8 @@ def cdata_getptr(p, size):
         return dbg.cast('void *', dbg.cast('uint64_t *', p)[0])
 
 
-# Get C type ID for a C type.
-def ctype_typeid(cts, ct):
-    return ct - cts['tab']
+def cdataptr(cd):
+    return dbg.cast('void *', (cd + 1))
 
 
 # JIT engine.
===================================================================

> 
> > 
> > + return info & CTMASK_CID
> > +
> > +
> > +def ctype_child(cts, ctype):
> > + return ctype_get(cts, ctype_cid(ctype['info']))
> > +
> > +
> > +def cdataptr(cd):
> > + return dbg.cast('void *', (cd + 1))
> > +
> > +
> > +def cdata_getptr(p, size):
> > + if LJ_64 and size == 4:
> > + return dbg.cast('void *', dbg.cast('uint32_t *', p)[0])
> > + else:
> > 
> 
> assert for size == 8 ?

Added since it may possibly lead to the incorrect (shrinked) pointer
result. If we ever see the 16-bit pointers. Also, support the 32-bit
systems (not LJ_64) as well.

================================================================
diff --git a/src/luajit_dbg.py b/src/luajit_dbg.py
index 183dda3b..de22d450 100644
--- a/src/luajit_dbg.py
+++ b/src/luajit_dbg.py
@@ -1463,9 +1463,10 @@ def ctype_typeid(cts, ct):
 
 
 def cdata_getptr(p, size):
-    if LJ_64 and size == 4:
+    if (LJ_64 and size == 4) or not LJ_64:
         return dbg.cast('void *', dbg.cast('uint32_t *', p)[0])
     else:
+        assert size == 8, 'incorrect pointer size'
         return dbg.cast('void *', dbg.cast('uint64_t *', p)[0])
 
 
================================================================

> 
> > 
> > + return dbg.cast('void *', dbg.cast('uint64_t *', p)[0])
> > +
> > +

<snipped>

> > +def ctype_prepnum(ctypestr, info, size):
> > 
> 
> Func proto differs with lj_ctype.c (static void ctype_prepnum(CTRepr *ctr, uint32_t n)).
> It seems, you move some of ctype_repr() code here. Let’s comment it?

===================================================================
diff --git a/src/luajit_dbg.py b/src/luajit_dbg.py
index de22d450..28cbe97d 100644
--- a/src/luajit_dbg.py
+++ b/src/luajit_dbg.py
@@ -2479,6 +2479,8 @@ def ctype_preptype(cts, ctypestr, ctype, qual, tp):
     return ctypestr
 
 
+# Partially moved the code from `ctype_repr()` here to make it
+# more readable.
 def ctype_prepnum(ctypestr, info, size):
     if info & CTF_BOOL:
         ctypestr = ctype_preplit(ctypestr, 'bool')
===================================================================

<snipped>

> > +def dump_ctype(ct):
> > 
> 
> Also, it seems, it will be easy to read to code, if it will be possible to distinguish between ported functions and extension itself ones. May be by use the ‘dbg_’ prefix for extension function names.

I suppose this refactoring can be done in the separate issue. Since it
is related to all functions. Also, the `dbg` is already used for the
instance of the corresponding class. `dump_` prefix looks common for all
dumpers of our extension.


<skipped>

> > +class TestLJCTypeBase(TestCaseBase):
> > + location = 'lj_cf_ffi_new'
> > + extension_cmds = (
> > + # Load `ct`. Skip inlined functions for LLDB.
> > 
> 
> The extension command set is common for GDB and LLDB. Does we skip for GDB also?

For GDB this function isn't inlined, but these n-s are harmless.
Adjusted the comment.

===================================================================
diff --git a/test/tarantool-debugger-tests/debug-extension-tests.py b/test/tarantool-debugger-tests/debug-extension-tests.py
index f17de27e..71b763d2 100644
--- a/test/tarantool-debugger-tests/debug-extension-tests.py
+++ b/test/tarantool-debugger-tests/debug-extension-tests.py
@@ -1033,7 +1033,9 @@ class TestLJCTypeFunc(TestCaseBase):
 class TestLJCTypeBase(TestCaseBase):
     location = 'lj_cf_ffi_new'
     extension_cmds = (
-        # Load `ct`. Skip inlined functions for LLDB.
+        # Load `ct`. Skip inlined functions for LLDB. The skip is
+        # harmless for GDB since we are still in the body of the
+        # function.
         'n\n'
         'n\n'
         'n\n'
===================================================================


> 
> > 
> > + 'n\n'
> > + 'n\n'
> > + 'n\n'
> > + 'n\n'
> > + 'n\n'
> > + 'n\n'
> > + 'lj-ctype ct\n'
> > + )

<snipped>

> > --
> > 2.54.0
> >

-- 
Best regards,
Sergey Kaplun

  reply	other threads:[~2026-06-29 19:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-25 20:29 [Tarantool-patches] [PATCH luajit 0/3] Extend debug extension Sergey Kaplun via Tarantool-patches
2026-06-25 20:29 ` [Tarantool-patches] [PATCH luajit 1/3] dbg: introduce lj-ir, lj-jslots, lj-trace dumpers Sergey Kaplun via Tarantool-patches
2026-06-28  1:03   ` Evgeniy Temirgaleev via Tarantool-patches
2026-06-28 16:32     ` Sergey Kaplun via Tarantool-patches
2026-06-29 16:35       ` Evgeniy Temirgaleev via Tarantool-patches
2026-06-25 20:29 ` [Tarantool-patches] [PATCH luajit 2/3] dbg: introduce lj-ctype command, extend cdata dump Sergey Kaplun via Tarantool-patches
2026-06-29 13:55   ` Evgeniy Temirgaleev via Tarantool-patches
2026-06-29 19:20     ` Sergey Kaplun via Tarantool-patches [this message]
2026-06-25 20:29 ` [Tarantool-patches] [PATCH luajit 3/3] test: add verbose mode for debug extension tests Sergey Kaplun via Tarantool-patches
2026-06-28  1:31   ` Evgeniy Temirgaleev via Tarantool-patches
2026-06-28 15:19     ` Sergey Kaplun via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akLFfMswD-uECeuZ@root \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=e.temirgaleev@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 2/3] dbg: introduce lj-ctype command, extend cdata dump' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox