From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 6BF593858D37 for ; Tue, 21 Mar 2023 14:45:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6BF593858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [172.16.0.192] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 16F611E0D3; Tue, 21 Mar 2023 10:45:14 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1679409915; bh=REw5ostxOHX1ag/gEjH9ecKPLMDtnh9MM9FuJB6E5qg=; h=Date:Subject:To:References:From:In-Reply-To:From; b=fP6uSj5UOO0fOxjDCKHVywrEqxp2OrRDgu5iju5vEaXU5SpUWUg6GgkIWx521FGTG p4bTTXpOpnB2KdBp8fEk21qNLaosjZq5zs3HKgYtu707guliYbU/GtPFeOn8iblzX+ PH4HrmsiDvwYBWxsffwHTsR0VevhQfKZovzzC0FE= Message-ID: <46b9208a-f051-786d-76db-956a3362e26b@simark.ca> Date: Tue, 21 Mar 2023 10:45:14 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [PATCHv2 1/4] gdb: more debug output for displaced stepping Content-Language: fr To: Andrew Burgess , gdb-patches@sourceware.org References: <09efdbfc7f9f0bf0402c222237c85f837b011082.1678984664.git.aburgess@redhat.com> From: Simon Marchi In-Reply-To: <09efdbfc7f9f0bf0402c222237c85f837b011082.1678984664.git.aburgess@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,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: On 3/16/23 12:39, Andrew Burgess via Gdb-patches wrote: > While investigating a displaced stepping issue I wanted an easy way to > see what GDB thought the original instruction was, and what > instruction GDB replaced that with when performing the displaced step. > > We do print out the address that is being stepped, so I can track down > the original instruction. > > And we do print out the bytes of the new instruction, so I can figure > out what the replacement instruction was, but it's not really easy. > > In this commit, within displaced_step_buffers::prepare, I add new > debug output which disassembles both the original instruction, and the > replacement instruction, and prints both these instructions, along > with the bytes that make these instructions, to the debug output. > > This new debug improved on some debug that was already present in > infrun.c, however, the old infrun.c debug (a) didn't actually > disassemble the instruction, it just printed the bytes, and (b) there > was an assumption that all instructions are 4-bytes long, which > clearly isn't correct. > > I think it makes more sense to have all this debug in the > displaced-stepping.c file, so I've removed the infrun.c code, and > added my new debug in displaced-stepping.c. > > Here's an example of what the output looks like on x86-64 (this is > with 'set debug displaced on'). The two interesting lines contain the > strings 'original insn' and 'replacement insn': > > (gdb) step > [displaced] displaced_step_prepare_throw: displaced-stepping 2073893.2073893.0 now > [displaced] prepare: selected buffer at 0x401052 > [displaced] prepare: saved 0x401052: 1e fa 31 ed 49 89 d1 5e 48 89 e2 48 83 e4 f0 50 > [displaced] prepare: original insn 0x401030: ff 25 e2 2f 00 00 jmp *0x2fe2(%rip) # 0x404018 > [displaced] fixup_riprel: %rip-relative addressing used. > [displaced] fixup_riprel: using temp reg 2, old value 0x7ffff7f8a578, new value 0x401036 > [displaced] amd64_displaced_step_copy_insn: copy 0x401030->0x401052: ff a1 e2 2f 00 00 68 00 00 00 00 e9 e0 ff ff ff > [displaced] prepare: replacement insn 0x401052: ff a1 e2 2f 00 00 jmp *0x2fe2(%rcx) > [displaced] displaced_step_prepare_throw: prepared successfully thread=2073893.2073893.0, original_pc=0x401030, displaced_pc=0x401052 > [displaced] finish: restored 2073893.2073893.0 0x401052 > [displaced] amd64_displaced_step_fixup: fixup (0x401030, 0x401052), insn = 0xff 0xa1 ... > [displaced] amd64_displaced_step_fixup: restoring reg 2 to 0x7ffff7f8a578 > 0x00007ffff7e402c0 in puts () from /lib64/libc.so.6 > (gdb) > > One final note. For many targets that support displaced stepping (in > fact all targets except ARM) the replacement instruction is always a > single instruction. But on ARM the replacement could actually be a > series of instructions. The debug code tries to handle this by > disassembling the entire displaced stepping buffer. Obviously this > might actually print more than is necessary, but there's (currently) > no easy way to know how many instructions to disassemble; that > knowledge is all locked in the architecture specific code. Still I > don't think it really hurts, if someone is looking at this debug then > hopefully they known what to expect. The obvious idea (although maybe not obvious to implement) is for the arch code to return the number of bytes it used in the buffer. > @@ -43,6 +44,24 @@ show_debug_displaced (struct ui_file *file, int from_tty, > gdb_printf (file, _("Displace stepping debugging is %s.\n"), value); > } > > +/* See displaced-stepping.h. */ > + > +std::string > +displaced_step_dump_bytes (const gdb_byte *buf, size_t len) > +{ > + std::string ret; > + > + for (size_t i = 0; i < len; i++) > + { > + if (i == 0) > + ret += string_printf ("%02x", buf[i]); > + else > + ret += string_printf (" %02x", buf[i]); > + } > + > + return ret; > +} Kinda unrelated to your change, but if you feel like it: this function isn't really specific to displaced stepping. I could see it living in some util file in gdbsupport, as "dump_bytes". It could also take a gdb::array_view. Simon