Add more styling to "disassemble"

Message ID 20190724222155.14274-1-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey July 24, 2019, 10:21 p.m. UTC
  This adds more styling to the disassemble command.  In particular,
addresses and function names in the disassembly are now styled.

This required fixing a small latent bug in set_output_style.  This
function always passed NULL to emit_style_escape; but when writing to
a file other than gdb_stdout, it should emit the style escape
directly.  (FWIW this is another argument for better integrating the
pager with ui_file and getting rid of this entire layer.)

gdb/ChangeLog
2019-07-24  Tom Tromey  <tom@tromey.com>

	* utils.c (set_output_style): Sometimes pass stream to
	emit_style_escape.
	* ui-out.h (class ui_out) <can_emit_style_escape>: Declare.
	* record-btrace.c (btrace_insn_history): Update.
	* mi/mi-out.h (class mi_ui_out) <can_emit_style_escape>: New
	method.
	* disasm.h (gdb_pretty_print_disassembler): Add uiout parameter.
	Update initializers.
	<m_uiout>: New field.
	<m_di>: Move lower.
	* disasm.c (gdb_pretty_print_disassembler::pretty_print_insn):
	Remove "uiout" parameter.
	(dump_insns): Update.
	* cli-out.h (class cli_ui_out) <can_emit_style_escape>: Declare.
	* cli-out.c (cli_ui_out::can_emit_style_escape): New method.

gdb/testsuite/ChangeLog
2019-07-24  Tom Tromey  <tom@tromey.com>

	* gdb.base/style.exp: Add disassemble test.
	* gdb.base/style.c (some_called_function): New function.
	(main): Use it.
---
 gdb/ChangeLog                    | 18 ++++++++++++
 gdb/cli-out.c                    |  6 ++++
 gdb/cli-out.h                    |  2 ++
 gdb/disasm.c                     | 49 ++++++++++++++++----------------
 gdb/disasm.h                     | 20 ++++++++-----
 gdb/mi/mi-out.h                  |  5 ++++
 gdb/record-btrace.c              |  4 +--
 gdb/testsuite/ChangeLog          |  6 ++++
 gdb/testsuite/gdb.base/style.c   |  7 ++++-
 gdb/testsuite/gdb.base/style.exp |  4 +++
 gdb/ui-out.h                     |  4 +++
 gdb/utils.c                      |  6 ++--
 12 files changed, 94 insertions(+), 37 deletions(-)
  

Comments

Tom Tromey Aug. 6, 2019, 5:15 p.m. UTC | #1
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> This adds more styling to the disassemble command.  In particular,
Tom> addresses and function names in the disassembly are now styled.

I'm checking this in now.

Tom
  

Patch

diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 75a2259ab0b..549d518d60f 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -316,6 +316,12 @@  cli_ui_out::set_stream (struct ui_file *stream)
   return old;
 }
 
+bool
+cli_ui_out::can_emit_style_escape () const
+{
+  return m_streams.back ()->can_emit_style_escape ();
+}
+
 /* CLI interface to display tab-completion matches.  */
 
 /* CLI version of displayer.crlf.  */
diff --git a/gdb/cli-out.h b/gdb/cli-out.h
index d28f468a3e7..f38c1cc0571 100644
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -32,6 +32,8 @@  public:
 
   ui_file *set_stream (ui_file *stream);
 
+  bool can_emit_style_escape () const override;
+
 protected:
 
   virtual void do_table_begin (int nbrofcols, int nr_rows,
diff --git a/gdb/disasm.c b/gdb/disasm.c
index e2b4fd63918..b79a39cc51f 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -191,8 +191,7 @@  compare_lines (const void *mle1p, const void *mle2p)
 /* See disasm.h.  */
 
 int
-gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
-						  const struct disasm_insn *insn,
+gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn,
 						  gdb_disassembly_flags flags)
 {
   /* parts of the symbolic representation of the address */
@@ -204,37 +203,37 @@  gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
   struct gdbarch *gdbarch = arch ();
 
   {
-    ui_out_emit_tuple tuple_emitter (uiout, NULL);
+    ui_out_emit_tuple tuple_emitter (m_uiout, NULL);
     pc = insn->addr;
 
     if (insn->number != 0)
       {
-	uiout->field_unsigned ("insn-number", insn->number);
-	uiout->text ("\t");
+	m_uiout->field_unsigned ("insn-number", insn->number);
+	m_uiout->text ("\t");
       }
 
     if ((flags & DISASSEMBLY_SPECULATIVE) != 0)
       {
 	if (insn->is_speculative)
 	  {
-	    uiout->field_string ("is-speculative", "?");
+	    m_uiout->field_string ("is-speculative", "?");
 
 	    /* The speculative execution indication overwrites the first
 	       character of the PC prefix.
 	       We assume a PC prefix length of 3 characters.  */
 	    if ((flags & DISASSEMBLY_OMIT_PC) == 0)
-	      uiout->text (pc_prefix (pc) + 1);
+	      m_uiout->text (pc_prefix (pc) + 1);
 	    else
-	      uiout->text ("  ");
+	      m_uiout->text ("  ");
 	  }
 	else if ((flags & DISASSEMBLY_OMIT_PC) == 0)
-	  uiout->text (pc_prefix (pc));
+	  m_uiout->text (pc_prefix (pc));
 	else
-	  uiout->text ("   ");
+	  m_uiout->text ("   ");
       }
     else if ((flags & DISASSEMBLY_OMIT_PC) == 0)
-      uiout->text (pc_prefix (pc));
-    uiout->field_core_addr ("address", gdbarch, pc);
+      m_uiout->text (pc_prefix (pc));
+    m_uiout->field_core_addr ("address", gdbarch, pc);
 
     std::string name, filename;
     if (!build_address_symbolic (gdbarch, pc, 0, &name, &offset, &filename,
@@ -242,16 +241,16 @@  gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
       {
 	/* We don't care now about line, filename and unmapped.  But we might in
 	   the future.  */
-	uiout->text (" <");
+	m_uiout->text (" <");
 	if ((flags & DISASSEMBLY_OMIT_FNAME) == 0)
-	  uiout->field_string ("func-name", name.c_str (),
-			       ui_out_style_kind::FUNCTION);
-	uiout->text ("+");
-	uiout->field_signed ("offset", offset);
-	uiout->text (">:\t");
+	  m_uiout->field_string ("func-name", name.c_str (),
+				 ui_out_style_kind::FUNCTION);
+	m_uiout->text ("+");
+	m_uiout->field_signed ("offset", offset);
+	m_uiout->text (">:\t");
       }
     else
-      uiout->text (":\t");
+      m_uiout->text (":\t");
 
     m_insn_stb.clear ();
 
@@ -275,15 +274,15 @@  gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
 	    spacer = " ";
 	  }
 
-	uiout->field_stream ("opcodes", m_opcode_stb);
-	uiout->text ("\t");
+	m_uiout->field_stream ("opcodes", m_opcode_stb);
+	m_uiout->text ("\t");
       }
     else
       size = m_di.print_insn (pc);
 
-    uiout->field_stream ("inst", m_insn_stb);
+    m_uiout->field_stream ("inst", m_insn_stb);
   }
-  uiout->text ("\n");
+  m_uiout->text ("\n");
 
   return size;
 }
@@ -299,13 +298,13 @@  dump_insns (struct gdbarch *gdbarch,
   memset (&insn, 0, sizeof (insn));
   insn.addr = low;
 
-  gdb_pretty_print_disassembler disasm (gdbarch);
+  gdb_pretty_print_disassembler disasm (gdbarch, uiout);
 
   while (insn.addr < high && (how_many < 0 || num_displayed < how_many))
     {
       int size;
 
-      size = disasm.pretty_print_insn (uiout, &insn, flags);
+      size = disasm.pretty_print_insn (&insn, flags);
       if (size <= 0)
 	break;
 
diff --git a/gdb/disasm.h b/gdb/disasm.h
index a9cfb21f872..5c1b30b8531 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -112,26 +112,32 @@  extern int gdb_print_insn (struct gdbarch *gdbarch, CORE_ADDR memaddr,
 class gdb_pretty_print_disassembler
 {
 public:
-  explicit gdb_pretty_print_disassembler (struct gdbarch *gdbarch)
-    : m_di (gdbarch, &m_insn_stb)
+  explicit gdb_pretty_print_disassembler (struct gdbarch *gdbarch,
+					  struct ui_out *uiout)
+    : m_uiout (uiout),
+      m_insn_stb (uiout->can_emit_style_escape ()),
+      m_di (gdbarch, &m_insn_stb)
   {}
 
-  /* Prints the instruction INSN into UIOUT and returns the length of
-     the printed instruction in bytes.  */
-  int pretty_print_insn (struct ui_out *uiout, const struct disasm_insn *insn,
+  /* Prints the instruction INSN into the saved ui_out and returns the
+     length of the printed instruction in bytes.  */
+  int pretty_print_insn (const struct disasm_insn *insn,
 			 gdb_disassembly_flags flags);
 
 private:
   /* Returns the architecture used for disassembling.  */
   struct gdbarch *arch () { return m_di.arch (); }
 
-  /* The disassembler used for instruction printing.  */
-  gdb_disassembler m_di;
+  /* The ui_out that is used by pretty_print_insn.  */
+  struct ui_out *m_uiout;
 
   /* The buffer used to build the instruction string.  The
      disassembler is initialized with this stream.  */
   string_file m_insn_stb;
 
+  /* The disassembler used for instruction printing.  */
+  gdb_disassembler m_di;
+
   /* The buffer used to build the raw opcodes string.  */
   string_file m_opcode_stb;
 };
diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index 01d15e38ba0..fe96658b59b 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -40,6 +40,11 @@  public:
   /* Return the version number of the current MI.  */
   int version ();
 
+  bool can_emit_style_escape () const override
+  {
+    return false;
+  }
+
 protected:
 
   virtual void do_table_begin (int nbrofcols, int nr_rows, const char *tblid)
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index fa89aa60a3c..c4197454266 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -778,7 +778,7 @@  btrace_insn_history (struct ui_out *uiout,
   gdb::optional<ui_out_emit_tuple> src_and_asm_tuple;
   gdb::optional<ui_out_emit_list> asm_list;
 
-  gdb_pretty_print_disassembler disasm (gdbarch);
+  gdb_pretty_print_disassembler disasm (gdbarch, uiout);
 
   for (btrace_insn_iterator it = *begin; btrace_insn_cmp (&it, end) != 0;
          btrace_insn_next (&it, 1))
@@ -841,7 +841,7 @@  btrace_insn_history (struct ui_out *uiout,
 	  if ((insn->flags & BTRACE_INSN_FLAG_SPECULATIVE) != 0)
 	    dinsn.is_speculative = 1;
 
-	  disasm.pretty_print_insn (uiout, &dinsn, flags);
+	  disasm.pretty_print_insn (&dinsn, flags);
 	}
     }
 }
diff --git a/gdb/testsuite/gdb.base/style.c b/gdb/testsuite/gdb.base/style.c
index a44936ed731..82dca7dfd15 100644
--- a/gdb/testsuite/gdb.base/style.c
+++ b/gdb/testsuite/gdb.base/style.c
@@ -15,8 +15,13 @@ 
 
 #define SOME_MACRO 23
 
+int some_called_function (void)
+{
+  return 0;
+}
+
 int
 main (int argc, char **argv)
 {
-  return 0; /* break here */
+  return some_called_function (); /* break here */
 }
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index a17f2014865..842c52e4891 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -83,6 +83,10 @@  save_vars { env(TERM) } {
 	    "Defined at $base_file_expr:16\r\n#define SOME_MACRO 23"
     }
 
+    set func [style some_called_function function]
+    # Somewhere should see the call to the function.
+    gdb_test "disassemble main" "[style $hex address].*$func.*"
+
     gdb_exit
     gdb_spawn
 
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 3398e432423..7041d9690e5 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -145,6 +145,10 @@  class ui_out
   bool query_table_field (int colno, int *width, int *alignment,
 			  const char **col_name);
 
+  /* Return true if this stream is prepared to handle style
+     escapes.  */
+  virtual bool can_emit_style_escape () const = 0;
+
  protected:
 
   virtual void do_table_begin (int nbrofcols, int nr_rows, const char *tblid)
diff --git a/gdb/utils.c b/gdb/utils.c
index 7584d5ab3af..398dd30f07b 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1442,9 +1442,11 @@  set_output_style (struct ui_file *stream, const ui_file_style &style)
   if (!stream->can_emit_style_escape ())
     return;
 
-  /* Note that we don't pass STREAM here, because we want to emit to
+  /* Note that we may not pass STREAM here, when we want to emit to
      the wrap buffer, not directly to STREAM.  */
-  emit_style_escape (style);
+  if (stream == gdb_stdout)
+    stream = nullptr;
+  emit_style_escape (style, stream);
 }
 
 /* See utils.h.  */