Re: [Tarantool-patches] [PATCH luajit 1/4] dbg: fix lj-stack command for LLDB

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 5622 bytes --]

Hi, Sergey,

thanks for the patch! LGTM

On 6/4/26 12:30, Sergey Kaplun wrote:
> This commit is the follow-up for the commit
> 593fef813b7d36060e0c1b1ae2df0fbc0d604d1f ("lldb: refactor extension").
> `GetValueForExpressionPath()` may produce the empty value for negative
> indexes. This leads to incorrect frame unwinding in the LLDB extension
> and errors like:
> | Failed to execute command `lj-stack`:
> | 'Q' format requires 0 <= number <= 18446744073709551615
>
> This patch adds special handling for the negative indices.
> ---
>   src/luajit_dbg.py                             | 49 ++++++++++++++++---
>   .../debug-extension-tests.py                  | 26 +++++++---
>   2 files changed, 62 insertions(+), 13 deletions(-)
>
> diff --git a/src/luajit_dbg.py b/src/luajit_dbg.py
> index d9196f06..410f0191 100644
> --- a/src/luajit_dbg.py
> +++ b/src/luajit_dbg.py
> @@ -255,15 +255,31 @@ class _GDBDebugger(Debugger):
>   
>   class _LLDBDebugger(Debugger):
>       def _lldb_tp_isfp(self, tp):
> -        return tp.GetBasicType() in [
> +        return tp.GetCanonicalType().GetBasicType() in [
>               lldb.eBasicTypeFloat,
>               lldb.eBasicTypeDouble,
>               lldb.eBasicTypeLongDouble
>           ]
>   
> +    def _lldb_tp_issigned(self, tp):
> +        return tp.GetCanonicalType().GetBasicType() in [
> +            lldb.eBasicTypeChar,
> +            lldb.eBasicTypeSignedChar,
> +            lldb.eBasicTypeShort,
> +            lldb.eBasicTypeInt,
> +            lldb.eBasicTypeLong,
> +            lldb.eBasicTypeLongLong,
> +            lldb.eBasicTypeInt128
> +        ]
> +
>       def _lldb_value_from_raw(self, raw_value, size, tp):
>           isfp = self._lldb_tp_isfp(tp)
> -        pack_flag = '<d' if isfp else '<Q'
> +        if isfp:
> +            pack_flag = '<d'
> +        elif self._lldb_tp_issigned(tp):
> +            pack_flag = '<q'
> +        else:
> +            pack_flag = '<Q'
>           raw_data = struct.pack(pack_flag, raw_value)
>           sbdata = lldb.SBData()
>           sbdata.SetData(
> @@ -308,9 +324,24 @@ class _LLDBDebugger(Debugger):
>                   key = int(key)
>               if type(key) is int:
>                   # Allow array access.
> -                return lldb.value(
> -                    lldbval.sbvalue.GetValueForExpressionPath('[%i]' % key)
> -                )
> +                if key >= 0:
> +                    return lldb.value(
> +                        lldbval.sbvalue.GetValueForExpressionPath('[%i]' % key)
> +                    )
> +                else:
> +                    # GetValueForExpressionPath doesn't work for
> +                    # negative offsets.
> +                    sbvalue = lldbval.sbvalue
> +                    assert sbvalue.TypeIsPointerType(), \
> +                        'attempt to get index of non-pointer type'
> +                    tp = sbvalue.GetType().GetPointeeType()
> +                    sz = sbvalue.deref.size
> +                    addr = sbvalue.GetValueAsUnsigned() + key * sz
> +                    return lldb.value(self.target.CreateValueFromAddress(
> +                            '({tp}){addr}'.format(tp=tp, addr=addr),
> +                            lldb.SBAddress(addr, self.target),
> +                            tp,
> +                    ))
>               elif type(key) is str:
>                   return lldb.value(lldbval.sbvalue.GetChildMemberWithName(key))
>               raise Exception(TypeError('No item of type %s' % str(type(key))))
> @@ -417,8 +448,12 @@ class _LLDBDebugger(Debugger):
>           # may take the 8 bytes of memory instead of 4, before the
>           # cast. Construct the value on the fly.
>           tp = self._dbgtype(typestr)
> -        is_fp = self._lldb_tp_isfp(tp)
> -        rawval = float(val.GetValue()) if is_fp else val.GetValueAsUnsigned()
> +        if self._lldb_tp_isfp(tp):
> +            rawval = float(val.GetValue())
> +        elif self._lldb_tp_issigned(tp):
> +            rawval = val.GetValueAsSigned()
> +        else:
> +            rawval = val.GetValueAsUnsigned()
>           return self._lldb_value_from_raw(rawval, val.GetByteSize(), tp)
>   
>       def sizeof(self, typestr):
> diff --git a/test/tarantool-debugger-tests/debug-extension-tests.py b/test/tarantool-debugger-tests/debug-extension-tests.py
> index 61529561..7cb60d84 100644
> --- a/test/tarantool-debugger-tests/debug-extension-tests.py
> +++ b/test/tarantool-debugger-tests/debug-extension-tests.py
> @@ -186,16 +186,30 @@ class TestLJGC(TestCaseBase):
>       )
>   
>   
> -class TestLJStack(TestCaseBase):
> +STACK_RX = (
> +    r'-+ Redzone:\s+\d+ slots -+\n'
> +    r'(' + RX_ADDR + r'\s+' + RX_FRAME + r' VALUE: nil\n?)*\n'
> +    r'-+Stack:\s+\d+ slots -+\n'
> +    r'(' + RX_ADDR + r'(:' + RX_ADDR + r')?\s+' + RX_FRAME + r'.*\n?)+\n'
> +)
> +
> +
> +class TestLJStackBase(TestCaseBase):
>       extension_cmds = 'lj-stack'
>       location = 'lj_cf_print'
>       lua_script = 'print(1)'
> -    pattern = (
> -        r'-+ Redzone:\s+\d+ slots -+\n'
> -        r'(' + RX_ADDR + r'\s+' + RX_FRAME + r' VALUE: nil\n?)*\n'
> -        r'-+Stack:\s+\d+ slots -+\n'
> -        r'(' + RX_ADDR + r'(:' + RX_ADDR + r')?\s+' + RX_FRAME + r'.*\n?)+\n'
> +    pattern = STACK_RX
> +
> +
> +# Check LLDB correctness for the specific stack.
> +class TestLJStackFunc(TestCaseBase):
> +    extension_cmds = 'lj-stack'
> +    location = 'lj_cf_print'
> +    lua_script = (
> +        'local function nop() end\n'
> +        'print()\n'
>       )
> +    pattern = STACK_RX
>   
>   
>   class TestLJTV(TestCaseBase):

[-- Attachment #2: Type: text/html, Size: 6023 bytes --]