From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id F18643858C62 for ; Mon, 28 Nov 2022 19:26:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F18643858C62 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669663568; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=eNiSyEFu9vwDWPv4+f8vsmeQoKIu3VmpwvasjFpGA/Y=; b=ZqxrnHSaxJNLZxjjT+GniX0lW9NvyO6RPDVLTliHPrbfQP6lHfYM96EypcqD4qoflrZB+D DGc0WhsYHNH6CnREKLFyc+OrROy6MkIomaJs1h+a4j6JI+PTKCvbCFQ5o/nhuKSOiscDVn HErcUrpMuIEafbgKFJMgOPXACTq7Lrs= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-635-dHVKA9jsNBeLoc_f2J8_jw-1; Mon, 28 Nov 2022 14:26:06 -0500 X-MC-Unique: dHVKA9jsNBeLoc_f2J8_jw-1 Received: by mail-wm1-f69.google.com with SMTP id c187-20020a1c35c4000000b003cfee3c91cdso6676062wma.6 for ; Mon, 28 Nov 2022 11:26:06 -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:message-id:date:references :in-reply-to:subject:to:from:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=eNiSyEFu9vwDWPv4+f8vsmeQoKIu3VmpwvasjFpGA/Y=; b=HNRoB23kU8noj1rHVAM8aehYCEf6zuA5W1Di9DmvQL8HuMc/M7KOIymx6qYpspoKK5 8w5bzAHrtxGNXRBBAd35enK+p3NZWxPPLsiX1bNAYp546TD754LvjR241XVrBIg0W2yE rM12xf64SNM6EZSiMBlwwmJzqejYZp3p00t2iNlZ2h3Lg7TTQ2Ku+4/oI1028T8e0t9h J3x0gt0wEify1L2tVhg6GYbsRR6xTwZcD1RwOMJZssvJVCOCvAG//NmkaVO91jGf/L13 ox/w86I9sWz7kIAC1M7jsKbhL6ea85j1295j9mkyTx5GXBIsqlc9tNflYvkefBOMs6qu 9Dyg== X-Gm-Message-State: ANoB5pmhL7v9G4A/t+LLC+DoDxVaLW+AkFaukh8upAShkPUCJB6N4AdC Ppe0gIcofYcrgP3DJCIJh+YoPfBYZW7sg76nD019lQut3xKIEZG/wXglRvXANFVmckLttLOmZc+ l+WIRho8Pl7dR1cTX0F3mTg== X-Received: by 2002:adf:eb90:0:b0:236:b8b9:b018 with SMTP id t16-20020adfeb90000000b00236b8b9b018mr33418576wrn.596.1669663565149; Mon, 28 Nov 2022 11:26:05 -0800 (PST) X-Google-Smtp-Source: AA0mqf5j1GgMX4Dg6he2RMSytDpX6sUpATverTpy37eh/gGoQmXWy7Vtah2RX47RCvlglgAeqDp8Zw== X-Received: by 2002:adf:eb90:0:b0:236:b8b9:b018 with SMTP id t16-20020adfeb90000000b00236b8b9b018mr33418566wrn.596.1669663564823; Mon, 28 Nov 2022 11:26:04 -0800 (PST) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id m16-20020a05600c3b1000b003cfd0bd8c0asm17693098wms.30.2022.11.28.11.26.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Nov 2022 11:26:03 -0800 (PST) From: Andrew Burgess To: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCHv2 1/3] gdb/python: avoid throwing an exception over libopcodes code In-Reply-To: References: <20221024125016.2823862-1-aburgess@redhat.com> <2924e27d733e1b398654bea3ae1e04b875e5fe43.1666884855.git.aburgess@redhat.com> Date: Mon, 28 Nov 2022 19:26:01 +0000 Message-ID: <87pmd62852.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 List-Id: Simon Marchi writes: > On 10/27/22 11:38, Andrew Burgess wrote: >> 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. >>=20 >> To explain what GDB is trying to do, consider the following trivial >> use of the Python disassembler API: >>=20 >> class ExampleDisassembler(gdb.disassembler.Disassembler): >>=20 >> class MyInfo(gdb.disassembler.DisassembleInfo): >> def __init__(self, info): >> super().__init__(info) >>=20 >> def read_memory(self, length, offset): >> return super().read_memory(length, offset) >>=20 >> def __init__(self): >> super().__init__("ExampleDisassembler") >>=20 >> def __call__(self, info): >> info =3D self.MyInfo(info) >> return gdb.disassembler.builtin_disassemble(info) >>=20 >> 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. >>=20 >> 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: >>=20 >> #0 gdbpy_disassembler::read_memory_func (memaddr=3D4198805, buff=3D0x= 7fff9ab9d2a8 "\220=D3=B9\232\377\177", len=3D1, info=3D0x7fff9ab9d558) at .= ./../src/gdb/python/py-disasm.c:510 >> #1 0x000000000104ba06 in fetch_data (info=3D0x7fff9ab9d558, addr=3D0x= 7fff9ab9d2a9 "=D3=B9\232\377\177") at ../../src/opcodes/i386-dis.c:305 >> #2 0x000000000104badb in ckprefix (ins=3D0x7fff9ab9d100) at ../../src= /opcodes/i386-dis.c:8571 >> #3 0x000000000104e28e in print_insn (pc=3D4198805, info=3D0x7fff9ab9d= 558, intel_syntax=3D-1) at ../../src/opcodes/i386-dis.c:9548 >> #4 0x000000000104f4d4 in print_insn_i386 (pc=3D4198805, info=3D0x7fff= 9ab9d558) at ../../src/opcodes/i386-dis.c:9949 >> #5 0x00000000004fa7ea in default_print_insn (memaddr=3D4198805, info= =3D0x7fff9ab9d558) at ../../src/gdb/arch-utils.c:1033 >> #6 0x000000000094fe5e in i386_print_insn (pc=3D4198805, info=3D0x7fff= 9ab9d558) at ../../src/gdb/i386-tdep.c:4072 >> #7 0x0000000000503d49 in gdbarch_print_insn (gdbarch=3D0x5335560, vma= =3D4198805, info=3D0x7fff9ab9d558) at ../../src/gdb/gdbarch.c:3351 >> #8 0x0000000000bcc8c6 in disasmpy_builtin_disassemble (self=3D0x7f2ab= 07f54d0, args=3D0x7f2ab0789790, kw=3D0x0) at ../../src/gdb/python/py-disasm= .c:324 >>=20 >> ### ... snip lots of frames as we pass through Python itself ... >>=20 >> #22 0x0000000000bcd860 in gdbpy_print_insn (gdbarch=3D0x5335560, memad= dr=3D0x401195, info=3D0x7fff9ab9e3c8) at ../../src/gdb/python/py-disasm.c:7= 83 >> #23 0x00000000008995a5 in ext_lang_print_insn (gdbarch=3D0x5335560, ad= dress=3D0x401195, info=3D0x7fff9ab9e3c8) at ../../src/gdb/extension.c:939 >> #24 0x0000000000741aaa in gdb_print_insn_1 (gdbarch=3D0x5335560, vma= =3D0x401195, info=3D0x7fff9ab9e3c8) at ../../src/gdb/disasm.c:1078 >> #25 0x0000000000741bab in gdb_disassembler::print_insn (this=3D0x7fff9= ab9e3c0, memaddr=3D0x401195, branch_delay_insns=3D0x0) at ../../src/gdb/dis= asm.c:1101 >>=20 >> 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. >>=20 >> If the user supplied Python code raises an gdb.MemoryError exception >> indicating the memory read failed, this this is fine. The C++ code > > "this this" > >> converts this exception back into a return value that libopcodes can >> understand, and returns to libopcodes. >>=20 >> 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. >>=20 >> 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. >>=20 >> 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. >>=20 >> This commit proposes an alternative solution that does not rely on >> throwing a C++ exception. >>=20 >> 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. >>=20 >> 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. >>=20 >> 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. >>=20 >> 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. > > After understanding this, I'm fine with the idea. You can add: > > Approved-By: Simon Marchi Thanks. I pushed this series. Andrew