Re: [Tarantool-patches] [PATCH luajit 2/2] gdb: introduce lj-bc, lj-func and lj-proto dumpers

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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


Hi!
Thanks for the patch!
LGTM
--
Best regards,
Maxim Kokryashkin
 
  
>Четверг, 14 июля 2022, 18:43 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:
> 
>Hi, Maxim!
>
>Thanks for the review!
>
>On 04.07.22, Maxim Kokryashkin wrote:
>>
>> Hi, Sergey!
>> Thanks for the patch!
>>  
>> Please consider my comments below.
>
>Fixed, see iterative patches below.
>Branch is force-pushed.
>
>Side note: I've fixed you suggestions from the last one to the top one,
>so don't be confused by diff in them.
>
>>  
>> >
>> >diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
>> >index 779a25f8..339b57ed 100644
>> >--- a/src/luajit-gdb.py
>> >+++ b/src/luajit-gdb.py
>> <snipped>
>> >+___ = 0
>> >+BC_NAME = 0
>> >+BC_A = 1
>> >+BC_B = 2
>> >+BC_CD = 3
>> >+BC_MM = 4
>> >+
>> >+BYTECODES = [
>> >+ # Comparison ops. ORDER OPR.
>> >+ ['ISLT', 'var', ___, 'var', 'lt'],
>
><snipped>
>
>> >+ ['FUNCCW', 'rbase', ___, ___, ___],
>> >+]
>> I suggest rewriting that chunk with named tuple[1]. That is more readable
>> and more pythonic way of doing stuff like this.
>
>I've rewrited it with tuple (instead namedtuple) to avoid usage of
>"collections", also it helps to read each bytecode declaration.
>
>See the iterative patch below.
>
>===================================================================
>diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
>index be86f599..98feebec 100644
>--- a/src/luajit-gdb.py
>+++ b/src/luajit-gdb.py
>@@ -129,136 +129,131 @@ def bc_d(ins):
>     return int(ins) >> 16
> 
> ___ = 0
>-BC_NAME = 0
>-BC_A = 1
>-BC_B = 2
>-BC_CD = 3
>-BC_MM = 4
> 
> BYTECODES = [
>     # Comparison ops. ORDER OPR.
>- ['ISLT', 'var', ___, 'var', 'lt'],
>- ['ISGE', 'var', ___, 'var', 'lt'],
>- ['ISLE', 'var', ___, 'var', 'le'],
>- ['ISGT', 'var', ___, 'var', 'le'],
>-
>- ['ISEQV', 'var', ___, 'var', 'eq'],
>- ['ISNEV', 'var', ___, 'var', 'eq'],
>- ['ISEQS', 'var', ___, 'str', 'eq'],
>- ['ISNES', 'var', ___, 'str', 'eq'],
>- ['ISEQN', 'var', ___, 'num', 'eq'],
>- ['ISNEN', 'var', ___, 'num', 'eq'],
>- ['ISEQP', 'var', ___, 'pri', 'eq'],
>- ['ISNEP', 'var', ___, 'pri', 'eq'],
>+ {'name': 'ISLT', 'ra': 'var', 'rb': ___, 'rcd': 'var', 'mm': 'lt'},
>+ {'name': 'ISGE', 'ra': 'var', 'rb': ___, 'rcd': 'var', 'mm': 'lt'},
>+ {'name': 'ISLE', 'ra': 'var', 'rb': ___, 'rcd': 'var', 'mm': 'le'},
>+ {'name': 'ISGT', 'ra': 'var', 'rb': ___, 'rcd': 'var', 'mm': 'le'},
>+
>+ {'name': 'ISEQV', 'ra': 'var', 'rb': ___, 'rcd': 'var', 'mm': 'eq'},
>+ {'name': 'ISNEV', 'ra': 'var', 'rb': ___, 'rcd': 'var', 'mm': 'eq'},
>+ {'name': 'ISEQS', 'ra': 'var', 'rb': ___, 'rcd': 'str', 'mm': 'eq'},
>+ {'name': 'ISNES', 'ra': 'var', 'rb': ___, 'rcd': 'str', 'mm': 'eq'},
>+ {'name': 'ISEQN', 'ra': 'var', 'rb': ___, 'rcd': 'num', 'mm': 'eq'},
>+ {'name': 'ISNEN', 'ra': 'var', 'rb': ___, 'rcd': 'num', 'mm': 'eq'},
>+ {'name': 'ISEQP', 'ra': 'var', 'rb': ___, 'rcd': 'pri', 'mm': 'eq'},
>+ {'name': 'ISNEP', 'ra': 'var', 'rb': ___, 'rcd': 'pri', 'mm': 'eq'},
> 
>     # Unary test and copy ops.
>- ['ISTC', 'dst', ___, 'var', ___],
>- ['ISFC', 'dst', ___, 'var', ___],
>- ['IST', ___, ___, 'var', ___],
>- ['ISF', ___, ___, 'var', ___],
>- ['ISTYPE', 'var', ___, 'lit', ___],
>- ['ISNUM', 'var', ___, 'lit', ___],
>- ['MOV', 'dst', ___, 'var', ___],
>- ['NOT', 'dst', ___, 'var', ___],
>- ['UNM', 'dst', ___, 'var', 'unm'],
>- ['LEN', 'dst', ___, 'var', 'len'],
>- ['ADDVN', 'dst', 'var', 'num', 'add'],
>- ['SUBVN', 'dst', 'var', 'num', 'sub'],
>- ['MULVN', 'dst', 'var', 'num', 'mul'],
>- ['DIVVN', 'dst', 'var', 'num', 'div'],
>- ['MODVN', 'dst', 'var', 'num', 'mod'],
>+ {'name': 'ISTC', 'ra': 'dst', 'rb': ___, 'rcd': 'var', 'mm': ___},
>+ {'name': 'ISFC', 'ra': 'dst', 'rb': ___, 'rcd': 'var', 'mm': ___},
>+ {'name': 'IST', 'ra': ___, 'rb': ___, 'rcd': 'var', 'mm': ___},
>+ {'name': 'ISF', 'ra': ___, 'rb': ___, 'rcd': 'var', 'mm': ___},
>+ {'name': 'ISTYPE', 'ra': 'var', 'rb': ___, 'rcd': 'lit', 'mm': ___},
>+ {'name': 'ISNUM', 'ra': 'var', 'rb': ___, 'rcd': 'lit', 'mm': ___},
>+ {'name': 'MOV', 'ra': 'dst', 'rb': ___, 'rcd': 'var', 'mm': ___},
>+ {'name': 'NOT', 'ra': 'dst', 'rb': ___, 'rcd': 'var', 'mm': ___},
>+ {'name': 'UNM', 'ra': 'dst', 'rb': ___, 'rcd': 'var', 'mm': 'unm'},
>+ {'name': 'LEN', 'ra': 'dst', 'rb': ___, 'rcd': 'var', 'mm': 'len'},
>+ {'name': 'ADDVN', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'add'},
>+ {'name': 'SUBVN', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'sub'},
>+ {'name': 'MULVN', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'mul'},
>+ {'name': 'DIVVN', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'div'},
>+ {'name': 'MODVN', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'mod'},
> 
>     # Binary ops. ORDER OPR.
>- ['ADDNV', 'dst', 'var', 'num', 'add'],
>- ['SUBNV', 'dst', 'var', 'num', 'sub'],
>- ['MULNV', 'dst', 'var', 'num', 'mul'],
>- ['DIVNV', 'dst', 'var', 'num', 'div'],
>- ['MODNV', 'dst', 'var', 'num', 'mod'],
>+ {'name': 'ADDNV', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'add'},
>+ {'name': 'SUBNV', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'sub'},
>+ {'name': 'MULNV', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'mul'},
>+ {'name': 'DIVNV', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'div'},
>+ {'name': 'MODNV', 'ra': 'dst', 'rb': 'var', 'rcd': 'num', 'mm': 'mod'},
> 
>- ['ADDVV', 'dst', 'var', 'var', 'add'],
>- ['SUBVV', 'dst', 'var', 'var', 'sub'],
>- ['MULVV', 'dst', 'var', 'var', 'mul'],
>- ['DIVVV', 'dst', 'var', 'var', 'div'],
>- ['MODVV', 'dst', 'var', 'var', 'mod'],
>+ {'name': 'ADDVV', 'ra': 'dst', 'rb': 'var', 'rcd': 'var', 'mm': 'add'},
>+ {'name': 'SUBVV', 'ra': 'dst', 'rb': 'var', 'rcd': 'var', 'mm': 'sub'},
>+ {'name': 'MULVV', 'ra': 'dst', 'rb': 'var', 'rcd': 'var', 'mm': 'mul'},
>+ {'name': 'DIVVV', 'ra': 'dst', 'rb': 'var', 'rcd': 'var', 'mm': 'div'},
>+ {'name': 'MODVV', 'ra': 'dst', 'rb': 'var', 'rcd': 'var', 'mm': 'mod'},
> 
>- ['POW', 'dst', 'var', 'var', 'pow'],
>- ['CAT', 'dst', 'rbase', 'rbase', 'concat'],
>+ {'name': 'POW', 'ra': 'dst', 'rb': 'var', 'rcd': 'var', 'mm': 'pow'},
>+ {'name': 'CAT', 'ra': 'dst', 'rb': 'rbase', 'rcd': 'rbase', 'mm': 'concat'},
> 
>     # Constant ops.
>- ['KSTR', 'dst', ___, 'str', ___],
>- ['KCDATA', 'dst', ___, 'cdata', ___],
>- ['KSHORT', 'dst', ___, 'lits', ___],
>- ['KNUM', 'dst', ___, 'num', ___],
>- ['KPRI', 'dst', ___, 'pri', ___],
>- ['KNIL', 'base', ___, 'base', ___],
>+ {'name': 'KSTR', 'ra': 'dst', 'rb': ___, 'rcd': 'str', 'mm': ___},
>+ {'name': 'KCDATA', 'ra': 'dst', 'rb': ___, 'rcd': 'cdata', 'mm': ___},
>+ {'name': 'KSHORT', 'ra': 'dst', 'rb': ___, 'rcd': 'lits', 'mm': ___},
>+ {'name': 'KNUM', 'ra': 'dst', 'rb': ___, 'rcd': 'num', 'mm': ___},
>+ {'name': 'KPRI', 'ra': 'dst', 'rb': ___, 'rcd': 'pri', 'mm': ___},
>+ {'name': 'KNIL', 'ra': 'base', 'rb': ___, 'rcd': 'base', 'mm': ___},
> 
>     # Upvalue and function ops.
>- ['UGET', 'dst', ___, 'uv', ___],
>- ['USETV', 'uv', ___, 'var', ___],
>- ['USETS', 'uv', ___, 'str', ___],
>- ['USETN', 'uv', ___, 'num', ___],
>- ['USETP', 'uv', ___, 'pri', ___],
>- ['UCLO', 'rbase', ___, 'jump', ___],
>- ['FNEW', 'dst', ___, 'func', ___],
>+ {'name': 'UGET', 'ra': 'dst', 'rb': ___, 'rcd': 'uv', 'mm': ___},
>+ {'name': 'USETV', 'ra': 'uv', 'rb': ___, 'rcd': 'var', 'mm': ___},
>+ {'name': 'USETS', 'ra': 'uv', 'rb': ___, 'rcd': 'str', 'mm': ___},
>+ {'name': 'USETN', 'ra': 'uv', 'rb': ___, 'rcd': 'num', 'mm': ___},
>+ {'name': 'USETP', 'ra': 'uv', 'rb': ___, 'rcd': 'pri', 'mm': ___},
>+ {'name': 'UCLO', 'ra': 'rbase', 'rb': ___, 'rcd': 'jump', 'mm': ___},
>+ {'name': 'FNEW', 'ra': 'dst', 'rb': ___, 'rcd': 'func', 'mm': ___},
> 
>     # Table ops.
>- ['TNEW', 'dst', ___, 'lit', ___],
>- ['TDUP', 'dst', ___, 'tab', ___],
>- ['GGET', 'dst', ___, 'str', 'index'],
>- ['GSET', 'var', ___, 'str', 'newindex'],
>- ['TGETV', 'dst', 'var', 'var', 'index'],
>- ['TGETS', 'dst', 'var', 'str', 'index'],
>- ['TGETB', 'dst', 'var', 'lit', 'index'],
>- ['TGETR', 'dst', 'var', 'var', 'index'],
>- ['TSETV', 'var', 'var', 'var', 'newindex'],
>- ['TSETS', 'var', 'var', 'str', 'newindex'],
>- ['TSETB', 'var', 'var', 'lit', 'newindex'],
>- ['TSETM', 'base', ___, 'num', 'newindex'],
>- ['TSETR', 'var', 'var', 'var', 'newindex'],
>+ {'name': 'TNEW', 'ra': 'dst', 'rb': ___, 'rcd': 'lit', 'mm': ___},
>+ {'name': 'TDUP', 'ra': 'dst', 'rb': ___, 'rcd': 'tab', 'mm': ___},
>+ {'name': 'GGET', 'ra': 'dst', 'rb': ___, 'rcd': 'str', 'mm': 'index'},
>+ {'name': 'GSET', 'ra': 'var', 'rb': ___, 'rcd': 'str', 'mm': 'newindex'},
>+ {'name': 'TGETV', 'ra': 'dst', 'rb': 'var', 'rcd': 'var', 'mm': 'index'},
>+ {'name': 'TGETS', 'ra': 'dst', 'rb': 'var', 'rcd': 'str', 'mm': 'index'},
>+ {'name': 'TGETB', 'ra': 'dst', 'rb': 'var', 'rcd': 'lit', 'mm': 'index'},
>+ {'name': 'TGETR', 'ra': 'dst', 'rb': 'var', 'rcd': 'var', 'mm': 'index'},
>+ {'name': 'TSETV', 'ra': 'var', 'rb': 'var', 'rcd': 'var', 'mm': 'newindex'},
>+ {'name': 'TSETS', 'ra': 'var', 'rb': 'var', 'rcd': 'str', 'mm': 'newindex'},
>+ {'name': 'TSETB', 'ra': 'var', 'rb': 'var', 'rcd': 'lit', 'mm': 'newindex'},
>+ {'name': 'TSETM', 'ra': 'base', 'rb': ___, 'rcd': 'num', 'mm': 'newindex'},
>+ {'name': 'TSETR', 'ra': 'var', 'rb': 'var', 'rcd': 'var', 'mm': 'newindex'},
> 
>     # Calls and vararg handling. T = tail call.
>- ['CALLM', 'base', 'lit', 'lit', 'call'],
>- ['CALL', 'base', 'lit', 'lit', 'call'],
>- ['CALLMT', 'base', ___, 'lit', 'call'],
>- ['CALLT', 'base', ___, 'lit', 'call'],
>- ['ITERC', 'base', 'lit', 'lit', 'call'],
>- ['ITERN', 'base', 'lit', 'lit', 'call'],
>- ['VARG', 'base', 'lit', 'lit', ___],
>- ['ISNEXT', 'base', ___, 'jump', ___],
>+ {'name': 'CALLM', 'ra': 'base', 'rb': 'lit', 'rcd': 'lit', 'mm': 'call'},
>+ {'name': 'CALL', 'ra': 'base', 'rb': 'lit', 'rcd': 'lit', 'mm': 'call'},
>+ {'name': 'CALLMT', 'ra': 'base', 'rb': ___, 'rcd': 'lit', 'mm': 'call'},
>+ {'name': 'CALLT', 'ra': 'base', 'rb': ___, 'rcd': 'lit', 'mm': 'call'},
>+ {'name': 'ITERC', 'ra': 'base', 'rb': 'lit', 'rcd': 'lit', 'mm': 'call'},
>+ {'name': 'ITERN', 'ra': 'base', 'rb': 'lit', 'rcd': 'lit', 'mm': 'call'},
>+ {'name': 'VARG', 'ra': 'base', 'rb': 'lit', 'rcd': 'lit', 'mm': ___},
>+ {'name': 'ISNEXT', 'ra': 'base', 'rb': ___, 'rcd': 'jump', 'mm': ___},
> 
>     # Returns.
>- ['RETM', 'base', ___, 'lit', ___],
>- ['RET', 'rbase', ___, 'lit', ___],
>- ['RET0', 'rbase', ___, 'lit', ___],
>- ['RET1', 'rbase', ___, 'lit', ___],
>+ {'name': 'RETM', 'ra': 'base', 'rb': ___, 'rcd': 'lit', 'mm': ___},
>+ {'name': 'RET', 'ra': 'rbase', 'rb': ___, 'rcd': 'lit', 'mm': ___},
>+ {'name': 'RET0', 'ra': 'rbase', 'rb': ___, 'rcd': 'lit', 'mm': ___},
>+ {'name': 'RET1', 'ra': 'rbase', 'rb': ___, 'rcd': 'lit', 'mm': ___},
> 
>     # Loops and branches. I/J = interp/JIT, I/C/L = init/call/loop.
>- ['FORI', 'base', ___, 'jump', ___],
>- ['JFORI', 'base', ___, 'jump', ___],
>+ {'name': 'FORI', 'ra': 'base', 'rb': ___, 'rcd': 'jump', 'mm': ___},
>+ {'name': 'JFORI', 'ra': 'base', 'rb': ___, 'rcd': 'jump', 'mm': ___},
> 
>- ['FORL', 'base', ___, 'jump', ___],
>- ['IFORL', 'base', ___, 'jump', ___],
>- ['JFORL', 'base', ___, 'lit', ___],
>+ {'name': 'FORL', 'ra': 'base', 'rb': ___, 'rcd': 'jump', 'mm': ___},
>+ {'name': 'IFORL', 'ra': 'base', 'rb': ___, 'rcd': 'jump', 'mm': ___},
>+ {'name': 'JFORL', 'ra': 'base', 'rb': ___, 'rcd': 'lit', 'mm': ___},
> 
>- ['ITERL', 'base', ___, 'jump', ___],
>- ['IITERL', 'base', ___, 'jump', ___],
>- ['JITERL', 'base', ___, 'lit', ___],
>+ {'name': 'ITERL', 'ra': 'base', 'rb': ___, 'rcd': 'jump', 'mm': ___},
>+ {'name': 'IITERL', 'ra': 'base', 'rb': ___, 'rcd': 'jump', 'mm': ___},
>+ {'name': 'JITERL', 'ra': 'base', 'rb': ___, 'rcd': 'lit', 'mm': ___},
> 
>- ['LOOP', 'rbase', ___, 'jump', ___],
>- ['ILOOP', 'rbase', ___, 'jump', ___],
>- ['JLOOP', 'rbase', ___, 'lit', ___],
>+ {'name': 'LOOP', 'ra': 'rbase', 'rb': ___, 'rcd': 'jump', 'mm': ___},
>+ {'name': 'ILOOP', 'ra': 'rbase', 'rb': ___, 'rcd': 'jump', 'mm': ___},
>+ {'name': 'JLOOP', 'ra': 'rbase', 'rb': ___, 'rcd': 'lit', 'mm': ___},
> 
>- ['JMP', 'rbase', ___, 'jump', ___],
>+ {'name': 'JMP', 'ra': 'rbase', 'rb': ___, 'rcd': 'jump', 'mm': ___},
> 
>     # Function headers. I/J = interp/JIT, F/V/C = fixarg/vararg/C func.
>- ['FUNCF', 'rbase', ___, ___, ___],
>- ['IFUNCF', 'rbase', ___, ___, ___],
>- ['JFUNCF', 'rbase', ___, 'lit', ___],
>- ['FUNCV', 'rbase', ___, ___, ___],
>- ['IFUNCV', 'rbase', ___, ___, ___],
>- ['JFUNCV', 'rbase', ___, 'lit', ___],
>- ['FUNCC', 'rbase', ___, ___, ___],
>- ['FUNCCW', 'rbase', ___, ___, ___],
>+ {'name': 'FUNCF', 'ra': 'rbase', 'rb': ___, 'rcd': ___, 'mm': ___},
>+ {'name': 'IFUNCF', 'ra': 'rbase', 'rb': ___, 'rcd': ___, 'mm': ___},
>+ {'name': 'JFUNCF', 'ra': 'rbase', 'rb': ___, 'rcd': 'lit', 'mm': ___},
>+ {'name': 'FUNCV', 'ra': 'rbase', 'rb': ___, 'rcd': ___, 'mm': ___},
>+ {'name': 'IFUNCV', 'ra': 'rbase', 'rb': ___, 'rcd': ___, 'mm': ___},
>+ {'name': 'JFUNCV', 'ra': 'rbase', 'rb': ___, 'rcd': 'lit', 'mm': ___},
>+ {'name': 'FUNCC', 'ra': 'rbase', 'rb': ___, 'rcd': ___, 'mm': ___},
>+ {'name': 'FUNCCW', 'ra': 'rbase', 'rb': ___, 'rcd': ___, 'mm': ___},
> ]
> 
> def proto_bc(proto):
>@@ -725,8 +720,7 @@ def funcuvname(pt, idx):
> 
>     return str(cast('char *', uvinfo))
> 
>-def dump_reg(bc, reg, value, jmp_format=None, jmp_ctx=None):
>- rtype = bc[reg]
>+def dump_reg(rtype, value, jmp_format=None, jmp_ctx=None):
>     is_jmp = rtype == 'jump'
> 
>     if rtype == 'jump':
>@@ -745,8 +739,7 @@ def dump_reg(bc, reg, value, jmp_format=None, jmp_ctx=None):
>         value = value,
>     )
> 
>-def dump_kc(bc, reg, value, proto):
>- rtype = bc[reg]
>+def dump_kc(rtype, value, proto):
>     kc = ''
>     if proto:
>         if rtype == 'str' or rtype == 'func':
>@@ -766,23 +759,21 @@ def dump_bc(ins, jmp_format=None, jmp_ctx=None, proto=None):
>         return 'INVALID'
> 
>     bc = BYTECODES[op]
>- bc_name = bc[BC_NAME]
> 
>- bc_hasa = bc[BC_A]
>- bc_hasb = bc[BC_B]
>-
>- kca = dump_kc(bc, BC_A, bc_a(ins), proto) if bc_hasa else ''
>- kcc = dump_kc(bc, BC_CD, bc_c(ins) if bc_hasb else bc_d(ins), proto) if bc[BC_CD] else ''
>+ kca = dump_kc(bc['ra'], bc_a(ins), proto) if bc['ra'] else ''
>+ kcc = dump_kc(
>+ bc['rcd'], bc_c(ins) if bc['rb'] else bc_d(ins), proto
>+ ) if bc['rcd'] else ''
> 
>     return '{name:6} {ra}{rb}{rcd}{kc}'.format(
>- name = bc_name,
>- ra = dump_reg(bc, BC_A, bc_a(ins)) + ' ' if bc_hasa else '',
>- rb = dump_reg(bc, BC_B, bc_b(ins)) + ' ' if bc_hasb else '',
>+ name = bc['name'],
>+ ra = dump_reg(bc['ra'], bc_a(ins)) + ' ' if bc['ra'] else '',
>+ rb = dump_reg(bc['rb'], bc_b(ins)) + ' ' if bc['rb'] else '',
>         rcd = dump_reg(
>- bc, BC_CD, bc_c(ins) if bc_hasb else bc_d(ins),
>+ bc['rcd'], bc_c(ins) if bc['rb'] else bc_d(ins),
>             jmp_format=jmp_format, jmp_ctx=jmp_ctx
>- ) if bc[BC_CD] else '',
>- kc=kca+kcc
>+ ) if bc['rcd'] else '',
>+ kc = kca + kcc
>     )
> 
> def dump_proto(proto):
>===================================================================
>
>>  
>> <snipped>
>> >+def dump_reg(bc, reg, value, jmp_format=None, jmp_ctx=None):
>> >+ rtype = bc[reg]
>> >+ is_jmp = rtype == 'jump'
>> >+ padding = ':' + ' ' * (5 - len(rtype))
>> >+
>> >+ if rtype == 'jump':
>> >+ # Destination of jump instruction encoded as offset from BCBIAS_J.
>> >+ delta = value - 0x7fff
>> >+ if jmp_format:
>> >+ value = jmp_format(jmp_ctx, delta)
>> >+ else:
>> >+ prefix = '+' if delta >= 0 else ''
>> >+ value = prefix + str(delta)
>> >+ else:
>> >+ value = '{:3d}'.format(value)
>> >+
>> >+ return '{rtype}{padding} {value}'.format(
>> >+ rtype = rtype,
>> >+ padding = padding,
>> >+ value = value,
>> >+ )
>> The padding variable is unnecessary. You can use format-string
>> width specifier[2] instead.
>
>Fixed! Thanks! See the iterative patch below.
>
>===================================================================
>diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
>index 0827ad45..be86f599 100644
>--- a/src/luajit-gdb.py
>+++ b/src/luajit-gdb.py
>@@ -728,7 +728,6 @@ def funcuvname(pt, idx):
> def dump_reg(bc, reg, value, jmp_format=None, jmp_ctx=None):
>     rtype = bc[reg]
>     is_jmp = rtype == 'jump'
>- padding = ':' + ' ' * (5 - len(rtype))
> 
>     if rtype == 'jump':
>         # Destination of jump instruction encoded as offset from BCBIAS_J.
>@@ -741,9 +740,8 @@ def dump_reg(bc, reg, value, jmp_format=None, jmp_ctx=None):
>     else:
>         value = '{:3d}'.format(value)
> 
>- return '{rtype}{padding} {value}'.format(
>- rtype = rtype,
>- padding = padding,
>+ return '{rtype:6} {value}'.format(
>+ rtype = rtype + ':',
>         value = value,
>     )
> 
>@@ -769,7 +767,6 @@ def dump_bc(ins, jmp_format=None, jmp_ctx=None, proto=None):
> 
>     bc = BYTECODES[op]
>     bc_name = bc[BC_NAME]
>- name_padding = ' ' * (6 - len(bc_name))
> 
>     bc_hasa = bc[BC_A]
>     bc_hasb = bc[BC_B]
>@@ -777,9 +774,8 @@ def dump_bc(ins, jmp_format=None, jmp_ctx=None, proto=None):
>     kca = dump_kc(bc, BC_A, bc_a(ins), proto) if bc_hasa else ''
>     kcc = dump_kc(bc, BC_CD, bc_c(ins) if bc_hasb else bc_d(ins), proto) if bc[BC_CD] else ''
> 
>- return '{name}{npad} {ra}{rb}{rcd}{kc}'.format(
>+ return '{name:6} {ra}{rb}{rcd}{kc}'.format(
>         name = bc_name,
>- npad = name_padding,
>         ra = dump_reg(bc, BC_A, bc_a(ins)) + ' ' if bc_hasa else '',
>         rb = dump_reg(bc, BC_B, bc_b(ins)) + ' ' if bc_hasb else '',
>         rcd = dump_reg(
>~
>===================================================================
>
>>  
>> <snipped>
>> >
>> >+class LJDumpFunc(LJBase):
>> >+ '''
>> >+lj-funcl <GCfunc *>
>> Typo: s/lj-funcl/lj-func
>
>Fixed! Thanks! See the iterative patch below.
>
>===================================================================
>diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
>index f976b758..0827ad45 100644
>--- a/src/luajit-gdb.py
>+++ b/src/luajit-gdb.py
>@@ -1051,7 +1051,7 @@ The constants or upvalues of the prototype are decoded after ';'.
> 
> class LJDumpFunc(LJBase):
>     '''
>-lj-funcl <GCfunc *>
>+lj-func <GCfunc *>
> 
> The command receives a <gcr> of the corresponding GCfunc object and dumps
> the chunk name, where the corresponding function is defined, corresponding
>===================================================================
>
>>  
>> <snipped>
>>  
>> Side note:
>> In the present moment, passing an invalid pointer to `lj-func`/`lj-proto` results in empty
>> output, which is not a gdb-way (and not a UNIX-way either) of reporting errors.
>> Maybe it is a good idea to check the type of the  value under the pointer and
>> report corresponding errors. 
>
>Yes, maybe. But I suggest now just use the same interface as for other
>lj-* dumpers. We can refactor them later.
>
>>  
>> [1]:   https://docs.python.org/3/library/collections.html#collections.namedtuple
>> [2]:   https://docs.python.org/3/library/string.html#format-examples
>>  
>> --
>> Best regards,
>> Maxim Kokryashkin
>
>--
>Best regards,
>Sergey Kaplun
 

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