From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 9L50LCtz82ljcgYAWB0awg (envelope-from ) for ; Thu, 30 Apr 2026 11:20:11 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=embecosm.com header.i=@embecosm.com header.a=rsa-sha256 header.s=google header.b=RVJTiUBp; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id A10D01E067; Thu, 30 Apr 2026 11:20:11 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H2,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 4979C1E067 for ; Thu, 30 Apr 2026 11:20:10 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 05205436F7C4 for ; Thu, 30 Apr 2026 15:20:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 05205436F7C4 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=embecosm.com header.i=@embecosm.com header.a=rsa-sha256 header.s=google header.b=RVJTiUBp Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by sourceware.org (Postfix) with ESMTPS id 323B74371D54 for ; Thu, 30 Apr 2026 15:19:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 323B74371D54 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 323B74371D54 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::431 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777562381; cv=none; b=vtczUOH4g8bUREggbnBkYkh0nZrJvWLk95eAM/X0KlFboI2zfdP/dHkGpVRzpA1HHi6ngllB1P5iyRSSw094tbJ8dQVUTsSf/shtyzEJ9Gv8gkQJc32WBbl2TIZRRb5UKlz2/3n74CCFLWvd9cEMcXQSPiRvkJ9k2SnAUpqcBZw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1777562381; c=relaxed/simple; bh=kkFiJdmZRPGZXGplYXbcz97+p8S1tybWlKUFhDOXmJQ=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=brbRan1ZEaF1Bo6vwiwAczpP5zq4yFiR3Cs937bM38U+D6LTx6Lu4sAIuo9b5CY43UCAoITcl8nA5sX2aeYT4vXDpIWA3B2P/84hgh892cje2BO/Tl230rRXogyRRifBRg+4XPxLlVCh89QJmUfNQThm+ffok14F7CaLobj5dts= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 323B74371D54 Received: by mail-wr1-x431.google.com with SMTP id ffacd0b85a97d-43fe3e22e33so648912f8f.0 for ; Thu, 30 Apr 2026 08:19:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; t=1777562380; x=1778167180; darn=sourceware.org; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to; bh=24qNezDSZrVBktUbrEKpgA5hFHwPqoLTtMMnRSg2VKI=; b=RVJTiUBp4D0d72NxDQEOJamaFF7wMOpoa8v8i8S5864cwm7jTgUlJdSGP8jav2kom8 qEVSc+m/jQICvYbcHvAT4Ig8I61sLPvdQPjFodL8dNizEfy4pZpp53kFoXq4V3xpDNEP 7Nh6l/C3LF6jVDVJafyR346/17zvp0uSiKFTFtDCRV2NwEcz26AzH0x3VWOUcni5/7Nz iF7jB02uQwdccG47GKjfcBHlLsZRJ86HP89RMz7oxnVa3iGXBa2eWtQPFSPUjCjLpRzN wQjeqWI0TaoPxyg4cFRgZzIvvVRRlUfKY1johyl2aadGSzyB0TZB6169Bj6jP5val9vM wgcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777562380; x=1778167180; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=24qNezDSZrVBktUbrEKpgA5hFHwPqoLTtMMnRSg2VKI=; b=QO9FADaVdO7spxntZkjZxmH7SZHZ7UOnB8zIcbVy55mQUxAVZ6dxAJ30++o8QAZGSx bFa48TO7IxLfEMyOIFfMH2/pfzOOCfyizHSoPTyaJo1UmE8DB/SVby7vKszYW2SJ8/0g PyIWn2novNKmDQlXHnMFPpikIqyiKnR2FitGF1gXTxLxyCbe8HlpnsSJ7RqLdOIkWp5B 5QQZvDQ4QWYQ6zzKSDxQj6IPjeYLUOml6DM53Y7qMwbqsfFoyN/afo93fr7X4RTQrNbe Ia6W8bANao6JrlyvT/VvbKoU+Nq993q+oWPz6UubH/YwKfY7BIZ72+syfK6awbadYBEl 5lpg== X-Forwarded-Encrypted: i=1; AFNElJ8noewFNekAdtYAvLYswyFO91mObICQGlnq9fmLrS3KOWc5yvIBE020ZtBGmOlcT0hCeECJNamn3pVtqg==@sourceware.org X-Gm-Message-State: AOJu0YyUl9928D1JbhCtrms+fFQ+ozrukPHZaU3MDj/ENOH2bkixpi5U CDwDK07ln3u/eqk+M9Gxpxp9HCk4fn0PEW6qzEVVS0C80VTB29mRXeek3QKGI/XQwsE/nHenJim TRnNw X-Gm-Gg: AeBDieuQX3VKiWtbNW4ffhWNMjG8sNVCxAKMGnNlzF0KmRzShtOPZLIPiEayd+rWgyM 3rcJj+mBVoI+K9t70IOqXEvR3NfVzNlQISpuK0E8xf0HQzPY5uXZkcuiNh7rXwiV+9FOc9fHBKj GC8t5KH42+GCnj2pNtRXGGGLtbD4oG+2SQZHUhY2DpcW62V9ya9T2pGlXNlGTbjxlujJuxBmPq9 T+olfoSxTThNL1qFo28ld/0ebWo1xj9dakuvcvbpPqNQU9D4AGEewB8Whm9/b1ZPAMA4glISyTT x6gDipN3KYryScxKAnqkaeVtJDoB0wBe/eDESqLlcQIUN+MGsLyJzFkogh8ReRLmzrdMLfLo2sO R+AmDhjWLPgzQrHm9Gg9ATjcINHytgESdh2A/X6xtbba63m/rCYu8yJFMqHD8JRtEAbdLfjCaTm NJP94KDF18Du2+Kg9dGXCT6ha7nqNixa7tKpqSJMrIIX2UiS5V X-Received: by 2002:a5d:5f89:0:b0:43d:7dc2:b655 with SMTP id ffacd0b85a97d-4493dcd518emr5876156f8f.15.1777562379128; Thu, 30 Apr 2026 08:19:39 -0700 (PDT) Received: from [192.168.0.192] ([212.69.42.53]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-447b7ca5fe6sm14041963f8f.32.2026.04.30.08.19.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 30 Apr 2026 08:19:38 -0700 (PDT) Message-ID: Date: Thu, 30 Apr 2026 16:19:38 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v2] gdb: Add python support for demangling register unwind values To: Andrew Burgess , gdb-patches@sourceware.org Cc: Eli Zaretskii References: <20250410110426.3488955-1-craig.blackmore@embecosm.com> <20250508101721.2000793-1-craig.blackmore@embecosm.com> <87wmamotqb.fsf@redhat.com> Content-Language: en-US From: Craig Blackmore Autocrypt: addr=craig.blackmore@embecosm.com; keydata= xsBNBFdIF8oBCACwrsvc6YVfzJRT+ZoBfL9jEb8ITwNahDxCGSG6sIWrJ9UFeTwE8fnNhMpz RyFRm0OXruS5k/8YHJHrxKxFY9cgZ3CWNftXEjRqURUWGtN/ESiw0J7nVfhSGQTo3LBzpXZ1 0JHk4ZHKDJKYa+fhybCHOs19BfP3HydHoTlc5QTKMfom0X/xo7WDdwUYeZsjD9u8IzHk7gNw 05Abk1vqni+J7Fghjp4RI8W3IsjpKOfV3f02OyO/MTSraXNyejO4JRl0A8b3q1Lq+G6Z7o5n LVief5JpkRyzWQSawTIBKmRZa9EzAKZXd6IJdY/sZt7pTir5EP7MHq4a+AtKfKuDkrDDABEB AAHNLkNyYWlnIEJsYWNrbW9yZSA8Y3JhaWcuYmxhY2ttb3JlQGVtYmVjb3NtLmNvbT7CwHgE EwECACIFAldIF8oCGwMGCwkIBwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJEGEeRQLLl5WtydMH /1nYd9jmOBaF8w5gGgjF5eOO5b/cdUegmO///VYj/5R7iF/zbB6KgF0Obo5h2gG9AIfsZG+T ybuTx7oU1DZYEIndw+YP9c9Yi5de5UzEHwbJiV57W0n+MP0Widgw7p6XJmUQ1XbHxdcWp7nY EJa8ASKLuuIhO8JFUXbQ8BcUiWbsA/JxgCzeid8iixGrzPWj6iFzoK2mX4GqP+24pXSDUamM TXmSQd2taYEsyUdJNiEkUC51ncRcMuThjdtfn6Ok+7lHjh3Zz8q0keJz5pnIp4EXdkAgKSjq U42PMrd3v1HoIFINTtr5F23OdkxoQzysu4GMO4pkw5pwz95Uckr08ojOwE0EV0gXygEIAKv/ luYHmCG/qefgzdbnegwMdG5753NJ+zGxFltFX6aaOPZ8go9Omf6zwjybUKv6Qx6AlDanwCl3 ewVQs+h9iW8uaQBRgeDmwAGMG/doBiFqs7X0jBf23exMiJezXlKb2ZlKzMAbzJ87408AzRaV sZdwEpXHVi2mRPoXtMrqL5iQEyG5hdx2ySj5164DIgVOs/ypFiaiFaDPkIcAQTzJrxsbt6pf iI9kT93DO9nRKVV0pPWztV8P5gKM8HY2rS0wQcfrqAU6T89Aa0VFw92J+w5d2spF8MUNPsvR NLm9ooCF3YME9STYHXrNH1U9fJUWpIC+b49UoWSWRD9nwl2h2i8AEQEAAcLAXwQYAQIACQUC V0gXygIbDAAKCRBhHkUCy5eVreLbB/sHPs1xu78uNV8O4UPTX7D5zBBS3nsrbDr+8stmXRap xbvo6kqKzIMAXuO3bYB/NyJ/tFzuFr9Tjd/2g56D2186bp01/kgxJ9CEl/m2T3lG3DlxIoLg pCExzTLTb8zH/7/6mdeJ17cdnrK+2QAKYctReVPAC67cq5KmUyU3bv5e1JzhV4ezz/i/O+Jv el112ZEsa54ya9KZOUHbgAR6hLnRWIa+8yQTtXqYRc3LxLRfS80Wn0Err1YvqFYzJsQMC8ND xAeEuqQ1gfk1b0jmv7tYljNqsHqzGVbuWz6hyzyLv5GjcdSDKpbw/797gRKQSY8Gty5ynfUH O4kKyuZPrE8P In-Reply-To: <87wmamotqb.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Hi Andrew, Thanks for the review and apologies for the very slow reply as it took longer to get back to this than I'd hoped. I've responded inline with how I've addressed your comments and I will post a v3 patch shortly. On 12/05/2025 14:15, Andrew Burgess wrote: > Craig Blackmore writes: > >> This patch addresses the scenario where a register value is saved in >> some mangled form on the target and gdb does not have sufficient >> information to demangle it. > I'm assuming this is some kind of security related thing, e.g. return > address is hashed or "encrypted" in some way? Some of my thoughts below > are based on this assumption. Yes it's security related. >> After getting a register value from an unwinder, call out to extension >> languages to allow the value to be demangled. This avoids having to >> write a new unwinder and allows the standard unwinders to be used. >> >> This approach is purely for demangling values that have been read, there >> is no similar support provided for mangling values being written back to >> the target as this was not something that I needed to be able to do. > OK, but, presumably, if you did try to modify one of these demangled > registers, and then resumed the inferior, things would not continue as > expected? Attempting to modify a demangled register results in an "Attempt to assign to an unmodifiable value" error which is checked for in py-unwind-demangler.exp. > If this is true, and right now you don't need to write the value back. > Would it not be worth designing an API now that _could_ support this > functionality? > > I have no problem, in theory, with having this functionality in the API, > the problem is, once things are added to the Python API, we're usually > pretty conservative about changing them, so we can only add new > features. > > Now, sure, if, in the future, we want to support writing registers, we > can add a whole new, parallel, API that supports the writing case. But > wouldn't it be nice if we spent a little time thinking, up front, what > that writing case might look like, even if it's not implemented? Maybe > we can just document in this commit message what that support would look > like, and how it would fit into the existing API. I've changed `gdb.unwinder.UnwindRegisterValueDemangler` to `gdb.unwinder.UnwindRegisterValueTransformer` which is a base class for transformers that provide methods for transforming unwind register values.  Instead of calling `__call__` to demangle we now call `demangle`.  In future, other transform methods such as `mangle` could be added. >> Only one demangler can be registered at any one time. In future, the >> implementation could be extended to support multiple demanglers executed >> in order of priority. > Again, have you thought about how this might look? For the same reasons > as before, while for changes to GDB's C++ it's fine to do "just enough", > the Python API has a nasty habit of becoming locked in. > > I notice, for example, that register demanglers are created and > registered by creating a UnwindRegisterValueDemangler instance. In > contrast, instruct, many other similar functionality is done via a > gdb.register_* or gdb.*.register_ function which take a locus that > allows handlers to be attached to specific objfiles or program spaces. > > I think it might be a good idea if you switched to using this same > approach. Currently, these register functions accept a locus of `None` > to indicate that this should be a global registration. You could, for > example, throw an error if a locus other than `None` is used. But this > would then leave the door open, in future, to support other locus, which > would allow per-program-space registrations for example. I have changed the registration to be similar to python unwinder registration so that registering multiple transformers and registering to other loci can be supported in future. > I also wonder if you should be more inline with the existing register > functions when it comes to supporting multiple handlers, while at the > same time, not actually changing what you support? For example, other > register functions take a `replace` flag which controls whether existing > handlers are replaced. You could just support replacing a handler with > the same name, or adding a single handler, anything else: raise an > error. Whilst changing the registration approach, I added support for the `replace` flag with the behaviour you suggested. > This would be a little more work up front, but not significantly, and in > the future. The benefit is the API is consistent, and future proof. > I'm not sure how you'd extend the existing API to handle > per-program-space registrations, which I think should be a minimum > requirement. > >> The demangler is registered globally. In future, the implementation >> could be extended to register per program space and object file too. > See above. Or maybe outline how you'd see that future design being. See above. >> gdbpy_get_register_descriptor is now externally visible as it is used >> to create a gdb.RegisterDescriptor object to pass to the demangler. >> >> This patch adds gdb.Value.is_lval_{register,memory} variables so that >> the demangler can choose to skip modifying values that did not come >> directly from the target and may not need demangling, for example, >> values taken directly from DWARF. > I do wonder if this is the right approach for dealing with lval types. > If we ever want to handle different lval types in the future we're now > stuck adding yet more is_lval_* methods. > > Wouldn't a better approach be to add a read-only lval type attribute? I > don't think you'd even need to support all the types necessarily, you > could add constants for maybe just: not_lval, lval_memory, > lval_register, and map anything not caught be the above to not_lval, and > document that other lval types might be added in the future. Though I'm > happy to be convinced that separate flags is right way to go... I agree that's a better approach. I've made this change. >> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi >> index 7bb650347f7..7fd0ba4f83b 100644 >> --- a/gdb/doc/python.texi >> +++ b/gdb/doc/python.texi >> @@ -938,6 +938,16 @@ fetched when the value is needed, or when the @code{fetch_lazy} >> method is invoked. >> @end defvar >> >> +@defvar Value.is_lval_register >> +The value of this read-only boolean attribute is @code{True} if this >> +@code{gdb.Value} is from a register on the inferior. >> +@end defvar >> + >> +@defvar Value.is_lval_memory >> +The value of this read-only boolean attribute is @code{True} if this >> +@code{gdb.Value} is from inferior memory. >> +@end defvar >> + >> @defvar Value.bytes >> The value of this attribute is a @code{bytes} object containing the >> bytes that make up this @code{Value}'s complete value in little endian >> @@ -3160,6 +3170,44 @@ the matching unwinders are enabled. The @code{enabled} field of each >> matching unwinder is set to @code{True}. >> @end table >> >> +@subheading Unwind Register Value Demangler >> + >> +After getting a register value from an unwinder, @value{GDBN} will call >> +out to extension language demanglers to allow them to modify the value. >> +This is useful in case a register value needs demangling before >> +@value{GDBN} uses it and avoids the need to write a new unwinder. >> + >> +Currently, only one demangler can be registered at any one time and it >> +is registered globally. >> + >> +@subheading Skeleton Code for a Register Value Demangler >> + >> +Here is an example of how to structure a user created demangler: >> + >> +@smallexample >> +from gdb.unwinder import UnwindRegisterValueDemangler >> + >> +class MyUnwindRegisterValueDemangler(UnwindRegisterValueDemangler): >> + def __init__(self): >> + # Register self as the one and only demangler >> + super().__init__("MyUnwinder_Name") >> + >> + def __call__(self, frame_type, reg, value): > I'll add this comment here just because: I think it is a mistake to pass > through the `frame_type` here. I think you'd be better off passing the > gdb.Frame object itself, then let the demangled grab the frame type if > that's what it needs. This is a pretty small change, but gives the API > greater flexibility for the future. Done. >> + """ >> + Return new value if demangled, otherwise None. >> + """ >> + if frame_type != : >> + return None >> + >> + if reg.name == : >> + new_value = ... compute demangled value ... >> + return new_value >> + >> + return None >> + >> +my_demangler = MyUnwindRegisterValueDemangler() >> +@end smallexample >> + >> @node Xmethods In Python >> @subsubsection Xmethods In Python >> @cindex xmethods in Python > >> diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c >> index d43d7e97d99..6ec239664f5 100644 >> --- a/gdb/python/py-unwind.c >> +++ b/gdb/python/py-unwind.c >> @@ -30,6 +30,7 @@ >> #include "stack.h" >> #include "charset.h" >> #include "block.h" >> +#include "value.h" >> >> >> /* Debugging of Python unwinders. */ >> @@ -1181,3 +1182,82 @@ PyTypeObject unwind_info_object_type = >> 0, /* tp_init */ >> 0, /* tp_alloc */ >> }; >> + >> +/* Call demangler and handle the result. */ >> + >> +enum ext_lang_rc >> +gdbpy_demangle_unwind_register_value (frame_info_ptr frame, int regnum, >> + struct value **val) >> +{ >> + if (!gdb_python_initialized) >> + return EXT_LANG_RC_ERROR; > If you checkout gdbpy_print_insn you'll notice that it uses this: > > if (!gdb_python_initialized || !python_print_insn_enabled) > return {}; > > The `python_print_insn_enabled` is a C++ global that is set from Python > code when any handlers are registered. > > The point here is that for the common case, where no handler is > registered, GDB doesn't need to enter the Python environment, or create, > access, or setup, any Python state at all. > > This is a pretty easy thing to add, but my thinking at the time, is that > this will give a small improvement (of no Python work) for the common > case. Do you think you could look at how hard it would be to do > something similar in this case too please? Done. I've followed a similar approach to how `python_print_insn_enabled` is set / used. > >> + >> + gdbarch *gdbarch = get_frame_arch (frame); >> + gdbpy_enter enter_py (gdbarch); >> + >> + gdbpy_ref<> py_frame_type >> + = gdb_py_object_from_longest (get_frame_type (frame)); >> + >> + gdbpy_ref<> py_reg_obj (gdbpy_get_register_descriptor (gdbarch, regnum)); >> + if (py_reg_obj == NULL) > Remember s/NULL/nullptr/ throughout this patch. Done. >> + { >> + gdbpy_print_stack (); >> + return EXT_LANG_RC_ERROR; >> + } >> + >> + gdbpy_ref<> py_value_obj (value_to_value_object (*val)); >> + if (py_value_obj == NULL) >> + { >> + gdbpy_print_stack (); >> + return EXT_LANG_RC_ERROR; >> + } >> + >> + /* Call demangler. */ >> + const char *func_name = "_execute_unwind_register_value_demangler"; >> + if (gdb_python_module == NULL >> + || ! PyObject_HasAttrString (gdb_python_module, func_name)) >> + { >> + PyErr_SetString (PyExc_NameError, >> + "Installation error: gdb._execute_unwind_register_value_demangler " >> + "function is missing"); >> + gdbpy_print_stack (); >> + return EXT_LANG_RC_ERROR; >> + } >> + gdbpy_ref<> pyo_execute ( >> + PyObject_GetAttrString (gdb_python_module, func_name)); >> + if (pyo_execute == nullptr) >> + { >> + gdbpy_print_stack (); >> + return EXT_LANG_RC_ERROR; >> + } >> + >> + /* Demangler should return a gdb.Value if value has been changed, otherwise >> + None. */ >> + gdbpy_ref<> pyo_execute_ret >> + (PyObject_CallFunctionObjArgs (pyo_execute.get (), py_frame_type.get (), >> + py_reg_obj.get (), py_value_obj.get (), >> + NULL)); >> + if (pyo_execute_ret == nullptr) >> + { >> + gdbpy_print_stack (); >> + return EXT_LANG_RC_ERROR; >> + } >> + /* Use original value. */ >> + if (pyo_execute_ret == Py_None) >> + return EXT_LANG_RC_NOP; >> + >> + /* Verify the return value is a gdb.Value. */ >> + struct value *new_val = convert_value_from_python (pyo_execute_ret.get ()); >> + >> + if (new_val == nullptr) >> + { >> + gdbpy_print_stack (); >> + error (_("An unwind register value demangler should return a gdb.Value " >> + "instance if the value is changed, otherwise None")); >> + return EXT_LANG_RC_ERROR; > There's no need for a return after a call to error(). I also notice > that this error message doesn't show up in the testsuite, so it would > appear that this case isn't being tested maybe? I've removed the return and added a testcase. Thanks, Craig > Thanks, > Andrew > >> + } >> + /* Finally update val to the demangled value. */ >> + *val = new_val; >> + >> + return EXT_LANG_RC_OK; >> +}