Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch] Allow "disassemble 'Foo::bar(char *)'"
@ 2009-11-14  0:10 Paul Pluzhnikov
  2009-11-15  8:54 ` Andreas Schwab
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-14  0:10 UTC (permalink / raw)
  To: gdb-patches; +Cc: ppluzhnikov

Greetings,

Consider this test:

struct Foo
{
  void bar(const char *s) { }
};

int main (void)
{
  Foo().bar("hello");
  return 0;
}


And the following GDB behavior:

(gdb) disas 'Foo::b<TAB>      ## completes to:
(gdb) disas 'Foo::bar(char const*)' 
Unmatched single quote.


This (IMHO very confusing behavior) is happening because disassemble_command
unconditionally splits the argument on the first space, and then tries to
parse "'Foo::bar(char" as the start address.

Attached patch fixes that (no regressions on Linux/x86_64).

Thanks,
--
Paul Pluzhnikov

2009-11-13  Paul Pluzhnikov  <ppluzhnikov@google.com>

	*  cli/cli-cmds.c (disassemble_command): Respect quotes.


Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.93
diff -u -p -u -r1.93 cli-cmds.c
--- cli/cli-cmds.c	23 Oct 2009 00:49:33 -0000	1.93
+++ cli/cli-cmds.c	13 Nov 2009 23:59:08 -0000
@@ -1025,7 +1025,8 @@ disassemble_command (char *arg, int from
   /* FIXME: 'twould be nice to allow spaces in the expression for the first
      arg.  Allow comma separater too?  */
 
-  if (!(space_index = (char *) strchr (arg, ' ')))
+  space_index = skip_quoted (arg);
+  if (!(space_index = (char *) strchr (space_index, ' ')))
     {
       /* One argument.  */
       pc = parse_and_eval_address (arg);


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-14  0:10 [patch] Allow "disassemble 'Foo::bar(char *)'" Paul Pluzhnikov
@ 2009-11-15  8:54 ` Andreas Schwab
  2009-11-15 17:17   ` Paul Pluzhnikov
  0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2009-11-15  8:54 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: gdb-patches

ppluzhnikov@google.com (Paul Pluzhnikov) writes:

> Index: cli/cli-cmds.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
> retrieving revision 1.93
> diff -u -p -u -r1.93 cli-cmds.c
> --- cli/cli-cmds.c	23 Oct 2009 00:49:33 -0000	1.93
> +++ cli/cli-cmds.c	13 Nov 2009 23:59:08 -0000
> @@ -1025,7 +1025,8 @@ disassemble_command (char *arg, int from
>    /* FIXME: 'twould be nice to allow spaces in the expression for the first
>       arg.  Allow comma separater too?  */
>  
> -  if (!(space_index = (char *) strchr (arg, ' ')))
> +  space_index = skip_quoted (arg);
> +  if (!(space_index = (char *) strchr (space_index, ' ')))

Looks like the comment is now obsolete?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-15  8:54 ` Andreas Schwab
@ 2009-11-15 17:17   ` Paul Pluzhnikov
  2009-11-15 17:26     ` Daniel Jacobowitz
  2009-11-16 18:26     ` Tom Tromey
  0 siblings, 2 replies; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-15 17:17 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: gdb-patches

On Sun, Nov 15, 2009 at 12:53 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:

> Looks like the comment is now obsolete?

No, not really:

(top-gdb) disas &main+1 &main+5
Dump of assembler code from 0x453c39 to 0x453c3d:
   0x0000000000453c39 <main+1>:	mov    %rsp,%rbp
   0x0000000000453c3c <main+4>:	sub    $0x30,%rsp
End of assembler dump.

(top-gdb) disas &main+1 &main + 5
Dump of assembler code from 0x453c39 to 0x453c3d:
   0x0000000000453c39 <main+1>:	mov    %rsp,%rbp
   0x0000000000453c3c <main+4>:	sub    $0x30,%rsp
End of assembler dump.

(top-gdb) disas &main + 1 &main + 5
Argument to arithmetic operation not a number or boolean.


The only way I can see to allow general expressions here is to require
a separator which can't happen in any expression in any language
(semicolon probably comes close), or for parse_expression() to consume
as much as it can and set a "end_of_parse" indicator.

But perhaps splitting on comma (after skipping any quoted expression
-- Foo::bar could have multiple parameters) is "good enough"? That
would allow 'disas &main + 1, &main + 5' to work.


Thanks,
-- 
Paul Pluzhnikov


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-15 17:17   ` Paul Pluzhnikov
@ 2009-11-15 17:26     ` Daniel Jacobowitz
  2009-11-16 18:26     ` Tom Tromey
  1 sibling, 0 replies; 20+ messages in thread
From: Daniel Jacobowitz @ 2009-11-15 17:26 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: Andreas Schwab, gdb-patches

On Sun, Nov 15, 2009 at 09:16:06AM -0800, Paul Pluzhnikov wrote:
> The only way I can see to allow general expressions here is to require
> a separator which can't happen in any expression in any language
> (semicolon probably comes close), or for parse_expression() to consume
> as much as it can and set a "end_of_parse" indicator.
> 
> But perhaps splitting on comma (after skipping any quoted expression
> -- Foo::bar could have multiple parameters) is "good enough"? That
> would allow 'disas &main + 1, &main + 5' to work.

See parse_to_comma_and_eval (which I always forget about in this
situation...).

It's not perfect, but it does handle parenthesis nesting.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-15 17:17   ` Paul Pluzhnikov
  2009-11-15 17:26     ` Daniel Jacobowitz
@ 2009-11-16 18:26     ` Tom Tromey
  2009-11-16 18:29       ` Paul Pluzhnikov
  1 sibling, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2009-11-16 18:26 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: Andreas Schwab, gdb-patches

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> But perhaps splitting on comma (after skipping any quoted expression
Paul> -- Foo::bar could have multiple parameters) is "good enough"? That
Paul> would allow 'disas &main + 1, &main + 5' to work.

I'm ok with changing this, mostly because I think the current approach
is bogus -- but I can understand that some people might not like it
because it is not backward compatible.

Are you planning to work on this?  If not, I think your original patch
is ok.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-16 18:26     ` Tom Tromey
@ 2009-11-16 18:29       ` Paul Pluzhnikov
  2009-11-19 23:41         ` Paul Pluzhnikov
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-16 18:29 UTC (permalink / raw)
  To: tromey; +Cc: Andreas Schwab, gdb-patches

On Mon, Nov 16, 2009 at 10:25 AM, Tom Tromey <tromey@redhat.com> wrote:

> Are you planning to work on this?

Yes: I will look at parse_to_comma_and_eval() (Daniel's suggestion),
and see if this all can be handled in backward-compatible manner.

Thanks,
-- 
Paul Pluzhnikov


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-16 18:29       ` Paul Pluzhnikov
@ 2009-11-19 23:41         ` Paul Pluzhnikov
  2009-11-20 17:28           ` Tom Tromey
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-19 23:41 UTC (permalink / raw)
  To: tromey; +Cc: Andreas Schwab, gdb-patches

On Mon, Nov 16, 2009 at 10:28 AM, Paul Pluzhnikov
<ppluzhnikov@google.com> wrote:

> Yes: I will look at parse_to_comma_and_eval() (Daniel's suggestion),
> and see if this all can be handled in backward-compatible manner.

It works, but I can't make it backward-compatible and not require a comma
for two-argument invocation :-(

This works:

(gdb) disas 'Foo::bar(char const*, char const*)'
Dump of assembler code for function _ZN3Foo3barEPKcS1_:
   0x000000000040055a <+0>:     push   %rbp
   0x000000000040055b <+1>:     mov    %rsp,%rbp
   0x000000000040055e <+4>:     mov    %rdi,-0x8(%rbp)
   0x0000000000400562 <+8>:     mov    %rsi,-0x10(%rbp)
   0x0000000000400566 <+12>:    mov    %rdx,-0x18(%rbp)
   0x000000000040056a <+16>:    leaveq
   0x000000000040056b <+17>:    retq
End of assembler dump.

(gdb) disas 0x40055a + 1
Dump of assembler code for function _ZN3Foo3barEPKcS1_:
   0x000000000040055a <+0>:     push   %rbp
   0x000000000040055b <+1>:     mov    %rsp,%rbp
   0x000000000040055e <+4>:     mov    %rdi,-0x8(%rbp)
   0x0000000000400562 <+8>:     mov    %rsi,-0x10(%rbp)
   0x0000000000400566 <+12>:    mov    %rdx,-0x18(%rbp)
   0x000000000040056a <+16>:    leaveq
   0x000000000040056b <+17>:    retq
End of assembler dump.

(gdb) disas 0x40055a + 1, 0x40055a + 5
Dump of assembler code from 0x40055b to 0x40055f:
   0x000000000040055b <_ZN3Foo3barEPKcS1_+1>:   mov    %rsp,%rbp
   0x000000000040055e <_ZN3Foo3barEPKcS1_+4>:   mov    %rdi,-0x8(%rbp)
End of assembler dump.


But this doesn't:

(gdb) disas 0x400540 0x400553
A syntax error in expression, near `0x400553'.


If it's acceptable to require comma for two-argument command (which IMO
it is), I'll update documentation and testsuites, and produce a "real" patch.

Attaching what I've got so far.

Thanks,
-- 
Paul Pluzhnikov




Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.93
diff -u -p -u -r1.93 cli-cmds.c
--- cli/cli-cmds.c	23 Oct 2009 00:49:33 -0000	1.93
+++ cli/cli-cmds.c	19 Nov 2009 23:37:39 -0000
@@ -983,7 +983,6 @@ disassemble_command (char *arg, int from
   CORE_ADDR low, high;
   char *name;
   CORE_ADDR pc, pc_masked;
-  char *space_index;
   int flags;

   name = NULL;
@@ -1022,13 +1021,12 @@ disassemble_command (char *arg, int from
       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, ' ')))
+  pc = value_as_address (parse_to_comma_and_eval (&arg));
+  if (arg[0] == ',')
+    ++arg;
+  if (arg[0] == '\0')
     {
       /* One argument.  */
-      pc = parse_and_eval_address (arg);
       if (find_pc_partial_function (pc, &name, &low, &high) == 0)
 	error (_("No function contains specified address."));
 #if defined(TUI)
@@ -1044,9 +1042,8 @@ disassemble_command (char *arg, int from
   else
     {
       /* Two arguments.  */
-      *space_index = '\0';
-      low = parse_and_eval_address (arg);
-      high = parse_and_eval_address (space_index + 1);
+      low = pc;
+      high = parse_and_eval_address (arg);
     }

   print_disassembly (gdbarch, name, low, high, flags);


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-19 23:41         ` Paul Pluzhnikov
@ 2009-11-20 17:28           ` Tom Tromey
  2009-11-20 17:41             ` Paul Pluzhnikov
  2009-11-21  0:44             ` Paul Pluzhnikov
  0 siblings, 2 replies; 20+ messages in thread
From: Tom Tromey @ 2009-11-20 17:28 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: Andreas Schwab, gdb-patches

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> It works, but I can't make it backward-compatible and not require a comma
Paul> for two-argument invocation :-(

Yeah.

Paul> If it's acceptable to require comma for two-argument command
Paul> (which IMO it is), I'll update documentation and testsuites, and
Paul> produce a "real" patch.

It is fine by me.

If anybody objects it would be helpful if they would speak up sooner
rather than later.

Paul> +  if (arg[0] == ',')
Paul> +    ++arg;
Paul> +  if (arg[0] == '\0')

You probably want to skip whitespace after the ",".

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-20 17:28           ` Tom Tromey
@ 2009-11-20 17:41             ` Paul Pluzhnikov
  2009-11-21  0:44             ` Paul Pluzhnikov
  1 sibling, 0 replies; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-20 17:41 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Andreas Schwab, gdb-patches

On Fri, Nov 20, 2009 at 9:27 AM, Tom Tromey <tromey@redhat.com> wrote:

> You probably want to skip whitespace after the ",".

parse_and_eval_address() does it on its own (as
"disas 0x40055a + 1, 0x40055a + 5" demonstrates).

I didn't see the point in adding extra "while (isspace)" loop.

Thanks,
-- 
Paul Pluzhnikov


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-20 17:28           ` Tom Tromey
  2009-11-20 17:41             ` Paul Pluzhnikov
@ 2009-11-21  0:44             ` Paul Pluzhnikov
  2009-11-21  1:00               ` Daniel Jacobowitz
  1 sibling, 1 reply; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-21  0:44 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Andreas Schwab, gdb-patches

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

On Fri, Nov 20, 2009 at 9:27 AM, Tom Tromey <tromey@redhat.com> wrote:

> Paul> If it's acceptable to require comma for two-argument command

> It is fine by me.

Here is the patch with documentation and test fixes.

Does this require a NEWS entry (incompatible change to disassemble
command) ?

Thanks,
-- 
Paul Pluzhnikov

gdb/ChangeLog:

2009-11-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* cli/cli-cmds.c (disassemble_command): Split on comma.
	(init_cli_cmds): Update help.

gdb/doc/ChangeLog:

2009-11-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.texinfo (Machine Code): Adjust.

gdb/testsuite/ChangeLog:

2009-11-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.asm/asm-source.exp: Adjust.
	* gdb.base/help.exp: Adjust.

[-- Attachment #2: gdb-disas-20091120.txt --]
[-- Type: text/plain, Size: 6519 bytes --]

Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.93
diff -u -p -u -r1.93 cli-cmds.c
--- cli/cli-cmds.c	23 Oct 2009 00:49:33 -0000	1.93
+++ cli/cli-cmds.c	21 Nov 2009 00:33:25 -0000
@@ -983,7 +983,6 @@ disassemble_command (char *arg, int from
   CORE_ADDR low, high;
   char *name;
   CORE_ADDR pc, pc_masked;
-  char *space_index;
   int flags;
 
   name = NULL;
@@ -1022,13 +1021,12 @@ disassemble_command (char *arg, int from
       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, ' ')))
+  pc = value_as_address (parse_to_comma_and_eval (&arg));
+  if (arg[0] == ',')
+    ++arg;
+  if (arg[0] == '\0')
     {
       /* One argument.  */
-      pc = parse_and_eval_address (arg);
       if (find_pc_partial_function (pc, &name, &low, &high) == 0)
 	error (_("No function contains specified address."));
 #if defined(TUI)
@@ -1044,9 +1042,8 @@ disassemble_command (char *arg, int from
   else
     {
       /* Two arguments.  */
-      *space_index = '\0';
-      low = parse_and_eval_address (arg);
-      high = parse_and_eval_address (space_index + 1);
+      low = pc;
+      high = parse_and_eval_address (arg);
     }
 
   print_disassembly (gdbarch, name, low, high, flags);
@@ -1461,7 +1458,7 @@ Default is the function surrounding the 
 With a /m modifier, source lines are included (if available).\n\
 With a /r modifier, raw instructions in hex 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."));
+Two arguments (separated by comma) are taken as a range of memory to dump."));
   set_cmd_completer (c, location_completer);
   if (xdb_commands)
     add_com_alias ("va", "disassemble", class_xdb, 0);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.641
diff -u -p -u -r1.641 gdb.texinfo
--- doc/gdb.texinfo	2 Nov 2009 14:59:52 -0000	1.641
+++ doc/gdb.texinfo	21 Nov 2009 00:33:25 -0000
@@ -6539,10 +6539,10 @@ in symbolic form by specifying the @code
 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
-(first inclusive, second exclusive) to dump.  In that case, the name of
-the function is also printed (since there could be several functions in
-the given range).
+surrounding this value.  Two arguments (separated by comma) specify a
+range of addresses (first inclusive, second exclusive) to dump.  In that
+case, the name of the function is also printed (since there could be
+several functions in the given range).
 
 If the range of memory being disassembled contains current program counter,
 the instruction at that location is shown with a @code{=>} marker.
Index: testsuite/gdb.asm/asm-source.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.asm/asm-source.exp,v
retrieving revision 1.75
diff -u -p -u -r1.75 asm-source.exp
--- testsuite/gdb.asm/asm-source.exp	23 Oct 2009 00:49:33 -0000	1.75
+++ testsuite/gdb.asm/asm-source.exp	21 Nov 2009 00:33:25 -0000
@@ -453,12 +453,12 @@ proc test_dis { command var } {
 # See if we can look at a global variable, three ways
 gdb_test "print globalvar" ".* = 11" "look at global variable"
 test_dis "x/i &globalvar" "globalvar"
-test_dis "disassem &globalvar &globalvar+1" "globalvar"
+test_dis "disassem &globalvar, &globalvar+1" "globalvar"
 
 # See if we can look at a static variable, three ways
 gdb_test "print staticvar" ".* = 5" "look at static variable"
 test_dis "x/i &staticvar" "staticvar"
-test_dis "disassem &staticvar &staticvar+1" "staticvar"
+test_dis "disassem &staticvar, &staticvar+1" "staticvar"
 
 # See if we can look at a static function
 gdb_test "disassem foostatic" ".*<\\+0>:.*End of assembler dump." \
Index: testsuite/gdb.base/help.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/help.exp,v
retrieving revision 1.35
diff -u -p -u -r1.35 help.exp
--- testsuite/gdb.base/help.exp	13 Aug 2009 14:58:27 -0000	1.35
+++ testsuite/gdb.base/help.exp	21 Nov 2009 00:33:25 -0000
@@ -124,7 +124,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 /m modifier, source lines are included \\(if available\\)\.\[\r\n\]+With a /r modifier, raw instructions in hex are included\.\[\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 /r modifier, raw instructions in hex are included\.\[\r\n\]+With a single argument, the function surrounding that address is dumped\.\[\r\n\]+Two arguments \\(separated by comma\\) 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-21  0:44             ` Paul Pluzhnikov
@ 2009-11-21  1:00               ` Daniel Jacobowitz
  2009-11-21  1:09                 ` Paul Pluzhnikov
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Jacobowitz @ 2009-11-21  1:00 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: Tom Tromey, Andreas Schwab, gdb-patches

On Fri, Nov 20, 2009 at 04:42:52PM -0800, Paul Pluzhnikov wrote:
> Does this require a NEWS entry (incompatible change to disassemble
> command) ?

I think yes.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-21  1:00               ` Daniel Jacobowitz
@ 2009-11-21  1:09                 ` Paul Pluzhnikov
  2009-11-21  8:36                   ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-21  1:09 UTC (permalink / raw)
  To: Paul Pluzhnikov, Tom Tromey, Andreas Schwab, gdb-patches

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

On Fri, Nov 20, 2009 at 4:59 PM, Daniel Jacobowitz <drow@false.org> wrote:

>> Does this require a NEWS entry (incompatible change to disassemble
>> command) ?
>
> I think yes.

Thanks, NEWS mention added.
-- 
Paul Pluzhnikov

gdb/ChangeLog:

2009-11-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

       * cli/cli-cmds.c (disassemble_command): Split on comma.
       (init_cli_cmds): Update help.
       * NEWS: Mention incompatible change to 'disassemble'.

gdb/doc/ChangeLog:

2009-11-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

       * gdb.texinfo (Machine Code): Adjust.

gdb/testsuite/ChangeLog:

2009-11-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

       * gdb.asm/asm-source.exp: Adjust.
       * gdb.base/help.exp: Adjust.

[-- Attachment #2: gdb-disas-20091120-2.txt --]
[-- Type: text/plain, Size: 7175 bytes --]

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.337
diff -u -p -u -r1.337 NEWS
--- NEWS	1 Nov 2009 21:20:18 -0000	1.337
+++ NEWS	21 Nov 2009 01:06:06 -0000
@@ -22,6 +22,12 @@ Xilinx MicroBlaze		microblaze
   lists inferiors that are not running yet or that have exited
   already.  See also "New commands" and "New options" below.
 
+* Changed commands
+
+disassemble
+  The disassemble command, when invoked with two arguments, now requires
+  the arguments to be comma-separated.
+
 * New commands (for set/show, see "New options" below)
 
 record save [<FILENAME>]
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.93
diff -u -p -u -r1.93 cli-cmds.c
--- cli/cli-cmds.c	23 Oct 2009 00:49:33 -0000	1.93
+++ cli/cli-cmds.c	21 Nov 2009 01:06:06 -0000
@@ -983,7 +983,6 @@ disassemble_command (char *arg, int from
   CORE_ADDR low, high;
   char *name;
   CORE_ADDR pc, pc_masked;
-  char *space_index;
   int flags;
 
   name = NULL;
@@ -1022,13 +1021,12 @@ disassemble_command (char *arg, int from
       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, ' ')))
+  pc = value_as_address (parse_to_comma_and_eval (&arg));
+  if (arg[0] == ',')
+    ++arg;
+  if (arg[0] == '\0')
     {
       /* One argument.  */
-      pc = parse_and_eval_address (arg);
       if (find_pc_partial_function (pc, &name, &low, &high) == 0)
 	error (_("No function contains specified address."));
 #if defined(TUI)
@@ -1044,9 +1042,8 @@ disassemble_command (char *arg, int from
   else
     {
       /* Two arguments.  */
-      *space_index = '\0';
-      low = parse_and_eval_address (arg);
-      high = parse_and_eval_address (space_index + 1);
+      low = pc;
+      high = parse_and_eval_address (arg);
     }
 
   print_disassembly (gdbarch, name, low, high, flags);
@@ -1461,7 +1458,7 @@ Default is the function surrounding the 
 With a /m modifier, source lines are included (if available).\n\
 With a /r modifier, raw instructions in hex 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."));
+Two arguments (separated by comma) are taken as a range of memory to dump."));
   set_cmd_completer (c, location_completer);
   if (xdb_commands)
     add_com_alias ("va", "disassemble", class_xdb, 0);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.641
diff -u -p -u -r1.641 gdb.texinfo
--- doc/gdb.texinfo	2 Nov 2009 14:59:52 -0000	1.641
+++ doc/gdb.texinfo	21 Nov 2009 01:06:07 -0000
@@ -6539,10 +6539,10 @@ in symbolic form by specifying the @code
 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
-(first inclusive, second exclusive) to dump.  In that case, the name of
-the function is also printed (since there could be several functions in
-the given range).
+surrounding this value.  Two arguments (separated by comma) specify a
+range of addresses (first inclusive, second exclusive) to dump.  In that
+case, the name of the function is also printed (since there could be
+several functions in the given range).
 
 If the range of memory being disassembled contains current program counter,
 the instruction at that location is shown with a @code{=>} marker.
Index: testsuite/gdb.asm/asm-source.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.asm/asm-source.exp,v
retrieving revision 1.75
diff -u -p -u -r1.75 asm-source.exp
--- testsuite/gdb.asm/asm-source.exp	23 Oct 2009 00:49:33 -0000	1.75
+++ testsuite/gdb.asm/asm-source.exp	21 Nov 2009 01:06:07 -0000
@@ -453,12 +453,12 @@ proc test_dis { command var } {
 # See if we can look at a global variable, three ways
 gdb_test "print globalvar" ".* = 11" "look at global variable"
 test_dis "x/i &globalvar" "globalvar"
-test_dis "disassem &globalvar &globalvar+1" "globalvar"
+test_dis "disassem &globalvar, &globalvar+1" "globalvar"
 
 # See if we can look at a static variable, three ways
 gdb_test "print staticvar" ".* = 5" "look at static variable"
 test_dis "x/i &staticvar" "staticvar"
-test_dis "disassem &staticvar &staticvar+1" "staticvar"
+test_dis "disassem &staticvar, &staticvar+1" "staticvar"
 
 # See if we can look at a static function
 gdb_test "disassem foostatic" ".*<\\+0>:.*End of assembler dump." \
Index: testsuite/gdb.base/help.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/help.exp,v
retrieving revision 1.35
diff -u -p -u -r1.35 help.exp
--- testsuite/gdb.base/help.exp	13 Aug 2009 14:58:27 -0000	1.35
+++ testsuite/gdb.base/help.exp	21 Nov 2009 01:06:07 -0000
@@ -124,7 +124,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 /m modifier, source lines are included \\(if available\\)\.\[\r\n\]+With a /r modifier, raw instructions in hex are included\.\[\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 /r modifier, raw instructions in hex are included\.\[\r\n\]+With a single argument, the function surrounding that address is dumped\.\[\r\n\]+Two arguments \\(separated by comma\\) 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-21  1:09                 ` Paul Pluzhnikov
@ 2009-11-21  8:36                   ` Eli Zaretskii
  2009-11-22  2:40                     ` Paul Pluzhnikov
  0 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2009-11-21  8:36 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: tromey, schwab, gdb-patches

> Date: Fri, 20 Nov 2009 17:08:21 -0800
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> 
> On Fri, Nov 20, 2009 at 4:59 PM, Daniel Jacobowitz <drow@false.org> wrote:
> 
> >> Does this require a NEWS entry (incompatible change to disassemble
> >> command) ?
> >
> > I think yes.
> 
> Thanks, NEWS mention added.

Thanks.  I have only one comment: You say everywhere in the docs that
the 2 arguments should be "separated by comma".  ("Separated by a
comma" would be more correct, English-wise.)  However, the test files
show that the arguments can in fact be separated by a combination of a
comma and whitespace:

> -test_dis "disassem &staticvar &staticvar+1" "staticvar"
> +test_dis "disassem &staticvar, &staticvar+1" "staticvar"

Is my interpretation of the test correct?  If it is, then the
documentation should say that whitespace is allowed around the comma.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-21  8:36                   ` Eli Zaretskii
@ 2009-11-22  2:40                     ` Paul Pluzhnikov
  2009-11-22  4:28                       ` Eli Zaretskii
  2009-11-23 17:30                       ` Tom Tromey
  0 siblings, 2 replies; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-22  2:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, schwab, gdb-patches

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

On Sat, Nov 21, 2009 at 12:35 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> You say everywhere in the docs that
> the 2 arguments should be "separated by comma".  ("Separated by a
> comma" would be more correct, English-wise.)

Fixed.

> However, the test files
> show that the arguments can in fact be separated by a combination of a
> comma and whitespace:
>
>> -test_dis "disassem &staticvar &staticvar+1" "staticvar"
>> +test_dis "disassem &staticvar, &staticvar+1" "staticvar"
>
> Is my interpretation of the test correct?  If it is, then the
> documentation should say that whitespace is allowed around the comma.

The comma separates expressions, and expressions may contain
{leading,trailing,in the middle} whitespace.

I've added a statement to gdb.texinfo to clarify that.

I also see that I didn't update the two-argument example there; now fixed.

Thanks,
-- 
Paul Pluzhnikov

gdb/ChangeLog:

2009-11-21  Paul Pluzhnikov  <ppluzhnikov@google.com>

      * cli/cli-cmds.c (disassemble_command): Split on comma.
      (init_cli_cmds): Update help.
      * NEWS: Mention incompatible change to 'disassemble'.

gdb/doc/ChangeLog:

2009-11-21  Paul Pluzhnikov  <ppluzhnikov@google.com>

      * gdb.texinfo (Machine Code): Adjust.

gdb/testsuite/ChangeLog:

2009-11-21  Paul Pluzhnikov  <ppluzhnikov@google.com>

      * gdb.asm/asm-source.exp: Adjust.
      * gdb.base/help.exp: Adjust.

[-- Attachment #2: gdb-disas-20091121.txt --]
[-- Type: text/plain, Size: 7617 bytes --]

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.337
diff -u -p -u -r1.337 NEWS
--- NEWS	1 Nov 2009 21:20:18 -0000	1.337
+++ NEWS	22 Nov 2009 02:32:57 -0000
@@ -22,6 +22,12 @@ Xilinx MicroBlaze		microblaze
   lists inferiors that are not running yet or that have exited
   already.  See also "New commands" and "New options" below.
 
+* Changed commands
+
+disassemble
+  The disassemble command, when invoked with two arguments, now requires
+  the arguments to be comma-separated.
+
 * New commands (for set/show, see "New options" below)
 
 record save [<FILENAME>]
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.93
diff -u -p -u -r1.93 cli-cmds.c
--- cli/cli-cmds.c	23 Oct 2009 00:49:33 -0000	1.93
+++ cli/cli-cmds.c	22 Nov 2009 02:32:57 -0000
@@ -983,7 +983,6 @@ disassemble_command (char *arg, int from
   CORE_ADDR low, high;
   char *name;
   CORE_ADDR pc, pc_masked;
-  char *space_index;
   int flags;
 
   name = NULL;
@@ -1022,13 +1021,12 @@ disassemble_command (char *arg, int from
       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, ' ')))
+  pc = value_as_address (parse_to_comma_and_eval (&arg));
+  if (arg[0] == ',')
+    ++arg;
+  if (arg[0] == '\0')
     {
       /* One argument.  */
-      pc = parse_and_eval_address (arg);
       if (find_pc_partial_function (pc, &name, &low, &high) == 0)
 	error (_("No function contains specified address."));
 #if defined(TUI)
@@ -1044,9 +1042,8 @@ disassemble_command (char *arg, int from
   else
     {
       /* Two arguments.  */
-      *space_index = '\0';
-      low = parse_and_eval_address (arg);
-      high = parse_and_eval_address (space_index + 1);
+      low = pc;
+      high = parse_and_eval_address (arg);
     }
 
   print_disassembly (gdbarch, name, low, high, flags);
@@ -1461,7 +1458,7 @@ Default is the function surrounding the 
 With a /m modifier, source lines are included (if available).\n\
 With a /r modifier, raw instructions in hex 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."));
+Two arguments (separated by a comma) are taken as a range of memory to dump."));
   set_cmd_completer (c, location_completer);
   if (xdb_commands)
     add_com_alias ("va", "disassemble", class_xdb, 0);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.641
diff -u -p -u -r1.641 gdb.texinfo
--- doc/gdb.texinfo	2 Nov 2009 14:59:52 -0000	1.641
+++ doc/gdb.texinfo	22 Nov 2009 02:32:57 -0000
@@ -6539,10 +6539,12 @@ in symbolic form by specifying the @code
 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
-(first inclusive, second exclusive) to dump.  In that case, the name of
-the function is also printed (since there could be several functions in
-the given range).
+surrounding this value.  Two arguments (separated by a comma) specify a
+range of addresses (first inclusive, second exclusive) to dump.  In that
+case, the name of the function is also printed (since there could be
+several functions in the given range).  The argument(s) can be
+any expression yielding a numeric value, such as @samp{0x32c4},
+@samp{&main+10} or @samp{$pc - 8}.
 
 If the range of memory being disassembled contains current program counter,
 the instruction at that location is shown with a @code{=>} marker.
@@ -6552,7 +6554,7 @@ The following example shows the disassem
 HP PA-RISC 2.0 code:
 
 @smallexample
-(@value{GDBP}) disas 0x32c4 0x32e4
+(@value{GDBP}) disas 0x32c4, 0x32e4
 Dump of assembler code from 0x32c4 to 0x32e4:
    0x32c4 <main+204>:      addil 0,dp
    0x32c8 <main+208>:      ldw 0x22c(sr0,r1),r26
Index: testsuite/gdb.asm/asm-source.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.asm/asm-source.exp,v
retrieving revision 1.75
diff -u -p -u -r1.75 asm-source.exp
--- testsuite/gdb.asm/asm-source.exp	23 Oct 2009 00:49:33 -0000	1.75
+++ testsuite/gdb.asm/asm-source.exp	22 Nov 2009 02:32:58 -0000
@@ -453,12 +453,12 @@ proc test_dis { command var } {
 # See if we can look at a global variable, three ways
 gdb_test "print globalvar" ".* = 11" "look at global variable"
 test_dis "x/i &globalvar" "globalvar"
-test_dis "disassem &globalvar &globalvar+1" "globalvar"
+test_dis "disassem &globalvar, &globalvar+1" "globalvar"
 
 # See if we can look at a static variable, three ways
 gdb_test "print staticvar" ".* = 5" "look at static variable"
 test_dis "x/i &staticvar" "staticvar"
-test_dis "disassem &staticvar &staticvar+1" "staticvar"
+test_dis "disassem &staticvar, &staticvar+1" "staticvar"
 
 # See if we can look at a static function
 gdb_test "disassem foostatic" ".*<\\+0>:.*End of assembler dump." \
Index: testsuite/gdb.base/help.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/help.exp,v
retrieving revision 1.35
diff -u -p -u -r1.35 help.exp
--- testsuite/gdb.base/help.exp	13 Aug 2009 14:58:27 -0000	1.35
+++ testsuite/gdb.base/help.exp	22 Nov 2009 02:32:58 -0000
@@ -124,7 +124,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 /m modifier, source lines are included \\(if available\\)\.\[\r\n\]+With a /r modifier, raw instructions in hex are included\.\[\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 /r modifier, raw instructions in hex are included\.\[\r\n\]+With a single argument, the function surrounding that address is dumped\.\[\r\n\]+Two arguments \\(separated by a comma\\) 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-22  2:40                     ` Paul Pluzhnikov
@ 2009-11-22  4:28                       ` Eli Zaretskii
  2009-11-23  7:45                         ` Paul Pluzhnikov
  2009-11-23 17:30                       ` Tom Tromey
  1 sibling, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2009-11-22  4:28 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: tromey, schwab, gdb-patches

> Date: Sat, 21 Nov 2009 18:38:52 -0800
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Cc: tromey@redhat.com, schwab@linux-m68k.org, gdb-patches@sourceware.org
> 
> > Is my interpretation of the test correct?  If it is, then the
> > documentation should say that whitespace is allowed around the comma.
> 
> The comma separates expressions, and expressions may contain
> {leading,trailing,in the middle} whitespace.
> 
> I've added a statement to gdb.texinfo to clarify that.

I still think that the whitespace should be explicitly mentioned.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-22  4:28                       ` Eli Zaretskii
@ 2009-11-23  7:45                         ` Paul Pluzhnikov
  2009-11-23 11:58                           ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-23  7:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, schwab, gdb-patches

On Sat, Nov 21, 2009 at 8:27 PM, Eli Zaretskii <eliz@gnu.org> wrote:

>> The comma separates expressions, and expressions may contain
>> {leading,trailing,in the middle} whitespace.

> I still think that the whitespace should be explicitly mentioned.

I am having hard time coming up with a description that makes sense
and doesn't restate the obvious :-(
"When parsing address expression(s), non-quoted whitespace is ignored."

Could you suggest what you would like to see there?

Thanks,
-- 
Paul Pluzhnikov


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-23  7:45                         ` Paul Pluzhnikov
@ 2009-11-23 11:58                           ` Eli Zaretskii
  2009-11-23 17:42                             ` Paul Pluzhnikov
  0 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2009-11-23 11:58 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: tromey, schwab, gdb-patches

> Date: Sun, 22 Nov 2009 23:43:51 -0800
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Cc: tromey@redhat.com, schwab@linux-m68k.org, gdb-patches@sourceware.org
> 
> On Sat, Nov 21, 2009 at 8:27 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> The comma separates expressions, and expressions may contain
> >> {leading,trailing,in the middle} whitespace.
> 
> > I still think that the whitespace should be explicitly mentioned.
> 
> I am having hard time coming up with a description that makes sense
> and doesn't restate the obvious :-(
> "When parsing address expression(s), non-quoted whitespace is ignored."
> 
> Could you suggest what you would like to see there?

How about

  When two arguments are given, they should be separated by a comma,
  possibly surrounded by whitespace.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-22  2:40                     ` Paul Pluzhnikov
  2009-11-22  4:28                       ` Eli Zaretskii
@ 2009-11-23 17:30                       ` Tom Tromey
  1 sibling, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2009-11-23 17:30 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: Eli Zaretskii, schwab, gdb-patches

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> 2009-11-21  Paul Pluzhnikov  <ppluzhnikov@google.com>
Paul>       * cli/cli-cmds.c (disassemble_command): Split on comma.
Paul>       (init_cli_cmds): Update help.
Paul>       * NEWS: Mention incompatible change to 'disassemble'.

The code bits are ok.  Thanks.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-23 11:58                           ` Eli Zaretskii
@ 2009-11-23 17:42                             ` Paul Pluzhnikov
  2009-11-23 18:40                               ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Paul Pluzhnikov @ 2009-11-23 17:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, schwab, gdb-patches

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

On Mon, Nov 23, 2009 at 3:57 AM, Eli Zaretskii <eliz@gnu.org> wrote:

>  When two arguments are given, they should be separated by a comma,
>  possibly surrounded by whitespace.

Sounds good to me. Thanks,
-- 
Paul Pluzhnikov

gdb/ChangeLog:

2009-11-23  Paul Pluzhnikov  <ppluzhnikov@google.com>

     * cli/cli-cmds.c (disassemble_command): Split on comma.
     (init_cli_cmds): Update help.
     * NEWS: Mention incompatible change to 'disassemble'.

gdb/doc/ChangeLog:

2009-11-23  Paul Pluzhnikov  <ppluzhnikov@google.com>

     * gdb.texinfo (Machine Code): Adjust.

gdb/testsuite/ChangeLog:

2009-11-23  Paul Pluzhnikov  <ppluzhnikov@google.com>

     * gdb.asm/asm-source.exp: Adjust.
     * gdb.base/help.exp: Adjust.

[-- Attachment #2: gdb-disas-20091123.txt --]
[-- Type: text/plain, Size: 7699 bytes --]

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.337
diff -u -p -u -r1.337 NEWS
--- NEWS	1 Nov 2009 21:20:18 -0000	1.337
+++ NEWS	23 Nov 2009 17:39:53 -0000
@@ -22,6 +22,12 @@ Xilinx MicroBlaze		microblaze
   lists inferiors that are not running yet or that have exited
   already.  See also "New commands" and "New options" below.
 
+* Changed commands
+
+disassemble
+  The disassemble command, when invoked with two arguments, now requires
+  the arguments to be comma-separated.
+
 * New commands (for set/show, see "New options" below)
 
 record save [<FILENAME>]
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.93
diff -u -p -u -r1.93 cli-cmds.c
--- cli/cli-cmds.c	23 Oct 2009 00:49:33 -0000	1.93
+++ cli/cli-cmds.c	23 Nov 2009 17:39:53 -0000
@@ -983,7 +983,6 @@ disassemble_command (char *arg, int from
   CORE_ADDR low, high;
   char *name;
   CORE_ADDR pc, pc_masked;
-  char *space_index;
   int flags;
 
   name = NULL;
@@ -1022,13 +1021,12 @@ disassemble_command (char *arg, int from
       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, ' ')))
+  pc = value_as_address (parse_to_comma_and_eval (&arg));
+  if (arg[0] == ',')
+    ++arg;
+  if (arg[0] == '\0')
     {
       /* One argument.  */
-      pc = parse_and_eval_address (arg);
       if (find_pc_partial_function (pc, &name, &low, &high) == 0)
 	error (_("No function contains specified address."));
 #if defined(TUI)
@@ -1044,9 +1042,8 @@ disassemble_command (char *arg, int from
   else
     {
       /* Two arguments.  */
-      *space_index = '\0';
-      low = parse_and_eval_address (arg);
-      high = parse_and_eval_address (space_index + 1);
+      low = pc;
+      high = parse_and_eval_address (arg);
     }
 
   print_disassembly (gdbarch, name, low, high, flags);
@@ -1461,7 +1458,7 @@ Default is the function surrounding the 
 With a /m modifier, source lines are included (if available).\n\
 With a /r modifier, raw instructions in hex 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."));
+Two arguments (separated by a comma) are taken as a range of memory to dump."));
   set_cmd_completer (c, location_completer);
   if (xdb_commands)
     add_com_alias ("va", "disassemble", class_xdb, 0);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.641
diff -u -p -u -r1.641 gdb.texinfo
--- doc/gdb.texinfo	2 Nov 2009 14:59:52 -0000	1.641
+++ doc/gdb.texinfo	23 Nov 2009 17:39:53 -0000
@@ -6539,10 +6539,14 @@ in symbolic form by specifying the @code
 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
-(first inclusive, second exclusive) to dump.  In that case, the name of
-the function is also printed (since there could be several functions in
-the given range).
+surrounding this value.  When two arguments are given, they should
+be separated by a comma, possibly surrounded by whitespace.  The
+arguments specify a range of addresses (first inclusive, second exclusive)
+to dump.  In that case, the name of the function is also printed (since
+there could be several functions in the given range).
+
+The argument(s) can be any expression yielding a numeric value, such as
+@samp{0x32c4}, @samp{&main+10} or @samp{$pc - 8}.
 
 If the range of memory being disassembled contains current program counter,
 the instruction at that location is shown with a @code{=>} marker.
@@ -6552,7 +6556,7 @@ The following example shows the disassem
 HP PA-RISC 2.0 code:
 
 @smallexample
-(@value{GDBP}) disas 0x32c4 0x32e4
+(@value{GDBP}) disas 0x32c4, 0x32e4
 Dump of assembler code from 0x32c4 to 0x32e4:
    0x32c4 <main+204>:      addil 0,dp
    0x32c8 <main+208>:      ldw 0x22c(sr0,r1),r26
Index: testsuite/gdb.asm/asm-source.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.asm/asm-source.exp,v
retrieving revision 1.75
diff -u -p -u -r1.75 asm-source.exp
--- testsuite/gdb.asm/asm-source.exp	23 Oct 2009 00:49:33 -0000	1.75
+++ testsuite/gdb.asm/asm-source.exp	23 Nov 2009 17:39:53 -0000
@@ -453,12 +453,12 @@ proc test_dis { command var } {
 # See if we can look at a global variable, three ways
 gdb_test "print globalvar" ".* = 11" "look at global variable"
 test_dis "x/i &globalvar" "globalvar"
-test_dis "disassem &globalvar &globalvar+1" "globalvar"
+test_dis "disassem &globalvar, &globalvar+1" "globalvar"
 
 # See if we can look at a static variable, three ways
 gdb_test "print staticvar" ".* = 5" "look at static variable"
 test_dis "x/i &staticvar" "staticvar"
-test_dis "disassem &staticvar &staticvar+1" "staticvar"
+test_dis "disassem &staticvar, &staticvar+1" "staticvar"
 
 # See if we can look at a static function
 gdb_test "disassem foostatic" ".*<\\+0>:.*End of assembler dump." \
Index: testsuite/gdb.base/help.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/help.exp,v
retrieving revision 1.35
diff -u -p -u -r1.35 help.exp
--- testsuite/gdb.base/help.exp	13 Aug 2009 14:58:27 -0000	1.35
+++ testsuite/gdb.base/help.exp	23 Nov 2009 17:39:53 -0000
@@ -124,7 +124,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 /m modifier, source lines are included \\(if available\\)\.\[\r\n\]+With a /r modifier, raw instructions in hex are included\.\[\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 /r modifier, raw instructions in hex are included\.\[\r\n\]+With a single argument, the function surrounding that address is dumped\.\[\r\n\]+Two arguments \\(separated by a comma\\) 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] Allow "disassemble 'Foo::bar(char *)'"
  2009-11-23 17:42                             ` Paul Pluzhnikov
@ 2009-11-23 18:40                               ` Eli Zaretskii
  0 siblings, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2009-11-23 18:40 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: tromey, schwab, gdb-patches

> Date: Mon, 23 Nov 2009 09:40:48 -0800
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Cc: tromey@redhat.com, schwab@linux-m68k.org, gdb-patches@sourceware.org
> 
> --0016e649c24e70a0f004790d530c
> Content-Type: text/plain; charset=ISO-8859-1
> 
> On Mon, Nov 23, 2009 at 3:57 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >  When two arguments are given, they should be separated by a comma,
> >  possibly surrounded by whitespace.
> 
> Sounds good to me. Thanks,

Thanks, the doco parts are approved.


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2009-11-23 18:40 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-14  0:10 [patch] Allow "disassemble 'Foo::bar(char *)'" Paul Pluzhnikov
2009-11-15  8:54 ` Andreas Schwab
2009-11-15 17:17   ` Paul Pluzhnikov
2009-11-15 17:26     ` Daniel Jacobowitz
2009-11-16 18:26     ` Tom Tromey
2009-11-16 18:29       ` Paul Pluzhnikov
2009-11-19 23:41         ` Paul Pluzhnikov
2009-11-20 17:28           ` Tom Tromey
2009-11-20 17:41             ` Paul Pluzhnikov
2009-11-21  0:44             ` Paul Pluzhnikov
2009-11-21  1:00               ` Daniel Jacobowitz
2009-11-21  1:09                 ` Paul Pluzhnikov
2009-11-21  8:36                   ` Eli Zaretskii
2009-11-22  2:40                     ` Paul Pluzhnikov
2009-11-22  4:28                       ` Eli Zaretskii
2009-11-23  7:45                         ` Paul Pluzhnikov
2009-11-23 11:58                           ` Eli Zaretskii
2009-11-23 17:42                             ` Paul Pluzhnikov
2009-11-23 18:40                               ` Eli Zaretskii
2009-11-23 17:30                       ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox