

<HTML><BODY><div>Hi, Sergey!</div><div>Thanks for the patch!</div><div> </div><div>Please consider my comments below.</div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><br>diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py<br>index 779a25f8..339b57ed 100644<br>--- a/src/luajit-gdb.py<br>+++ b/src/luajit-gdb.py</div></blockquote><div><snipped></div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">+___ = 0<br>+BC_NAME = 0<br>+BC_A = 1<br>+BC_B = 2<br>+BC_CD = 3<br>+BC_MM = 4<br>+</blockquote><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">+BYTECODES = [<br>+ # Comparison ops. ORDER OPR.<br>+ ['ISLT', 'var', ___, 'var', 'lt'],<br>+ ['ISGE', 'var', ___, 'var', 'lt'],<br>+ ['ISLE', 'var', ___, 'var', 'le'],<br>+ ['ISGT', 'var', ___, 'var', 'le'],<br>+<br>+ ['ISEQV', 'var', ___, 'var', 'eq'],<br>+ ['ISNEV', 'var', ___, 'var', 'eq'],<br>+ ['ISEQS', 'var', ___, 'str', 'eq'],<br>+ ['ISNES', 'var', ___, 'str', 'eq'],<br>+ ['ISEQN', 'var', ___, 'num', 'eq'],<br>+ ['ISNEN', 'var', ___, 'num', 'eq'],<br>+ ['ISEQP', 'var', ___, 'pri', 'eq'],<br>+ ['ISNEP', 'var', ___, 'pri', 'eq'],<br>+<br>+ # Unary test and copy ops.<br>+ ['ISTC', 'dst', ___, 'var', ___],<br>+ ['ISFC', 'dst', ___, 'var', ___],<br>+ ['IST', ___, ___, 'var', ___],<br>+ ['ISF', ___, ___, 'var', ___],<br>+ ['ISTYPE', 'var', ___, 'lit', ___],<br>+ ['ISNUM', 'var', ___, 'lit', ___],<br>+ ['MOV', 'dst', ___, 'var', ___],<br>+ ['NOT', 'dst', ___, 'var', ___],<br>+ ['UNM', 'dst', ___, 'var', 'unm'],<br>+ ['LEN', 'dst', ___, 'var', 'len'],<br>+ ['ADDVN', 'dst', 'var', 'num', 'add'],<br>+ ['SUBVN', 'dst', 'var', 'num', 'sub'],<br>+ ['MULVN', 'dst', 'var', 'num', 'mul'],<br>+ ['DIVVN', 'dst', 'var', 'num', 'div'],<br>+ ['MODVN', 'dst', 'var', 'num', 'mod'],<br>+<br>+ # Binary ops. ORDER OPR.<br>+ ['ADDNV', 'dst', 'var', 'num', 'add'],<br>+ ['SUBNV', 'dst', 'var', 'num', 'sub'],<br>+ ['MULNV', 'dst', 'var', 'num', 'mul'],<br>+ ['DIVNV', 'dst', 'var', 'num', 'div'],<br>+ ['MODNV', 'dst', 'var', 'num', 'mod'],<br>+<br>+ ['ADDVV', 'dst', 'var', 'var', 'add'],<br>+ ['SUBVV', 'dst', 'var', 'var', 'sub'],<br>+ ['MULVV', 'dst', 'var', 'var', 'mul'],<br>+ ['DIVVV', 'dst', 'var', 'var', 'div'],<br>+ ['MODVV', 'dst', 'var', 'var', 'mod'],<br>+<br>+ ['POW', 'dst', 'var', 'var', 'pow'],<br>+ ['CAT', 'dst', 'rbase', 'rbase', 'concat'],<br>+<br>+ # Constant ops.<br>+ ['KSTR', 'dst', ___, 'str', ___],<br>+ ['KCDATA', 'dst', ___, 'cdata', ___],<br>+ ['KSHORT', 'dst', ___, 'lits', ___],<br>+ ['KNUM', 'dst', ___, 'num', ___],<br>+ ['KPRI', 'dst', ___, 'pri', ___],<br>+ ['KNIL', 'base', ___, 'base', ___],<br>+<br>+ # Upvalue and function ops.<br>+ ['UGET', 'dst', ___, 'uv', ___],<br>+ ['USETV', 'uv', ___, 'var', ___],<br>+ ['USETS', 'uv', ___, 'str', ___],<br>+ ['USETN', 'uv', ___, 'num', ___],<br>+ ['USETP', 'uv', ___, 'pri', ___],<br>+ ['UCLO', 'rbase', ___, 'jump', ___],<br>+ ['FNEW', 'dst', ___, 'func', ___],<br>+<br>+ # Table ops.<br>+ ['TNEW', 'dst', ___, 'lit', ___],<br>+ ['TDUP', 'dst', ___, 'tab', ___],<br>+ ['GGET', 'dst', ___, 'str', 'index'],<br>+ ['GSET', 'var', ___, 'str', 'newindex'],<br>+ ['TGETV', 'dst', 'var', 'var', 'index'],<br>+ ['TGETS', 'dst', 'var', 'str', 'index'],<br>+ ['TGETB', 'dst', 'var', 'lit', 'index'],<br>+ ['TGETR', 'dst', 'var', 'var', 'index'],<br>+ ['TSETV', 'var', 'var', 'var', 'newindex'],<br>+ ['TSETS', 'var', 'var', 'str', 'newindex'],<br>+ ['TSETB', 'var', 'var', 'lit', 'newindex'],<br>+ ['TSETM', 'base', ___, 'num', 'newindex'],<br>+ ['TSETR', 'var', 'var', 'var', 'newindex'],<br>+<br>+ # Calls and vararg handling. T = tail call.<br>+ ['CALLM', 'base', 'lit', 'lit', 'call'],<br>+ ['CALL', 'base', 'lit', 'lit', 'call'],<br>+ ['CALLMT', 'base', ___, 'lit', 'call'],<br>+ ['CALLT', 'base', ___, 'lit', 'call'],<br>+ ['ITERC', 'base', 'lit', 'lit', 'call'],<br>+ ['ITERN', 'base', 'lit', 'lit', 'call'],<br>+ ['VARG', 'base', 'lit', 'lit', ___],<br>+ ['ISNEXT', 'base', ___, 'jump', ___],<br>+<br>+ # Returns.<br>+ ['RETM', 'base', ___, 'lit', ___],<br>+ ['RET', 'rbase', ___, 'lit', ___],<br>+ ['RET0', 'rbase', ___, 'lit', ___],<br>+ ['RET1', 'rbase', ___, 'lit', ___],<br>+<br>+ # Loops and branches. I/J = interp/JIT, I/C/L = init/call/loop.<br>+ ['FORI', 'base', ___, 'jump', ___],<br>+ ['JFORI', 'base', ___, 'jump', ___],<br>+<br>+ ['FORL', 'base', ___, 'jump', ___],<br>+ ['IFORL', 'base', ___, 'jump', ___],<br>+ ['JFORL', 'base', ___, 'lit', ___],<br>+<br>+ ['ITERL', 'base', ___, 'jump', ___],<br>+ ['IITERL', 'base', ___, 'jump', ___],<br>+ ['JITERL', 'base', ___, 'lit', ___],<br>+<br>+ ['LOOP', 'rbase', ___, 'jump', ___],<br>+ ['ILOOP', 'rbase', ___, 'jump', ___],<br>+ ['JLOOP', 'rbase', ___, 'lit', ___],<br>+<br>+ ['JMP', 'rbase', ___, 'jump', ___],<br>+<br>+ # Function headers. I/J = interp/JIT, F/V/C = fixarg/vararg/C func.<br>+ ['FUNCF', 'rbase', ___, ___, ___],<br>+ ['IFUNCF', 'rbase', ___, ___, ___],<br>+ ['JFUNCF', 'rbase', ___, 'lit', ___],<br>+ ['FUNCV', 'rbase', ___, ___, ___],<br>+ ['IFUNCV', 'rbase', ___, ___, ___],<br>+ ['JFUNCV', 'rbase', ___, 'lit', ___],<br>+ ['FUNCC', 'rbase', ___, ___, ___],<br>+ ['FUNCCW', 'rbase', ___, ___, ___],<br>+]</blockquote><div>I suggest rewriting that chunk with named tuple[1]. That is more readable</div><div>and more pythonic way of doing stuff like this.</div><div> </div><div><snipped></div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">+def dump_reg(bc, reg, value, jmp_format=None, jmp_ctx=None):<br>+ rtype = bc[reg]<br>+ is_jmp = rtype == 'jump'<br>+ padding = ':' + ' ' * (5 - len(rtype))<br>+<br>+ if rtype == 'jump':<br>+ # Destination of jump instruction encoded as offset from BCBIAS_J.<br>+ delta = value - 0x7fff<br>+ if jmp_format:<br>+ value = jmp_format(jmp_ctx, delta)<br>+ else:<br>+ prefix = '+' if delta >= 0 else ''<br>+ value = prefix + str(delta)<br>+ else:<br>+ value = '{:3d}'.format(value)<br>+<br>+ return '{rtype}{padding} {value}'.format(<br>+ rtype = rtype,<br>+ padding = padding,<br>+ value = value,<br>+ )</blockquote><div>The padding variable is unnecessary. You can use format-string</div><div>width specifier[2] instead.</div><div> </div><div><snipped></div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><br>+class LJDumpFunc(LJBase):<br>+ '''<br>+lj-funcl <GCfunc *></div></blockquote><div>Typo: s/lj-funcl/lj-func</div><div> </div><div><snipped></div><div> </div><div>Side note:<div>In the present moment, passing an invalid pointer to `lj-func`/`lj-proto` results in empty</div><div>output, which is not a gdb-way (and not a UNIX-way either) of reporting errors.</div><div><span style="font-family: var(--vkui--octavius_font_family_global,var(--vkui--font_family_base,Helvetica,Arial,sans-serif)); letter-spacing: var(--vkui--font_text--letter_spacing--regular,normal);">Maybe it is a good idea to check the type of the</span> value under the pointer and</div><div>report corresponding errors. </div></div><div> </div><div>[1]: <a href="https://docs.python.org/3/library/collections.html#collections.namedtuple">https://docs.python.org/3/library/collections.html#collections.namedtuple</a></div><div>[2]: <a href="https://docs.python.org/3/library/string.html#format-examples">https://docs.python.org/3/library/string.html#format-examples</a></div><div> </div><div><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div></BODY></HTML>