From patchwork Fri Nov 18 16:57:30 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 60848 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2415F384D9B6 for ; Fri, 18 Nov 2022 16:58:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2415F384D9B6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1668790692; bh=+k+u8SnGHRwPjpEQAdcymDgECMW3aW+e29n+L5QemtQ=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=Ual3O4ocmsFAy9xSg7GWWKOk49knIR8MVy2pU+jUy7Ge+mTmXzDW9BdnVlBVcsNRq RuF2REt0pOwMAllReU/lg7g9JtiSGNHKEJanlPPmokJVkcbAsfihpWdgBRXFcBFyfz gzHDRavEkWZGBZAvU+E802yroB3308wZagNRBMj0= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id A5FEF3857C51 for ; Fri, 18 Nov 2022 16:57:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A5FEF3857C51 Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-70-0-X1cnPLOjKkV7TY_OzZpw-1; Fri, 18 Nov 2022 11:57:39 -0500 X-MC-Unique: 0-X1cnPLOjKkV7TY_OzZpw-1 Received: by mail-wr1-f69.google.com with SMTP id q2-20020adfab02000000b00241b8f7efc5so1634770wrc.19 for ; Fri, 18 Nov 2022 08:57:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+k+u8SnGHRwPjpEQAdcymDgECMW3aW+e29n+L5QemtQ=; b=uIV0TO4oYOGK6yz+hfAoPv2C/sAGpw/ISwNF5pO0Yg/22gnbPQQFue2CgYZaCvI+NI FJlGchgitXDxvUUGbnVwV0vGsmlfS+EuGWOW677/o9pP1n+beLNFuw+s3ni3+NT6IigU haqysRZShZ99O5gB+7Zphxy5YWF10D8Q7ea9HkxeA9segucqFkfb1+A/egTj9d0/WmGE 6zAQ9oTwR4QUqjV4qA0JLhdI91cRoEQ58OD4vSo2Rp6Y7kRqLkf4jzT1oNmHub64o07C HM6XyRagffeefnTl3+DvuBwzxvGP6Qoqri0Mfj4RbzA4BbU3hMAWAmfQ9gj6cKNZR+iY qJOQ== X-Gm-Message-State: ANoB5pl/26qOQDwOGO/DJdUXrZF4YTs9DPpIDv0Qc1VZEchUVJQRL4ha gcFot5BRrvWr8RvRV1OVLuhG4YykbADFoo7ocDec7OzFhWbAdRKbJ6BQnFW4ARKv3qhBBlWIJ5U XqiA2ZWUIcvxScO4orcY1k99hduu4X3lrHsxMC4C++WvxoOa42QtwFf3ZbVs8bAfTvPgE7gTHmg == X-Received: by 2002:a05:600c:1f1b:b0:3c5:a867:e59f with SMTP id bd27-20020a05600c1f1b00b003c5a867e59fmr8736773wmb.146.1668790657608; Fri, 18 Nov 2022 08:57:37 -0800 (PST) X-Google-Smtp-Source: AA0mqf62HvvwcdTMBEvo+VjpirTvhi5M6UWIOd5OqpALBMV/QahdQCpsKGcuYPGggTdCnK9HFR9MvA== X-Received: by 2002:a05:600c:1f1b:b0:3c5:a867:e59f with SMTP id bd27-20020a05600c1f1b00b003c5a867e59fmr8736751wmb.146.1668790657179; Fri, 18 Nov 2022 08:57:37 -0800 (PST) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id a8-20020a5d5088000000b00236863c02f5sm3884243wrt.96.2022.11.18.08.57.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Nov 2022 08:57:36 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv3 1/3] gdb/python: avoid throwing an exception over libopcodes code Date: Fri, 18 Nov 2022 16:57:30 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" Bug gdb/29712 identifies a problem with the Python disassembler API. In some cases GDB will try to throw an exception through the libopcodes disassembler code, however, not all targets include exception unwind information when compiling C code, for targets that don't include this information GDB will terminate when trying to pass the exception through libopcodes. To explain what GDB is trying to do, consider the following trivial use of the Python disassembler API: class ExampleDisassembler(gdb.disassembler.Disassembler): class MyInfo(gdb.disassembler.DisassembleInfo): def __init__(self, info): super().__init__(info) def read_memory(self, length, offset): return super().read_memory(length, offset) def __init__(self): super().__init__("ExampleDisassembler") def __call__(self, info): info = self.MyInfo(info) return gdb.disassembler.builtin_disassemble(info) This disassembler doesn't add any value, it defers back to GDB to do all the actual work, but it serves to allow us to discuss the problem. The problem occurs when a Python exception is raised by the MyInfo.read_memory method. The MyInfo.read_memory method is called from the C++ function gdbpy_disassembler::read_memory_func. The C++ stack at the point this function is called looks like this: #0 gdbpy_disassembler::read_memory_func (memaddr=4198805, buff=0x7fff9ab9d2a8 "\220ӹ\232\377\177", len=1, info=0x7fff9ab9d558) at ../../src/gdb/python/py-disasm.c:510 #1 0x000000000104ba06 in fetch_data (info=0x7fff9ab9d558, addr=0x7fff9ab9d2a9 "ӹ\232\377\177") at ../../src/opcodes/i386-dis.c:305 #2 0x000000000104badb in ckprefix (ins=0x7fff9ab9d100) at ../../src/opcodes/i386-dis.c:8571 #3 0x000000000104e28e in print_insn (pc=4198805, info=0x7fff9ab9d558, intel_syntax=-1) at ../../src/opcodes/i386-dis.c:9548 #4 0x000000000104f4d4 in print_insn_i386 (pc=4198805, info=0x7fff9ab9d558) at ../../src/opcodes/i386-dis.c:9949 #5 0x00000000004fa7ea in default_print_insn (memaddr=4198805, info=0x7fff9ab9d558) at ../../src/gdb/arch-utils.c:1033 #6 0x000000000094fe5e in i386_print_insn (pc=4198805, info=0x7fff9ab9d558) at ../../src/gdb/i386-tdep.c:4072 #7 0x0000000000503d49 in gdbarch_print_insn (gdbarch=0x5335560, vma=4198805, info=0x7fff9ab9d558) at ../../src/gdb/gdbarch.c:3351 #8 0x0000000000bcc8c6 in disasmpy_builtin_disassemble (self=0x7f2ab07f54d0, args=0x7f2ab0789790, kw=0x0) at ../../src/gdb/python/py-disasm.c:324 ### ... snip lots of frames as we pass through Python itself ... #22 0x0000000000bcd860 in gdbpy_print_insn (gdbarch=0x5335560, memaddr=0x401195, info=0x7fff9ab9e3c8) at ../../src/gdb/python/py-disasm.c:783 #23 0x00000000008995a5 in ext_lang_print_insn (gdbarch=0x5335560, address=0x401195, info=0x7fff9ab9e3c8) at ../../src/gdb/extension.c:939 #24 0x0000000000741aaa in gdb_print_insn_1 (gdbarch=0x5335560, vma=0x401195, info=0x7fff9ab9e3c8) at ../../src/gdb/disasm.c:1078 #25 0x0000000000741bab in gdb_disassembler::print_insn (this=0x7fff9ab9e3c0, memaddr=0x401195, branch_delay_insns=0x0) at ../../src/gdb/disasm.c:1101 So gdbpy_disassembler::read_memory_func is called from the libopcodes disassembler to read memory, this C++ function then calls into user supplied Python code to do the work. If the user supplied Python code raises an gdb.MemoryError exception indicating the memory read failed, this this is fine. The C++ code converts this exception back into a return value that libopcodes can understand, and returns to libopcodes. However, if the user supplied Python code raises some other exception, what we want is for this exception to propagate through GDB and appear as if raised by the call to gdb.disassembler.builtin_disassemble. To achieve this, when gdbpy_disassembler::read_memory_func spots an unknown Python exception, we must pass the information about this exception from frame #0 to frame #8 in the above backtrace. Frame #8 is the C++ implementation of gdb.disassembler.builtin_disassemble, and so it is this function that we want to re-raise the unknown Python exception, so the user can, if they want, catch the exception in their code. The previous mechanism by which the exception was passed was to pack the details of the Python exception into a C++ exception, then throw the exception from frame #0, and catch the exception in frame #8, unpack the details of the Python exception, and re-raise it. However, this relies on the exception passing through frames #1 to #7, some of which are in libopcodes, which is C code, and so, might not be compiled with exception support. This commit proposes an alternative solution that does not rely on throwing a C++ exception. When we spot an unhandled Python exception in frame #0, we will store the details of this exception within the gdbpy_disassembler object currently in use. Then we return to libopcodes a value indicating that the memory_read failed. libopcodes will now continue to disassemble as though that memory read failed (with one special case described below), then, when we eventually return to disasmpy_builtin_disassemble we check to see if there is an exception stored in the gdbpy_disassembler object. If there is then this exception can immediately be installed, and then we return back to Python, when the user will be able to catch the exception. There is one extra change in gdbpy_disassembler::read_memory_func. After the first call that results in an exception being stored on the gdbpy_disassembler object, any future calls to the ::read_memory_func function will immediately return as if the read failed. This avoids any additional calls into user supplied Python code. My thinking here is that should the first call fail with some unknown error, GDB should not keep trying with any additional calls. This maintains the illusion that the exception raised from MyInfo.read_memory is immediately raised by gdb.disassembler.builtin_disassemble. I have no tests for this change though - to trigger this issue would rely on a libopcodes disassembler that will try to read further memory even after the first failed read. I'm not aware of any such disassembler that currently does this, but that doesn't mean such a disassembler couldn't exist in the future. With this change in place the gdb.python/py-disasm.exp test should now pass on AArch64. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29712 --- gdb/python/py-disasm.c | 79 +++++++++++++++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 16 deletions(-) diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c index c37452fcf72..1d460997831 100644 --- a/gdb/python/py-disasm.c +++ b/gdb/python/py-disasm.c @@ -122,6 +122,28 @@ struct gdbpy_disassembler : public gdb_printing_disassembler return m_string_file.release (); } + /* If there is a Python exception stored in this disassembler then + restore it (i.e. set the PyErr_* state), clear the exception within + this disassembler, and return true. There must be no current + exception set (i.e. !PyErr_Occurred()) when this function is called, + as any such exception might get lost. + + Otherwise, there is no exception stored in this disassembler, return + false. */ + bool restore_exception () + { + gdb_assert (!PyErr_Occurred ()); + if (m_stored_exception.has_value ()) + { + gdbpy_err_fetch ex = std::move (*m_stored_exception); + m_stored_exception.reset (); + ex.restore (); + return true; + } + + return false; + } + private: /* Where the disassembler result is written. */ @@ -138,6 +160,25 @@ struct gdbpy_disassembler : public gdb_printing_disassembler memory source object then a pointer to the object is placed in here, otherwise, this field is nullptr. */ PyObject *m_memory_source; + + /* Move the exception EX into this disassembler object. */ + void store_exception (gdbpy_err_fetch &&ex) + { + /* The only calls to store_exception are from read_memory_func, which + will return early if there's already an exception stored. */ + gdb_assert (!m_stored_exception.has_value ()); + m_stored_exception.emplace (std::move (ex)); + } + + /* Return true if there is an exception stored in this disassembler. */ + bool has_stored_exception () const + { + return m_stored_exception.has_value (); + } + + /* Store a single exception. This is used to pass Python exceptions back + from ::memory_read to disasmpy_builtin_disassemble. */ + gdb::optional m_stored_exception; }; /* Return true if OBJ is still valid, otherwise, return false. A valid OBJ @@ -288,20 +329,15 @@ disasmpy_builtin_disassemble (PyObject *self, PyObject *args, PyObject *kw) the disassembled instruction, or -1 if there was a memory-error encountered while disassembling. See below more more details on handling of -1 return value. */ - int length; - try - { - length = gdbarch_print_insn (disasm_info->gdbarch, disasm_info->address, + int length = gdbarch_print_insn (disasm_info->gdbarch, disasm_info->address, disassembler.disasm_info ()); - } - catch (gdbpy_err_fetch &pyerr) - { - /* Reinstall the Python exception held in PYERR. This clears to - pointers held in PYERR, hence the need to catch as a non-const - reference. */ - pyerr.restore (); - return nullptr; - } + + /* It is possible that, while calling a user overridden memory read + function, a Python exception was raised that couldn't be + translated into a standard memory-error. In this case the first such + exception is stored in the disassembler and restored here. */ + if (disassembler.restore_exception ()) + return nullptr; if (length == -1) { @@ -483,6 +519,14 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff, = static_cast (info->application_data); disasm_info_object *obj = dis->py_disasm_info (); + /* If a previous read attempt resulted in an exception, then we don't + allow any further reads to succeed. We only do this check for the + read_memory_func as this is the only one the user can hook into, + thus, this check prevents us calling back into user code if a + previous call has already thrown an error. */ + if (dis->has_stored_exception ()) + return -1; + /* The DisassembleInfo.read_memory method expects an offset from the address stored within the DisassembleInfo object; calculate that offset here. */ @@ -513,7 +557,8 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff, exception and throw it, this will then be caught in disasmpy_builtin_disassemble, at which point the exception will be restored. */ - throw gdbpy_err_fetch (); + dis->store_exception (gdbpy_err_fetch ()); + return -1; } /* Convert the result to a buffer. */ @@ -523,7 +568,8 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff, { PyErr_Format (PyExc_TypeError, _("Result from read_memory is not a buffer")); - throw gdbpy_err_fetch (); + dis->store_exception (gdbpy_err_fetch ()); + return -1; } /* Wrap PY_BUFF so that it is cleaned up correctly at the end of this @@ -536,7 +582,8 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff, PyErr_Format (PyExc_ValueError, _("Buffer returned from read_memory is sized %d instead of the expected %d"), py_buff.len, len); - throw gdbpy_err_fetch (); + dis->store_exception (gdbpy_err_fetch ()); + return -1; } /* Copy the data out of the Python buffer and return success. */ From patchwork Fri Nov 18 16:57:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 60850 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 105A5385222B for ; Fri, 18 Nov 2022 16:58:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 105A5385222B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1668790719; bh=YaSdimHe/Z0Gvpd/Mh+bh4FqcfBPkcYt5ZwWmPUs+Dc=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=b+i7lJPkHUpT34NJ6g/ZLaHeHA928a/Xz1/LXGWHngT6S9mE2rreKeiHPL3LKZZe4 3wEJcf6kkBfRp5IXohxax2X6hhEDB1EUV1LCIFINNELVL10mMOdxub1pAV7dT3UlwZ HxkEcYuxqDUIgLxilxPQWBmn+Z/VEUsGMA/ZIKA8= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 4A1373854579 for ; Fri, 18 Nov 2022 16:57:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4A1373854579 Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-630-FWRTM5qlM4Cm6pcGPP1bdw-1; Fri, 18 Nov 2022 11:57:40 -0500 X-MC-Unique: FWRTM5qlM4Cm6pcGPP1bdw-1 Received: by mail-wr1-f70.google.com with SMTP id v12-20020adfa1cc000000b00236eaee7197so1793271wrv.0 for ; Fri, 18 Nov 2022 08:57:40 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YaSdimHe/Z0Gvpd/Mh+bh4FqcfBPkcYt5ZwWmPUs+Dc=; b=Gv6RBFEm/g8pJt4tPb9nvn5R6lGpJI7SiFPJeAjjgTa71J8evpwKQ5+bWEahGV3Zvr xQWLqaHiU+wJRHut3P0AHIO8AUHGNDCeuLIAICBTYg/MV0kKlVAkip+TdeD0y0USiL2c cq3Ym0kLsu6Dlm9qIiXJZ8BfLz6GYqOLF/H+wvxUZwLEOUPOM0yt1NXQ6ZQUQ0+Oo4cv 7cfpvu7wwEovx125yQS4S3pes4i+RTkfgTKTpjFtZSNQBLPdepVqVyuXdW49ag941foU KQBS0ZGKjztjYda2gfjsNjeQJJhqu/qrpXz6PtYS5E+h5QWzZ6goMCwpMieIoL3sZeQR 6ZBQ== X-Gm-Message-State: ANoB5plF3wWclX+NxS2dEIIAaX7uvrkzvn0Cwj+s1GLbbtGecs4apcuQ cZuNQOyi5J9CahFNTPMQeNl6I2v6RTmV8/IFYQw4toZLGjIzpq7M/EXSA8Lf4gEUBBEdrd1c6O3 V6esAmnshXNfSJjjmcIqA5YEPGZfDcIZZ4RBl/G4pcALztWdnj3Kkgo1a9VQYIFmwuddf1IFZsw == X-Received: by 2002:a05:600c:2296:b0:3cf:baa6:8ca5 with SMTP id 22-20020a05600c229600b003cfbaa68ca5mr8932793wmf.178.1668790659232; Fri, 18 Nov 2022 08:57:39 -0800 (PST) X-Google-Smtp-Source: AA0mqf5wxKW6ZzQXu88/kTVkMLQH9azz83Se7hVXQr9+Y9E/+dM0+PBPZiSzbySEOX0u6FmI7YrZ/w== X-Received: by 2002:a05:600c:2296:b0:3cf:baa6:8ca5 with SMTP id 22-20020a05600c229600b003cfbaa68ca5mr8932767wmf.178.1668790658730; Fri, 18 Nov 2022 08:57:38 -0800 (PST) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id c3-20020adffb43000000b00225307f43fbsm3922488wrs.44.2022.11.18.08.57.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Nov 2022 08:57:38 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv3 2/3] gdb/disasm: mark functions passed to the disassembler noexcept Date: Fri, 18 Nov 2022 16:57:31 +0000 Message-Id: <2513c776a59b7d7e1446a81c10157bc40e3e8ba3.1668790576.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" While working on another patch, Simon pointed out that GDB could be improved by marking the functions passed to the disassembler as noexcept. https://sourceware.org/pipermail/gdb-patches/2022-October/193084.html The reason this is important is the on some hosts, libopcodes, being C code, will not be compiled with support for handling exceptions. As such, an attempt to throw an exception over libopcodes code will cause GDB to terminate. See bug gdb/29712 for an example of when this happened. In this commit all the functions that are passed to the disassembler, and which might be used as callbacks by libopcodes are marked noexcept. Ideally, I would have liked to change these typedefs: using read_memory_ftype = decltype (disassemble_info::read_memory_func); using memory_error_ftype = decltype (disassemble_info::memory_error_func); using print_address_ftype = decltype (disassemble_info::print_address_func); using fprintf_ftype = decltype (disassemble_info::fprintf_func); using fprintf_styled_ftype = decltype (disassemble_info::fprintf_styled_func); which are declared in disasm.h, as including the noexcept keyword. However, when I tried this, I ran into this warning/error: In file included from ../../src/gdb/disasm.c:25: ../../src/gdb/disasm.h: In constructor ‘gdb_printing_disassembler::gdb_printing_disassembler(gdbarch*, ui_file*, gdb_disassemble_info::read_memory_ftype, gdb_disassemble_info::memory_error_ftype, gdb_disassemble_info::print_address_ftype)’: ../../src/gdb/disasm.h:116:3: error: mangled name for ‘gdb_printing_disassembler::gdb_printing_disassembler(gdbarch*, ui_file*, gdb_disassemble_info::read_memory_ftype, gdb_disassemble_info::memory_error_ftype, gdb_disassemble_info::print_address_ftype)’ will change in C++17 because the exception specification is part of a function type [-Werror=noexcept-type] 116 | gdb_printing_disassembler (struct gdbarch *gdbarch, | ^~~~~~~~~~~~~~~~~~~~~~~~~ So I've left that change out. This does mean that if somebody adds a new use of the disassembler classes in the future, and forgets to mark the callbacks as noexcept, this will compile fine. We'll just have to manually check for that during review. --- gdb/disasm-selftests.c | 5 +++-- gdb/disasm.c | 25 +++++++++++++------------ gdb/disasm.h | 22 ++++++++++++++-------- gdb/guile/scm-disasm.c | 2 +- gdb/python/py-disasm.c | 12 ++++++------ 5 files changed, 37 insertions(+), 29 deletions(-) diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c index db2d1e0ac59..10df1a1980d 100644 --- a/gdb/disasm-selftests.c +++ b/gdb/disasm-selftests.c @@ -234,7 +234,8 @@ print_one_insn_test (struct gdbarch *gdbarch) size_t m_len; static int read_memory (bfd_vma memaddr, gdb_byte *myaddr, - unsigned int len, struct disassemble_info *info) + unsigned int len, + struct disassemble_info *info) noexcept { gdb_disassembler_test *self = static_cast(info->application_data); @@ -296,7 +297,7 @@ memory_error_test (struct gdbarch *gdbarch) static int read_memory (bfd_vma memaddr, gdb_byte *myaddr, unsigned int len, - struct disassemble_info *info) + struct disassemble_info *info) noexcept { /* Always return an error. */ return -1; diff --git a/gdb/disasm.c b/gdb/disasm.c index 60f95c398a9..e64cf69b250 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -191,7 +191,7 @@ line_has_code_p (htab_t table, struct symtab *symtab, int line) int gdb_disassembler_memory_reader::dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr, unsigned int len, - struct disassemble_info *info) + struct disassemble_info *info) noexcept { return target_read_code (memaddr, myaddr, len); } @@ -199,8 +199,8 @@ gdb_disassembler_memory_reader::dis_asm_read_memory /* Wrapper of memory_error. */ void -gdb_disassembler::dis_asm_memory_error (int err, bfd_vma memaddr, - struct disassemble_info *info) +gdb_disassembler::dis_asm_memory_error + (int err, bfd_vma memaddr, struct disassemble_info *info) noexcept { gdb_disassembler *self = static_cast(info->application_data); @@ -211,8 +211,8 @@ gdb_disassembler::dis_asm_memory_error (int err, bfd_vma memaddr, /* Wrapper of print_address. */ void -gdb_disassembler::dis_asm_print_address (bfd_vma addr, - struct disassemble_info *info) +gdb_disassembler::dis_asm_print_address + (bfd_vma addr, struct disassemble_info *info) noexcept { gdb_disassembler *self = static_cast(info->application_data); @@ -256,7 +256,7 @@ gdb_printing_disassembler::stream_from_gdb_disassemble_info (void *dis_info) int gdb_printing_disassembler::fprintf_func (void *dis_info, - const char *format, ...) + const char *format, ...) noexcept { ui_file *stream = stream_from_gdb_disassemble_info (dis_info); @@ -272,9 +272,9 @@ gdb_printing_disassembler::fprintf_func (void *dis_info, /* See disasm.h. */ int -gdb_printing_disassembler::fprintf_styled_func (void *dis_info, - enum disassembler_style style, - const char *format, ...) +gdb_printing_disassembler::fprintf_styled_func + (void *dis_info, enum disassembler_style style, + const char *format, ...) noexcept { ui_file *stream = stream_from_gdb_disassemble_info (dis_info); gdb_printing_disassembler *dis = (gdb_printing_disassembler *) dis_info; @@ -1220,8 +1220,8 @@ gdb_insn_length (struct gdbarch *gdbarch, CORE_ADDR addr) /* See disasm.h. */ int -gdb_non_printing_disassembler::null_fprintf_func (void *stream, - const char *format, ...) +gdb_non_printing_disassembler::null_fprintf_func + (void *stream, const char *format, ...) noexcept { return 0; } @@ -1230,7 +1230,8 @@ gdb_non_printing_disassembler::null_fprintf_func (void *stream, int gdb_non_printing_disassembler::null_fprintf_styled_func - (void *stream, enum disassembler_style style, const char *format, ...) + (void *stream, enum disassembler_style style, + const char *format, ...) noexcept { return 0; } diff --git a/gdb/disasm.h b/gdb/disasm.h index b7d16744c20..58c6c623098 100644 --- a/gdb/disasm.h +++ b/gdb/disasm.h @@ -51,7 +51,13 @@ struct gdb_disassemble_info protected: - /* Types for the function callbacks within m_di. */ + /* Types for the function callbacks within m_di. It would be nice if + these function types were all defined to include the noexcept + keyword, as every implementation of these must be noexcept. However, + using noexcept within a function typedef like this is a C++17 + feature, trying to do this for earlier C++ versions results in a + warning from GCC/Clang, and the noexcept isn't checked. After we + move to C++17 these should be updated to add noexcept. */ using read_memory_ftype = decltype (disassemble_info::read_memory_func); using memory_error_ftype = decltype (disassemble_info::memory_error_func); using print_address_ftype = decltype (disassemble_info::print_address_func); @@ -127,7 +133,7 @@ struct gdb_printing_disassembler : public gdb_disassemble_info /* Callback used as the disassemble_info's fprintf_func callback. The DIS_INFO pointer is a pointer to a gdb_printing_disassembler object. Content is written to the m_stream extracted from DIS_INFO. */ - static int fprintf_func (void *dis_info, const char *format, ...) + static int fprintf_func (void *dis_info, const char *format, ...) noexcept ATTRIBUTE_PRINTF(2,3); /* Callback used as the disassemble_info's fprintf_styled_func callback. @@ -135,7 +141,7 @@ struct gdb_printing_disassembler : public gdb_disassemble_info object. Content is written to the m_stream extracted from DIS_INFO. */ static int fprintf_styled_func (void *dis_info, enum disassembler_style style, - const char *format, ...) + const char *format, ...) noexcept ATTRIBUTE_PRINTF(3,4); /* Return true if the disassembler is considered inside a comment, false @@ -187,14 +193,14 @@ struct gdb_non_printing_disassembler : public gdb_disassemble_info /* Callback used as the disassemble_info's fprintf_func callback, this doesn't write anything to STREAM, but just returns 0. */ - static int null_fprintf_func (void *stream, const char *format, ...) + static int null_fprintf_func (void *stream, const char *format, ...) noexcept ATTRIBUTE_PRINTF(2,3); /* Callback used as the disassemble_info's fprintf_styled_func callback, , this doesn't write anything to STREAM, but just returns 0. */ static int null_fprintf_styled_func (void *stream, enum disassembler_style style, - const char *format, ...) + const char *format, ...) noexcept ATTRIBUTE_PRINTF(3,4); }; @@ -208,7 +214,7 @@ struct gdb_disassembler_memory_reader /* Implements the read_memory_func disassemble_info callback. */ static int dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr, unsigned int len, - struct disassemble_info *info); + struct disassemble_info *info) noexcept; }; /* A non-printing disassemble_info management class. The disassemble_info @@ -281,9 +287,9 @@ struct gdb_disassembler : public gdb_printing_disassembler, static bool use_ext_lang_colorization_p; static void dis_asm_memory_error (int err, bfd_vma memaddr, - struct disassemble_info *info); + struct disassemble_info *info) noexcept; static void dis_asm_print_address (bfd_vma addr, - struct disassemble_info *info); + struct disassemble_info *info) noexcept; /* Return true if we should use the extension language to apply disassembler styling. This requires disassembler styling to be on diff --git a/gdb/guile/scm-disasm.c b/gdb/guile/scm-disasm.c index 992a64b9331..c0957559136 100644 --- a/gdb/guile/scm-disasm.c +++ b/gdb/guile/scm-disasm.c @@ -106,7 +106,7 @@ gdbscm_disasm_read_memory_worker (void *datap) static int gdbscm_disasm_read_memory (bfd_vma memaddr, bfd_byte *myaddr, unsigned int length, - struct disassemble_info *dinfo) + struct disassemble_info *dinfo) noexcept { gdbscm_disassembler *self = static_cast (dinfo->application_data); diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c index 1d460997831..a25252b4306 100644 --- a/gdb/python/py-disasm.c +++ b/gdb/python/py-disasm.c @@ -101,12 +101,12 @@ struct gdbpy_disassembler : public gdb_printing_disassembler /* Callbacks used by disassemble_info. */ static void memory_error_func (int status, bfd_vma memaddr, - struct disassemble_info *info); + struct disassemble_info *info) noexcept; static void print_address_func (bfd_vma addr, - struct disassemble_info *info); + struct disassemble_info *info) noexcept; static int read_memory_func (bfd_vma memaddr, gdb_byte *buff, unsigned int len, - struct disassemble_info *info); + struct disassemble_info *info) noexcept; /* Return a reference to an optional that contains the address at which a memory error occurred. The optional will only have a value if a @@ -513,7 +513,7 @@ disasmpy_info_progspace (PyObject *self, void *closure) int gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff, unsigned int len, - struct disassemble_info *info) + struct disassemble_info *info) noexcept { gdbpy_disassembler *dis = static_cast (info->application_data); @@ -658,7 +658,7 @@ disasmpy_result_init (PyObject *self, PyObject *args, PyObject *kwargs) void gdbpy_disassembler::memory_error_func (int status, bfd_vma memaddr, - struct disassemble_info *info) + struct disassemble_info *info) noexcept { gdbpy_disassembler *dis = static_cast (info->application_data); @@ -669,7 +669,7 @@ gdbpy_disassembler::memory_error_func (int status, bfd_vma memaddr, void gdbpy_disassembler::print_address_func (bfd_vma addr, - struct disassemble_info *info) + struct disassemble_info *info) noexcept { gdbpy_disassembler *dis = static_cast (info->application_data); From patchwork Fri Nov 18 16:57:32 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 60849 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 89A603853D74 for ; Fri, 18 Nov 2022 16:58:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 89A603853D74 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1668790694; bh=5mYZ3SEMVaCedVDqcHzd/EyR3rD23SLkfhEFu9t0LhQ=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=EHlG02jUnhow4qgCxA+7lmlAkahWcAyvj+HlgURLDlqGTN5hgs5nkOtx+U5DEjf5F h6APohMYJkd/IaKKSCCpNV64hKWjoWopeo/TrhVySZssiyEE0rQb9X07uGInJb+vI6 qz5V0W5zvqRu3s1KdNrG9RyhNdGILygRDwkDvQKk= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id A796A385457F for ; Fri, 18 Nov 2022 16:57:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A796A385457F Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-338-_JytBC5kPoWgDk-g69kY_Q-1; Fri, 18 Nov 2022 11:57:42 -0500 X-MC-Unique: _JytBC5kPoWgDk-g69kY_Q-1 Received: by mail-wr1-f72.google.com with SMTP id w23-20020adf8bd7000000b002358f733307so1805853wra.17 for ; Fri, 18 Nov 2022 08:57:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5mYZ3SEMVaCedVDqcHzd/EyR3rD23SLkfhEFu9t0LhQ=; b=Oh9g/KTzm8hdxasscYAXvhtx/W5CerpgvqjOutC9MkK4K1dbLNtSRYkdci6Lm465Wf VLLtkRrzNNdBbOw/qVd22Q2nNwwIG/ZyLfCnSm52+Q0O5wFbmYfe40rY+mXqhpsRmIh+ rFjIIMe6aT5lrbwoG1Sln0pajU/olnm0IBVa324e4syN1WQHN9YY8CPrR8WCFtOX1BQJ g3iGPs0hPm3HOU9vYgQRapx44jml6kEjaWM3dVaGr4QNEEHXoKDu1QjTUA2E2yrpQ8vJ OjvciA8e2p40jkIfvxdf/3Xzq5s0KfOeWZPpEMq9tx/YEqz7IvrGTivIS38F1wGAd5M5 ssxQ== X-Gm-Message-State: ANoB5pkskenoBfygIXpEYJB6JlyuK+5/CbLoBe4i9d60bQfqwCHBBkMf F7tcTmD4yKqXiontbRPyJbgOBtpie96jlz4PHqIXMT/eaIMSGIPGa/57TMPgNarBvlEGQV9J1xB TPEvGRwOs7mfxiC1NdVcGd5KKvEZ+vxnTsm1rpwuOcZ39OG/a6jwICNVYV0gziEII853E+GArEA == X-Received: by 2002:a05:600c:4f51:b0:3b4:a6c4:70fb with SMTP id m17-20020a05600c4f5100b003b4a6c470fbmr9277819wmq.79.1668790660712; Fri, 18 Nov 2022 08:57:40 -0800 (PST) X-Google-Smtp-Source: AA0mqf6qvHGZJG/rldsfmyvfV2glvXARQB/BWhj5kKRxFMn3NlgIfO1ADbvkyWHYY6vPWameJXwPzw== X-Received: by 2002:a05:600c:4f51:b0:3b4:a6c4:70fb with SMTP id m17-20020a05600c4f5100b003b4a6c470fbmr9277797wmq.79.1668790660383; Fri, 18 Nov 2022 08:57:40 -0800 (PST) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id n27-20020a05600c3b9b00b003cfa81e2eb4sm5868569wms.38.2022.11.18.08.57.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Nov 2022 08:57:39 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv3 3/3] gdb: mark disassembler function callback types as noexcept Date: Fri, 18 Nov 2022 16:57:32 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" In disasm.h we define a set of types that are used by the various disassembler classes to hold callback functions before passing the callbacks into libopcodes. Because libopcodes is C code, and on some (many?) targets, C code is compiled without exception support, it is important that GDB not try to throw an exception over libopcode code. In the previous commit all the existing callbacks were marked as noexcept, however, this doesn't protect us from a future change to GDB either adding a new callback that is not noexcept, or removing the noexcept keyword from an existing callback. In this commit I mark all the callback types as noexcept. This means that GDB's disassembler classes will no longer compile if we try to pass a callback that is not marked as noexcept. At least, that's the idea. Unfortunately, it's not that easy. Before C++17, the noexcept keyword on a function typedef would be ignored, thus: using func_type = void (*) (void) noexcept; void a_func () { throw 123; } void some_func (func_type f) { f (); } int main () { some_func (a_func); return 0; } Will compile just fine for C++11 and C++14 with GCC. Clang on the other hand complains that 'noexcept' should not appear on function types, but then does appear to correctly complain that passing a_func is a mismatch in the set of exceptions that could be thrown. Switching to C++17 and both GCC and Clang correctly point out that passing a_func is an invalid conversion relating to the noexcept keyword. Changing a_func to: void a_func () noexcept { /* Nothing. */ } And for C++17 both GCC and Clang compile this just fine. My conclusion then is that adding the noexcept keyword to the function types is pointless while GDB is not compiled as C++17, and silencing the warnings would require us to jump through a bunch of hoops. And so, in this commit, I define a macro LIBOPCODE_CALLBACK_NOEXCEPT, this macro expands to noexcept when compiling for C++17, but otherwise expands to nothing. I then add this macro to the function types. I've compiled GDB as the default C++11 and also forced the compile to C++17. When compiling as C++17 I spotted a few additional places where callbacks needed to be marked noexcept (these fixes were merged into the previous commit, but this confirmed to be that the macro is working as expected). --- gdb/disasm.h | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/gdb/disasm.h b/gdb/disasm.h index 58c6c623098..d6aec9a4705 100644 --- a/gdb/disasm.h +++ b/gdb/disasm.h @@ -26,6 +26,12 @@ struct gdbarch; struct ui_out; struct ui_file; +#if __cplusplus >= 201703L +#define LIBOPCODE_CALLBACK_NOEXCEPT noexcept +#else +#define LIBOPCODE_CALLBACK_NOEXCEPT +#endif + /* A wrapper around a disassemble_info and a gdbarch. This is the core set of data that all disassembler sub-classes will need. This class doesn't actually implement the disassembling process, that is something @@ -51,18 +57,28 @@ struct gdb_disassemble_info protected: - /* Types for the function callbacks within m_di. It would be nice if - these function types were all defined to include the noexcept - keyword, as every implementation of these must be noexcept. However, - using noexcept within a function typedef like this is a C++17 - feature, trying to do this for earlier C++ versions results in a - warning from GCC/Clang, and the noexcept isn't checked. After we - move to C++17 these should be updated to add noexcept. */ - using read_memory_ftype = decltype (disassemble_info::read_memory_func); - using memory_error_ftype = decltype (disassemble_info::memory_error_func); - using print_address_ftype = decltype (disassemble_info::print_address_func); - using fprintf_ftype = decltype (disassemble_info::fprintf_func); - using fprintf_styled_ftype = decltype (disassemble_info::fprintf_styled_func); + /* Types for the function callbacks within m_di. The actual function + signatures here are taken from include/dis-asm.h. The noexcept macro + expands to 'noexcept' for C++17 and later, otherwise, it expands to + nothing. This is because including noexcept was ignored for function + types before C++17, but both GCC and Clang warn that the noexcept + will become relevant when you switch to C++17, and this warning + causes the build to fail. */ + using read_memory_ftype + = int (*) (bfd_vma, bfd_byte *, unsigned int, struct disassemble_info *) + LIBOPCODE_CALLBACK_NOEXCEPT; + using memory_error_ftype + = void (*) (int, bfd_vma, struct disassemble_info *) + LIBOPCODE_CALLBACK_NOEXCEPT; + using print_address_ftype + = void (*) (bfd_vma, struct disassemble_info *) + LIBOPCODE_CALLBACK_NOEXCEPT; + using fprintf_ftype + = int (*) (void *, const char *, ...) + LIBOPCODE_CALLBACK_NOEXCEPT; + using fprintf_styled_ftype + = int (*) (void *, enum disassembler_style, const char *, ...) + LIBOPCODE_CALLBACK_NOEXCEPT; /* Constructor, many fields in m_di are initialized from GDBARCH. The remaining arguments are function callbacks that are written into m_di.