gdb: use try/catch around a gdb_disassembler::print_insn call
authorAndrew Burgess <aburgess@redhat.com>
Sat, 6 Nov 2021 11:25:12 +0000 (11:25 +0000)
committerAndrew Burgess <aburgess@redhat.com>
Wed, 8 Dec 2021 13:26:33 +0000 (13:26 +0000)
commita5d8391846052cde835015c894237c799089c8cd
treea4faef1db7d2e0a1da79d259cb22d0a5af52789d
parent2bd64d21094f9527c9a6019f668c16ca897b2631
gdb: use try/catch around a gdb_disassembler::print_insn call

While investigating some disassembler problems I ran into this case;
GDB compiled on a 32-bit arm target, with --enable-targets=all.  Then
in GDB:

  (gdb) set architecture i386
  (gdb) disassemble 0x0,+4
  unknown disassembler error (error = -1)

This is interesting because it shows a case where the libopcodes
disassembler is returning -1 without first calling the
memory_error_func callback.  Indeed, the return from libopcodes
happens from this code snippet in i386-dis.c in the print_insn
function:

  if (address_mode == mode_64bit && sizeof (bfd_vma) < 8)
    {
      (*info->fprintf_func) (info->stream,
     _("64-bit address is disabled"));
      return -1;
    }

Notice how, prior to the return the disassembler tries to print a
helpful message out, but GDB doesn't print this message.

The reason this message goes missing is the call stack, it looks like
this:

  gdb_pretty_print_disassembler::pretty_print_insn
    gdb_disassembler::print_insn
      gdbarch_print_insn
        ...
          i386-dis.c:print_insn

When i386-dis.c:print_insn returns -1 this is handled in
gdb_disassembler::print_insn, where an exception is thrown.  However,
the actual printing of the disassembler output is done in
gdb_pretty_print_disassembler::pretty_print_insn, and is only done if
an exception is not thrown.

In this commit I change this.  The pretty_print_insn now uses
try/catch around the call to gdb_disassembler::print_insn, if we catch
an error then we first print any pending output in the instruction
buffer, before rethrowing the exception.  As a result, even if an
exception is thrown we still print any pending disassembler output to
the screen; in the above case the helpful message will now be shown.

Before my patch we might expect to see this output:

  (gdb) disassemble 0x0,+4
  Dump of assembler code from 0x0 to 0x4:
     0x0000000000000000: unknown disassembler error (error = -1)
  (gdb)

But now we see this:

  (gdb) disassemble 0x0,+4
  Dump of assembler code from 0x0 to 0x4:
     0x0000000000000000: 64-bit address is disabled
  unknown disassembler error (error = -1)

If the disassembler returns -1 without printing a helpful message then
we would still expect a change in output, something like:

  (gdb) disassemble 0x0,+4
  Dump of assembler code from 0x0 to 0x4:
     0x0000000000000000:
  unknown disassembler error (error = -1)

Which I think is still acceptable, though at this point I think a
strong case can be made that this is a disassembler bug (not printing
anything, but still returning -1).

Notice however, that the error message is always printed on a new line
now.  This is also true for the memory error case, where before we
might see this:

  (gdb) disassemble 0x0,+4
  Dump of assembler code from 0x0 to 0x4:
     0x00000000: Cannot access memory at address 0x0

We now get this:

  (gdb) disassemble 0x0,+4
  Dump of assembler code from 0x0 to 0x4:
     0x00000000:
  Cannot access memory at address 0x0

For me, I'm happy to accept this change, having the error on a line by
itself, rather than just appended to the end of the previous line,
seems like an improvement, but I'm aware others might feel
differently, so I'd appreciate any feedback.
gdb/disasm.c