[RFA] mixed source+assembly from cli disassemble

[RFA] mixed source+assembly from cli disassemble

[Doug Evans]

The functionality is there, it seems a shame to not provide
access to it.

This adds a /s modifier to disassemble.

2008-04-03  Doug Evans  <dje@google.com>

	* cli/cli-cmds.c (print_disassembly): New fn.
	(disassemble_current_function): New fn.
	(disassemble_command): Recognize /s modifier, print mixed
	source+assembly.
	(init_cli_cmds): Update disassemble help text.

Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.73
diff -u -p -u -p -r1.73 cli-cmds.c
--- cli/cli-cmds.c	1 Jan 2008 22:53:14 -0000	1.73
+++ cli/cli-cmds.c	4 Apr 2008 00:33:42 -0000
@@ -892,10 +892,76 @@ list_command (char *arg, int from_tty)
 			0);
 }
 
+/* Subroutine of disassemble_command to simplify it.
+   Perform the disassembly.
+   NAME is the name of the function if known, or NULL.
+   [LOW,HIGH) are the range of addresses to disassemble.
+   MIXED is non-zero to print source with the assembler.  */
+
+static void
+print_disassembly (const char *name, CORE_ADDR low, CORE_ADDR high, int mixed)
+{
+#if defined(TUI)
+  if (!tui_is_window_visible (DISASSEM_WIN))
+#endif
+    {
+      printf_filtered ("Dump of assembler code ");
+      if (name != NULL)
+	{
+	  printf_filtered ("for function %s:\n", name);
+	}
+      else
+	{
+	  printf_filtered ("from ");
+	  deprecated_print_address_numeric (low, 1, gdb_stdout);
+	  printf_filtered (" to ");
+	  deprecated_print_address_numeric (high, 1, gdb_stdout);
+	  printf_filtered (":\n");
+	}
+
+      /* Dump the specified range.  */
+      gdb_disassembly (uiout, 0, 0, mixed, -1, low, high);
+
+      printf_filtered ("End of assembler dump.\n");
+      gdb_flush (gdb_stdout);
+    }
+#if defined(TUI)
+  else
+    {
+      tui_show_assembly (low);
+    }
+#endif
+}
+
+/* Subroutine of disassemble_command to simplify it.
+   Print a disassembly of the current function.
+   MIXED is non-zero to print source with the assembler.  */
+
+static void
+disassemble_current_function (int mixed)
+{
+  CORE_ADDR low, high, pc;
+  char *name;
+
+  pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
+  if (find_pc_partial_function (pc, &name, &low, &high) == 0)
+    error (_("No function contains program counter for selected frame."));
+#if defined(TUI)
+  /* NOTE: cagney/2003-02-13 The `tui_active' was previously
+     `tui_version'.  */
+  if (tui_active)
+    /* FIXME: cagney/2004-02-07: This should be an observer.  */
+    low = tui_get_low_disassembly_address (low, pc);
+#endif
+  low += gdbarch_deprecated_function_start_offset (current_gdbarch);
+
+  print_disassembly (name, low, high, mixed);
+}
+
 /* Dump a specified section of assembly code.  With no command line
    arguments, this command will dump the assembly code for the
    function surrounding the pc value in the selected frame.  With one
-   argument, it will dump the assembly code surrounding that pc value.
+
    Two arguments are interpeted as bounds within which to dump
    assembly.  */
 
@@ -906,26 +972,44 @@ disassemble_command (char *arg, int from
   char *name;
   CORE_ADDR pc, pc_masked;
   char *space_index;
-#if 0
-  asection *section;
-#endif
+  int mixed_source_and_assembly;
 
   name = NULL;
-  if (!arg)
+  mixed_source_and_assembly = 0;
+
+  if (arg && *arg == '/')
     {
-      pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
-      if (find_pc_partial_function (pc, &name, &low, &high) == 0)
-	error (_("No function contains program counter for selected frame."));
-#if defined(TUI)
-      /* NOTE: cagney/2003-02-13 The `tui_active' was previously
-	 `tui_version'.  */
-      if (tui_active)
-	/* FIXME: cagney/2004-02-07: This should be an observer.  */
-	low = tui_get_low_disassembly_address (low, pc);
-#endif
-      low += gdbarch_deprecated_function_start_offset (current_gdbarch);
+      ++arg;
+
+      if (*arg == '\0')
+	error (_("Missing modifier."));
+
+      while (*arg && ! isspace (*arg))
+	{
+	  switch (*arg++)
+	    {
+	    case 's':
+	      mixed_source_and_assembly = 1;
+	      break;
+	    default:
+	      error (_("Invalid disassembly modifier."));
+	    }
+	}
+
+      while (isspace (*arg))
+	++arg;
+    }
+
+  if (! arg || ! *arg)
+    {
+      disassemble_current_function (mixed_source_and_assembly);
+      return;
     }
-  else if (!(space_index = (char *) strchr (arg, ' ')))
+
+  /* FIXME: 'twould be nice to allow spaces in the expression for the first
+     arg.  Allow comma separater too?  */
+
+  if (!(space_index = (char *) strchr (arg, ' ')))
     {
       /* One argument.  */
       pc = parse_and_eval_address (arg);
@@ -948,36 +1032,7 @@ disassemble_command (char *arg, int from
       high = parse_and_eval_address (space_index + 1);
     }
 
-#if defined(TUI)
-  if (!tui_is_window_visible (DISASSEM_WIN))
-#endif
-    {
-      printf_filtered ("Dump of assembler code ");
-      if (name != NULL)
-	{
-	  printf_filtered ("for function %s:\n", name);
-	}
-      else
-	{
-	  printf_filtered ("from ");
-	  deprecated_print_address_numeric (low, 1, gdb_stdout);
-	  printf_filtered (" to ");
-	  deprecated_print_address_numeric (high, 1, gdb_stdout);
-	  printf_filtered (":\n");
-	}
-
-      /* Dump the specified range.  */
-      gdb_disassembly (uiout, 0, 0, 0, -1, low, high);
-
-      printf_filtered ("End of assembler dump.\n");
-      gdb_flush (gdb_stdout);
-    }
-#if defined(TUI)
-  else
-    {
-      tui_show_assembly (low);
-    }
-#endif
+  print_disassembly (name, low, high, mixed_source_and_assembly);
 }
 
 static void
@@ -1383,6 +1438,7 @@ With two args if one is empty it stands 
   c = add_com ("disassemble", class_vars, disassemble_command, _("\
 Disassemble a specified section of memory.\n\
 Default is the function surrounding the pc of the selected frame.\n\
+With a leading /s modifier source lines, if available, are included.\n\
 With a single argument, the function surrounding that address is dumped.\n\
 Two arguments are taken as a range of memory to dump."));
   set_cmd_completer (c, location_completer);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.479
diff -u -p -u -p -r1.479 gdb.texinfo
--- doc/gdb.texinfo	3 Apr 2008 21:51:11 -0000	1.479
+++ doc/gdb.texinfo	4 Apr 2008 00:33:42 -0000
@@ -5419,7 +5419,9 @@ Variables}).
 @cindex listing machine instructions
 @item disassemble
 This specialized command dumps a range of memory as machine
-instructions.  The default memory range is the function surrounding the
+instructions.  It can also print mixed source+disassembly by specifying
+the @code{/s} modifier.
+The default memory range is the function surrounding the
 program counter of the selected frame.  A single argument to this
 command is a program counter value; @value{GDBN} dumps the function
 surrounding this value.  Two arguments specify a range of addresses

Re: [RFA] mixed source+assembly from cli disassemble

[Michael Snyder]

On Thu, 2008-04-03 at 17:38 -0700, Doug Evans wrote:
> The functionality is there, it seems a shame to not provide
> access to it.
> 
> This adds a /s modifier to disassemble.

Sweet!!!  Thanks, Doug...

Looks well done, but I'm sure others may have input.

Re: [RFA] mixed source+assembly from cli disassemble

[Doug Evans]

[-- Attachment #1: Type: text/plain, Size: 113 bytes --]

Not well enough I'm afraid.  Messed up a comment (in front of
disassemble_command).  Here's an improved version.

[-- Attachment #2: gdb-080403-disass.patch.txt --]
[-- Type: text/plain, Size: 7304 bytes --]

2008-04-03  Doug Evans  <dje@google.com>

	* cli/cli-cmds.c (print_disassembly): New fn.
	(disassemble_current_function): New fn.
	(disassemble_command): Recognize /s modifier, print mixed
	source+assembly.
	(init_cli_cmds): Update disassemble help text.

Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.73
diff -u -p -u -p -r1.73 cli-cmds.c
--- cli/cli-cmds.c	1 Jan 2008 22:53:14 -0000	1.73
+++ cli/cli-cmds.c	4 Apr 2008 03:19:45 -0000
@@ -892,12 +892,83 @@ list_command (char *arg, int from_tty)
 			0);
 }
 
-/* Dump a specified section of assembly code.  With no command line
-   arguments, this command will dump the assembly code for the
-   function surrounding the pc value in the selected frame.  With one
-   argument, it will dump the assembly code surrounding that pc value.
-   Two arguments are interpeted as bounds within which to dump
-   assembly.  */
+/* Subroutine of disassemble_command to simplify it.
+   Perform the disassembly.
+   NAME is the name of the function if known, or NULL.
+   [LOW,HIGH) are the range of addresses to disassemble.
+   MIXED is non-zero to print source with the assembler.  */
+
+static void
+print_disassembly (const char *name, CORE_ADDR low, CORE_ADDR high, int mixed)
+{
+#if defined(TUI)
+  if (!tui_is_window_visible (DISASSEM_WIN))
+#endif
+    {
+      printf_filtered ("Dump of assembler code ");
+      if (name != NULL)
+	{
+	  printf_filtered ("for function %s:\n", name);
+	}
+      else
+	{
+	  printf_filtered ("from ");
+	  deprecated_print_address_numeric (low, 1, gdb_stdout);
+	  printf_filtered (" to ");
+	  deprecated_print_address_numeric (high, 1, gdb_stdout);
+	  printf_filtered (":\n");
+	}
+
+      /* Dump the specified range.  */
+      gdb_disassembly (uiout, 0, 0, mixed, -1, low, high);
+
+      printf_filtered ("End of assembler dump.\n");
+      gdb_flush (gdb_stdout);
+    }
+#if defined(TUI)
+  else
+    {
+      tui_show_assembly (low);
+    }
+#endif
+}
+
+/* Subroutine of disassemble_command to simplify it.
+   Print a disassembly of the current function.
+   MIXED is non-zero to print source with the assembler.  */
+
+static void
+disassemble_current_function (int mixed)
+{
+  CORE_ADDR low, high, pc;
+  char *name;
+
+  pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
+  if (find_pc_partial_function (pc, &name, &low, &high) == 0)
+    error (_("No function contains program counter for selected frame."));
+#if defined(TUI)
+  /* NOTE: cagney/2003-02-13 The `tui_active' was previously
+     `tui_version'.  */
+  if (tui_active)
+    /* FIXME: cagney/2004-02-07: This should be an observer.  */
+    low = tui_get_low_disassembly_address (low, pc);
+#endif
+  low += gdbarch_deprecated_function_start_offset (current_gdbarch);
+
+  print_disassembly (name, low, high, mixed);
+}
+
+/* Dump a specified section of assembly code.
+
+   Usage:
+     disassemble [/s]
+       - dump the assembly code for the function of the current pc
+     disassemble [/s] addr
+       - dump the assembly code for the function at ADDR
+     disassemble [/s] low high
+       - dump the assembly code in the range [LOW,HIGH)
+
+   A leading /s modifier will include source code with the assembly.  */
 
 static void
 disassemble_command (char *arg, int from_tty)
@@ -906,26 +977,44 @@ disassemble_command (char *arg, int from
   char *name;
   CORE_ADDR pc, pc_masked;
   char *space_index;
-#if 0
-  asection *section;
-#endif
+  int mixed_source_and_assembly;
 
   name = NULL;
-  if (!arg)
+  mixed_source_and_assembly = 0;
+
+  if (arg && *arg == '/')
     {
-      pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
-      if (find_pc_partial_function (pc, &name, &low, &high) == 0)
-	error (_("No function contains program counter for selected frame."));
-#if defined(TUI)
-      /* NOTE: cagney/2003-02-13 The `tui_active' was previously
-	 `tui_version'.  */
-      if (tui_active)
-	/* FIXME: cagney/2004-02-07: This should be an observer.  */
-	low = tui_get_low_disassembly_address (low, pc);
-#endif
-      low += gdbarch_deprecated_function_start_offset (current_gdbarch);
+      ++arg;
+
+      if (*arg == '\0')
+	error (_("Missing modifier."));
+
+      while (*arg && ! isspace (*arg))
+	{
+	  switch (*arg++)
+	    {
+	    case 's':
+	      mixed_source_and_assembly = 1;
+	      break;
+	    default:
+	      error (_("Invalid disassembly modifier."));
+	    }
+	}
+
+      while (isspace (*arg))
+	++arg;
+    }
+
+  if (! arg || ! *arg)
+    {
+      disassemble_current_function (mixed_source_and_assembly);
+      return;
     }
-  else if (!(space_index = (char *) strchr (arg, ' ')))
+
+  /* FIXME: 'twould be nice to allow spaces in the expression for the first
+     arg.  Allow comma separater too?  */
+
+  if (!(space_index = (char *) strchr (arg, ' ')))
     {
       /* One argument.  */
       pc = parse_and_eval_address (arg);
@@ -948,36 +1037,7 @@ disassemble_command (char *arg, int from
       high = parse_and_eval_address (space_index + 1);
     }
 
-#if defined(TUI)
-  if (!tui_is_window_visible (DISASSEM_WIN))
-#endif
-    {
-      printf_filtered ("Dump of assembler code ");
-      if (name != NULL)
-	{
-	  printf_filtered ("for function %s:\n", name);
-	}
-      else
-	{
-	  printf_filtered ("from ");
-	  deprecated_print_address_numeric (low, 1, gdb_stdout);
-	  printf_filtered (" to ");
-	  deprecated_print_address_numeric (high, 1, gdb_stdout);
-	  printf_filtered (":\n");
-	}
-
-      /* Dump the specified range.  */
-      gdb_disassembly (uiout, 0, 0, 0, -1, low, high);
-
-      printf_filtered ("End of assembler dump.\n");
-      gdb_flush (gdb_stdout);
-    }
-#if defined(TUI)
-  else
-    {
-      tui_show_assembly (low);
-    }
-#endif
+  print_disassembly (name, low, high, mixed_source_and_assembly);
 }
 
 static void
@@ -1383,6 +1443,7 @@ With two args if one is empty it stands 
   c = add_com ("disassemble", class_vars, disassemble_command, _("\
 Disassemble a specified section of memory.\n\
 Default is the function surrounding the pc of the selected frame.\n\
+With a leading /s modifier source lines, if available, are included.\n\
 With a single argument, the function surrounding that address is dumped.\n\
 Two arguments are taken as a range of memory to dump."));
   set_cmd_completer (c, location_completer);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.479
diff -u -p -u -p -r1.479 gdb.texinfo
--- doc/gdb.texinfo	3 Apr 2008 21:51:11 -0000	1.479
+++ doc/gdb.texinfo	4 Apr 2008 03:19:45 -0000
@@ -5419,7 +5419,9 @@ Variables}).
 @cindex listing machine instructions
 @item disassemble
 This specialized command dumps a range of memory as machine
-instructions.  The default memory range is the function surrounding the
+instructions.  It can also print mixed source+disassembly by specifying
+the @code{/s} modifier.
+The default memory range is the function surrounding the
 program counter of the selected frame.  A single argument to this
 command is a program counter value; @value{GDBN} dumps the function
 surrounding this value.  Two arguments specify a range of addresses

Re: [RFA] mixed source+assembly from cli disassemble

[Eli Zaretskii]

> Date: Thu, 3 Apr 2008 20:25:44 -0700
> From: "Doug Evans" <dje@google.com>
> Cc: gdb-patches@sourceware.org
> 
> Not well enough I'm afraid.  Messed up a comment (in front of
> disassemble_command).  Here's an improved version.

Thanks!

>    c = add_com ("disassemble", class_vars, disassemble_command, _("\
>  Disassemble a specified section of memory.\n\
>  Default is the function surrounding the pc of the selected frame.\n\
> +With a leading /s modifier source lines, if available, are included.\n\

Why do you say ``leading /s''?  What would /s be leading?

>  This specialized command dumps a range of memory as machine
> -instructions.  The default memory range is the function surrounding the
> +instructions.  It can also print mixed source+disassembly by specifying
> +the @code{/s} modifier.

An example would be good here, and the optional modifier should be
mentioned in the @item line above.

Other than that, the patch for gdb.texinfo is approved.  Thanks.

Re: [RFA] mixed source+assembly from cli disassemble

[Michael Snyder]

Going on a week...
No other comments, pro or con?


On Thu, 2008-04-03 at 17:38 -0700, Doug Evans wrote:
> The functionality is there, it seems a shame to not provide
> access to it.
> 
> This adds a /s modifier to disassemble.
> 
> 2008-04-03  Doug Evans  <dje@google.com>
> 
> 	* cli/cli-cmds.c (print_disassembly): New fn.
> 	(disassemble_current_function): New fn.
> 	(disassemble_command): Recognize /s modifier, print mixed
> 	source+assembly.
> 	(init_cli_cmds): Update disassemble help text.
> 
> Index: cli/cli-cmds.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
> retrieving revision 1.73
> diff -u -p -u -p -r1.73 cli-cmds.c
> --- cli/cli-cmds.c	1 Jan 2008 22:53:14 -0000	1.73
> +++ cli/cli-cmds.c	4 Apr 2008 00:33:42 -0000
> @@ -892,10 +892,76 @@ list_command (char *arg, int from_tty)
>  			0);
>  }
>  
> +/* Subroutine of disassemble_command to simplify it.
> +   Perform the disassembly.
> +   NAME is the name of the function if known, or NULL.
> +   [LOW,HIGH) are the range of addresses to disassemble.
> +   MIXED is non-zero to print source with the assembler.  */
> +
> +static void
> +print_disassembly (const char *name, CORE_ADDR low, CORE_ADDR high, int mixed)
> +{
> +#if defined(TUI)
> +  if (!tui_is_window_visible (DISASSEM_WIN))
> +#endif
> +    {
> +      printf_filtered ("Dump of assembler code ");
> +      if (name != NULL)
> +	{
> +	  printf_filtered ("for function %s:\n", name);
> +	}
> +      else
> +	{
> +	  printf_filtered ("from ");
> +	  deprecated_print_address_numeric (low, 1, gdb_stdout);
> +	  printf_filtered (" to ");
> +	  deprecated_print_address_numeric (high, 1, gdb_stdout);
> +	  printf_filtered (":\n");
> +	}
> +
> +      /* Dump the specified range.  */
> +      gdb_disassembly (uiout, 0, 0, mixed, -1, low, high);
> +
> +      printf_filtered ("End of assembler dump.\n");
> +      gdb_flush (gdb_stdout);
> +    }
> +#if defined(TUI)
> +  else
> +    {
> +      tui_show_assembly (low);
> +    }
> +#endif
> +}
> +
> +/* Subroutine of disassemble_command to simplify it.
> +   Print a disassembly of the current function.
> +   MIXED is non-zero to print source with the assembler.  */
> +
> +static void
> +disassemble_current_function (int mixed)
> +{
> +  CORE_ADDR low, high, pc;
> +  char *name;
> +
> +  pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
> +  if (find_pc_partial_function (pc, &name, &low, &high) == 0)
> +    error (_("No function contains program counter for selected frame."));
> +#if defined(TUI)
> +  /* NOTE: cagney/2003-02-13 The `tui_active' was previously
> +     `tui_version'.  */
> +  if (tui_active)
> +    /* FIXME: cagney/2004-02-07: This should be an observer.  */
> +    low = tui_get_low_disassembly_address (low, pc);
> +#endif
> +  low += gdbarch_deprecated_function_start_offset (current_gdbarch);
> +
> +  print_disassembly (name, low, high, mixed);
> +}
> +
>  /* Dump a specified section of assembly code.  With no command line
>     arguments, this command will dump the assembly code for the
>     function surrounding the pc value in the selected frame.  With one
> -   argument, it will dump the assembly code surrounding that pc value.
> +
>     Two arguments are interpeted as bounds within which to dump
>     assembly.  */
>  
> @@ -906,26 +972,44 @@ disassemble_command (char *arg, int from
>    char *name;
>    CORE_ADDR pc, pc_masked;
>    char *space_index;
> -#if 0
> -  asection *section;
> -#endif
> +  int mixed_source_and_assembly;
>  
>    name = NULL;
> -  if (!arg)
> +  mixed_source_and_assembly = 0;
> +
> +  if (arg && *arg == '/')
>      {
> -      pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
> -      if (find_pc_partial_function (pc, &name, &low, &high) == 0)
> -	error (_("No function contains program counter for selected frame."));
> -#if defined(TUI)
> -      /* NOTE: cagney/2003-02-13 The `tui_active' was previously
> -	 `tui_version'.  */
> -      if (tui_active)
> -	/* FIXME: cagney/2004-02-07: This should be an observer.  */
> -	low = tui_get_low_disassembly_address (low, pc);
> -#endif
> -      low += gdbarch_deprecated_function_start_offset (current_gdbarch);
> +      ++arg;
> +
> +      if (*arg == '\0')
> +	error (_("Missing modifier."));
> +
> +      while (*arg && ! isspace (*arg))
> +	{
> +	  switch (*arg++)
> +	    {
> +	    case 's':
> +	      mixed_source_and_assembly = 1;
> +	      break;
> +	    default:
> +	      error (_("Invalid disassembly modifier."));
> +	    }
> +	}
> +
> +      while (isspace (*arg))
> +	++arg;
> +    }
> +
> +  if (! arg || ! *arg)
> +    {
> +      disassemble_current_function (mixed_source_and_assembly);
> +      return;
>      }
> -  else if (!(space_index = (char *) strchr (arg, ' ')))
> +
> +  /* FIXME: 'twould be nice to allow spaces in the expression for the first
> +     arg.  Allow comma separater too?  */
> +
> +  if (!(space_index = (char *) strchr (arg, ' ')))
>      {
>        /* One argument.  */
>        pc = parse_and_eval_address (arg);
> @@ -948,36 +1032,7 @@ disassemble_command (char *arg, int from
>        high = parse_and_eval_address (space_index + 1);
>      }
>  
> -#if defined(TUI)
> -  if (!tui_is_window_visible (DISASSEM_WIN))
> -#endif
> -    {
> -      printf_filtered ("Dump of assembler code ");
> -      if (name != NULL)
> -	{
> -	  printf_filtered ("for function %s:\n", name);
> -	}
> -      else
> -	{
> -	  printf_filtered ("from ");
> -	  deprecated_print_address_numeric (low, 1, gdb_stdout);
> -	  printf_filtered (" to ");
> -	  deprecated_print_address_numeric (high, 1, gdb_stdout);
> -	  printf_filtered (":\n");
> -	}
> -
> -      /* Dump the specified range.  */
> -      gdb_disassembly (uiout, 0, 0, 0, -1, low, high);
> -
> -      printf_filtered ("End of assembler dump.\n");
> -      gdb_flush (gdb_stdout);
> -    }
> -#if defined(TUI)
> -  else
> -    {
> -      tui_show_assembly (low);
> -    }
> -#endif
> +  print_disassembly (name, low, high, mixed_source_and_assembly);
>  }
>  
>  static void
> @@ -1383,6 +1438,7 @@ With two args if one is empty it stands 
>    c = add_com ("disassemble", class_vars, disassemble_command, _("\
>  Disassemble a specified section of memory.\n\
>  Default is the function surrounding the pc of the selected frame.\n\
> +With a leading /s modifier source lines, if available, are included.\n\
>  With a single argument, the function surrounding that address is dumped.\n\
>  Two arguments are taken as a range of memory to dump."));
>    set_cmd_completer (c, location_completer);
> Index: doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.479
> diff -u -p -u -p -r1.479 gdb.texinfo
> --- doc/gdb.texinfo	3 Apr 2008 21:51:11 -0000	1.479
> +++ doc/gdb.texinfo	4 Apr 2008 00:33:42 -0000
> @@ -5419,7 +5419,9 @@ Variables}).
>  @cindex listing machine instructions
>  @item disassemble
>  This specialized command dumps a range of memory as machine
> -instructions.  The default memory range is the function surrounding the
> +instructions.  It can also print mixed source+disassembly by specifying
> +the @code{/s} modifier.
> +The default memory range is the function surrounding the
>  program counter of the selected frame.  A single argument to this
>  command is a program counter value; @value{GDBN} dumps the function
>  surrounding this value.  Two arguments specify a range of addresses

Re: [RFA] mixed source+assembly from cli disassemble

[Joel Brobecker]

> Going on a week...
> No other comments, pro or con?

I really like the idea, it's something that I often wished we'd have
(and often asked for in graphical front-ends). I was going to review
the patch, but wanted to ask others to comment on the user interface
first. Once we agree on the syntax, then I can review the patch itself.

Doug suggests to use an optional "/s" modifier. For instance:

   (gdb) disassemble ADDR     -> regular assembly listing
   (gdb) disassemble /s ADDR  -> intermixed source+assembly listing

Is that OK with everyone? It's pretty consistent with what we do
for the print command, so I like it.

-- 
Joel

Re: [RFA] mixed source+assembly from cli disassemble

[Michael Snyder]

On Thu, 2008-04-10 at 01:15 -0700, Joel Brobecker wrote:
> > Going on a week...
> > No other comments, pro or con?
> 
> I really like the idea, it's something that I often wished we'd have
> (and often asked for in graphical front-ends). I was going to review
> the patch, but wanted to ask others to comment on the user interface
> first. Once we agree on the syntax, then I can review the patch itself.
> 
> Doug suggests to use an optional "/s" modifier. For instance:
> 
>    (gdb) disassemble ADDR     -> regular assembly listing
>    (gdb) disassemble /s ADDR  -> intermixed source+assembly listing
> 
> Is that OK with everyone? It's pretty consistent with what we do
> for the print command, so I like it.

I think using a '/' modifier is good, since that's what 
we use for print and examine and so on.

The choice of the letter 's' is probably as good as any.
The only other one that comes to my mind is 'm' (for "mixed").
And I'm not expressing a preference or objection.

Re: [RFA] mixed source+assembly from cli disassemble

[Michael Snyder]

On Thu, 2008-04-10 at 16:13 -0700, Michael Snyder wrote:
> On Thu, 2008-04-10 at 01:15 -0700, Joel Brobecker wrote:
> > > Going on a week...
> > > No other comments, pro or con?
> > 
> > I really like the idea, it's something that I often wished we'd have
> > (and often asked for in graphical front-ends). I was going to review
> > the patch, but wanted to ask others to comment on the user interface
> > first. Once we agree on the syntax, then I can review the patch itself.
> > 
> > Doug suggests to use an optional "/s" modifier. For instance:
> > 
> >    (gdb) disassemble ADDR     -> regular assembly listing
> >    (gdb) disassemble /s ADDR  -> intermixed source+assembly listing
> > 
> > Is that OK with everyone? It's pretty consistent with what we do
> > for the print command, so I like it.
> 
> I think using a '/' modifier is good, since that's what 
> we use for print and examine and so on.
> 
> The choice of the letter 's' is probably as good as any.
> The only other one that comes to my mind is 'm' (for "mixed").
> And I'm not expressing a preference or objection.

OK, in the absence of further discussion, patch approved.

Doug, as the author, you get to choose the modifier flag.
;-)

Re: [RFA] mixed source+assembly from cli disassemble

[Joel Brobecker]

Doug,

> 2008-04-03  Doug Evans  <dje@google.com>
> 
> 	* cli/cli-cmds.c (print_disassembly): New fn.
> 	(disassemble_current_function): New fn.
> 	(disassemble_command): Recognize /s modifier, print mixed
> 	source+assembly.
> 	(init_cli_cmds): Update disassemble help text.

MichaelS already approved the patch, and I agree with him. I had some
comments, and a request so I'm sending them now.

First, the request: Can you also write up a NEWS entry? I think this
is a great addition and it deserves a mention in NEWS.

The ChangeLog is missing the entry for the documentation update. Please
make sure that EliZ has seen it before checking in.  I'm not sure
whether this is the case or not anymore.

I also personally prefer to avoid abbreviations such as "fn", although
the GNU coding standards say nothing against them - so it may only be
a personally preference. I see that it has been use a few times in
the past, but most uses is in the 90s. Don't change it unless other
maintainers agree that it should be changed.

> +	  printf_filtered ("from ");
> +	  deprecated_print_address_numeric (low, 1, gdb_stdout);
> +	  printf_filtered (" to ");
> +	  deprecated_print_address_numeric (high, 1, gdb_stdout);
> +	  printf_filtered (":\n");

While you are modifying this code, I think the following should work too:

    printf_filtered ("form %s to %s:\n", paddress (low), paddress (high));

Not only does this help us get rid of a couple of uses of deprecated
functions, I believe it's also better for i18n.

Other than that, thanks for doing the work! I'll definitely enjoy
this new feature :).

-- 
Joel

Re: [RFA] mixed source+assembly from cli disassemble

[Doug Evans]

On Wed, Apr 16, 2008 at 11:55 AM, Joel Brobecker <brobecker@adacore.com> wrote:
> Doug,
>
>
>  > 2008-04-03  Doug Evans  <dje@google.com>
>  >
>  >       * cli/cli-cmds.c (print_disassembly): New fn.
>  >       (disassemble_current_function): New fn.
>  >       (disassemble_command): Recognize /s modifier, print mixed
>  >       source+assembly.
>  >       (init_cli_cmds): Update disassemble help text.
>
>  MichaelS already approved the patch, and I agree with him. I had some
>  comments, and a request so I'm sending them now.
>
>  First, the request: Can you also write up a NEWS entry? I think this
>  is a great addition and it deserves a mention in NEWS.

Ok.

>
>  The ChangeLog is missing the entry for the documentation update. Please
>  make sure that EliZ has seen it before checking in.  I'm not sure
>  whether this is the case or not anymore.

Sorry, creature of habit.  Way back when such changelog entries were
never done (per gnu coding standards).

>  I also personally prefer to avoid abbreviations such as "fn", although
>  the GNU coding standards say nothing against them - so it may only be
>  a personally preference. I see that it has been use a few times in
>  the past, but most uses is in the 90s. Don't change it unless other
>  maintainers agree that it should be changed.
>
>
>  > +       printf_filtered ("from ");
>  > +       deprecated_print_address_numeric (low, 1, gdb_stdout);
>  > +       printf_filtered (" to ");
>  > +       deprecated_print_address_numeric (high, 1, gdb_stdout);
>  > +       printf_filtered (":\n");
>
>  While you are modifying this code, I think the following should work too:
>
>     printf_filtered ("form %s to %s:\n", paddress (low), paddress (high));
>
>  Not only does this help us get rid of a couple of uses of deprecated
>  functions, I believe it's also better for i18n.

Righto.

Re: [RFA] mixed source+assembly from cli disassemble

[Doug Evans]

[-- Attachment #1: Type: text/plain, Size: 161 bytes --]

Here is a revised patch with all requested changed.

I changed the modifier to /m.  I know it was only a suggestion
Michael, but it grew on me.

Ok to check in?

[-- Attachment #2: gdb-080416-disass-3.patch.txt --]
[-- Type: text/plain, Size: 8784 bytes --]

2008-04-16  Doug Evans  <dje@google.com>

	* NEWS: Mention new /m modifier for disassemble command.
	* cli/cli-cmds.c (print_disassembly): New function.
	(disassemble_current_function): New function
	(disassemble_command): Recognize /m modifier, print mixed
	source+assembly.
	(init_cli_cmds): Update disassemble help text.

	* gdb.texinfo (disassemble): Document /m modifier.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.264
diff -u -p -u -p -r1.264 NEWS
--- NEWS	4 Apr 2008 15:51:15 -0000	1.264
+++ NEWS	16 Apr 2008 22:14:34 -0000
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 6.8
 
+* The "disassemble" command now supports an optional /m modifier to print mixed
+  source+assembly.
+
 * Watchpoints can now be set on unreadable memory locations, e.g. addresses
 which will be allocated using malloc later in program execution.
 
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.73
diff -u -p -u -p -r1.73 cli-cmds.c
--- cli/cli-cmds.c	1 Jan 2008 22:53:14 -0000	1.73
+++ cli/cli-cmds.c	16 Apr 2008 22:14:38 -0000
@@ -892,12 +892,79 @@ list_command (char *arg, int from_tty)
 			0);
 }
 
-/* Dump a specified section of assembly code.  With no command line
-   arguments, this command will dump the assembly code for the
-   function surrounding the pc value in the selected frame.  With one
-   argument, it will dump the assembly code surrounding that pc value.
-   Two arguments are interpeted as bounds within which to dump
-   assembly.  */
+/* Subroutine of disassemble_command to simplify it.
+   Perform the disassembly.
+   NAME is the name of the function if known, or NULL.
+   [LOW,HIGH) are the range of addresses to disassemble.
+   MIXED is non-zero to print source with the assembler.  */
+
+static void
+print_disassembly (const char *name, CORE_ADDR low, CORE_ADDR high, int mixed)
+{
+#if defined(TUI)
+  if (!tui_is_window_visible (DISASSEM_WIN))
+#endif
+    {
+      printf_filtered ("Dump of assembler code ");
+      if (name != NULL)
+	{
+	  printf_filtered ("for function %s:\n", name);
+	}
+      else
+	{
+	  printf_filtered ("from %s to %s:\n", paddress (low), paddress (high));
+	}
+
+      /* Dump the specified range.  */
+      gdb_disassembly (uiout, 0, 0, mixed, -1, low, high);
+
+      printf_filtered ("End of assembler dump.\n");
+      gdb_flush (gdb_stdout);
+    }
+#if defined(TUI)
+  else
+    {
+      tui_show_assembly (low);
+    }
+#endif
+}
+
+/* Subroutine of disassemble_command to simplify it.
+   Print a disassembly of the current function.
+   MIXED is non-zero to print source with the assembler.  */
+
+static void
+disassemble_current_function (int mixed)
+{
+  CORE_ADDR low, high, pc;
+  char *name;
+
+  pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
+  if (find_pc_partial_function (pc, &name, &low, &high) == 0)
+    error (_("No function contains program counter for selected frame."));
+#if defined(TUI)
+  /* NOTE: cagney/2003-02-13 The `tui_active' was previously
+     `tui_version'.  */
+  if (tui_active)
+    /* FIXME: cagney/2004-02-07: This should be an observer.  */
+    low = tui_get_low_disassembly_address (low, pc);
+#endif
+  low += gdbarch_deprecated_function_start_offset (current_gdbarch);
+
+  print_disassembly (name, low, high, mixed);
+}
+
+/* Dump a specified section of assembly code.
+
+   Usage:
+     disassemble [/m]
+       - dump the assembly code for the function of the current pc
+     disassemble [/m] addr
+       - dump the assembly code for the function at ADDR
+     disassemble [/m] low high
+       - dump the assembly code in the range [LOW,HIGH)
+
+   A /m modifier will print mixed source+assembly.  */
 
 static void
 disassemble_command (char *arg, int from_tty)
@@ -906,26 +973,44 @@ disassemble_command (char *arg, int from
   char *name;
   CORE_ADDR pc, pc_masked;
   char *space_index;
-#if 0
-  asection *section;
-#endif
+  int mixed_source_and_assembly;
 
   name = NULL;
-  if (!arg)
+  mixed_source_and_assembly = 0;
+
+  if (arg && *arg == '/')
     {
-      pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
-      if (find_pc_partial_function (pc, &name, &low, &high) == 0)
-	error (_("No function contains program counter for selected frame."));
-#if defined(TUI)
-      /* NOTE: cagney/2003-02-13 The `tui_active' was previously
-	 `tui_version'.  */
-      if (tui_active)
-	/* FIXME: cagney/2004-02-07: This should be an observer.  */
-	low = tui_get_low_disassembly_address (low, pc);
-#endif
-      low += gdbarch_deprecated_function_start_offset (current_gdbarch);
+      ++arg;
+
+      if (*arg == '\0')
+	error (_("Missing modifier."));
+
+      while (*arg && ! isspace (*arg))
+	{
+	  switch (*arg++)
+	    {
+	    case 'm':
+	      mixed_source_and_assembly = 1;
+	      break;
+	    default:
+	      error (_("Invalid disassembly modifier."));
+	    }
+	}
+
+      while (isspace (*arg))
+	++arg;
     }
-  else if (!(space_index = (char *) strchr (arg, ' ')))
+
+  if (! arg || ! *arg)
+    {
+      disassemble_current_function (mixed_source_and_assembly);
+      return;
+    }
+
+  /* FIXME: 'twould be nice to allow spaces in the expression for the first
+     arg.  Allow comma separater too?  */
+
+  if (!(space_index = (char *) strchr (arg, ' ')))
     {
       /* One argument.  */
       pc = parse_and_eval_address (arg);
@@ -948,36 +1033,7 @@ disassemble_command (char *arg, int from
       high = parse_and_eval_address (space_index + 1);
     }
 
-#if defined(TUI)
-  if (!tui_is_window_visible (DISASSEM_WIN))
-#endif
-    {
-      printf_filtered ("Dump of assembler code ");
-      if (name != NULL)
-	{
-	  printf_filtered ("for function %s:\n", name);
-	}
-      else
-	{
-	  printf_filtered ("from ");
-	  deprecated_print_address_numeric (low, 1, gdb_stdout);
-	  printf_filtered (" to ");
-	  deprecated_print_address_numeric (high, 1, gdb_stdout);
-	  printf_filtered (":\n");
-	}
-
-      /* Dump the specified range.  */
-      gdb_disassembly (uiout, 0, 0, 0, -1, low, high);
-
-      printf_filtered ("End of assembler dump.\n");
-      gdb_flush (gdb_stdout);
-    }
-#if defined(TUI)
-  else
-    {
-      tui_show_assembly (low);
-    }
-#endif
+  print_disassembly (name, low, high, mixed_source_and_assembly);
 }
 
 static void
@@ -1383,6 +1439,7 @@ With two args if one is empty it stands 
   c = add_com ("disassemble", class_vars, disassemble_command, _("\
 Disassemble a specified section of memory.\n\
 Default is the function surrounding the pc of the selected frame.\n\
+With a /m modifier source lines, if available, are included.\n\
 With a single argument, the function surrounding that address is dumped.\n\
 Two arguments are taken as a range of memory to dump."));
   set_cmd_completer (c, location_completer);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.481
diff -u -p -u -p -r1.481 gdb.texinfo
--- doc/gdb.texinfo	16 Apr 2008 13:14:18 -0000	1.481
+++ doc/gdb.texinfo	16 Apr 2008 22:14:40 -0000
@@ -5418,8 +5418,11 @@ Variables}).
 @cindex machine instructions
 @cindex listing machine instructions
 @item disassemble
+@itemx disassemble /@var{m}
 This specialized command dumps a range of memory as machine
-instructions.  The default memory range is the function surrounding the
+instructions.  It can also print mixed source+disassembly by specifying
+the @code{/m} modifier.
+The default memory range is the function surrounding the
 program counter of the selected frame.  A single argument to this
 command is a program counter value; @value{GDBN} dumps the function
 surrounding this value.  Two arguments specify a range of addresses
@@ -5443,6 +5446,31 @@ Dump of assembler code from 0x32c4 to 0x
 End of assembler dump.
 @end smallexample
 
+Here is an example showing mixed source+assembly for Intel x86:
+
+@smallexample
+(@value{GDBP}) disas /m main
+Dump of assembler code for function main:
+5       @{
+0x08048330 <main+0>:    push   %ebp
+0x08048331 <main+1>:    mov    %esp,%ebp
+0x08048333 <main+3>:    sub    $0x8,%esp
+0x08048336 <main+6>:    and    $0xfffffff0,%esp
+0x08048339 <main+9>:    sub    $0x10,%esp
+
+6         printf ("Hello.\n");
+0x0804833c <main+12>:   movl   $0x8048440,(%esp)
+0x08048343 <main+19>:   call   0x8048284 <puts@@plt>
+
+7         return 0;
+8       @}
+0x08048348 <main+24>:   mov    $0x0,%eax
+0x0804834d <main+29>:   leave
+0x0804834e <main+30>:   ret
+
+End of assembler dump.
+@end smallexample
+
 Some architectures have more than one commonly-used set of instruction
 mnemonics or other syntax.
 

Re: [RFA] mixed source+assembly from cli disassemble

[Joel Brobecker]

> 2008-04-16  Doug Evans  <dje@google.com>
> 
> 	* NEWS: Mention new /m modifier for disassemble command.
> 	* cli/cli-cmds.c (print_disassembly): New function.
> 	(disassemble_current_function): New function
> 	(disassemble_command): Recognize /m modifier, print mixed
> 	source+assembly.
> 	(init_cli_cmds): Update disassemble help text.

The code part of your patch is good to go, with a couple of tiny
adjustments. Let's wait for Eli's feedback before checking in, since
there is some documentation being adjusted in the code too.

> +      printf_filtered ("Dump of assembler code ");
> +      if (name != NULL)
> +	{
> +	  printf_filtered ("for function %s:\n", name);
> +	}
> +      else
> +	{
> +	  printf_filtered ("from %s to %s:\n", paddress (low), paddress (high));
> +	}

We don't need the curly braces in this case. Can you remove them?

> +      if (*arg == '\0')
> +	error (_("Missing modifier."));
> +
> +      while (*arg && ! isspace (*arg))
> +	{
> +	  switch (*arg++)
> +	    {
> +	    case 'm':
> +	      mixed_source_and_assembly = 1;
> +	      break;
> +	    default:
> +	      error (_("Invalid disassembly modifier."));
> +	    }
> +	}
> +
> +      while (isspace (*arg))
> +	++arg;

The formatting looks weird, because some of the lines (the "while" lines
for instance) are using spaces instead of tabs. Could you fix that to
use TABs, please?

> @@ -1383,6 +1439,7 @@ With two args if one is empty it stands 
>    c = add_com ("disassemble", class_vars, disassemble_command, _("\
>  Disassemble a specified section of memory.\n\
>  Default is the function surrounding the pc of the selected frame.\n\
> +With a /m modifier source lines, if available, are included.\n\

I'd like to have Eli's feedback on this change. I would phrase
differently (the current form is missing a coma to make it
intelligible):

  With a /m modifier, source lines are included (if available).

-- 
Joel

Re: [RFA] mixed source+assembly from cli disassemble

[Daniel Jacobowitz]

On Thu, Apr 17, 2008 at 02:25:19PM -0700, Joel Brobecker wrote:
> > +      if (*arg == '\0')
> > +	error (_("Missing modifier."));
> > +
> > +      while (*arg && ! isspace (*arg))
> > +	{
> > +	  switch (*arg++)
> > +	    {
> > +	    case 'm':
> > +	      mixed_source_and_assembly = 1;
> > +	      break;
> > +	    default:
> > +	      error (_("Invalid disassembly modifier."));
> > +	    }
> > +	}
> > +
> > +      while (isspace (*arg))
> > +	++arg;
> 
> The formatting looks weird, because some of the lines (the "while" lines
> for instance) are using spaces instead of tabs. Could you fix that to
> use TABs, please?

It looks right to me.  It just gets weirder and weirder as you add
more levels of email quoting.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] mixed source+assembly from cli disassemble

[Joel Brobecker]

> It looks right to me.  It just gets weirder and weirder as you add
> more levels of email quoting.

Sorry - sometimes, if feels like I'll never manage to get used to
this tabs/spaces mix. Everything would be so much simpler if it were
all spaces... (just a wish, not a request to rediscuss this)

-- 
Joel

Fwd: [RFA] mixed source+assembly from cli disassemble

[Doug Evans]

[-- Attachment #1: Type: text/plain, Size: 2789 bytes --]

Ping.  Eli, Joel requests review of the doc parts of this patch.

On Thu, Apr 17, 2008 at 2:25 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> > 2008-04-16  Doug Evans  <dje@google.com>
>  >
>  >       * NEWS: Mention new /m modifier for disassemble command.
>  >       * cli/cli-cmds.c (print_disassembly): New function.
>  >       (disassemble_current_function): New function
>  >       (disassemble_command): Recognize /m modifier, print mixed
>
> >       source+assembly.
>  >       (init_cli_cmds): Update disassemble help text.
>
>  The code part of your patch is good to go, with a couple of tiny
>  adjustments. Let's wait for Eli's feedback before checking in, since
>  there is some documentation being adjusted in the code too.
>
>
>  > +      printf_filtered ("Dump of assembler code ");
>  > +      if (name != NULL)
>  > +     {
>  > +       printf_filtered ("for function %s:\n", name);
>  > +     }
>  > +      else
>  > +     {
>  > +       printf_filtered ("from %s to %s:\n", paddress (low), paddress (high));
>  > +     }
>
>  We don't need the curly braces in this case. Can you remove them?
>
>
>  > +      if (*arg == '\0')
>  > +     error (_("Missing modifier."));
>  > +
>  > +      while (*arg && ! isspace (*arg))
>  > +     {
>  > +       switch (*arg++)
>  > +         {
>  > +         case 'm':
>
> > +           mixed_source_and_assembly = 1;
>  > +           break;
>  > +         default:
>  > +           error (_("Invalid disassembly modifier."));
>  > +         }
>  > +     }
>  > +
>  > +      while (isspace (*arg))
>  > +     ++arg;
>
>  The formatting looks weird, because some of the lines (the "while" lines
>  for instance) are using spaces instead of tabs. Could you fix that to
>  use TABs, please?
>
>  > @@ -1383,6 +1439,7 @@ With two args if one is empty it stands
>
> >    c = add_com ("disassemble", class_vars, disassemble_command, _("\
>  >  Disassemble a specified section of memory.\n\
>  >  Default is the function surrounding the pc of the selected frame.\n\
>  > +With a /m modifier source lines, if available, are included.\n\
>
>  I'd like to have Eli's feedback on this change. I would phrase
>  differently (the current form is missing a coma to make it
>  intelligible):
>
>   With a /m modifier, source lines are included (if available).


---------- Forwarded message ----------
From: Doug Evans <dje@google.com>
Date: Wed, Apr 16, 2008 at 3:25 PM
Subject: Re: [RFA] mixed source+assembly from cli disassemble
To: Joel Brobecker <brobecker@adacore.com>, Michael Snyder
<msnyder@specifix.com>, Eli Zaretskii <eliz@gnu.org>
Cc: gdb-patches@sourceware.org


Here is a revised patch with all requested changed.

 I changed the modifier to /m.  I know it was only a suggestion
 Michael, but it grew on me.

 Ok to check in?

[-- Attachment #2: gdb-080416-disass-3.patch.txt --]
[-- Type: text/plain, Size: 8784 bytes --]

2008-04-16  Doug Evans  <dje@google.com>

	* NEWS: Mention new /m modifier for disassemble command.
	* cli/cli-cmds.c (print_disassembly): New function.
	(disassemble_current_function): New function
	(disassemble_command): Recognize /m modifier, print mixed
	source+assembly.
	(init_cli_cmds): Update disassemble help text.

	* gdb.texinfo (disassemble): Document /m modifier.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.264
diff -u -p -u -p -r1.264 NEWS
--- NEWS	4 Apr 2008 15:51:15 -0000	1.264
+++ NEWS	16 Apr 2008 22:14:34 -0000
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 6.8
 
+* The "disassemble" command now supports an optional /m modifier to print mixed
+  source+assembly.
+
 * Watchpoints can now be set on unreadable memory locations, e.g. addresses
 which will be allocated using malloc later in program execution.
 
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.73
diff -u -p -u -p -r1.73 cli-cmds.c
--- cli/cli-cmds.c	1 Jan 2008 22:53:14 -0000	1.73
+++ cli/cli-cmds.c	16 Apr 2008 22:14:38 -0000
@@ -892,12 +892,79 @@ list_command (char *arg, int from_tty)
 			0);
 }
 
-/* Dump a specified section of assembly code.  With no command line
-   arguments, this command will dump the assembly code for the
-   function surrounding the pc value in the selected frame.  With one
-   argument, it will dump the assembly code surrounding that pc value.
-   Two arguments are interpeted as bounds within which to dump
-   assembly.  */
+/* Subroutine of disassemble_command to simplify it.
+   Perform the disassembly.
+   NAME is the name of the function if known, or NULL.
+   [LOW,HIGH) are the range of addresses to disassemble.
+   MIXED is non-zero to print source with the assembler.  */
+
+static void
+print_disassembly (const char *name, CORE_ADDR low, CORE_ADDR high, int mixed)
+{
+#if defined(TUI)
+  if (!tui_is_window_visible (DISASSEM_WIN))
+#endif
+    {
+      printf_filtered ("Dump of assembler code ");
+      if (name != NULL)
+	{
+	  printf_filtered ("for function %s:\n", name);
+	}
+      else
+	{
+	  printf_filtered ("from %s to %s:\n", paddress (low), paddress (high));
+	}
+
+      /* Dump the specified range.  */
+      gdb_disassembly (uiout, 0, 0, mixed, -1, low, high);
+
+      printf_filtered ("End of assembler dump.\n");
+      gdb_flush (gdb_stdout);
+    }
+#if defined(TUI)
+  else
+    {
+      tui_show_assembly (low);
+    }
+#endif
+}
+
+/* Subroutine of disassemble_command to simplify it.
+   Print a disassembly of the current function.
+   MIXED is non-zero to print source with the assembler.  */
+
+static void
+disassemble_current_function (int mixed)
+{
+  CORE_ADDR low, high, pc;
+  char *name;
+
+  pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
+  if (find_pc_partial_function (pc, &name, &low, &high) == 0)
+    error (_("No function contains program counter for selected frame."));
+#if defined(TUI)
+  /* NOTE: cagney/2003-02-13 The `tui_active' was previously
+     `tui_version'.  */
+  if (tui_active)
+    /* FIXME: cagney/2004-02-07: This should be an observer.  */
+    low = tui_get_low_disassembly_address (low, pc);
+#endif
+  low += gdbarch_deprecated_function_start_offset (current_gdbarch);
+
+  print_disassembly (name, low, high, mixed);
+}
+
+/* Dump a specified section of assembly code.
+
+   Usage:
+     disassemble [/m]
+       - dump the assembly code for the function of the current pc
+     disassemble [/m] addr
+       - dump the assembly code for the function at ADDR
+     disassemble [/m] low high
+       - dump the assembly code in the range [LOW,HIGH)
+
+   A /m modifier will print mixed source+assembly.  */
 
 static void
 disassemble_command (char *arg, int from_tty)
@@ -906,26 +973,44 @@ disassemble_command (char *arg, int from
   char *name;
   CORE_ADDR pc, pc_masked;
   char *space_index;
-#if 0
-  asection *section;
-#endif
+  int mixed_source_and_assembly;
 
   name = NULL;
-  if (!arg)
+  mixed_source_and_assembly = 0;
+
+  if (arg && *arg == '/')
     {
-      pc = get_frame_pc (get_selected_frame (_("No frame selected.")));
-      if (find_pc_partial_function (pc, &name, &low, &high) == 0)
-	error (_("No function contains program counter for selected frame."));
-#if defined(TUI)
-      /* NOTE: cagney/2003-02-13 The `tui_active' was previously
-	 `tui_version'.  */
-      if (tui_active)
-	/* FIXME: cagney/2004-02-07: This should be an observer.  */
-	low = tui_get_low_disassembly_address (low, pc);
-#endif
-      low += gdbarch_deprecated_function_start_offset (current_gdbarch);
+      ++arg;
+
+      if (*arg == '\0')
+	error (_("Missing modifier."));
+
+      while (*arg && ! isspace (*arg))
+	{
+	  switch (*arg++)
+	    {
+	    case 'm':
+	      mixed_source_and_assembly = 1;
+	      break;
+	    default:
+	      error (_("Invalid disassembly modifier."));
+	    }
+	}
+
+      while (isspace (*arg))
+	++arg;
     }
-  else if (!(space_index = (char *) strchr (arg, ' ')))
+
+  if (! arg || ! *arg)
+    {
+      disassemble_current_function (mixed_source_and_assembly);
+      return;
+    }
+
+  /* FIXME: 'twould be nice to allow spaces in the expression for the first
+     arg.  Allow comma separater too?  */
+
+  if (!(space_index = (char *) strchr (arg, ' ')))
     {
       /* One argument.  */
       pc = parse_and_eval_address (arg);
@@ -948,36 +1033,7 @@ disassemble_command (char *arg, int from
       high = parse_and_eval_address (space_index + 1);
     }
 
-#if defined(TUI)
-  if (!tui_is_window_visible (DISASSEM_WIN))
-#endif
-    {
-      printf_filtered ("Dump of assembler code ");
-      if (name != NULL)
-	{
-	  printf_filtered ("for function %s:\n", name);
-	}
-      else
-	{
-	  printf_filtered ("from ");
-	  deprecated_print_address_numeric (low, 1, gdb_stdout);
-	  printf_filtered (" to ");
-	  deprecated_print_address_numeric (high, 1, gdb_stdout);
-	  printf_filtered (":\n");
-	}
-
-      /* Dump the specified range.  */
-      gdb_disassembly (uiout, 0, 0, 0, -1, low, high);
-
-      printf_filtered ("End of assembler dump.\n");
-      gdb_flush (gdb_stdout);
-    }
-#if defined(TUI)
-  else
-    {
-      tui_show_assembly (low);
-    }
-#endif
+  print_disassembly (name, low, high, mixed_source_and_assembly);
 }
 
 static void
@@ -1383,6 +1439,7 @@ With two args if one is empty it stands 
   c = add_com ("disassemble", class_vars, disassemble_command, _("\
 Disassemble a specified section of memory.\n\
 Default is the function surrounding the pc of the selected frame.\n\
+With a /m modifier source lines, if available, are included.\n\
 With a single argument, the function surrounding that address is dumped.\n\
 Two arguments are taken as a range of memory to dump."));
   set_cmd_completer (c, location_completer);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.481
diff -u -p -u -p -r1.481 gdb.texinfo
--- doc/gdb.texinfo	16 Apr 2008 13:14:18 -0000	1.481
+++ doc/gdb.texinfo	16 Apr 2008 22:14:40 -0000
@@ -5418,8 +5418,11 @@ Variables}).
 @cindex machine instructions
 @cindex listing machine instructions
 @item disassemble
+@itemx disassemble /@var{m}
 This specialized command dumps a range of memory as machine
-instructions.  The default memory range is the function surrounding the
+instructions.  It can also print mixed source+disassembly by specifying
+the @code{/m} modifier.
+The default memory range is the function surrounding the
 program counter of the selected frame.  A single argument to this
 command is a program counter value; @value{GDBN} dumps the function
 surrounding this value.  Two arguments specify a range of addresses
@@ -5443,6 +5446,31 @@ Dump of assembler code from 0x32c4 to 0x
 End of assembler dump.
 @end smallexample
 
+Here is an example showing mixed source+assembly for Intel x86:
+
+@smallexample
+(@value{GDBP}) disas /m main
+Dump of assembler code for function main:
+5       @{
+0x08048330 <main+0>:    push   %ebp
+0x08048331 <main+1>:    mov    %esp,%ebp
+0x08048333 <main+3>:    sub    $0x8,%esp
+0x08048336 <main+6>:    and    $0xfffffff0,%esp
+0x08048339 <main+9>:    sub    $0x10,%esp
+
+6         printf ("Hello.\n");
+0x0804833c <main+12>:   movl   $0x8048440,(%esp)
+0x08048343 <main+19>:   call   0x8048284 <puts@@plt>
+
+7         return 0;
+8       @}
+0x08048348 <main+24>:   mov    $0x0,%eax
+0x0804834d <main+29>:   leave
+0x0804834e <main+30>:   ret
+
+End of assembler dump.
+@end smallexample
+
 Some architectures have more than one commonly-used set of instruction
 mnemonics or other syntax.
 

Re: Fwd: [RFA] mixed source+assembly from cli disassemble

[Eli Zaretskii]

> Date: Mon, 28 Apr 2008 07:13:05 -0700
> From: "Doug Evans" <dje@google.com>
> Cc: "Joel Brobecker" <brobecker@adacore.com>,
>         "GDB Patches" <gdb-patches@sourceware.org>
> 
> Ping.  Eli, Joel requests review of the doc parts of this patch.

Yes, sorry I missed it.

> > >    c = add_com ("disassemble", class_vars, disassemble_command, _("\
> >  >  Disassemble a specified section of memory.\n\
> >  >  Default is the function surrounding the pc of the selected frame.\n\
> >  > +With a /m modifier source lines, if available, are included.\n\
> >
> >  I'd like to have Eli's feedback on this change. I would phrase
> >  differently (the current form is missing a coma to make it
> >  intelligible):
> >
> >   With a /m modifier, source lines are included (if available).

I like the latter wording better, too.

>  @item disassemble
> +@itemx disassemble /@var{m}

It is wrong to use @var here, because "/m" is a literal string, it
doesn't stand for anything else.

Other than that, I'm okay with the doco changes.  Thanks.

Re: Fwd: [RFA] mixed source+assembly from cli disassemble

[Doug Evans]

[-- Attachment #1: Type: text/plain, Size: 68 bytes --]

fyi, I have checked the disassemble /m patch in with this addendum.

[-- Attachment #2: gdb-080505-help-disass-1.patch.txt --]
[-- Type: text/plain, Size: 2136 bytes --]

2008-05-05  Doug Evans  <dje@google.com>

	* gdb.base/help.exp (disassemble): Update expected help text.

Index: gdb.base/help.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/help.exp,v
retrieving revision 1.27
diff -u -p -u -p -r1.27 help.exp
--- gdb.base/help.exp	1 Jan 2008 22:53:19 -0000	1.27
+++ gdb.base/help.exp	5 May 2008 22:53:40 -0000
@@ -126,7 +126,7 @@ gdb_test "help disable breakpoints" "Dis
 # test help disable display
 gdb_test "help disable display" "Disable some expressions to be displayed when program stops\.\[\r\n\]+Arguments are the code numbers of the expressions to stop displaying\.\[\r\n\]+No argument means disable all automatic-display expressions\.\[\r\n\]+Do \"info display\" to see current list of code numbers\." "help disable display"
 # test help disassemble
-gdb_test "help disassemble" "Disassemble a specified section of memory\.\[\r\n\]+Default is the function surrounding the pc of the selected frame\.\[\r\n\]+With a single argument, the function surrounding that address is dumped\.\[\r\n\]+Two arguments are taken as a range of memory to dump\." "help disassemble"
+gdb_test "help disassemble" "Disassemble a specified section of memory\.\[\r\n\]+Default is the function surrounding the pc of the selected frame\.\[\r\n\]+With a /m modifier, source lines are included \\(if available\\)\.\[\r\n\]+With a single argument, the function surrounding that address is dumped\.\[\r\n\]+Two arguments are taken as a range of memory to dump\." "help disassemble"
 # test help display
 gdb_test "help display" "Print value of expression EXP each time the program stops\.\[\r\n\]+/FMT may be used before EXP as in the \"print\" command\.\[\r\n\]+/FMT \"i\" or \"s\" or including a size-letter is allowed,\[\r\n\]+as in the \"x\" command, and then EXP is used to get the address to examine\[\r\n\]+and examining is done as in the \"x\" command\.\[\r\n\]+With no argument, display all currently requested auto-display expressions\.\[\r\n\]+Use \"undisplay\" to cancel display requests previously made\." "help display"
 # test help do