From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 42A106ECDB; Mon, 21 Sep 2026 21:47:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 42A106ECDB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1790016478; bh=4krHPouAibxFSbVDg06FMcXxp7WYd4Vi97eWj4303gc=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=yYRpTOf8JKlR9nSVkKeoXMyFqGncpWj8g/1VF5wCvkADPI3x6OZDOiZRck3sWbj+6 7cItiDaJo/0fhW/Fl9x3PH3+RGIFoqLhCdOm3ggGxVkSCV6YoyJl3lfPfsAlGh0sDe nA5izH5unOI3c8US4IjVkK4NJva9saaAbkLJiIQ4= Received: from send194.i.mail.ru (send194.i.mail.ru [95.163.59.33]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 48CFF6ECDB for ; Mon, 21 Sep 2026 21:47:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 48CFF6ECDB Received: by exim-smtp-7cfc745659-qtl65 with esmtpa (envelope-from ) id 1x8j3T-000000001Lh-0Ob1; Mon, 21 Sep 2026 21:47:55 +0300 Date: Mon, 21 Sep 2026 21:47:34 +0300 To: Mikhail Elhimov Message-ID: References: <20260909084908.354159-1-m.elhimov@vk.team> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260909084908.354159-1-m.elhimov@vk.team> X-Mailru-Src: smtp X-4EC0790: 10 X-618D5548: A2F95B4D945BC00E9487ABAC94A94B5402B560673A7072B0DDB9BCA94691BC7BDC9B8C3B7ADA3463 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9B0E16AC5A4192F0E8E2A7D02ACAA921810B5B44D55A265A000894C459B0CD1B9A2F95B4D945BC00E9487ABAC94A94B546BD65EAB4E7C0748DDB9BCA94691BC7BE1C7414A75FA687D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7AD2F2D6F6013FF7FC2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6759CC434672EE6371C2A783ECEC0211ADC4224003CC836476D5A39DEEDB180909611E41BBFE2FEB2BC661CC960CEA0142CA2E1FFA880DA7E93284C9301639AEFFA8D5E84D045C5B2A9FA2833FD35BB23D9E625A9149C048EE33AC447995A7AD18618001F51B5FD3F9D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B3A703B70628EAD7BA471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249957A4DEDD2346B42E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B67ECBC18655D52CDF089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A564461FCABAF27C435002B1117B3ED69688C500C9B1BE8B88F09842853758E9E5823CB91A9FED034534781492E4B8EEAD29D60DE4F26800BE X-C8649E89: 1C3962B70DF3F0ADB58128AB1E6D661A8E10F71CB4DF9F96AB70F9BE574AE9C625B6776AC983F447FC0B9F89525902EE6F57B2FD27647F25E66C117BDB76D659254228069334BA8FEB16A7D997AF03AE33D65302146623F44A751A48778CA13A5FEED8F0AB2605B8B8341EE9D5BE9A0A78C61CE79E62B2742DAC478AB2CDD33026AF7D72802079C8C7CEAA0681F5848F4C41F94D744909CECFA6C6B0C050A61A8CAF69B82BA93681CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXOcIETfglQORZ0zpDET4Zrk3igikrdHlWP5gtAGnp+Stm5/lFOSIJYM= X-Mailru-Sender: 689FA8AB762F73937C9FA53A4753B3133BFED0712B2FBE690BF155A36BDA6387E5416FB1D22947C4E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] dbg: avoid hardcoded enums (get them from target) X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Mikhail! Thanks for the patch! I really like this! It helps to avoid copy-pasting and makes the code much more robust. Brilliant idea to use type of enums! I left some stillistic comments and asked for some patch clean-up below. On 09.09.26, Mikhail Elhimov wrote: > Besides reducing lines of code this way the extension become compatible Typo: s/become/becomes/ > with various version of luajit because different version might use Typo: s/version/versions/g > different set of enum members (newer version might introduce additional Typo: s/set/sets/ Typo: s/newer/a newer/ > BC/IR/etc.) > > Prior to this patch string was used as a debugger-agnostic way to Typo: s/this/this, the/ > specify type, but this way might not work in case of enum because Typo: s/case of enum/the case of an enum/ > debugging information might be optimized out if no variable of such enum Typo: s/enum/an enum/ > type is declared and its members are used only as a predefined Typo: s/a // > constants. The type of enum is needed to be able to get human readable Typo: s/human readable/human-readable/ > value (member name instead of number). The alternative way to get enum > type is get it from the value, i.e. somehow obtain the value that is of Typo: s/is get/is to get/ > enum type and then get its type object. > > To do that, separate method to create enum value was introduced in Typo: s/separate/a separate/ > Debugger API, LLDB value class was monkey-patched to get value type in > the same way as GDB value class does and 'cast' method was adjusted to > accept also type object, not only string. I would prefer to keep the `cast()` API to be used only for the string as the first parameter, since it is very helpful for reading of the code. May I suggest the little bit different approach here? Introduce the `dbg.cast_typeof()` API to have the following semantics: | dbg.cast_typeof(obj_with_type, value) The idea is the same, but it doesn't allow getting and working with internal types somehow in the debugger extension. I'll mention this in the cast usages below. > > Other adjustments: > - [lldb] 'eval' adjusted to return object of the same type as 'cast' > method (this improves consistency) > - [gdb] in 'eval' method dropped check of the value returned by > gdb.parse_and_eval() as it would fail also for expression like '0' > (looks like a kind of legacy code that is not needed now). > - [gdb/lldb] renamed eval argument to reflect its meaning (it is > an expression rather than a command). > > Closes tarantool/tarantool#13094 Nit: s/Closes/Resolves/ Since technically speaking it will be closed after LuaJIT's bump in Tarantool. > --- > This patch is to be applied after the 'fix mapping of FPMATHOP' patch. > > Branch: https://github.com/tarantool/luajit/tree/elhimov/gh-13094-dbg-use-enums-from-inferior > Related issue: https://github.com/tarantool/tarantool/issues/13094 > > src/luajit_dbg.py | 628 +++++++++++----------------------------------- > 1 file changed, 141 insertions(+), 487 deletions(-) > > diff --git a/src/luajit_dbg.py b/src/luajit_dbg.py > index 76001b7d..1ac1d275 100644 > --- a/src/luajit_dbg.py > +++ b/src/luajit_dbg.py > @@ -110,9 +110,21 @@ class Debugger(object): > self.write('{} command initialized\n'.format(name)) > self.write('LuaJIT debug extension is successfully loaded\n') > > + def cast(self, tp, val): > + '''Cast the value to the given type (it is either C type string > + or the debugger-specific type object).''' > + if isinstance(tp, str): > + tp = self._dbgtype(tp) I would prefer to avoid casting to the given debugger-specific type. It is better to introduce `cast_typeof()` method to be used with another object given as the first argument (it should be read approximately as `cast(typeof(x), y))`. | def cast_typeof(self, typed_object, val): | '''Cast the value to the type of the given object. | It is used mostly for casting when the string name of the type isn't | accessible (for example, anonymous enums).''' > + return self._cast(tp, val) > + > + @abc.abstractmethod > + def _cast(self, tp, val): > + '''Cast the value to the debugger-specific type.''' > + pass > + > @abc.abstractmethod > - def cast(self, typestr, val): > - '''Cast the value to the required C type.''' > + def _dbgtype(self, typestr): > + '''Convert C type string into debugger-specific type object.''' > pass I suppose since it is internal for each debugger, this should not be declared as the required type in the parent class. > > @abc.abstractmethod > @@ -146,8 +158,9 @@ class Debugger(object): > pass > > @abc.abstractmethod > - def eval(self, command): > - '''Parse and evaluate the given debugger command.''' > + def eval(self, expr): > + '''Parse and evaluate the given debugger expression. > + Return debugger-specific value.''' Nit: This clarification looks unrelated to the patch itself. It is better to move it to the separate commit if you really want it. > pass > > @abc.abstractmethod > @@ -187,6 +200,13 @@ class Debugger(object): > '''Register the command with the corresponding name.''' > pass > I suggest adding some comments with motivations about such a specific method (mostly described in the first paragraph in the commit message). > + @abc.abstractmethod > + def create_enum_value(self, enum_name, enum_member_name): > + '''Return debugger-specific value object that represents > + the given enum member. > + ''' > + pass > + > @abc.abstractproperty > def LJBase(self): > '''Base command class. > @@ -211,8 +231,9 @@ class _GDBDebugger(Debugger): > super(_GDBDebugger, self).__init__() > self.CONNECTED = False > > - def cast(self, typestr, val): > - return gdb.Value(val).cast(self._dbgtype(typestr)) > + def _cast(self, tp, val): > + assert isinstance(tp, gdb.Type) > + return gdb.Value(val).cast(tp) > > def sizeof(self, typestr): > return self._dbgtype(typestr).sizeof > @@ -268,14 +289,11 @@ class _GDBDebugger(Debugger): > else: > return None > > - def eval(self, command): > - if not command: > + def eval(self, expr): > + if not expr: > return None > > - ret = gdb.parse_and_eval(command) > - if not ret: > - raise gdb.GdbError('table argument empty') Hmm, indeed, I can't find an example when this returns something not evaluated as `True`... Probably it may be removed. But it should be done in the separate clean up patch since it is unrelated to the refactoring of the enums. > - return ret > + return gdb.parse_and_eval(expr) > > def detect_arch(self): > if hasattr(self, 'arch'): > @@ -340,6 +358,9 @@ class _GDBDebugger(Debugger): > def register_command(self, command, name): > command(name) > > + def create_enum_value(self, enum_name, enum_member_name): > + return self.eval(enum_member_name) > + > class LJBase(gdb and gdb.Command or object): > def __init__(ljbase, name): > # XXX Fragile: Though the command initialization looks > @@ -378,6 +399,10 @@ class _LLDBDebugger(Debugger): > lldb.eBasicTypeInt128 > ] > > + def _lldb_tp_isenum(self, tp): > + return tp.GetCanonicalType().GetTypeClass() == \ > + lldb.eTypeClassEnumeration > + > def _lldb_value_from_raw(self, raw_value, size, tp): > isfp = self._lldb_tp_isfp(tp) > if isfp: > @@ -491,8 +516,7 @@ class _LLDBDebugger(Debugger): > # Instead of default GetSummary. > if not lldbval.sbvalue.TypeIsPointerType(): > tp = lldbval.sbvalue.GetType() > - is_float = self._lldb_tp_isfp(tp) > - if is_float: > + if self._lldb_tp_isfp(tp) or self._lldb_tp_isenum(tp): > return lldbval.sbvalue.GetValue() > else: > return str(int(lldbval)) > @@ -530,6 +554,9 @@ class _LLDBDebugger(Debugger): > else: > return int(lldbval) - int(other) > > + def lldb_gettype(lldbval): > + return lldbval.sbvalue.type > + I prefer to drop this method in favor of `cast_typeof()`. > super(_LLDBDebugger, self).__init__() > self.target = lldb.debugger.GetSelectedTarget() > # Monkey-patch the lldb.value class. > @@ -545,6 +572,7 @@ class _LLDBDebugger(Debugger): > lldb.value.__ror__ = lldb__or__ # Same semantics. > lldb.value.__str__ = lldb__str__ > lldb.value.__sub__ = lldb__sub__ > + lldb.value.type = property(lldb_gettype) I prefer to drop this field in favor of `cast_typeof()`. > > def lldb_major_version(): > version_string = lldb.SBDebugger.GetVersionString() > @@ -568,11 +596,11 @@ class _LLDBDebugger(Debugger): > self.dbgtype_cache[typestr] = dbgtype > return dbgtype > > - def cast(self, typestr, val): > + def _cast(self, tp, val): > + assert isinstance(tp, lldb.SBType) > if isinstance(val, lldb.value): > val = val.sbvalue > elif type(val) is int: > - tp = self._dbgtype(typestr) > return self._lldb_value_from_raw(val, tp.GetByteSize(), tp) > elif not isinstance(val, lldb.SBValue): > raise Exception( > @@ -582,7 +610,6 @@ class _LLDBDebugger(Debugger): > # XXX: Simply SBValue.Cast() works incorrectly since it > # may take the 8 bytes of memory instead of 4, before the > # cast. Construct the value on the fly. > - tp = self._dbgtype(typestr) > if self._lldb_tp_isfp(tp): > rawval = float(val.GetValue()) > elif self._lldb_tp_issigned(tp): > @@ -670,15 +697,15 @@ class _LLDBDebugger(Debugger): > else: > return None > > - def eval(self, command): > - if not command: > + def eval(self, expr): > + if not expr: > return None > > process = self.target.GetProcess() > thread = process.GetSelectedThread() > frame = thread.GetSelectedFrame() > - ret = frame.EvaluateExpression(command) > - return ret > + ret = frame.EvaluateExpression(expr) > + return lldb.value(ret) Nit: Lets drop unneeded variable `ret`: | return lldb.value(frame.EvaluateExpression(expr)) > > def detect_arch(self): > if hasattr(self, 'arch'): > @@ -724,6 +751,37 @@ class _LLDBDebugger(Debugger): > ) > ) > > + def create_enum_value(self, enum_name, enum_member_name): > + val = self.eval(enum_name + "::" + enum_member_name) Please add the comment about this eval construction. > + if val.sbvalue.IsValid() and val.sbvalue.error.Success(): Why do we need to check `val.sbvalue.error.Success()` value? Why is `val.sbvalue.IsValid()` not enough? > + return val > + > + # LLDB uses enum name in expression above but debugging information Typo: s/enum/the enum/ Typo: s/expression/the expression/ > + # about enum name migth be optimized out if no variable of the given Typo: s/enum/the enum/ > + # enum type is declared and its members are only used as the predefined > + # constants (like IRFieldID). Nit: Please, use 66 comment line width. > + > + # In this case the above method doesn't work so trying to discover > + # enum type by the given enum member. Typo: s/enum/the enum/ Nit: Please, use 66 comment line width. > + > + def find_enum_type_member(enum_type, enum_member_name): > + # SBTypeEnumMemberList supports members iteration and [] access > + # (both by index and by member name) only starting from lldb-12 > + # so this implementation is used to handle earlier versions. Nit: Please, use 66 comment line width. > + members = enum_type.GetEnumMembers() > + for i in range(members.GetSize()): > + item = members.GetTypeEnumMemberAtIndex(i) > + if item.name == enum_member_name: > + return item > + return None > + > + for m in self.target.modules: > + for et in m.GetTypes(lldb.eTypeClassEnumeration): > + et_member = find_enum_type_member(et, enum_member_name) > + if et_member is not None: > + return self.cast(et, et_member.unsigned) `self.cast_typeof(et, et_member.unsigned)` > + return None > + > class LJBase(object): > # Ignore given parameters by LLDB. > def __init__(ljbase, debugger, unused): > @@ -800,6 +858,45 @@ def strx64(val): > return re.sub('L?$', '', hex(int(tou64(val)))) > > > +class EnumBasedList(object): > + def __init__(self, enum_name, max_enum_member, map_func=None, > + *map_func_extra_args): > + self.__enum_name = enum_name > + self.__max_enum_member = max_enum_member > + self.__map_func = map_func > + self.__map_func_extra_args = map_func_extra_args > + # Lazy initialization (see __get_items method) as the required Nit: Please, use 66 comment line width. Side note: I like this approach, it helps to avoid performance issues. > + # information might be unavailable at this moment. > + self.__items = None > + > + def __iter__(self): > + return iter(self.__get_items()) > + > + def __getitem__(self, key): > + return self.__get_items()[key] > + > + def __len__(self): > + return len(self.__get_items()) > + > + def __get_items(self): > + if self.__items is None: > + max_enum_value = dbg.create_enum_value( > + self.__enum_name, self.__max_enum_member > + ) > + items = [] > + for i in range(dbg.cast('int', max_enum_value)): Can it be just: | for i in range(int(max_enum_value)) instead? > + item = str(dbg.cast(max_enum_value.type, dbg.eval(str(i)))) | item = str(cast_typeof(max_enum_value, dbg.eval(str(i)))) Be aware, that debugger-specific type usage should be part of the `cast_typeof()`. Hence, it allows dropping `.type` setting for LLDB. > + if self.__map_func: > + item = self.__map_func(item, *self.__map_func_extra_args) > + items.append(item) > + self.__items = items > + return self.__items > +IRTYPES = EnumBasedList('IRType', 'IRT__MAX', lambda x: { > + 'IRT_LIGHTUD': 'lud', > + 'IRT_CDATA': 'cdt', > + 'IRT_UDATA': 'udt', > + 'IRT_FLOAT': 'flt', > + 'IRT_SOFTFP': 'sfp', Minor: since this is not the full spectrum of values, lets sort them alphabetically. > + }.get(x, cut_prefix(x, 'IRT_')[:3].ljust(3).lower())) > -- > 2.43.0 > -- Best regards, Sergey Kaplun