Allow -data-disassemble to run with only -s option

Allow -data-disassemble to run with only -s option

[John Lindal]

Since the "disassemble" command only requires the function name, I  
figured it would be nice if -data-disassemble did not require the  
ending address.  The attached patch makes the end address (-e option)  
optional.  This is backward compatible, since all existing code that  
calls -data-disassemble passes both -s and -e.

Sincerely,
John Lindal
New Planet Software
http://www.newplanetsoftware.com/
twitter: newplanetsw

-----

2011-08-06 John Lindal <gdb@newplanetsoftware.com>

	* mi-cmd-disas.c: Accept only -s option.  Automatically set end to end
	of function, or complain that start address is not part of any  
function.

-----

*** mi/mi-cmd-disas.c~	2011-03-09 12:31:50.000000000 -0800
--- mi/mi-cmd-disas.c	2011-08-06 15:14:55.000000000 -0700
***************
*** 37,42 ****
--- 37,47 ----

      or:

+    START-ADDRESS: address to start the disassembly at.
+    (END-ADDRESS is the end of the function.)
+
+    or:
+
      FILENAME: The name of the file where we want disassemble from.
      LINE: The line around which we want to disassemble. It will
      disassemble the function that contins that line.
*************** mi_cmd_disassemble (char *command, char
*** 127,132 ****
--- 132,147 ----
     argv += optind;
     argc -= optind;

+   if (!line_seen && !file_seen && !num_seen && start_seen && ! 
end_seen)
+     {
+       char* ignore_name;
+       CORE_ADDR ignore_low;
+       if (find_pc_partial_function (low, &ignore_name, &ignore_low,  
&high) == 0)
+ 	error (_("No function contains specified address"));
+
+       end_seen = 1;
+     }
+
     /* Allow only filename + linenum (with how_many which is not
        required) OR start_addr + and_addr */

*************** mi_cmd_disassemble (char *command, char
*** 134,144 ****
   	|| (line_seen && file_seen && !num_seen && !start_seen && !end_seen)
   	|| (!line_seen && !file_seen && !num_seen && start_seen &&  
end_seen)))
       error (_("-data-disassemble: Usage: ( [-f filename -l linenum [- 
n "
! 	     "howmany]] | [-s startaddr -e endaddr]) [--] mode."));

     if (argc != 1)
       error (_("-data-disassemble: Usage: [-f filename -l linenum "
! 	     "[-n howmany]] [-s startaddr -e endaddr] [--] mode."));

     mode = atoi (argv[0]);
     if (mode < 0 || mode > 3)
--- 149,159 ----
   	|| (line_seen && file_seen && !num_seen && !start_seen && !end_seen)
   	|| (!line_seen && !file_seen && !num_seen && start_seen &&  
end_seen)))
       error (_("-data-disassemble: Usage: ( [-f filename -l linenum [- 
n "
! 	     "howmany]] | [-s startaddr [-e endaddr]]) [--] mode."));

     if (argc != 1)
       error (_("-data-disassemble: Usage: [-f filename -l linenum "
! 	     "[-n howmany]] [-s startaddr [-e endaddr]] [--] mode."));

     mode = atoi (argv[0]);
     if (mode < 0 || mode > 3)

Re: Allow -data-disassemble to run with only -s option

[Tom Tromey]

>>>>> "John" == John Lindal <support_0384@newplanetsoftware.com> writes:

John> Since the "disassemble" command only requires the function name, I
John> figured it would be nice if -data-disassemble did not require the
John> ending address.  The attached patch makes the end address (-e option)
John> optional.  This is backward compatible, since all existing code that
John> calls -data-disassemble passes both -s and -e.

Do you have a copyright assignment in place?
If not, contact me off-list and I will get you started.

This patch needs a documentation update.

A test case would be nice.

A few minor nits.

John> +   if (!line_seen && !file_seen && !num_seen && start_seen &&
John> !end_seen)

Your mailer mangled the patch.

John> +     {
John> +       char* ignore_name;

Wrong formatting, "char *ignore_name".

John> +       CORE_ADDR ignore_low;
John> +       if (find_pc_partial_function (low, &ignore_name, &ignore_low,

Blank line between declarations and code.

Tom

Re: Allow -data-disassemble to run with only -s option

[John Lindal]

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

> This patch needs a documentation update.

Attached are the updated patches, which hopefully won't get mangled in  
MIME format.

> A test case would be nice.

Without the patch, you have to specify both -s and -e.  With the  
patch, you can do this:

   -data-disassemble -s main

Thanks,
John

[-- Attachment #2: ChangeLog --]
[-- Type: application/octet-stream, Size: 198 bytes --]

2011-08-06 John Lindal <gdb@newplanetsoftware.com>

	* mi-cmd-disas.c: Accept only -s option.  Automatically set end to end
	of function, or complain that start address is not part of any function.

[-- Attachment #3: patch --]
[-- Type: application/octet-stream, Size: 2165 bytes --]

*** gdb-7.3-orig/gdb/mi/mi-cmd-disas.c	2011-03-09 12:31:50.000000000 -0800
--- gdb-7.3/gdb/mi/mi-cmd-disas.c	2011-08-08 15:18:49.000000000 -0700
***************
*** 37,42 ****
--- 37,47 ----
  
     or:
  
+    START-ADDRESS: address to start the disassembly at.
+    (END-ADDRESS is the end of the function.)
+ 
+    or:
+ 
     FILENAME: The name of the file where we want disassemble from.
     LINE: The line around which we want to disassemble. It will
     disassemble the function that contins that line.
*************** mi_cmd_disassemble (char *command, char 
*** 127,132 ****
--- 132,148 ----
    argv += optind;
    argc -= optind;
  
+   if (!line_seen && !file_seen && !num_seen && start_seen && !end_seen)
+     {
+       char *ignore_name;
+       CORE_ADDR ignore_low;
+ 
+       if (find_pc_partial_function (low, &ignore_name, &ignore_low, &high) == 0)
+ 	error (_("No function contains specified address"));
+ 
+       end_seen = 1;
+     }
+ 
    /* Allow only filename + linenum (with how_many which is not
       required) OR start_addr + and_addr */
  
*************** mi_cmd_disassemble (char *command, char 
*** 134,144 ****
  	|| (line_seen && file_seen && !num_seen && !start_seen && !end_seen)
  	|| (!line_seen && !file_seen && !num_seen && start_seen && end_seen)))
      error (_("-data-disassemble: Usage: ( [-f filename -l linenum [-n "
! 	     "howmany]] | [-s startaddr -e endaddr]) [--] mode."));
  
    if (argc != 1)
      error (_("-data-disassemble: Usage: [-f filename -l linenum "
! 	     "[-n howmany]] [-s startaddr -e endaddr] [--] mode."));
  
    mode = atoi (argv[0]);
    if (mode < 0 || mode > 3)
--- 150,160 ----
  	|| (line_seen && file_seen && !num_seen && !start_seen && !end_seen)
  	|| (!line_seen && !file_seen && !num_seen && start_seen && end_seen)))
      error (_("-data-disassemble: Usage: ( [-f filename -l linenum [-n "
! 	     "howmany]] | [-s startaddr [-e endaddr]]) [--] mode."));
  
    if (argc != 1)
      error (_("-data-disassemble: Usage: [-f filename -l linenum "
! 	     "[-n howmany]] [-s startaddr [-e endaddr]] [--] mode."));
  
    mode = atoi (argv[0]);
    if (mode < 0 || mode > 3)

[-- Attachment #4: patch_doc --]
[-- Type: application/octet-stream, Size: 1185 bytes --]

*** doc/gdb.texinfo-orig	2011-08-08 17:51:19.000000000 -0700
--- doc/gdb.texinfo	2011-08-08 17:55:37.000000000 -0700
*************** examine memory and registers, evaluate e
*** 28271,28277 ****
  
  @smallexample
   -data-disassemble
!     [ -s @var{start-addr} -e @var{end-addr} ]
    | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
    -- @var{mode}
  @end smallexample
--- 28271,28277 ----
  
  @smallexample
   -data-disassemble
!     [ -s @var{start-addr} [ -e @var{end-addr} ] ]
    | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
    -- @var{mode}
  @end smallexample
*************** Where:
*** 28283,28289 ****
  @item @var{start-addr}
  is the beginning address (or @code{$pc})
  @item @var{end-addr}
! is the end address
  @item @var{filename}
  is the name of the file to disassemble
  @item @var{linenum}
--- 28283,28289 ----
  @item @var{start-addr}
  is the beginning address (or @code{$pc})
  @item @var{end-addr}
! is the end address.  If only @var{start-addr} is specified, then @var{end-addr} is set to the end of the function containing @var{start-addr}.
  @item @var{filename}
  is the name of the file to disassemble
  @item @var{linenum}

Re: Allow -data-disassemble to run with only -s option

[Sergio Durigan Junior]

Hi John,

Thanks for your work on this.  Some comments below.

John Lindal <support_0384@newplanetsoftware.com> writes:

>> A test case would be nice.
>
> Without the patch, you have to specify both -s and -e.  With the
> patch, you can do this:
>
>   -data-disassemble -s main

What Tom meant is that you should write a testcase for it.  See the
directory gdb/testsuite for examples.

> 2011-08-06 John Lindal <gdb@newplanetsoftware.com>
>
> 	* mi-cmd-disas.c: Accept only -s option.  Automatically set end to end
> 	of function, or complain that start address is not part of any function.

I think those lines are too long.  We try to limit the lines on 80
characteres (some people set a hard limit at 72, IIRC).

> +   if (!line_seen && !file_seen && !num_seen && start_seen && !end_seen)

Same issue here.

> +       if (find_pc_partial_function (low, &ignore_name, &ignore_low, &high) == 0)

Same issue here.

> *** doc/gdb.texinfo-orig	2011-08-08 17:51:19.000000000 -0700
> --- doc/gdb.texinfo	2011-08-08 17:55:37.000000000 -0700

> ! is the end address.  If only @var{start-addr} is specified, then @var{end-addr} is set to the end of the function containing @var{start-addr}.

Same issue here.

Thanks,

Sergio.

Re: Allow -data-disassemble to run with only -s option

[Sergio Durigan Junior]

Sorry, I made some mistakes.  See below.

Sergio Durigan Junior <sergiodj@redhat.com> writes:

> John Lindal <support_0384@newplanetsoftware.com> writes:
>> 2011-08-06 John Lindal <gdb@newplanetsoftware.com>
>>
>> 	* mi-cmd-disas.c: Accept only -s option.  Automatically set end to end
>> 	of function, or complain that start address is not part of any function.
>
> I think those lines are too long.  We try to limit the lines on 80
> characteres (some people set a hard limit at 72, IIRC).

Actually the length is OK here.

>> +   if (!line_seen && !file_seen && !num_seen && start_seen && !end_seen)
>
> Same issue here.

And here too.

Sorry about the confusion.

Re: Allow -data-disassemble to run with only -s option

[John Lindal]

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

> What Tom meant is that you should write a testcase for it.  See the
> directory gdb/testsuite for examples.

Attached are the updated patches.  Thanks for your patience.

John


[-- Attachment #2: ChangeLog --]
[-- Type: application/octet-stream, Size: 198 bytes --]

2011-08-06 John Lindal <gdb@newplanetsoftware.com>

	* mi-cmd-disas.c: Accept only -s option.  Automatically set end to end
	of function, or complain that start address is not part of any function.

[-- Attachment #3: patch_code --]
[-- Type: application/octet-stream, Size: 2161 bytes --]

*** gdb-7.3-orig/gdb/mi/mi-cmd-disas.c	2011-03-09 12:31:50.000000000 -0800
--- gdb-7.3/gdb/mi/mi-cmd-disas.c	2011-08-08 20:18:25.000000000 -0700
***************
*** 37,42 ****
--- 37,47 ----
  
     or:
  
+    START-ADDRESS: address to start the disassembly at.
+    (END-ADDRESS is the end of the function.)
+ 
+    or:
+ 
     FILENAME: The name of the file where we want disassemble from.
     LINE: The line around which we want to disassemble. It will
     disassemble the function that contins that line.
*************** mi_cmd_disassemble (char *command, char 
*** 127,132 ****
--- 132,148 ----
    argv += optind;
    argc -= optind;
  
+   if (!line_seen && !file_seen && !num_seen && start_seen && !end_seen)
+     {
+       char *ignore_name;
+       CORE_ADDR ignore_low;
+ 
+       if (!find_pc_partial_function (low, &ignore_name, &ignore_low, &high))
+ 	error (_("No function contains specified address"));
+ 
+       end_seen = 1;
+     }
+ 
    /* Allow only filename + linenum (with how_many which is not
       required) OR start_addr + and_addr */
  
*************** mi_cmd_disassemble (char *command, char 
*** 134,144 ****
  	|| (line_seen && file_seen && !num_seen && !start_seen && !end_seen)
  	|| (!line_seen && !file_seen && !num_seen && start_seen && end_seen)))
      error (_("-data-disassemble: Usage: ( [-f filename -l linenum [-n "
! 	     "howmany]] | [-s startaddr -e endaddr]) [--] mode."));
  
    if (argc != 1)
      error (_("-data-disassemble: Usage: [-f filename -l linenum "
! 	     "[-n howmany]] [-s startaddr -e endaddr] [--] mode."));
  
    mode = atoi (argv[0]);
    if (mode < 0 || mode > 3)
--- 150,160 ----
  	|| (line_seen && file_seen && !num_seen && !start_seen && !end_seen)
  	|| (!line_seen && !file_seen && !num_seen && start_seen && end_seen)))
      error (_("-data-disassemble: Usage: ( [-f filename -l linenum [-n "
! 	     "howmany]] | [-s startaddr [-e endaddr]]) [--] mode."));
  
    if (argc != 1)
      error (_("-data-disassemble: Usage: [-f filename -l linenum "
! 	     "[-n howmany]] [-s startaddr [-e endaddr]] [--] mode."));
  
    mode = atoi (argv[0]);
    if (mode < 0 || mode > 3)

[-- Attachment #4: patch_doc --]
[-- Type: application/octet-stream, Size: 1205 bytes --]

*** gdb-7.3/doc/gdb.texinfo-orig	2011-08-08 17:51:19.000000000 -0700
--- gdb-7.3/doc/gdb.texinfo	2011-08-08 20:15:15.000000000 -0700
*************** examine memory and registers, evaluate e
*** 28271,28277 ****
  
  @smallexample
   -data-disassemble
!     [ -s @var{start-addr} -e @var{end-addr} ]
    | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
    -- @var{mode}
  @end smallexample
--- 28271,28277 ----
  
  @smallexample
   -data-disassemble
!     [ -s @var{start-addr} [ -e @var{end-addr} ] ]
    | [ -f @var{filename} -l @var{linenum} [ -n @var{lines} ] ]
    -- @var{mode}
  @end smallexample
*************** Where:
*** 28283,28289 ****
  @item @var{start-addr}
  is the beginning address (or @code{$pc})
  @item @var{end-addr}
! is the end address
  @item @var{filename}
  is the name of the file to disassemble
  @item @var{linenum}
--- 28283,28291 ----
  @item @var{start-addr}
  is the beginning address (or @code{$pc})
  @item @var{end-addr}
! is the end address.  If only @var{start-addr} is specified, then
! @var{end-addr} is set to the end of the function containing
! @var{start-addr}.
  @item @var{filename}
  is the name of the file to disassemble
  @item @var{linenum}

[-- Attachment #5: patch_test --]
[-- Type: application/octet-stream, Size: 878 bytes --]

*** mi2-disassemble.exp-orig	2011-08-08 20:22:09.000000000 -0700
--- mi2-disassemble.exp	2011-08-09 09:29:52.000000000 -0700
*************** proc test_disassembly_only {} {
*** 58,63 ****
--- 58,67 ----
      mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -- 0" \
  	    "222\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]" \
                "data-disassemble file & line, assembly only"
+ 
+     mi_gdb_test "333-data-disassemble -s \$pc -- 0" \
+ 	    "333\\^done,asm_insns=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\},\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\]" \
+              "data-disassemble from pc to end of function"
  }
  
  proc test_disassembly_with_opcodes {} {

Re: Allow -data-disassemble to run with only -s option

[Tom Tromey]

>>>>> "John" == John Lindal <support_0384@newplanetsoftware.com> writes:

John> Attached are the updated patches.  Thanks for your patience.

It all looks good to me.  Thanks for doing this.

It still needs a documentation review.

When your paperwork is all in place, let us know by pinging the patch on
the list and mentioning that fact.  Then we can put the patch in.

Tom