Disassemble branch delay slot instructions automatically

Disassemble branch delay slot instructions automatically

[Maciej W. Rozycki]

Hello,

 This is a change that implements automatic disassembly of instructions in 
a branch delay slot if a branch is the last instruction to be output.  It 
is especially useful with the "display /i" command when single-stepping, 
where you may want to see a single instruction normally not to get 
distracted, but with a branch an intruction that follows would be executed 
without having been printed.

 Here is an example for MIPS machine code -- this is with our current 
implementation:

(gdb) x /i 0x801018a4
0x801018a4 <pthread_create+52>: beqz    v0,0x80101a10 <pthread_create+416>

and here -- the same command with the patch applied:

(gdb) x /i 0x801018a4
0x801018a4 <pthread_create+52>: beqz    v0,0x80101a10 <pthread_create+416>
0x801018a8 <pthread_create+56>: li      v1,11

This change has been tested natively for mips-unknown-linux-gnu and
remotely for mipsisa32-sde-elf, using mips-sim-sde32/-EB and
mips-sim-sde32/-EL as the targets, with no regressions.

2007-05-15  Nigel Stephens  <nigel@mips.com>
            Maciej W. Rozycki  <macro@mips.com>

	* disasm.c (gdb_set_disassemble_info): New public function to
	set up a disassemble_info structure, ready for a call to
	TARGET_PRINT_INSN.
	* disasm.h (gdb_set_disassemble_info): Add prototype.
	* printcmd.c (branch_delay_insns): New variable to record number
	of delay slots after disassembling a branch.
	(print_formatted): When disassembling don't call gdb_print_insn,
	but call TARGET_PRINT_INSN directly.  Then pick up the resulting
	branch_delay_insns from the disassemble_info structure and store
	in local variable of same name.
	(do_examine): When disassembling, if the last instruction
	disassembled has any branch delay slots, then bump the count so
	that they get disassembled too.

 OK to apply?

  Maciej

12235.diff
Index: binutils-quilt/src/gdb/printcmd.c
===================================================================
--- binutils-quilt.orig/src/gdb/printcmd.c	2007-05-15 18:59:05.000000000 +0100
+++ binutils-quilt/src/gdb/printcmd.c	2007-05-15 18:59:46.000000000 +0100
@@ -43,6 +43,7 @@
 #include "gdb_assert.h"
 #include "block.h"
 #include "disasm.h"
+#include "dis-asm.h"
 
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_active et.al.   */
@@ -70,6 +71,10 @@
 
 static CORE_ADDR next_address;
 
+/* Number of delay instructions following current disassembled insn.  */
+
+static int branch_delay_insns;
+
 /* Last address examined.  */
 
 static CORE_ADDR last_examine_address;
@@ -277,8 +282,17 @@
 
       /* We often wrap here if there are long symbolic names.  */
       wrap_here ("    ");
-      next_address = VALUE_ADDRESS (val)
-	+ gdb_print_insn (VALUE_ADDRESS (val), stream);
+      {
+	static struct disassemble_info di;
+
+	gdb_set_disassemble_info (current_gdbarch, stream, &di);
+	next_address = (VALUE_ADDRESS (val)
+			+ TARGET_PRINT_INSN (VALUE_ADDRESS (val), &di));
+	if (di.insn_info_valid)
+	  branch_delay_insns = di.branch_delay_insns;
+	else
+	  branch_delay_insns = 0;
+      }
       break;
 
     default:
@@ -800,6 +814,10 @@
 	    release_value (last_examine_value);
 
 	  print_formatted (last_examine_value, format, size, gdb_stdout);
+
+	  /* Display any branch delay slots following the final insn.  */
+	  if (format == 'i' && count == 1)
+	    count += branch_delay_insns;
 	}
       printf_filtered ("\n");
       gdb_flush (gdb_stdout);
Index: binutils-quilt/src/gdb/disasm.c
===================================================================
--- binutils-quilt.orig/src/gdb/disasm.c	2007-05-15 18:59:05.000000000 +0100
+++ binutils-quilt/src/gdb/disasm.c	2007-05-15 18:59:46.000000000 +0100
@@ -349,6 +349,15 @@
   return di;
 }
 
+/* Publicly callable version of gdb_disassemble_info which doesn't
+   return a structure, but stores the result via a pointer.  */
+void
+gdb_set_disassemble_info (struct gdbarch *gdbarch, struct ui_file *file,
+			  struct disassemble_info *di)
+{
+  *di = gdb_disassemble_info (gdbarch, file);
+}
+
 void
 gdb_disassembly (struct ui_out *uiout,
 		char *file_string,
Index: binutils-quilt/src/gdb/disasm.h
===================================================================
--- binutils-quilt.orig/src/gdb/disasm.h	2007-05-15 18:59:05.000000000 +0100
+++ binutils-quilt/src/gdb/disasm.h	2007-05-15 18:59:46.000000000 +0100
@@ -35,4 +35,13 @@
 
 extern int gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream);
 
+/* Fill in a disassemble_info structure, ready for a call to
+   TARGET_PRINT_INSN.  */
+
+struct disassemble_info;
+
+extern void gdb_set_disassemble_info (struct gdbarch *,
+				      struct ui_file *,
+				      struct disassemble_info *);
+
 #endif

Re: Disassemble branch delay slot instructions automatically

[Daniel Jacobowitz]

On Tue, May 15, 2007 at 07:13:41PM +0100, Maciej W. Rozycki wrote:
> Hello,
> 
>  This is a change that implements automatic disassembly of instructions in 
> a branch delay slot if a branch is the last instruction to be output.  It 
> is especially useful with the "display /i" command when single-stepping, 
> where you may want to see a single instruction normally not to get 
> distracted, but with a branch an intruction that follows would be executed 
> without having been printed.
> 
>  Here is an example for MIPS machine code -- this is with our current 
> implementation:
> 
> (gdb) x /i 0x801018a4
> 0x801018a4 <pthread_create+52>: beqz    v0,0x80101a10 <pthread_create+416>
> 
> and here -- the same command with the patch applied:
> 
> (gdb) x /i 0x801018a4
> 0x801018a4 <pthread_create+52>: beqz    v0,0x80101a10 <pthread_create+416>
> 0x801018a8 <pthread_create+56>: li      v1,11
> 
> This change has been tested natively for mips-unknown-linux-gnu and
> remotely for mipsisa32-sde-elf, using mips-sim-sde32/-EB and
> mips-sim-sde32/-EL as the targets, with no regressions.

I would like additional opinions on this patch.

My first reaction was "hey, that would be useful to me", but my second
was concern that this would upset a GDB/MI frontend.  We should at
least try to provoke a problem in one such frontend first.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Disassemble branch delay slot instructions automatically

[Maciej W. Rozycki]

On Wed, 16 May 2007, Daniel Jacobowitz wrote:

> I would like additional opinions on this patch.

 I wouldn't mind either.

> My first reaction was "hey, that would be useful to me", but my second
> was concern that this would upset a GDB/MI frontend.  We should at
> least try to provoke a problem in one such frontend first.

 Well, do_examine() is used by the "display" and "x" commands only which 
are not used by any other frontend it would seem.  Of course you can issue 
these commands from MI (or from the Insight console window), but that 
should not be an issue as these are not commands meant to be used by 
programs interfacing to this frontend (that would be "-data-disassemble" 
which uses some other code).

 I have a follow-on patch that makes the output of "display /i" look 
better, which I will submit straight away.

  Maciej

Re: Disassemble branch delay slot instructions automatically

[Daniel Jacobowitz]

On Fri, May 18, 2007 at 04:46:55PM +0100, Maciej W. Rozycki wrote:
> On Wed, 16 May 2007, Daniel Jacobowitz wrote:
> 
> > I would like additional opinions on this patch.
> 
>  I wouldn't mind either.

Well, we didn't get any.

I think the patch is pretty much OK, except for the use of
TARGET_PRINT_INSN.  We're trying to eliminate the gdbarch macros now.
I think the best solution would be to add the extra argument to
gdb_print_insn; it's only used here and in the TUI.

Why did you need the new function that modified an existing
disassemble_info, instead of using the existing one?

This probably deserves a NEWS entry.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Disassemble branch delay slot instructions automatically

[Maciej W. Rozycki]

On Wed, 13 Jun 2007, Daniel Jacobowitz wrote:

> I think the patch is pretty much OK, except for the use of
> TARGET_PRINT_INSN.  We're trying to eliminate the gdbarch macros now.
> I think the best solution would be to add the extra argument to
> gdb_print_insn; it's only used here and in the TUI.

 And also in Insight; I have updated that too.  I suppose the plan is to 
substitute TARGET_PRINT_INSN with gdbarch_print_insn(), but that should be 
done separately, so that it is not mixed with functional changes.

> Why did you need the new function that modified an existing
> disassemble_info, instead of using the existing one?

 Well, I guess Nigel could answer this question, and my feeling is it is 
not a particularly useful complication, but it is not relevant anymore.

> This probably deserves a NEWS entry.

 Done.

 Here is my new version, which has been tested natively for 
mips-unknown-linux-gnu and remotely for mipsisa32-sde-elf, using 
mips-sim-sde32/-EB, mips-sim-sde32/-mips16/-EB, mips-sim-sde32/-EL and 
mips-sim-sde32/-mips16/-EL as the targets, with no regressions.

gdb/:
2007-06-20  Nigel Stephens  <nigel@mips.com>
            Maciej W. Rozycki  <macro@mips.com>

	* disasm.c (gdb_print_insn): Return the number of branch delay
	slot instructions too.
	* disasm.h (gdb_print_insn): Update prototype.
	* printcmd.c (branch_delay_insns): New variable to record the
	number of delay slot instructions after disassembling a branch.
	(print_formatted): Record the number of branch delay slot
	instructions.
	(do_examine): When disassembling, if the last instruction
	disassembled has any branch delay slots, then bump the count so
	that they get disassembled too.
	* tui/tui-disasm.c (tui_disassemble): Update the call to
	gdb_print_insn().
	* NEWS: Document the new behaviour.

gdb/gdbtk/:
2007-06-20  Maciej W. Rozycki  <macro@mips.com>

	* generic/gdbtk-cmds.c (gdbtk_load_asm): Update the call to
	gdb_print_insn().

 OK to apply?

  Maciej

12235.diff
Index: binutils-quilt/src/gdb/printcmd.c
===================================================================
--- binutils-quilt.orig/src/gdb/printcmd.c	2007-06-19 14:49:26.000000000 +0100
+++ binutils-quilt/src/gdb/printcmd.c	2007-06-19 14:53:12.000000000 +0100
@@ -43,6 +43,7 @@
 #include "gdb_assert.h"
 #include "block.h"
 #include "disasm.h"
+#include "dis-asm.h"
 
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_active et.al.   */
@@ -70,6 +71,10 @@
 
 static CORE_ADDR next_address;
 
+/* Number of delay instructions following current disassembled insn.  */
+
+static int branch_delay_insns;
+
 /* Last address examined.  */
 
 static CORE_ADDR last_examine_address;
@@ -277,8 +282,9 @@
 
       /* We often wrap here if there are long symbolic names.  */
       wrap_here ("    ");
-      next_address = VALUE_ADDRESS (val)
-	+ gdb_print_insn (VALUE_ADDRESS (val), stream);
+      next_address = (VALUE_ADDRESS (val)
+		      + gdb_print_insn (VALUE_ADDRESS (val), stream,
+					&branch_delay_insns));
       break;
 
     default:
@@ -800,6 +806,10 @@
 	    release_value (last_examine_value);
 
 	  print_formatted (last_examine_value, format, size, gdb_stdout);
+
+	  /* Display any branch delay slots following the final insn.  */
+	  if (format == 'i' && count == 1)
+	    count += branch_delay_insns;
 	}
       printf_filtered ("\n");
       gdb_flush (gdb_stdout);
Index: binutils-quilt/src/gdb/disasm.c
===================================================================
--- binutils-quilt.orig/src/gdb/disasm.c	2007-06-19 14:49:26.000000000 +0100
+++ binutils-quilt/src/gdb/disasm.c	2007-06-19 14:53:12.000000000 +0100
@@ -387,11 +387,24 @@
 }
 
 /* Print the instruction at address MEMADDR in debugged memory,
-   on STREAM.  Returns length of the instruction, in bytes.  */
+   on STREAM.  Returns the length of the instruction, in bytes,
+   and, if requested, the number of branch delay slot instructions.  */
 
 int
-gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream)
+gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream,
+		int *branch_delay_insns)
 {
-  struct disassemble_info di = gdb_disassemble_info (current_gdbarch, stream);
-  return TARGET_PRINT_INSN (memaddr, &di);
+  struct disassemble_info di;
+  int length;
+
+  di = gdb_disassemble_info (current_gdbarch, stream);
+  length = TARGET_PRINT_INSN (memaddr, &di);
+  if (branch_delay_insns)
+    {
+      if (di.insn_info_valid)
+	*branch_delay_insns = di.branch_delay_insns;
+      else
+	*branch_delay_insns = 0;
+    }
+  return length;
 }
Index: binutils-quilt/src/gdb/disasm.h
===================================================================
--- binutils-quilt.orig/src/gdb/disasm.h	2007-06-19 14:49:26.000000000 +0100
+++ binutils-quilt/src/gdb/disasm.h	2007-06-19 14:53:12.000000000 +0100
@@ -30,9 +30,12 @@
 			     int mixed_source_and_assembly,
 			     int how_many, CORE_ADDR low, CORE_ADDR high);
 
-/* Print the instruction at address MEMADDR in debugged memory, on
-   STREAM.  Returns length of the instruction, in bytes.  */
+/* Print the instruction at address MEMADDR in debugged memory,
+   on STREAM.  Returns the length of the instruction, in bytes,
+   and, if requested, the number of branch delay slot instructions.  */
 
-extern int gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream);
+extern int gdb_print_insn (CORE_ADDR memaddr,
+			   struct ui_file *stream,
+			   int *branch_delay_insns);
 
 #endif
Index: binutils-quilt/src/gdb/tui/tui-disasm.c
===================================================================
--- binutils-quilt.orig/src/gdb/tui/tui-disasm.c	2007-06-19 14:49:26.000000000 +0100
+++ binutils-quilt/src/gdb/tui/tui-disasm.c	2007-06-19 14:53:12.000000000 +0100
@@ -72,7 +72,7 @@
 
       ui_file_rewind (gdb_dis_out);
 
-      pc = pc + gdb_print_insn (pc, gdb_dis_out);
+      pc = pc + gdb_print_insn (pc, gdb_dis_out, NULL);
 
       asm_lines->insn = xstrdup (tui_file_get_strbuf (gdb_dis_out));
 
Index: binutils-quilt/src/gdb/gdbtk/generic/gdbtk-cmds.c
===================================================================
--- binutils-quilt.orig/src/gdb/gdbtk/generic/gdbtk-cmds.c	2007-06-19 14:49:26.000000000 +0100
+++ binutils-quilt/src/gdb/gdbtk/generic/gdbtk-cmds.c	2007-06-19 14:53:44.000000000 +0100
@@ -1895,7 +1895,7 @@
 
   result_ptr->obj_ptr = client_data->result_obj[2];
   /* FIXME: cagney/2003-09-08: This should use gdb_disassembly.  */
-  insn = gdb_print_insn (pc, gdb_stdout);
+  insn = gdb_print_insn (pc, gdb_stdout, NULL);
   gdb_flush (gdb_stdout);
 
   client_data->widget_line_no++;
Index: binutils-quilt/src/gdb/NEWS
===================================================================
--- binutils-quilt.orig/src/gdb/NEWS	2007-06-19 12:24:32.000000000 +0100
+++ binutils-quilt/src/gdb/NEWS	2007-06-19 15:16:19.000000000 +0100
@@ -41,6 +41,9 @@
 layout.  It also supports a TextSeg= and DataSeg= response when only
 segment base addresses (rather than offsets) are available.
 
+* The /i format now outputs any trailing branch delay slot instructions 
+immediately following the last instruction within the count specified.
+
 * New commands
 
 set remoteflow

Re: Disassemble branch delay slot instructions automatically

[Daniel Jacobowitz]

On Wed, Jun 20, 2007 at 02:56:20PM +0100, Maciej W. Rozycki wrote:
> On Wed, 13 Jun 2007, Daniel Jacobowitz wrote:
> 
> > I think the patch is pretty much OK, except for the use of
> > TARGET_PRINT_INSN.  We're trying to eliminate the gdbarch macros now.
> > I think the best solution would be to add the extra argument to
> > gdb_print_insn; it's only used here and in the TUI.
> 
>  And also in Insight; I have updated that too.  I suppose the plan is to 
> substitute TARGET_PRINT_INSN with gdbarch_print_insn(), but that should be 
> done separately, so that it is not mixed with functional changes.

In fact, it happened yesterday.

2007-06-19  Markus Deuling  <deuling@de.ibm.com>

        * gdbarch.sh (TARGET_PRINT_INSN): Replace by
	gdbarch_print_insn.
        * disasm.c (dump_insns, gdb_print_insn): Likewise.
        * gdbarch.c, gdbarch.h: Regenerate.

So I imagine you need to refresh this patch.  I was actually
suggesting you add the disassemble_info argument to gdb_print_insn,
not the number of delay slots; but this way seems fine too.

This version is OK, if Eli likes the NEWS entry and you add a
Makefile.in update (since you added #include's).  Eli, is the below
OK?

> Index: binutils-quilt/src/gdb/NEWS
> ===================================================================
> --- binutils-quilt.orig/src/gdb/NEWS	2007-06-19 12:24:32.000000000 +0100
> +++ binutils-quilt/src/gdb/NEWS	2007-06-19 15:16:19.000000000 +0100
> @@ -41,6 +41,9 @@
>  layout.  It also supports a TextSeg= and DataSeg= response when only
>  segment base addresses (rather than offsets) are available.
>  
> +* The /i format now outputs any trailing branch delay slot instructions 
> +immediately following the last instruction within the count specified.
> +
>  * New commands
>  
>  set remoteflow
> 

-- 
Daniel Jacobowitz
CodeSourcery

Re: Disassemble branch delay slot instructions automatically

[Maciej W. Rozycki]

On Wed, 20 Jun 2007, Daniel Jacobowitz wrote:

> In fact, it happened yesterday.

 Well, it seems to be always the case that something changes under your 
feet during a test cycle. ;-)

> So I imagine you need to refresh this patch.  I was actually
> suggesting you add the disassemble_info argument to gdb_print_insn,
> not the number of delay slots; but this way seems fine too.

 I have thought of it at once, but then concluded copying the whole 
structure again for its just one member would not make much sense and 
would effectively make gdb_print_insn() of questionable use -- the three 
callers could use an explicit sequence of gdb_disassemble_info(); 
gdbarch_print_insn() instead.

> This version is OK, if Eli likes the NEWS entry and you add a
> Makefile.in update (since you added #include's).  Eli, is the below
> OK?

 Sigh... -- I always seem to forget about this bit (even though it has 
bitten me a couple of times already).  But wait! -- it is actually not 
needed anymore now that "struct disassemble_info" is not used here.

 For the record -- here's my current version that I am going to commit 
except for possible NEWS entry adjustments.

gdb/:
2007-06-20  Nigel Stephens  <nigel@mips.com>
            Maciej W. Rozycki  <macro@mips.com>

	* disasm.c (gdb_print_insn): Return the number of branch delay
	slot instructions too.
	* disasm.h (gdb_print_insn): Update prototype.
	* printcmd.c (branch_delay_insns): New variable to record the
	number of delay slot instructions after disassembling a branch.
	(print_formatted): Record the number of branch delay slot
	instructions.
	(do_examine): When disassembling, if the last instruction
	disassembled has any branch delay slots, then bump the count so
	that they get disassembled too.
	* tui/tui-disasm.c (tui_disassemble): Update the call to
	gdb_print_insn().
	* NEWS: Document the new behaviour.

gdb/gdbtk/:
2007-06-20  Maciej W. Rozycki  <macro@mips.com>

	* generic/gdbtk-cmds.c (gdbtk_load_asm): Update the call to
	gdb_print_insn().

  Maciej

12235.diff
Index: binutils-quilt/src/gdb/printcmd.c
===================================================================
--- binutils-quilt.orig/src/gdb/printcmd.c	2007-06-20 15:03:30.000000000 +0100
+++ binutils-quilt/src/gdb/printcmd.c	2007-06-20 15:45:33.000000000 +0100
@@ -70,6 +70,10 @@
 
 static CORE_ADDR next_address;
 
+/* Number of delay instructions following current disassembled insn.  */
+
+static int branch_delay_insns;
+
 /* Last address examined.  */
 
 static CORE_ADDR last_examine_address;
@@ -277,8 +281,9 @@
 
       /* We often wrap here if there are long symbolic names.  */
       wrap_here ("    ");
-      next_address = VALUE_ADDRESS (val)
-	+ gdb_print_insn (VALUE_ADDRESS (val), stream);
+      next_address = (VALUE_ADDRESS (val)
+		      + gdb_print_insn (VALUE_ADDRESS (val), stream,
+					&branch_delay_insns));
       break;
 
     default:
@@ -800,6 +805,10 @@
 	    release_value (last_examine_value);
 
 	  print_formatted (last_examine_value, format, size, gdb_stdout);
+
+	  /* Display any branch delay slots following the final insn.  */
+	  if (format == 'i' && count == 1)
+	    count += branch_delay_insns;
 	}
       printf_filtered ("\n");
       gdb_flush (gdb_stdout);
Index: binutils-quilt/src/gdb/disasm.c
===================================================================
--- binutils-quilt.orig/src/gdb/disasm.c	2007-06-20 15:03:30.000000000 +0100
+++ binutils-quilt/src/gdb/disasm.c	2007-06-20 15:42:24.000000000 +0100
@@ -387,11 +387,24 @@
 }
 
 /* Print the instruction at address MEMADDR in debugged memory,
-   on STREAM.  Returns length of the instruction, in bytes.  */
+   on STREAM.  Returns the length of the instruction, in bytes,
+   and, if requested, the number of branch delay slot instructions.  */
 
 int
-gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream)
+gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream,
+		int *branch_delay_insns)
 {
-  struct disassemble_info di = gdb_disassemble_info (current_gdbarch, stream);
-  return gdbarch_print_insn (current_gdbarch, memaddr, &di);
+  struct disassemble_info di;
+  int length;
+
+  di = gdb_disassemble_info (current_gdbarch, stream);
+  length = gdbarch_print_insn (current_gdbarch, memaddr, &di);
+  if (branch_delay_insns)
+    {
+      if (di.insn_info_valid)
+	*branch_delay_insns = di.branch_delay_insns;
+      else
+	*branch_delay_insns = 0;
+    }
+  return length;
 }
Index: binutils-quilt/src/gdb/disasm.h
===================================================================
--- binutils-quilt.orig/src/gdb/disasm.h	2007-06-20 15:03:30.000000000 +0100
+++ binutils-quilt/src/gdb/disasm.h	2007-06-20 15:42:24.000000000 +0100
@@ -30,9 +30,12 @@
 			     int mixed_source_and_assembly,
 			     int how_many, CORE_ADDR low, CORE_ADDR high);
 
-/* Print the instruction at address MEMADDR in debugged memory, on
-   STREAM.  Returns length of the instruction, in bytes.  */
+/* Print the instruction at address MEMADDR in debugged memory,
+   on STREAM.  Returns the length of the instruction, in bytes,
+   and, if requested, the number of branch delay slot instructions.  */
 
-extern int gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream);
+extern int gdb_print_insn (CORE_ADDR memaddr,
+			   struct ui_file *stream,
+			   int *branch_delay_insns);
 
 #endif
Index: binutils-quilt/src/gdb/tui/tui-disasm.c
===================================================================
--- binutils-quilt.orig/src/gdb/tui/tui-disasm.c	2007-06-20 15:03:30.000000000 +0100
+++ binutils-quilt/src/gdb/tui/tui-disasm.c	2007-06-20 15:42:24.000000000 +0100
@@ -72,7 +72,7 @@
 
       ui_file_rewind (gdb_dis_out);
 
-      pc = pc + gdb_print_insn (pc, gdb_dis_out);
+      pc = pc + gdb_print_insn (pc, gdb_dis_out, NULL);
 
       asm_lines->insn = xstrdup (tui_file_get_strbuf (gdb_dis_out));
 
Index: binutils-quilt/src/gdb/gdbtk/generic/gdbtk-cmds.c
===================================================================
--- binutils-quilt.orig/src/gdb/gdbtk/generic/gdbtk-cmds.c	2007-06-20 15:03:30.000000000 +0100
+++ binutils-quilt/src/gdb/gdbtk/generic/gdbtk-cmds.c	2007-06-20 15:42:24.000000000 +0100
@@ -1895,7 +1895,7 @@
 
   result_ptr->obj_ptr = client_data->result_obj[2];
   /* FIXME: cagney/2003-09-08: This should use gdb_disassembly.  */
-  insn = gdb_print_insn (pc, gdb_stdout);
+  insn = gdb_print_insn (pc, gdb_stdout, NULL);
   gdb_flush (gdb_stdout);
 
   client_data->widget_line_no++;
Index: binutils-quilt/src/gdb/NEWS
===================================================================
--- binutils-quilt.orig/src/gdb/NEWS	2007-06-20 15:03:30.000000000 +0100
+++ binutils-quilt/src/gdb/NEWS	2007-06-20 15:42:24.000000000 +0100
@@ -41,6 +41,9 @@
 layout.  It also supports a TextSeg= and DataSeg= response when only
 segment base addresses (rather than offsets) are available.
 
+* The /i format now outputs any trailing branch delay slot instructions 
+immediately following the last instruction within the count specified.
+
 * New commands
 
 set remoteflow

Re: Disassemble branch delay slot instructions automatically

[Eli Zaretskii]

> Date: Wed, 20 Jun 2007 14:56:20 +0100 (BST)
> From: "Maciej W. Rozycki" <macro@mips.com>
> cc: gdb-patches@sourceware.org, insight@sourceware.org,      Nigel Stephens <nigel@mips.com>,      "Maciej W. Rozycki" <macro@linux-mips.org>
> 
> Index: binutils-quilt/src/gdb/NEWS
> ===================================================================
> --- binutils-quilt.orig/src/gdb/NEWS	2007-06-19 12:24:32.000000000 +0100
> +++ binutils-quilt/src/gdb/NEWS	2007-06-19 15:16:19.000000000 +0100
> @@ -41,6 +41,9 @@
>  layout.  It also supports a TextSeg= and DataSeg= response when only
>  segment base addresses (rather than offsets) are available.
>  
> +* The /i format now outputs any trailing branch delay slot instructions 
> +immediately following the last instruction within the count specified.
> +

This is fine, but I think we should also document this in the user
manual.

Re: Disassemble branch delay slot instructions automatically

[Maciej W. Rozycki]

On Wed, 20 Jun 2007, Eli Zaretskii wrote:

> This is fine, but I think we should also document this in the user
> manual.

 Good point -- I hope my note is clear enough -- see below.

gdb/:
2007-06-21  Nigel Stephens  <nigel@mips.com>
            Maciej W. Rozycki  <macro@mips.com>

	* disasm.c (gdb_print_insn): Return the number of branch delay
	slot instructions too.
	* disasm.h (gdb_print_insn): Update prototype.
	* printcmd.c (branch_delay_insns): New variable to record the
	number of delay slot instructions after disassembling a branch.
	(print_formatted): Record the number of branch delay slot
	instructions.
	(do_examine): When disassembling, if the last instruction
	disassembled has any branch delay slots, then bump the count so
	that they get disassembled too.
	* tui/tui-disasm.c (tui_disassemble): Update the call to
	gdb_print_insn().
	* NEWS: Document the new behaviour.

gdb/doc/:
2007-06-21  Maciej W. Rozycki  <macro@mips.com>

	* gdb.texinfo (Examining Memory): Document the new behaviour.

gdb/gdbtk/:
2007-06-21  Maciej W. Rozycki  <macro@mips.com>

	* generic/gdbtk-cmds.c (gdbtk_load_asm): Update the call to
	gdb_print_insn().

 I have checked it in.

  Maciej

12235.diff
Index: binutils-quilt/src/gdb/printcmd.c
===================================================================
--- binutils-quilt.orig/src/gdb/printcmd.c	2007-06-21 13:39:03.000000000 +0100
+++ binutils-quilt/src/gdb/printcmd.c	2007-06-21 14:10:21.000000000 +0100
@@ -70,6 +70,10 @@
 
 static CORE_ADDR next_address;
 
+/* Number of delay instructions following current disassembled insn.  */
+
+static int branch_delay_insns;
+
 /* Last address examined.  */
 
 static CORE_ADDR last_examine_address;
@@ -277,8 +281,9 @@
 
       /* We often wrap here if there are long symbolic names.  */
       wrap_here ("    ");
-      next_address = VALUE_ADDRESS (val)
-	+ gdb_print_insn (VALUE_ADDRESS (val), stream);
+      next_address = (VALUE_ADDRESS (val)
+		      + gdb_print_insn (VALUE_ADDRESS (val), stream,
+					&branch_delay_insns));
       break;
 
     default:
@@ -800,6 +805,10 @@
 	    release_value (last_examine_value);
 
 	  print_formatted (last_examine_value, format, size, gdb_stdout);
+
+	  /* Display any branch delay slots following the final insn.  */
+	  if (format == 'i' && count == 1)
+	    count += branch_delay_insns;
 	}
       printf_filtered ("\n");
       gdb_flush (gdb_stdout);
Index: binutils-quilt/src/gdb/disasm.c
===================================================================
--- binutils-quilt.orig/src/gdb/disasm.c	2007-06-21 13:39:03.000000000 +0100
+++ binutils-quilt/src/gdb/disasm.c	2007-06-21 14:10:21.000000000 +0100
@@ -387,11 +387,24 @@
 }
 
 /* Print the instruction at address MEMADDR in debugged memory,
-   on STREAM.  Returns length of the instruction, in bytes.  */
+   on STREAM.  Returns the length of the instruction, in bytes,
+   and, if requested, the number of branch delay slot instructions.  */
 
 int
-gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream)
+gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream,
+		int *branch_delay_insns)
 {
-  struct disassemble_info di = gdb_disassemble_info (current_gdbarch, stream);
-  return gdbarch_print_insn (current_gdbarch, memaddr, &di);
+  struct disassemble_info di;
+  int length;
+
+  di = gdb_disassemble_info (current_gdbarch, stream);
+  length = gdbarch_print_insn (current_gdbarch, memaddr, &di);
+  if (branch_delay_insns)
+    {
+      if (di.insn_info_valid)
+	*branch_delay_insns = di.branch_delay_insns;
+      else
+	*branch_delay_insns = 0;
+    }
+  return length;
 }
Index: binutils-quilt/src/gdb/disasm.h
===================================================================
--- binutils-quilt.orig/src/gdb/disasm.h	2007-06-21 13:39:03.000000000 +0100
+++ binutils-quilt/src/gdb/disasm.h	2007-06-21 14:10:21.000000000 +0100
@@ -30,9 +30,12 @@
 			     int mixed_source_and_assembly,
 			     int how_many, CORE_ADDR low, CORE_ADDR high);
 
-/* Print the instruction at address MEMADDR in debugged memory, on
-   STREAM.  Returns length of the instruction, in bytes.  */
+/* Print the instruction at address MEMADDR in debugged memory,
+   on STREAM.  Returns the length of the instruction, in bytes,
+   and, if requested, the number of branch delay slot instructions.  */
 
-extern int gdb_print_insn (CORE_ADDR memaddr, struct ui_file *stream);
+extern int gdb_print_insn (CORE_ADDR memaddr,
+			   struct ui_file *stream,
+			   int *branch_delay_insns);
 
 #endif
Index: binutils-quilt/src/gdb/tui/tui-disasm.c
===================================================================
--- binutils-quilt.orig/src/gdb/tui/tui-disasm.c	2007-06-21 13:39:03.000000000 +0100
+++ binutils-quilt/src/gdb/tui/tui-disasm.c	2007-06-21 14:10:21.000000000 +0100
@@ -72,7 +72,7 @@
 
       ui_file_rewind (gdb_dis_out);
 
-      pc = pc + gdb_print_insn (pc, gdb_dis_out);
+      pc = pc + gdb_print_insn (pc, gdb_dis_out, NULL);
 
       asm_lines->insn = xstrdup (tui_file_get_strbuf (gdb_dis_out));
 
Index: binutils-quilt/src/gdb/gdbtk/generic/gdbtk-cmds.c
===================================================================
--- binutils-quilt.orig/src/gdb/gdbtk/generic/gdbtk-cmds.c	2007-06-21 13:39:03.000000000 +0100
+++ binutils-quilt/src/gdb/gdbtk/generic/gdbtk-cmds.c	2007-06-21 14:10:21.000000000 +0100
@@ -1895,7 +1895,7 @@
 
   result_ptr->obj_ptr = client_data->result_obj[2];
   /* FIXME: cagney/2003-09-08: This should use gdb_disassembly.  */
-  insn = gdb_print_insn (pc, gdb_stdout);
+  insn = gdb_print_insn (pc, gdb_stdout, NULL);
   gdb_flush (gdb_stdout);
 
   client_data->widget_line_no++;
Index: binutils-quilt/src/gdb/NEWS
===================================================================
--- binutils-quilt.orig/src/gdb/NEWS	2007-06-21 13:39:03.000000000 +0100
+++ binutils-quilt/src/gdb/NEWS	2007-06-21 14:10:21.000000000 +0100
@@ -41,6 +41,9 @@
 layout.  It also supports a TextSeg= and DataSeg= response when only
 segment base addresses (rather than offsets) are available.
 
+* The /i format now outputs any trailing branch delay slot instructions 
+immediately following the last instruction within the count specified.
+
 * New commands
 
 set remoteflow
Index: binutils-quilt/src/gdb/doc/gdb.texinfo
===================================================================
--- binutils-quilt.orig/src/gdb/doc/gdb.texinfo	2007-06-21 13:39:03.000000000 +0100
+++ binutils-quilt/src/gdb/doc/gdb.texinfo	2007-06-21 14:19:07.000000000 +0100
@@ -5861,9 +5861,12 @@
 Even though the unit size @var{u} is ignored for the formats @samp{s}
 and @samp{i}, you might still want to use a count @var{n}; for example,
 @samp{3i} specifies that you want to see three machine instructions,
-including any operands.  The command @code{disassemble} gives an
-alternative way of inspecting machine instructions; see @ref{Machine
-Code,,Source and Machine Code}.
+including any operands.  For convenience, especially when used with
+the @code{display} command, the @samp{i} format also prints branch delay
+slot instructions, if any, beyond the count specified, which immediately
+follow the last instruction that is within the count.  The command
+@code{disassemble} gives an alternative way of inspecting machine
+instructions; see @ref{Machine Code,,Source and Machine Code}.
 
 All the defaults for the arguments to @code{x} are designed to make it
 easy to continue scanning memory with minimal specifications each time

Re: Disassemble branch delay slot instructions automatically

[Eli Zaretskii]

> Date: Thu, 21 Jun 2007 16:19:03 +0100 (BST)
> From: "Maciej W. Rozycki" <macro@mips.com>
> cc: drow@false.org, gdb-patches@sourceware.org, insight@sourceware.org, 
>     nigel@mips.com, "Maciej W. Rozycki" <macro@linux-mips.org>
> 
> On Wed, 20 Jun 2007, Eli Zaretskii wrote:
> 
> > This is fine, but I think we should also document this in the user
> > manual.
> 
>  Good point -- I hope my note is clear enough -- see below.

Your patch for the manual is fine with me.  Thanks.