Re: [PATCHv2 1/4] gdb: more debug output for displaced stepping

[Simon Marchi <simark@simark.ca>]

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 <puts@got.plt>
>   [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