[PATCH 2/2] gdb: use SCOPE_EXIT to write out disassembler instructions

[Andrew Burgess]

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 makes use of
SCOPE_EXIT to print the disassembler output.  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 | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index c045dfc94a6..29c85ea1c13 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -270,7 +270,31 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
     else
       m_uiout->text (":\t");
 
+    /* Clear the buffer into which we will disassemble the instruction,
+       then arrange to write this buffer out, followed by a newline, when
+       we exit this scope.
+
+       If the disassembler returns an error then the print_insn call below
+       will throw an exception.  However, it is possible, that, if the
+       disassembler returns an error it has still written something useful
+       into its output stream, for example, additional details about what
+       caused the error.
+
+       By always printing the instruction buffer, even on exception, we
+       ensure that these additional messages are displayed to the user.  */
     m_insn_stb.clear ();
+    SCOPE_EXIT {
+      m_uiout->field_stream ("inst", m_insn_stb);
+      m_uiout->text ("\n");
+    };
+
+    /* Now we can disassemble the instruction.  If the disassembler returns
+       a negative value this indicates an error and is handled within the
+       print_insn call, resulting in an exception being thrown.  Returning
+       zero makes no sense, as this indicates we disassembled something
+       successfully, but it was something of no size?  */
+    size = m_di.print_insn (pc);
+    gdb_assert (size > 0);
 
     if (flags & DISASSEMBLY_RAW_INSN)
       {
@@ -282,7 +306,6 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
 	   write them out in a single go for the MI.  */
 	m_opcode_stb.clear ();
 
-	size = m_di.print_insn (pc);
 	end_pc = pc + size;
 
 	for (;pc < end_pc; ++pc)
@@ -295,12 +318,7 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn
 	m_uiout->field_stream ("opcodes", m_opcode_stb);
 	m_uiout->text ("\t");
       }
-    else
-      size = m_di.print_insn (pc);
-
-    m_uiout->field_stream ("inst", m_insn_stb);
   }
-  m_uiout->text ("\n");
 
   return size;
 }
-- 
2.25.4