Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* RFC (gdb/mi): -stack-list-locals
@ 2003-11-23  0:16 Nick Roberts
  2003-11-26  0:48 ` Andrew Cagney
  0 siblings, 1 reply; 20+ messages in thread
From: Nick Roberts @ 2003-11-23  0:16 UTC (permalink / raw)
  To: gdb-patches


Attached is a patch for -stack-list-locals which roughly modifies this command
as I described previously on gdb@sources.redhat.com (Thu, 6 Nov 2003 22:04:22
+0000). It actually does the following:

1) Display the name, type and value for simple data types.
2) Display the name and type for complex data types.

I don't really know what make_cleanup_ui_out_tuple_begin_end and do_cleanups
do and I've approximated a simple data type to something that isn't
TYPE_CODE_ARRAY or TYPE_CODE_STRUCT so it's probably a pretty gross hack.

The idea is that the user can see the value of simple data types immediately
and can create variable objects for complex data types if he wishes to explore
their values in more detail.

Any comments?


    Nick                                         http://www.nick.uklinux.net



*** mi-cmd-stack.c.~1.19.~	2003-06-12 23:29:37.000000000 +0100
--- mi-cmd-stack.c	2003-11-22 23:49:24.000000000 +0000
***************
*** 273,292 ****
  		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
  	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
  
! 	      if (values)
! 		{
! 		  struct symbol *sym2;
! 		  if (!locals)
! 		    sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
! 					  block, VAR_DOMAIN,
! 					  (int *) NULL,
! 				2	  (struct symtab **) NULL);
! 		  else
  		    sym2 = sym;
  		  print_variable_value (sym2, fi, stb->stream);
  		  ui_out_field_stream (uiout, "value", stb);
- 		  do_cleanups (cleanup_tuple);
  		}
  	    }
  	}
        if (BLOCK_FUNCTION (block))
--- 273,303 ----
  		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
  	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
  
! 	      struct symbol *sym2;
! 	      if (!locals)
! 		sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
! 				      block, VAR_DOMAIN,
! 				      (int *) NULL,
! 				      (struct symtab **) NULL);
! 	      else
  		    sym2 = sym;
+ 	      if (values == 2)
+ 		{
+ 		  type_print (sym2->type, "", stb->stream, -1);
+ 		  ui_out_field_stream (uiout, "type", stb);
+ 		  if (TYPE_CODE (sym2->type) != TYPE_CODE_ARRAY &&
+ 		      TYPE_CODE (sym2->type) != TYPE_CODE_STRUCT)
+ 		    {
+ 		      print_variable_value (sym2, fi, stb->stream);
+ 		      ui_out_field_stream (uiout, "value", stb);
+ 		    }
+ 		}
+ 	      else if (values)
+ 		{
  		  print_variable_value (sym2, fi, stb->stream);
  		  ui_out_field_stream (uiout, "value", stb);
  		}
+ 	      if (values) do_cleanups (cleanup_tuple);
  	    }
  	}
        if (BLOCK_FUNCTION (block))


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

* Re: RFC (gdb/mi): -stack-list-locals
  2003-11-23  0:16 RFC (gdb/mi): -stack-list-locals Nick Roberts
@ 2003-11-26  0:48 ` Andrew Cagney
  2003-12-02  3:14   ` RFC (gdb/mi): -stack-list-locals + PATCH Nick Roberts
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Cagney @ 2003-11-26  0:48 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

> Attached is a patch for -stack-list-locals which roughly modifies this command
> as I described previously on gdb@sources.redhat.com (Thu, 6 Nov 2003 22:04:22
> +0000). It actually does the following:
> 
> 1) Display the name, type and value for simple data types.
> 2) Display the name and type for complex data types.
> 
> I don't really know what make_cleanup_ui_out_tuple_begin_end and do_cleanups
> do and I've approximated a simple data type to something that isn't
> TYPE_CODE_ARRAY or TYPE_CODE_STRUCT so it's probably a pretty gross hack.
> 
> The idea is that the user can see the value of simple data types immediately
> and can create variable objects for complex data types if he wishes to explore
> their values in more detail.
> 
> Any comments?

Looks like its time to cleanup "values" changing it to an enum or 
bitmask - too many magic numbers.

Would is_integral_type() give you what you want?

For the cleanups try the restructured form:
	{
	  struct cleanup = cleanups = make_cleanup (NULL, null_cleanup);
	  ..
           do_cleanups (cleanups);
	}
http://sources.redhat.com/gdb/current/onlinedocs/gdbint_13.html#SEC112

make_cleanup_ui_out_tuple_begin_end opens the tuple and then closes it 
via a cleanup (if an error is thrown the tuple under construction is 
still finished).

Andrew

>     Nick                                         http://www.nick.uklinux.net
> 
> 
> 
> *** mi-cmd-stack.c.~1.19.~	2003-06-12 23:29:37.000000000 +0100
> --- mi-cmd-stack.c	2003-11-22 23:49:24.000000000 +0000
> ***************
> *** 273,292 ****
>   		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
>   	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
>   
> ! 	      if (values)
> ! 		{
> ! 		  struct symbol *sym2;
> ! 		  if (!locals)
> ! 		    sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
> ! 					  block, VAR_DOMAIN,
> ! 					  (int *) NULL,
> ! 				2	  (struct symtab **) NULL);
> ! 		  else
>   		    sym2 = sym;
>   		  print_variable_value (sym2, fi, stb->stream);
>   		  ui_out_field_stream (uiout, "value", stb);
> - 		  do_cleanups (cleanup_tuple);
>   		}
>   	    }
>   	}
>         if (BLOCK_FUNCTION (block))
> --- 273,303 ----
>   		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
>   	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
>   
> ! 	      struct symbol *sym2;
> ! 	      if (!locals)
> ! 		sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
> ! 				      block, VAR_DOMAIN,
> ! 				      (int *) NULL,
> ! 				      (struct symtab **) NULL);
> ! 	      else
>   		    sym2 = sym;
> + 	      if (values == 2)
> + 		{
> + 		  type_print (sym2->type, "", stb->stream, -1);
> + 		  ui_out_field_stream (uiout, "type", stb);
> + 		  if (TYPE_CODE (sym2->type) != TYPE_CODE_ARRAY &&
> + 		      TYPE_CODE (sym2->type) != TYPE_CODE_STRUCT)
> + 		    {
> + 		      print_variable_value (sym2, fi, stb->stream);
> + 		      ui_out_field_stream (uiout, "value", stb);
> + 		    }
> + 		}
> + 	      else if (values)
> + 		{
>   		  print_variable_value (sym2, fi, stb->stream);
>   		  ui_out_field_stream (uiout, "value", stb);
>   		}
> + 	      if (values) do_cleanups (cleanup_tuple);
>   	    }
>   	}
>         if (BLOCK_FUNCTION (block))
> 



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

* Re: RFC (gdb/mi): -stack-list-locals + PATCH
  2003-11-26  0:48 ` Andrew Cagney
@ 2003-12-02  3:14   ` Nick Roberts
  2003-12-09  2:42     ` RFC (gdb/mi): -stack-list-locals + REVISED PATCH Nick Roberts
  0 siblings, 1 reply; 20+ messages in thread
From: Nick Roberts @ 2003-12-02  3:14 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

 ...
 > > I don't really know what make_cleanup_ui_out_tuple_begin_end and do_cleanups
 > > do and I've approximated a simple data type to something that isn't
 > > TYPE_CODE_ARRAY or TYPE_CODE_STRUCT so it's probably a pretty gross hack.
 > > 
 > > The idea is that the user can see the value of simple data types immediately
 > > and can create variable objects for complex data types if he wishes to explore
 > > their values in more detail.
 > > 
 > > Any comments?
 > 
 > Looks like its time to cleanup "values" changing it to an enum or 
 > bitmask - too many magic numbers.

I see now that I've also inadvertantly defined "-stack-list-arguments 2". I don't
know what magic numbers are but clearly better structure is desirable.

 > Would is_integral_type() give you what you want?

For a simple data type? I don't think so. What about float? Perhaps I'm missing
your point.

 > 
 > For the cleanups try the restructured form:
 > 	{
 > 	  struct cleanup = cleanups = make_cleanup (NULL, null_cleanup);
 > 	  ..
 >            do_cleanups (cleanups);
 > 	}
 > http://sources.redhat.com/gdb/current/onlinedocs/gdbint_13.html#SEC112
 > 
 > make_cleanup_ui_out_tuple_begin_end opens the tuple and then closes it 
 > via a cleanup (if an error is thrown the tuple under construction is 
 > still finished).

I understand make_cleanup_ui_out_tuple_begin_end but not the above
restructured form. Do I have to call make_cleanup (or relatives) each time I
allocated memory i.e for each call to ui_out_field_stream?

While I'm thinking about the above, here is a basic patch for
mi-cmd-stack.c. Currently if -stack-list-locals is invoked then it gives a
segmentation fault (Segmentation fault (core dumped)). This is inconvenient
and the patch changes this to give an output similar to that of cli:

&"No frame selected.\n"
^error,msg="No frame selected."
(gdb) 


    Nick                                         http://www.nick.uklinux.net



*** mi-cmd-stack.c.~1.19.~      2003-06-12 23:29:37.000000000 +0100
--- mi-cmd-stack.c      2003-12-02 02:43:04.000000000 +0000
***************
*** 138,147 ****
--- 138,150 ----
  mi_cmd_stack_list_locals (char *command, char **argv, int argc)
  {
    if (argc != 1)
      error ("mi_cmd_stack_list_locals: Usage: PRINT_VALUES");
  
+   if (!deprecated_selected_frame)
+     error ("No frame selected.");
+ 
    list_args_or_locals (1, atoi (argv[0]), deprecated_selected_frame);
    return MI_CMD_DONE;
  }


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

* Re: RFC (gdb/mi): -stack-list-locals + REVISED PATCH
  2003-12-02  3:14   ` RFC (gdb/mi): -stack-list-locals + PATCH Nick Roberts
@ 2003-12-09  2:42     ` Nick Roberts
  2003-12-10 17:56       ` Andrew Cagney
  0 siblings, 1 reply; 20+ messages in thread
From: Nick Roberts @ 2003-12-09  2:42 UTC (permalink / raw)
  To: gdb-patches


This patch:

1) Follows Jim Ingham's advice of using check_typedef to guard against the
   case of TYPE_CODE_TYPEDEF.

2) Avoids a segmentation fault if -stack-list-locals is invoked before the
   inferior has started execution.

3) Still introduces "-stack-list-locals 2". I'm not sure how to simplify this
   as any change must presumably be beackward compatible. I don't think using
   different numbers to mean different things is a problem here as mi commands
   are not intended for the user and so don't need to be remembered by
   him/her.


    Nick                                         http://www.nick.uklinux.net



*** mi-cmd-stack.c.~1.19.~	2003-06-12 23:29:37.000000000 +0100
--- mi-cmd-stack.c	2003-12-09 02:12:45.000000000 +0000
***************
*** 140,145 ****
--- 140,148 ----
    if (argc != 1)
      error ("mi_cmd_stack_list_locals: Usage: PRINT_VALUES");
  
+   if (!deprecated_selected_frame)
+     error ("No frame selected.");
+ 
    list_args_or_locals (1, atoi (argv[0]), deprecated_selected_frame);
    return MI_CMD_DONE;
  }
***************
*** 273,288 ****
  		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
  	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
  
! 	      if (values)
! 		{
! 		  struct symbol *sym2;
! 		  if (!locals)
! 		    sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
! 					  block, VAR_DOMAIN,
! 					  (int *) NULL,
! 					  (struct symtab **) NULL);
! 		  else
  		    sym2 = sym;
  		  print_variable_value (sym2, fi, stb->stream);
  		  ui_out_field_stream (uiout, "value", stb);
  		  do_cleanups (cleanup_tuple);
--- 276,304 ----
  		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
  	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
  
! 	      struct symbol *sym2;
! 	      if (!locals)
! 		sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
! 				      block, VAR_DOMAIN,
! 				      (int *) NULL,
! 				      (struct symtab **) NULL);
! 	      else
  		    sym2 = sym;
+ 	      if (values == 2)
+ 		{
+ 		  type_print (sym2->type, "", stb->stream, -1);
+ 		  ui_out_field_stream (uiout, "type", stb);
+ 		  if (TYPE_CODE (check_typedef (sym2->type)) != TYPE_CODE_ARRAY
+ 		      &&
+ 		      TYPE_CODE (check_typedef (sym2->type)) != TYPE_CODE_STRUCT)
+ 		    {
+ 		      print_variable_value (sym2, fi, stb->stream);
+ 		      ui_out_field_stream (uiout, "value", stb);
+ 		    }
+ 		  do_cleanups (cleanup_tuple);
+ 		}
+ 	      else if (values)
+ 		{
  		  print_variable_value (sym2, fi, stb->stream);
  		  ui_out_field_stream (uiout, "value", stb);
  		  do_cleanups (cleanup_tuple);


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

* Re: RFC (gdb/mi): -stack-list-locals + REVISED PATCH
  2003-12-09  2:42     ` RFC (gdb/mi): -stack-list-locals + REVISED PATCH Nick Roberts
@ 2003-12-10 17:56       ` Andrew Cagney
  2003-12-10 22:26         ` Jason Molenda
  2003-12-12 20:51         ` RFC (gdb/mi): -stack-list-locals Nick Roberts
  0 siblings, 2 replies; 20+ messages in thread
From: Andrew Cagney @ 2003-12-10 17:56 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

> This patch:
> 
> 1) Follows Jim Ingham's advice of using check_typedef to guard against the
>    case of TYPE_CODE_TYPEDEF.
> 
> 2) Avoids a segmentation fault if -stack-list-locals is invoked before the
>    inferior has started execution.
> 
> 3) Still introduces "-stack-list-locals 2". I'm not sure how to simplify this
>    as any change must presumably be beackward compatible. I don't think using
>    different numbers to mean different things is a problem here as mi commands
>    are not intended for the user and so don't need to be remembered by
>    him/her.

True, but they also need to be fairly self documenting.  Anyway, see below:

>     Nick                                         http://www.nick.uklinux.net
> 
> 
> 
> *** mi-cmd-stack.c.~1.19.~	2003-06-12 23:29:37.000000000 +0100
> --- mi-cmd-stack.c	2003-12-09 02:12:45.000000000 +0000
> ***************
> *** 140,145 ****
> --- 140,148 ----
>     if (argc != 1)
>       error ("mi_cmd_stack_list_locals: Usage: PRINT_VALUES");
>   
> +   if (!deprecated_selected_frame)
> +     error ("No frame selected.");

   struct frame_info *frame;
   ...
   frame = get_selected_frame ();
is better.  It throws an error if there is no frame, and follow on code 
can use "frame" instead of "deprecated_selected_frame" (and would be a 
really appreciated cleanup!).

>     list_args_or_locals (1, atoi (argv[0]), deprecated_selected_frame);

I was thinking of something as simple as:

enum print_values {
   PRINT_NO_VALUES,
   PRINT_ALL_VALUES,
   PRINT_SIMPLE_VALUES
};

and then the very mechanical:

   enum print_values print_values;

   if (strcmp (argv[0], "0") == 0
       || strcmp (argv[0], "no-values") == 0)
     print_values = PRINT_NO_VALUES;
   else if (strcmp (argv[0], "1") == 0
            || strcmp (argv[0], "all-values") == 0)
     print_values = PRINT_ALL_VALUES;
   else if (strcmp (argv[0], "simple-values") == 0)
     print_values = PRINT_SIMPLE_VALUES;
   else
     error ("...");

   list_args_or_locals (1, print_values, ...);

(the names aren't the best so feel free to improve).

>     return MI_CMD_DONE;
>   }
> ***************
> *** 273,288 ****
>   		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
>   	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
>   
> ! 	      if (values)

if (values != PRINT_NO_VALUES)

> ! 		{
> ! 		  struct symbol *sym2;
> ! 		  if (!locals)
> ! 		    sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
> ! 					  block, VAR_DOMAIN,
> ! 					  (int *) NULL,
> ! 					  (struct symtab **) NULL);
> ! 		  else
>   		    sym2 = sym;
>   		  print_variable_value (sym2, fi, stb->stream);
>   		  ui_out_field_stream (uiout, "value", stb);
>   		  do_cleanups (cleanup_tuple);
> --- 276,304 ----
>   		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
>   	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
>   
> ! 	      struct symbol *sym2;
> ! 	      if (!locals)
> ! 		sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),

Since you're here, I'd change this to SYMBOL_PRINT_NAME.  The comment 
below, from symtab.h, hopefully explains the difference (I also hope I 
picked the correct winner):

/* Now come lots of name accessor macros.  Short version as to when to
    use which: Use SYMBOL_NATURAL_NAME to refer to the name of the
    symbol in the original source code.  Use SYMBOL_LINKAGE_NAME if you
    want to know what the linker thinks the symbol's name is.  Use
    SYMBOL_PRINT_NAME for output.  Use SYMBOL_DEMANGLED_NAME if you
    specifically need to know whether SYMBOL_NATURAL_NAME and
    SYMBOL_LINKAGE_NAME are different.  Don't use
    DEPRECATED_SYMBOL_NAME at all: instances of that macro should be
    replaced by SYMBOL_NATURAL_NAME, SYMBOL_LINKAGE_NAME, or perhaps
    SYMBOL_PRINT_NAME.  */

> ! 				      block, VAR_DOMAIN,
> ! 				      (int *) NULL,
> ! 				      (struct symtab **) NULL);
> ! 	      else
>   		    sym2 = sym;
> + 	      if (values == 2)

if (values == PRINT_SIMPLE_VALUES)

> + 		{
> + 		  type_print (sym2->type, "", stb->stream, -1);
> + 		  ui_out_field_stream (uiout, "type", stb);
> + 		  if (TYPE_CODE (check_typedef (sym2->type)) != TYPE_CODE_ARRAY
> + 		      &&
> + 		      TYPE_CODE (check_typedef (sym2->type)) != TYPE_CODE_STRUCT)

You may also want to consider TYPE_CODE_UNION?  Your choice.

> + 		    {
> + 		      print_variable_value (sym2, fi, stb->stream);
> + 		      ui_out_field_stream (uiout, "value", stb);
> + 		    }
> + 		  do_cleanups (cleanup_tuple);
> + 		}
> + 	      else if (values)

if (values == PRINT_ALL_VALUES)

> + 		{
>   		  print_variable_value (sym2, fi, stb->stream);
>   		  ui_out_field_stream (uiout, "value", stb);
>   		  do_cleanups (cleanup_tuple);
> 

Anyway, it's basicly there.  Just the doco update (which is eli's call) 
(and for the assignment to pop up), and testcase.  For the testcase, 
just edit mi-stack.exp.  You don't need to edit mi[12]-stack.exp as 
there's no expectation that this new feature will work with old MI versions.

(since I'm traveling, I may be slow in responding, hopefully though, 
between my self and elena someone will be able to give a thumbs up and 
organize your account).

Andrew



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

* Re: RFC (gdb/mi): -stack-list-locals + REVISED PATCH
  2003-12-10 17:56       ` Andrew Cagney
@ 2003-12-10 22:26         ` Jason Molenda
  2003-12-12 20:51         ` RFC (gdb/mi): -stack-list-locals Nick Roberts
  1 sibling, 0 replies; 20+ messages in thread
From: Jason Molenda @ 2003-12-10 22:26 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Nick Roberts, gdb-patches


On Dec 10, 2003, at 9:56 AM, Andrew Cagney wrote:

> and then the very mechanical:
>
>   enum print_values print_values;
>
>   if (strcmp (argv[0], "0") == 0
>       || strcmp (argv[0], "no-values") == 0)
>     print_values = PRINT_NO_VALUES;
>   else if (strcmp (argv[0], "1") == 0
>            || strcmp (argv[0], "all-values") == 0)
>     print_values = PRINT_ALL_VALUES;
>   else if (strcmp (argv[0], "simple-values") == 0)
>     print_values = PRINT_SIMPLE_VALUES;
>   else
>     error ("...");


This would be a help for us at Apple, as well.  Right now we have a 
non-standard meaning for -stack-list-locals 2, and the FSF gdb will 
have a different meaning for 2 with this patch going in.  No complaints 
or anything, but it's unpleasant.  If 1 == all-values and there is a 
new 'simple-values' (or whatever) added, we can easily add a 
'apple-values' which are the particular things our GUI wants without 
fear of conflicting.


Jason


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

* Re: RFC (gdb/mi): -stack-list-locals
  2003-12-10 17:56       ` Andrew Cagney
  2003-12-10 22:26         ` Jason Molenda
@ 2003-12-12 20:51         ` Nick Roberts
  2003-12-12 21:09           ` David Carlton
  2003-12-12 23:01           ` RFC (gdb/mi): -stack-list-locals Jason Molenda
  1 sibling, 2 replies; 20+ messages in thread
From: Nick Roberts @ 2003-12-12 20:51 UTC (permalink / raw)
  To: Andrew Cagney, jmolenda; +Cc: gdb-patches


These (two) patches uses

 1) get_selected_frame instead of deprecated_selected_frame.

 2) SYMBOL_PRINT_NAME instead of DEPRECATED_SYMBOL_NAME.

 3) enum for print_values.


Jason Molenda writes:

>                                         ...Right now we have a 
> non-standard meaning for -stack-list-locals 2, and the FSF gdb will 
> have a different meaning for 2 with this patch going in.  No complaints 
> or anything, but it's unpleasant.

I don't see why both our changes can't be accommodated. This patch uses a
switch statement for each value of print_values. If for some reason Apple need
-stack-list-locals 2, I dont mind using another value.  Everybody seems to
want Apple's changes, including their management. Rather than being
unpleasant, perhaps this is an opportunity to make the case to that management
for resources to contribute back to the FSF.


    Nick                                         http://www.nick.uklinux.net



*** mi-cmd-stack.c.~1.19.~	2003-06-12 23:29:37.000000000 +0100
--- mi-cmd-stack.c	2003-12-11 00:53:34.000000000 +0000
***************
*** 29,34 ****
--- 29,35 ----
  #include "block.h"
  #include "stack.h"
  #include "dictionary.h"
+ #include "gdb_string.h"
  
  static void list_args_or_locals (int locals, int values, struct frame_info *fi);
  
***************
*** 137,146 ****
  enum mi_cmd_result
  mi_cmd_stack_list_locals (char *command, char **argv, int argc)
  {
    if (argc != 1)
      error ("mi_cmd_stack_list_locals: Usage: PRINT_VALUES");
  
!   list_args_or_locals (1, atoi (argv[0]), deprecated_selected_frame);
    return MI_CMD_DONE;
  }
  
--- 138,159 ----
  enum mi_cmd_result
  mi_cmd_stack_list_locals (char *command, char **argv, int argc)
  {
+   struct frame_info *frame;
+   enum print_values print_values;
+ 
    if (argc != 1)
      error ("mi_cmd_stack_list_locals: Usage: PRINT_VALUES");
  
!    frame = get_selected_frame ();
! 
!    if (strcmp (argv[0], "1") == 0 || strcmp (argv[0], "all-values") == 0)
!      print_values = PRINT_ALL_VALUES;
!    else if (strcmp (argv[0], "2") == 0 || strcmp (argv[0], "simple-values") == 0)
!      print_values = PRINT_SIMPLE_VALUES;
!    else
!      print_values = PRINT_NO_VALUES;
! 
!   list_args_or_locals (1, print_values, frame);
    return MI_CMD_DONE;
  }
  
***************
*** 218,223 ****
--- 231,237 ----
    int nsyms;
    struct cleanup *cleanup_list;
    static struct ui_stream *stb = NULL;
+   struct type *type;
  
    stb = ui_out_stream_new (uiout);
  
***************
*** 268,291 ****
  	  if (print_me)
  	    {
  	      struct cleanup *cleanup_tuple = NULL;
! 	      if (values)
  		cleanup_tuple = 
  		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
! 	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
  
! 	      if (values)
! 		{
! 		  struct symbol *sym2;
! 		  if (!locals)
! 		    sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
! 					  block, VAR_DOMAIN,
! 					  (int *) NULL,
! 					  (struct symtab **) NULL);
! 		  else
  		    sym2 = sym;
  		  print_variable_value (sym2, fi, stb->stream);
  		  ui_out_field_stream (uiout, "value", stb);
  		  do_cleanups (cleanup_tuple);
  		}
  	    }
  	}
--- 282,320 ----
  	  if (print_me)
  	    {
  	      struct cleanup *cleanup_tuple = NULL;
! 	      if (values != PRINT_NO_VALUES)
  		cleanup_tuple = 
  		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
! 	      ui_out_field_string (uiout, "name", SYMBOL_PRINT_NAME (sym));
  
! 	      struct symbol *sym2;
! 	      if (!locals)
! 		sym2 = lookup_symbol (SYMBOL_PRINT_NAME (sym),
! 				      block, VAR_DOMAIN,
! 				      (int *) NULL,
! 				      (struct symtab **) NULL);
! 	      else
  		    sym2 = sym;
+ 	      switch (values)
+ 		{
+ 		case PRINT_SIMPLE_VALUES:
+ 		  type = check_typedef (sym2->type);
+ 		  type_print (sym2->type, "", stb->stream, -1);
+ 		  ui_out_field_stream (uiout, "type", stb);
+ 		  if (TYPE_CODE (type) != TYPE_CODE_ARRAY &&
+ 		      TYPE_CODE (type) != TYPE_CODE_STRUCT &&
+ 		      TYPE_CODE (type) != TYPE_CODE_UNION)
+ 		    {
+ 		      print_variable_value (sym2, fi, stb->stream);
+ 		      ui_out_field_stream (uiout, "value", stb);
+ 		    }
+ 		  do_cleanups (cleanup_tuple);
+ 		  break;
+ 		case PRINT_ALL_VALUES:
  		  print_variable_value (sym2, fi, stb->stream);
  		  ui_out_field_stream (uiout, "value", stb);
  		  do_cleanups (cleanup_tuple);
+ 		  break;
  		}
  	    }
  	}

*** mi-cmds.h.~1.10.~	2003-10-24 22:30:52.000000000 +0100
--- mi-cmds.h	2003-12-10 20:09:22.000000000 +0000
***************
*** 48,53 ****
--- 48,59 ----
      MI_CMD_QUIET
    };
  
+ enum print_values {
+    PRINT_NO_VALUES,
+    PRINT_ALL_VALUES,
+    PRINT_SIMPLE_VALUES
+ };
+ 
  typedef enum mi_cmd_result (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
  
  /* Older MI commands have this interface. Retained until all old


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

* Re: RFC (gdb/mi): -stack-list-locals
  2003-12-12 20:51         ` RFC (gdb/mi): -stack-list-locals Nick Roberts
@ 2003-12-12 21:09           ` David Carlton
  2003-12-17  2:29             ` [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children Nick Roberts
  2003-12-12 23:01           ` RFC (gdb/mi): -stack-list-locals Jason Molenda
  1 sibling, 1 reply; 20+ messages in thread
From: David Carlton @ 2003-12-12 21:09 UTC (permalink / raw)
  To: Nick Roberts; +Cc: Andrew Cagney, jmolenda, gdb-patches

On Fri, 12 Dec 2003 20:43:16 +0000, Nick Roberts <nick@nick.uklinux.net> said:

>  2) SYMBOL_PRINT_NAME instead of DEPRECATED_SYMBOL_NAME.

It should be SYMBOL_NATURAL_NAME.  At least for the call to
lookup_symbol; maybe SYMBOL_PRINT_NAME is correct for the
ui_out_field_string call.

David Carlton
carlton@kealia.com


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

* Re: RFC (gdb/mi): -stack-list-locals
  2003-12-12 20:51         ` RFC (gdb/mi): -stack-list-locals Nick Roberts
  2003-12-12 21:09           ` David Carlton
@ 2003-12-12 23:01           ` Jason Molenda
  1 sibling, 0 replies; 20+ messages in thread
From: Jason Molenda @ 2003-12-12 23:01 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches


On Dec 12, 2003, at 12:43 PM, Nick Roberts wrote:

> Jason Molenda writes:
>
>>                                         ...Right now we have a
>> non-standard meaning for -stack-list-locals 2, and the FSF gdb will
>> have a different meaning for 2 with this patch going in.  No 
>> complaints
>> or anything, but it's unpleasant.
>
> I don't see why both our changes can't be accommodated.


Well, if we're both implementing a different meaning for "2" that's a 
pretty tough thing to accommodate. :-)

I didn't mean my statement as a criticism of this change or the work 
you're doing -- this is the sort of thing we inevitably have come up 
when we have lots of changes vs. the mainline.  We made our bed, etc.

> This patch uses a
> switch statement for each value of print_values. If for some reason 
> Apple need
> -stack-list-locals 2, I dont mind using another value.

"3" would be very nice. :)  But really, I think Andrew's suggestion is 
a better approach.  Instead of numeric values which represent an 
arbitrary collection of return values ("2" is "the things Apple's UI 
would like to get", "3" is "a smaller set of things that don't involve 
so much communication with the inferior"), text strings are a little 
less likely to conflict.


> Everybody seems to
> want Apple's changes, including their management. Rather than being
> unpleasant, perhaps this is an opportunity to make the case to that 
> management
> for resources to contribute back to the FSF.

Yeah, it's the usual problem.  Lots to do, not so much time in which to 
do it, sigh.


J


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

* [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children
  2003-12-12 21:09           ` David Carlton
@ 2003-12-17  2:29             ` Nick Roberts
  2003-12-17  2:32               ` David Carlton
                                 ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Nick Roberts @ 2003-12-17  2:29 UTC (permalink / raw)
  To: gdb-patches


This patch covers four files mi-cmds.h, mi-cmd-stack.c, mi-cmd-var.c
and gdb.texinfo and:

1) Uses SYMBOL_PRINT_NAME instead of DEPRECATED_SYMBOL_NAME for lookup_symbol
   (in mi-cmd-stack.c) as advised by David Carlton.

2) Uses a similar syntax to that suggested by Andrew Cagney for
   -var-list-children.

AC> I think the syntax should be: -var-list-children --print-values @var{name}

   I actually use --novalues and --all-values. This is to give some
   consistency with my changes to -stack-list-locals.


    Nick                                         http://www.nick.uklinux.net



*** mi-cmds.h.~1.10.~	2003-10-24 22:30:52.000000000 +0100
--- mi-cmds.h	2003-12-10 20:09:22.000000000 +0000
***************
*** 48,53 ****
--- 48,59 ----
      MI_CMD_QUIET
    };
  
+ enum print_values {
+    PRINT_NO_VALUES,
+    PRINT_ALL_VALUES,
+    PRINT_SIMPLE_VALUES
+ };
+ 
  typedef enum mi_cmd_result (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
  
  /* Older MI commands have this interface. Retained until all old
*** mi-cmd-var.c	2003-12-17 02:08:57.000000000 +0000
--- mi-cmd-var.c.~1.17~	2003-12-17 00:07:15.000000000 +0000
***************
*** 257,285 ****
    struct cleanup *cleanup_children;
    int numchild;
    char *type;
-   enum print_values print_values;
  
!   if (argc != 1 && argc != 2)
!     error ("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME");
  
    /* Get varobj handle, if a valid var obj name was specified */
!   if (argc == 1) var = varobj_get_handle (argv[0]);
!   else var = varobj_get_handle (argv[1]);
    if (var == NULL)
      error ("mi_cmd_var_list_children: Variable object not found");
  
    numchild = varobj_list_children (var, &childlist);
    ui_out_field_int (uiout, "numchild", numchild);
-   if (argc == 2) 
-     if (strcmp (argv[0], "0") == 0 
- 	|| strcmp (argv[0], "--no-values") == 0)
-       print_values = PRINT_NO_VALUES;
-     else if (strcmp (argv[0], "1") == 0 
- 	     || strcmp (argv[0], "--all-values") == 0)
-       print_values = PRINT_ALL_VALUES;
-     else
-      error ("mi_cmd_var_list_children: Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\"");
-   else print_values = PRINT_NO_VALUES;
  
    if (numchild <= 0)
      return MI_CMD_DONE;
--- 257,273 ----
    struct cleanup *cleanup_children;
    int numchild;
    char *type;
  
!   if (argc != 1)
!     error ("mi_cmd_var_list_children: Usage: NAME.");
  
    /* Get varobj handle, if a valid var obj name was specified */
!   var = varobj_get_handle (argv[0]);
    if (var == NULL)
      error ("mi_cmd_var_list_children: Variable object not found");
  
    numchild = varobj_list_children (var, &childlist);
    ui_out_field_int (uiout, "numchild", numchild);
  
    if (numchild <= 0)
      return MI_CMD_DONE;
***************
*** 296,303 ****
        ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
        ui_out_field_string (uiout, "exp", varobj_get_expression (*cc));
        ui_out_field_int (uiout, "numchild", varobj_get_num_children (*cc));
-       if (print_values)
- 	ui_out_field_string (uiout, "value", varobj_get_value (*cc));
        type = varobj_get_type (*cc);
        /* C++ pseudo-variables (public, private, protected) do not have a type */
        if (type)
--- 284,289 ----
*** gdb.texinfo.~1.185~	2003-12-17 00:06:41.000000000 +0000
--- gdb.texinfo	2003-12-17 02:13:43.000000000 +0000
***************
*** 17202,17209 ****
  @end smallexample
  
  Display the local variable names for the current frame.  With an
! argument of 0 prints only the names of the variables, with argument of 1
! prints also their values.
  
  @subsubheading @value{GDBN} Command
  
--- 17202,17215 ----
  @end smallexample
  
  Display the local variable names for the current frame.  With an
! argument of 0 or @code{--no-values}, prints only the names of the variables.
! With argument of 1 or @code{--all-values}, prints also their values.  With
! argument of 2 or @code{--simple-values}, prints the name, type and value for
! simple data types and the name and type for arrays, structures and
! unions.  In this last case, the idea is that the user can see the
! value of simple data types immediately and he can create variable
! objects for other data types if he wishes to explore their values in
! more detail.
  
  @subsubheading @value{GDBN} Command
  
***************
*** 17216,17224 ****
  -stack-list-locals 0
  ^done,locals=[name="A",name="B",name="C"]
  (@value{GDBP})
! -stack-list-locals 1
  ^done,locals=[@{name="A",value="1"@},@{name="B",value="2"@},
!   @{name="C",value="3"@}]
  (@value{GDBP})
  @end smallexample
  
--- 17222,17233 ----
  -stack-list-locals 0
  ^done,locals=[name="A",name="B",name="C"]
  (@value{GDBP})
! -stack-list-locals --all-values
  ^done,locals=[@{name="A",value="1"@},@{name="B",value="2"@},
!   @{name="C",value="@{1, 2, 3@}"@}]
! -stack-list-locals --simple-values
! ^done,locals=[@{name="A",type="int",value="1"@},
!   @{name="B",type="int",value="2"@},@{name="C",type="int [3]"@}]
  (@value{GDBP})
  @end smallexample
  
***************
*** 18162,18175 ****
  @subsubheading Synopsis
  
  @smallexample
!  -var-list-children @var{name}
  @end smallexample
  
! Returns a list of the children of the specified variable object:
  
  @smallexample
   numchild=@var{n},children=[@{name=@var{name},
   numchild=@var{n},type=@var{type}@},@r{(repeats N times)}]
  @end smallexample
  
  
--- 18171,18196 ----
  @subsubheading Synopsis
  
  @smallexample
!  -var-list-children [@var{print-values}] @var{name}
  @end smallexample
  
! Returns a list of the children of the specified variable object.  With
! just the variable object name as an argument or with an optional
! preceding argument of 0 or @code{--no-values}, prints only the names of the
! variables.  With an optional preceding argument of 1 or @code{--all-values},
! also prints their values.
! 
! @subsubheading Example
  
  @smallexample
+ (@value{GDBP})
+  -var-list-children n
   numchild=@var{n},children=[@{name=@var{name},
   numchild=@var{n},type=@var{type}@},@r{(repeats N times)}]
+ (@value{GDBP})
+  -var-list-children --all-values n
+  numchild=@var{n},children=[@{name=@var{name},
+  numchild=@var{n},value=@var{value},type=@var{type}@},@r{(repeats N times)}]
  @end smallexample
  
  


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

* Re: [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children
  2003-12-17  2:29             ` [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children Nick Roberts
@ 2003-12-17  2:32               ` David Carlton
  2003-12-17 18:51                 ` [PATCH: gdb/mi] -stack-list-locals Nick Roberts
  2003-12-17 18:05               ` [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children Eli Zaretskii
  2004-01-05 21:43               ` Andrew Cagney
  2 siblings, 1 reply; 20+ messages in thread
From: David Carlton @ 2003-12-17  2:32 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Wed, 17 Dec 2003 02:21:25 +0000, Nick Roberts <nick@nick.uklinux.net> said:

> 1) Uses SYMBOL_PRINT_NAME instead of DEPRECATED_SYMBOL_NAME for lookup_symbol
>    (in mi-cmd-stack.c) as advised by David Carlton.

I actually suggested SYMBOL_NATURAL_NAME for the call to
lookup_symbol.  But it's not in the patch below - I think you left out
that bit of the patch?

David Carlton
carlton@kealia.com


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

* Re: [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children
  2003-12-17  2:29             ` [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children Nick Roberts
  2003-12-17  2:32               ` David Carlton
@ 2003-12-17 18:05               ` Eli Zaretskii
  2004-01-05 21:43               ` Andrew Cagney
  2 siblings, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2003-12-17 18:05 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

> From: Nick Roberts <nick@nick.uklinux.net>
> Date: Wed, 17 Dec 2003 02:21:25 +0000
> 
> This patch covers four files mi-cmds.h, mi-cmd-stack.c, mi-cmd-var.c
> and gdb.texinfo and:

Thanks, the documentation patch is approved.


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

* Re: [PATCH: gdb/mi] -stack-list-locals
  2003-12-17  2:32               ` David Carlton
@ 2003-12-17 18:51                 ` Nick Roberts
  2003-12-17 21:21                   ` David Carlton
  0 siblings, 1 reply; 20+ messages in thread
From: Nick Roberts @ 2003-12-17 18:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: David Carlton


 > I actually suggested SYMBOL_NATURAL_NAME for the call to
 > lookup_symbol.  But it's not in the patch below - I think you left out
 > that bit of the patch?

Yes..Oops! I actually had the right macro in the patch but forgot to include
it.  I guess the moral is don't send in patches at 2 in the morning! Sorry
about that.

Take 2 on mi-cmd-stack.c:

1) Uses SYMBOL_NATURAL_NAME instead of DEPRECATED_SYMBOL_NAME for lookup_symbol
   (in mi-cmd-stack.c) as advised by David Carlton.

2) Shares syntax with earlier changes for -stack-list-locals.

    Nick                                         http://www.nick.uklinux.net



*** mi-cmd-stack.c.~1.19.~	2003-06-12 23:29:37.000000000 +0100
--- mi-cmd-stack.c	2003-12-16 23:14:00.000000000 +0000
***************
*** 29,34 ****
--- 29,35 ----
  #include "block.h"
  #include "stack.h"
  #include "dictionary.h"
+ #include "gdb_string.h"
  
  static void list_args_or_locals (int locals, int values, struct frame_info *fi);
  
***************
*** 137,146 ****
  enum mi_cmd_result
  mi_cmd_stack_list_locals (char *command, char **argv, int argc)
  {
    if (argc != 1)
      error ("mi_cmd_stack_list_locals: Usage: PRINT_VALUES");
  
!   list_args_or_locals (1, atoi (argv[0]), deprecated_selected_frame);
    return MI_CMD_DONE;
  }
  
--- 138,163 ----
  enum mi_cmd_result
  mi_cmd_stack_list_locals (char *command, char **argv, int argc)
  {
+   struct frame_info *frame;
+   enum print_values print_values;
+ 
    if (argc != 1)
      error ("mi_cmd_stack_list_locals: Usage: PRINT_VALUES");
  
!    frame = get_selected_frame ();
! 
!    if (strcmp (argv[0], "0") == 0
!        || strcmp (argv[0], "--no-values") == 0)
!      print_values = PRINT_NO_VALUES;
!    else if (strcmp (argv[0], "1") == 0 
! 	    || strcmp (argv[0], "--all-values") == 0)
!      print_values = PRINT_ALL_VALUES;
!    else if (strcmp (argv[0], "2") == 0
! 	    || strcmp (argv[0], "--simple-values") == 0)
!      print_values = PRINT_SIMPLE_VALUES;
!    else 
!      error ("mi_cmd_stack_list_locals: Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\", 2 or \"--simple-values\"");
!   list_args_or_locals (1, print_values, frame);
    return MI_CMD_DONE;
  }
  
***************
*** 218,223 ****
--- 235,241 ----
    int nsyms;
    struct cleanup *cleanup_list;
    static struct ui_stream *stb = NULL;
+   struct type *type;
  
    stb = ui_out_stream_new (uiout);
  
***************
*** 268,291 ****
  	  if (print_me)
  	    {
  	      struct cleanup *cleanup_tuple = NULL;
! 	      if (values)
  		cleanup_tuple = 
  		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
! 	      ui_out_field_string (uiout, "name", DEPRECATED_SYMBOL_NAME (sym));
  
! 	      if (values)
! 		{
! 		  struct symbol *sym2;
! 		  if (!locals)
! 		    sym2 = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
! 					  block, VAR_DOMAIN,
! 					  (int *) NULL,
! 					  (struct symtab **) NULL);
! 		  else
  		    sym2 = sym;
  		  print_variable_value (sym2, fi, stb->stream);
  		  ui_out_field_stream (uiout, "value", stb);
  		  do_cleanups (cleanup_tuple);
  		}
  	    }
  	}
--- 286,324 ----
  	  if (print_me)
  	    {
  	      struct cleanup *cleanup_tuple = NULL;
! 	      if (values != PRINT_NO_VALUES)
  		cleanup_tuple = 
  		  make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
! 	      ui_out_field_string (uiout, "name", SYMBOL_PRINT_NAME (sym));
  
! 	      struct symbol *sym2;
! 	      if (!locals)
! 		sym2 = lookup_symbol (SYMBOL_NATURAL_NAME (sym),
! 				      block, VAR_DOMAIN,
! 				      (int *) NULL,
! 				      (struct symtab **) NULL);
! 	      else
  		    sym2 = sym;
+ 	      switch (values)
+ 		{
+ 		case PRINT_SIMPLE_VALUES:
+ 		  type = check_typedef (sym2->type);
+ 		  type_print (sym2->type, "", stb->stream, -1);
+ 		  ui_out_field_stream (uiout, "type", stb);
+ 		  if (TYPE_CODE (type) != TYPE_CODE_ARRAY &&
+ 		      TYPE_CODE (type) != TYPE_CODE_STRUCT &&
+ 		      TYPE_CODE (type) != TYPE_CODE_UNION)
+ 		    {
+ 		      print_variable_value (sym2, fi, stb->stream);
+ 		      ui_out_field_stream (uiout, "value", stb);
+ 		    }
+ 		  do_cleanups (cleanup_tuple);
+ 		  break;
+ 		case PRINT_ALL_VALUES:
  		  print_variable_value (sym2, fi, stb->stream);
  		  ui_out_field_stream (uiout, "value", stb);
  		  do_cleanups (cleanup_tuple);
+ 		  break;
  		}
  	    }
  	}


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

* Re: [PATCH: gdb/mi] -stack-list-locals
  2003-12-17 18:51                 ` [PATCH: gdb/mi] -stack-list-locals Nick Roberts
@ 2003-12-17 21:21                   ` David Carlton
  0 siblings, 0 replies; 20+ messages in thread
From: David Carlton @ 2003-12-17 21:21 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Wed, 17 Dec 2003 18:41:01 +0000, Nick Roberts <nick@nick.uklinux.net> said:

> Take 2 on mi-cmd-stack.c:

> 1) Uses SYMBOL_NATURAL_NAME instead of DEPRECATED_SYMBOL_NAME for
>    lookup_symbol (in mi-cmd-stack.c) as advised by David Carlton.

Much better, thanks. :-)

David Carlton
carlton@kealia.com


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

* Re: [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children
  2003-12-17  2:29             ` [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children Nick Roberts
  2003-12-17  2:32               ` David Carlton
  2003-12-17 18:05               ` [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children Eli Zaretskii
@ 2004-01-05 21:43               ` Andrew Cagney
  2004-01-06  0:14                 ` [PATCH: gdb/mi] -stack-list-locals testcase Nick Roberts
  2 siblings, 1 reply; 20+ messages in thread
From: Andrew Cagney @ 2004-01-05 21:43 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

Nick, just the testcase is missing.  Suggest tweaking "mi-stack.exp" 
(you do not need to tweak mi[12]-stack.exp since they test older protocols).

Andrew


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

* Re: [PATCH: gdb/mi] -stack-list-locals testcase
  2004-01-05 21:43               ` Andrew Cagney
@ 2004-01-06  0:14                 ` Nick Roberts
  2004-01-06  1:07                   ` Andrew Cagney
  0 siblings, 1 reply; 20+ messages in thread
From: Nick Roberts @ 2004-01-06  0:14 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches


 > Nick, just the testcase is missing.  Suggest tweaking "mi-stack.exp" 
 > (you do not need to tweak mi[12]-stack.exp since they test older protocols).

mi-stack.exp had three fails to start with. Replacing line=\"7\" with
line=\"8\" and "-exec-next 3" with "-exec-next 4" removed them.

I have added one simple test for `-stack-list-locals 2' that seems to
work. It does not test complex data types but this would require changing
basics.c, which is probably not worth it.

Andrew, if you tell me that this patch does the right thing, then I'll do one
for -var-list-children.

Nick


*** mi-stack.exp.~1.10.~	2002-11-05 15:43:18.000000000 +0000
--- mi-stack.exp	2004-01-05 23:38:27.000000000 +0000
***************
*** 57,63 ****
      # -stack-list-frames 1 3
  
      mi_gdb_test "231-stack-list-frames" \
! 	    "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"8\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
                  "stack frame listing"
      mi_gdb_test "232-stack-list-frames 1 1" \
  	    "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
--- 57,63 ----
      # -stack-list-frames 1 3
  
      mi_gdb_test "231-stack-list-frames" \
! 	    "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"7\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
                  "stack frame listing"
      mi_gdb_test "232-stack-list-frames 1 1" \
  	    "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
***************
*** 156,162 ****
                  "stack locals listing 0"
  
  # step until A, B, C, have some reasonable values.
! send_gdb "-exec-next 3\n"
  gdb_expect {
      -re "\\^running\r\n${mi_gdb_prompt}\\*stopped,reason=\"end-stepping-range\",thread-id=\"\[01\]\",frame=\{addr=\"$hex\",func=\"callee4\",args=\\\[\\\],file=\".*basics.c\",line=\"13\"\}\r\n$mi_gdb_prompt$" {
  	pass "next's in callee4"
--- 156,162 ----
                  "stack locals listing 0"
  
  # step until A, B, C, have some reasonable values.
! send_gdb "-exec-next 4\n"
  gdb_expect {
      -re "\\^running\r\n${mi_gdb_prompt}\\*stopped,reason=\"end-stepping-range\",thread-id=\"\[01\]\",frame=\{addr=\"$hex\",func=\"callee4\",args=\\\[\\\],file=\".*basics.c\",line=\"13\"\}\r\n$mi_gdb_prompt$" {
  	pass "next's in callee4"
***************
*** 168,173 ****
--- 168,177 ----
  	    "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\}\\\]" \
                  "stack locals listing 1"
  
+     mi_gdb_test "232-stack-list-locals 2" \
+ 	    "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\}\\\]" \
+                 "stack locals listing 2"
+ 
      mi_gdb_test "234-stack-list-locals" \
  	    "234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
  	    "stack locals listing wrong"


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

* Re: [PATCH: gdb/mi] -stack-list-locals testcase
  2004-01-06  0:14                 ` [PATCH: gdb/mi] -stack-list-locals testcase Nick Roberts
@ 2004-01-06  1:07                   ` Andrew Cagney
  2004-01-07 16:31                     ` Andrew Cagney
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Cagney @ 2004-01-06  1:07 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

>  > Nick, just the testcase is missing.  Suggest tweaking "mi-stack.exp" 
>  > (you do not need to tweak mi[12]-stack.exp since they test older protocols).
> 
> mi-stack.exp had three fails to start with. Replacing line=\"7\" with
> line=\"8\" and "-exec-next 3" with "-exec-next 4" removed them.
> 
> I have added one simple test for `-stack-list-locals 2' that seems to
> work. It does not test complex data types but this would require changing
> basics.c, which is probably not worth it.
> 
> Andrew, if you tell me that this patch does the right thing, then I'll do one
> for -var-list-children.
> 
> Nick
> 
> 
> *** mi-stack.exp.~1.10.~	2002-11-05 15:43:18.000000000 +0000
> --- mi-stack.exp	2004-01-05 23:38:27.000000000 +0000
> ***************
> *** 57,63 ****
>       # -stack-list-frames 1 3
>   
>       mi_gdb_test "231-stack-list-frames" \
> ! 	    "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"8\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
>                   "stack frame listing"
>       mi_gdb_test "232-stack-list-frames 1 1" \
>   	    "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
> --- 57,63 ----
>       # -stack-list-frames 1 3
>   
>       mi_gdb_test "231-stack-list-frames" \
> ! 	    "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"7\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
>                   "stack frame listing"
>       mi_gdb_test "232-stack-list-frames 1 1" \
>   	    "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \

This test (and the rest of mi-stack.exp) is passing on a PPC/NetBSD gcc 
2.96 stabs system.  We'll need to look at it more carefully (or check 
michaelc's test matrix) to see why there's a failure.  MI is more strict 
  with its test results.

> ***************
> *** 156,162 ****
>                   "stack locals listing 0"
>   
>   # step until A, B, C, have some reasonable values.
> ! send_gdb "-exec-next 3\n"
>   gdb_expect {
>       -re "\\^running\r\n${mi_gdb_prompt}\\*stopped,reason=\"end-stepping-range\",thread-id=\"\[01\]\",frame=\{addr=\"$hex\",func=\"callee4\",args=\\\[\\\],file=\".*basics.c\",line=\"13\"\}\r\n$mi_gdb_prompt$" {
>   	pass "next's in callee4"
> --- 156,162 ----
>                   "stack locals listing 0"
>   
>   # step until A, B, C, have some reasonable values.
> ! send_gdb "-exec-next 4\n"
>   gdb_expect {
>       -re "\\^running\r\n${mi_gdb_prompt}\\*stopped,reason=\"end-stepping-range\",thread-id=\"\[01\]\",frame=\{addr=\"$hex\",func=\"callee4\",args=\\\[\\\],file=\".*basics.c\",line=\"13\"\}\r\n$mi_gdb_prompt$" {
>   	pass "next's in callee4"
> ***************
> *** 168,173 ****
> --- 168,177 ----
>   	    "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\}\\\]" \
>                   "stack locals listing 1"
>   
> +     mi_gdb_test "232-stack-list-locals 2" \
> + 	    "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\}\\\]" \
> +                 "stack locals listing 2"
> + 

Yes, just like that.

Andrew


>       mi_gdb_test "234-stack-list-locals" \
>   	    "234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
>   	    "stack locals listing wrong"
> 



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

* Re: [PATCH: gdb/mi] -stack-list-locals testcase
  2004-01-06  1:07                   ` Andrew Cagney
@ 2004-01-07 16:31                     ` Andrew Cagney
  2004-01-07 17:34                       ` Mark Kettenis
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Cagney @ 2004-01-07 16:31 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

> *** mi-stack.exp.~1.10.~    2002-11-05 15:43:18.000000000 +0000
> --- mi-stack.exp    2004-01-05 23:38:27.000000000 +0000
> ***************
> *** 57,63 ****
>       # -stack-list-frames 1 3
>         mi_gdb_test "231-stack-list-frames" \
> !         "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"8\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
>                   "stack frame listing"
>       mi_gdb_test "232-stack-list-frames 1 1" \
>           "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
> --- 57,63 ----
>       # -stack-list-frames 1 3
>         mi_gdb_test "231-stack-list-frames" \
> !         "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"7\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
>                   "stack frame listing"
>       mi_gdb_test "232-stack-list-frames 1 1" \
>           "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
> 
> This test (and the rest of mi-stack.exp) is passing on a PPC/NetBSD gcc 2.96 stabs system.  We'll need to look at it more carefully (or check michaelc's test matrix) to see why there's a failure.  MI is more strict  with its test results. 

Nick, what's your exact system?  On amd64 and i386 GNU/Linux systems 
(RHEL 3, dwarf 2, gcc 2.3.2 based) mi-stack.exp I'm also seeing this 
pass (puzzled).

Andrew


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

* Re: [PATCH: gdb/mi] -stack-list-locals testcase
  2004-01-07 16:31                     ` Andrew Cagney
@ 2004-01-07 17:34                       ` Mark Kettenis
  2004-01-07 17:40                         ` Daniel Jacobowitz
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Kettenis @ 2004-01-07 17:34 UTC (permalink / raw)
  To: cagney; +Cc: nick, gdb-patches

   Date: Wed, 07 Jan 2004 11:31:54 -0500
   From: Andrew Cagney <cagney@gnu.org>

   > *** mi-stack.exp.~1.10.~    2002-11-05 15:43:18.000000000 +0000
   > --- mi-stack.exp    2004-01-05 23:38:27.000000000 +0000
   > ***************
   > *** 57,63 ****
   >       # -stack-list-frames 1 3
   >         mi_gdb_test "231-stack-list-frames" \
   > !         "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"8\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
   >                   "stack frame listing"
   >       mi_gdb_test "232-stack-list-frames 1 1" \
   >           "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
   > --- 57,63 ----
   >       # -stack-list-frames 1 3
   >         mi_gdb_test "231-stack-list-frames" \
   > !         "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"7\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
   >                   "stack frame listing"
   >       mi_gdb_test "232-stack-list-frames 1 1" \
   >           "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
   > 
   > This test (and the rest of mi-stack.exp) is passing on a PPC/NetBSD gcc 2.96 stabs system.  We'll need to look at it more carefully (or check michaelc's test matrix) to see why there's a failure.  MI is more strict  with its test results. 

   Nick, what's your exact system?  On amd64 and i386 GNU/Linux systems 
   (RHEL 3, dwarf 2, gcc 2.3.2 based) mi-stack.exp I'm also seeing this 
   pass (puzzled).

I think it would be useful if Nick could post the output from "objdump
--stabs" on the generated binary.  The output of "gcc -g -S" on the
relevant source file would be even more useful.

I know for a fact that there are serious problems with the way GDB
reads line numbers for stabs.  In order to work around the bad
line-number info generated by GCC 2.95.3, we relocate the first
N_SLINE stab, which leads to incorrect behaviour, such as skipping the
first line of a function, with other compilers (such as Sun's) and
when debugging optimized code.  It's not unlikely that it influences
non-IA-32 platforms too.

I'll post a more detailed explanation in the near future.

Mark


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

* Re: [PATCH: gdb/mi] -stack-list-locals testcase
  2004-01-07 17:34                       ` Mark Kettenis
@ 2004-01-07 17:40                         ` Daniel Jacobowitz
  0 siblings, 0 replies; 20+ messages in thread
From: Daniel Jacobowitz @ 2004-01-07 17:40 UTC (permalink / raw)
  To: gdb-patches

On Wed, Jan 07, 2004 at 06:34:13PM +0100, Mark Kettenis wrote:
>    Date: Wed, 07 Jan 2004 11:31:54 -0500
>    From: Andrew Cagney <cagney@gnu.org>
> 
>    > *** mi-stack.exp.~1.10.~    2002-11-05 15:43:18.000000000 +0000
>    > --- mi-stack.exp    2004-01-05 23:38:27.000000000 +0000
>    > ***************
>    > *** 57,63 ****
>    >       # -stack-list-frames 1 3
>    >         mi_gdb_test "231-stack-list-frames" \
>    > !         "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"8\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
>    >                   "stack frame listing"
>    >       mi_gdb_test "232-stack-list-frames 1 1" \
>    >           "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
>    > --- 57,63 ----
>    >       # -stack-list-frames 1 3
>    >         mi_gdb_test "231-stack-list-frames" \
>    > !         "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"7\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
>    >                   "stack frame listing"
>    >       mi_gdb_test "232-stack-list-frames 1 1" \
>    >           "232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
>    > 
>    > This test (and the rest of mi-stack.exp) is passing on a PPC/NetBSD gcc 2.96 stabs system.  We'll need to look at it more carefully (or check michaelc's test matrix) to see why there's a failure.  MI is more strict  with its test results. 
> 
>    Nick, what's your exact system?  On amd64 and i386 GNU/Linux systems 
>    (RHEL 3, dwarf 2, gcc 2.3.2 based) mi-stack.exp I'm also seeing this 
>    pass (puzzled).
> 
> I think it would be useful if Nick could post the output from "objdump
> --stabs" on the generated binary.  The output of "gcc -g -S" on the
> relevant source file would be even more useful.
> 
> I know for a fact that there are serious problems with the way GDB
> reads line numbers for stabs.  In order to work around the bad
> line-number info generated by GCC 2.95.3, we relocate the first
> N_SLINE stab, which leads to incorrect behaviour, such as skipping the
> first line of a function, with other compilers (such as Sun's) and
> when debugging optimized code.  It's not unlikely that it influences
> non-IA-32 platforms too.
> 
> I'll post a more detailed explanation in the near future.

*sigh* it may be time to remove that hack.  I went to a lot of trouble
to get it to work, but lately a number of legitimate inputs have been
getting confused by it.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

end of thread, other threads:[~2004-01-07 17:40 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-11-23  0:16 RFC (gdb/mi): -stack-list-locals Nick Roberts
2003-11-26  0:48 ` Andrew Cagney
2003-12-02  3:14   ` RFC (gdb/mi): -stack-list-locals + PATCH Nick Roberts
2003-12-09  2:42     ` RFC (gdb/mi): -stack-list-locals + REVISED PATCH Nick Roberts
2003-12-10 17:56       ` Andrew Cagney
2003-12-10 22:26         ` Jason Molenda
2003-12-12 20:51         ` RFC (gdb/mi): -stack-list-locals Nick Roberts
2003-12-12 21:09           ` David Carlton
2003-12-17  2:29             ` [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children Nick Roberts
2003-12-17  2:32               ` David Carlton
2003-12-17 18:51                 ` [PATCH: gdb/mi] -stack-list-locals Nick Roberts
2003-12-17 21:21                   ` David Carlton
2003-12-17 18:05               ` [PATCH: gdb/mi + doco] -stack-list-locals and -var-list-children Eli Zaretskii
2004-01-05 21:43               ` Andrew Cagney
2004-01-06  0:14                 ` [PATCH: gdb/mi] -stack-list-locals testcase Nick Roberts
2004-01-06  1:07                   ` Andrew Cagney
2004-01-07 16:31                     ` Andrew Cagney
2004-01-07 17:34                       ` Mark Kettenis
2004-01-07 17:40                         ` Daniel Jacobowitz
2003-12-12 23:01           ` RFC (gdb/mi): -stack-list-locals Jason Molenda

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