[PATCH] -data-list-changed-registers

[PATCH] -data-list-changed-registers

[Nick Roberts]

Currently the GDB/MI command -data-list-changed-registers gives an internal
error if there is no stack i.e when there is no inferior process:

nickrob/91 ~/src/gdb/gdb -i=mi myprog
~"GNU gdb 6.3.50.20050527-cvs\n"
~"Copyright 2004 Free Software Foundation, Inc.\n"
~"GDB is free software, covered by the GNU General Public License, and you are\n
"
~"welcome to change it and/or distribute copies of it under certain conditions.\
n"
~"Type \"show copying\" to see the conditions.\n"
~"There is absolutely no warranty for GDB.  Type \"show warranty\" for details.\
n"
~"This GDB was configured as \"i586-pc-linux-gnu\"..."
~"Using host libthread_db library \"/lib/libthread_db.so.1\".\n"
~"\n"
(gdb) 
-data-list-changed-registers
~"frame.c:616: internal-error: frame_register: Assertion `frame != NULL && frame
->next != NULL' failed.\n"
~"A problem internal to GDB has been detected,\n"
~"further debugging may prove unreliable.\n"
~"Quit this debugging session? (y or n) "
~"\n"
~"frame.c:616: internal-error: frame_register: Assertion `frame != NULL && frame
->next != NULL' failed.\n"
~"A problem internal to GDB has been detected,\n"
~"further debugging may prove unreliable.\n"
~"Create a core file of GDB? (y or n) "

This patch changes the output to:

-data-list-changed-registers
^error,msg="mi_cmd_data_list_changed_registers: No registers."
(gdb) 


Nick


2005-06-01  Nick Roberts  <nickrob@snap.net.nz>

	* mi/mi-main.c (mi_cmd_data_list_changed_registers): Test for
	registers.


*** /home/nick/src/gdb/mi/mi-main.c.~1.78.~	2005-05-27 12:27:29.000000000 +1200
--- /home/nick/src/gdb/mi/mi-main.c	2005-06-01 22:59:47.000000000 +1200
***************
*** 331,336 ****
--- 331,342 ----
  
    cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changed-registers");
  
+   if (!target_has_registers)
+     {
+       mi_error_message = xstrprintf ("mi_cmd_data_list_changed_registers: No registers.");
+       return MI_CMD_ERROR;
+     }
+ 
    if (argc == 0)		/* No args, just do all the regs */
      {
        for (regnum = 0;

Re: [PATCH] -data-list-changed-registers

[Daniel Jacobowitz]

On Wed, Jun 01, 2005 at 11:12:01PM +1200, Nick Roberts wrote:
> 
> Currently the GDB/MI command -data-list-changed-registers gives an internal
> error if there is no stack i.e when there is no inferior process:

This has come up before; the right solution is elsewhere.  I believe
that updating register_changed_p and get_register to use
get_selected_frame() will work.

I have a big patch for this sort of thing lying around, but the problem
is that many of the references to deprecated_selected_frame should
really by fixed by adding a frame parameter to the function, so that
they can be used on non-selected frames.  They need to be looked over
one by one and I haven't gotten round to it yet.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -data-list-changed-registers

[Nick Roberts]

Daniel Jacobowitz writes:
 > On Wed, Jun 01, 2005 at 11:12:01PM +1200, Nick Roberts wrote:
 > > 
 > > Currently the GDB/MI command -data-list-changed-registers gives an internal
 > > error if there is no stack i.e when there is no inferior process:
 > 
 > This has come up before; the right solution is elsewhere.  I believe
 > that updating register_changed_p and get_register to use
 > get_selected_frame() will work.
 > 
 > I have a big patch for this sort of thing lying around, but the problem
 > is that many of the references to deprecated_selected_frame should
 > really by fixed by adding a frame parameter to the function, so that
 > they can be used on non-selected frames.  They need to be looked over
 > one by one and I haven't gotten round to it yet.

Could this patch be applied as an interim measure? Currently any front end
that uses -data-list-changed-registers without a stack crashes.

Nick

Re: [PATCH] -data-list-changed-registers

[Daniel Jacobowitz]

On Sat, Jun 04, 2005 at 10:36:05AM +1200, Nick Roberts wrote:
> Daniel Jacobowitz writes:
>  > On Wed, Jun 01, 2005 at 11:12:01PM +1200, Nick Roberts wrote:
>  > > 
>  > > Currently the GDB/MI command -data-list-changed-registers gives an internal
>  > > error if there is no stack i.e when there is no inferior process:
>  > 
>  > This has come up before; the right solution is elsewhere.  I believe
>  > that updating register_changed_p and get_register to use
>  > get_selected_frame() will work.
>  > 
>  > I have a big patch for this sort of thing lying around, but the problem
>  > is that many of the references to deprecated_selected_frame should
>  > really by fixed by adding a frame parameter to the function, so that
>  > they can be used on non-selected frames.  They need to be looked over
>  > one by one and I haven't gotten round to it yet.
> 
> Could this patch be applied as an interim measure? Currently any front end
> that uses -data-list-changed-registers without a stack crashes.

I would prefer not to; right now the presence of
deprecated_selected_frame is a good warning sign.  I can find you a
copy if you want to look at individual places that should be
investigated.

I think the changes are right in this case, though.  Could you take a
look at those two functions (the change to make is obvious) and see if
that fixes your problem?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -data-list-changed-registers

[Nick Roberts]

 > I think the changes are right in this case, though.  Could you take a
 > look at those two functions (the change to make is obvious) and see if
 > that fixes your problem?

If you mean the changes below, then these seem fine and give:

(gdb) 
-data-list-changed-registers
&"No registers.\n"
^error,msg="No registers."
(gdb) 

I have presumed that similarly this is wrong:

(gdb) 
-data-list-register-values x
^error,msg="mi_cmd_data_list_register_values: No registers."
(gdb) 

and removed the same error check (if (!target_has_registers)...) so that
this also gives:

(gdb) 
-data-list-register-values x
&"No registers.\n"
^error,msg="No registers."
(gdb) 

mi_cmd_data_write_register_values has the same error check.  Should I remove
this too?


Nick


*** /home/nick/src/gdb/mi/mi-main.c.~1.78.~	2005-05-27 12:27:29.000000000 +1200
--- /home/nick/src/gdb/mi/mi-main.c	2005-06-04 23:34:21.000000000 +1200
***************
*** 388,394 ****
  {
    gdb_byte raw_buffer[MAX_REGISTER_SIZE];
  
!   if (! frame_register_read (deprecated_selected_frame, regnum, raw_buffer))
      return -1;
  
    if (memcmp (&old_regs[DEPRECATED_REGISTER_BYTE (regnum)], raw_buffer,
--- 388,394 ----
  {
    gdb_byte raw_buffer[MAX_REGISTER_SIZE];
  
!   if (! frame_register_read (get_selected_frame (NULL), regnum, raw_buffer))
      return -1;
  
    if (memcmp (&old_regs[DEPRECATED_REGISTER_BYTE (regnum)], raw_buffer,
***************
*** 433,444 ****
  
    format = (int) argv[0][0];
  
-   if (!target_has_registers)
-     {
-       mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: No registers.");
-       return MI_CMD_ERROR;
-     }
- 
    list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "register-values");
  
    if (argc == 1)		/* No args, beside the format: do all the regs */
--- 433,438 ----
***************
*** 509,515 ****
    if (format == 'N')
      format = 0;
  
!   frame_register (deprecated_selected_frame, regnum, &optim, &lval, &addr,
  		  &realnum, buffer);
  
    if (optim)
--- 503,509 ----
    if (format == 'N')
      format = 0;
  
!   frame_register (get_selected_frame (NULL), regnum, &optim, &lval, &addr,
  		  &realnum, buffer);
  
    if (optim)

Re: [PATCH] -data-list-changed-registers

[Nick Roberts]

Nick Roberts writes:
 >  > I think the changes are right in this case, though.  Could you take a
 >  > look at those two functions (the change to make is obvious) and see if
 >  > that fixes your problem?
 > 
 > If you mean the changes below, then these seem fine and give:

Actually this isn't quite right:

(gdb) 
-break-insert main
^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x080484d9",func="main",file="myprog.c",line="55",times="0"}
(gdb) 
-data-list-changed-registers
&"No registers.\n"
^error,msg="No registers."
(gdb) 
-exec-run
^running
(gdb) 
*stopped,changed-registers=[],reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x080484d9",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbffff794"}],file="myprog.c",fullname="/home/nick/myprog.c",line="55"}
(gdb) 
-data-list-changed-registers
^done,changed-registers=["1","3","4","5","6","7","8","9","10","11","12","13","24","26","40","41"]
(gdb) 


I think the changed-registers=[] after *stopped occurs because an exception is
raised in register_changed_p in the _first_ call to
mi_cmd_data_list_changed_registers and control doesn't return to to call
do_cleanups (cleanup).

Nick

Re: [PATCH] -data-list-changed-registers

[Nick Roberts]

 > Actually this isn't quite right:
 > 
 > (gdb) 
 > -break-insert main
 > ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x080484d9",func="main",file="myprog.c",line="55",times="0"}
 > (gdb) 
 > -data-list-changed-registers
 > &"No registers.\n"
 > ^error,msg="No registers."
 > (gdb) 
 > -exec-run
 > ^running
 > (gdb) 
 > *stopped,changed-registers=[],reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x080484d9",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbffff794"}],file="myprog.c",fullname="/home/nick/myprog.c",line="55"}
 > (gdb) 
 > -data-list-changed-registers
 > ^done,changed-registers=["1","3","4","5","6","7","8","9","10","11","12","13","24","26","40","41"]
 > (gdb) 
 > 
 > 
 > I think the changed-registers=[] after *stopped occurs because an exception
 > is raised in register_changed_p in the _first_ call to
 > mi_cmd_data_list_changed_registers and control doesn't return to to call
 > do_cleanups (cleanup).

Well it turms out that throw_exception calls do_cleanups, but clearly the
above output isn't acceptable. With CLI everything is output immediately.
With MI it's stored in memory to be output later. This can mixed with the
output of the next MI command and break the syntax, as above.

This patch addresses this problem and is not as lightweight as my earlier
patches. If its not the right fix then its pretty damn close.

Nick


*** /home/nick/src/gdb/ui-file.h.~1.4.~	2003-06-11 02:37:04.000000000 +1200
--- /home/nick/src/gdb/ui-file.h	2005-06-11 14:16:15.000000000 +1200
***************
*** 83,88 ****
--- 83,93 ----
  
  extern long ui_file_read (struct ui_file *file, char *buf, long length_buf);
  
+ struct mem_file;
+ extern void mem_init_stream_buffer (struct mem_file *stream);
+ 
+ extern void mem_free_stream_contents (void *ptr);
+ 
  /* Create/open a memory based file. Can be used as a scratch buffer
     for collecting output. */
  extern struct ui_file *mem_fileopen (void);

*** /home/nick/src/gdb/ui-file.h.~1.4.~	2003-06-11 02:37:04.000000000 +1200
--- /home/nick/src/gdb/ui-file.h	2005-06-11 14:16:15.000000000 +1200
***************
*** 83,88 ****
--- 83,93 ----
  
  extern long ui_file_read (struct ui_file *file, char *buf, long length_buf);
  
+ struct mem_file;
+ extern void mem_init_stream_buffer (struct mem_file *stream);
+ 
+ extern void mem_free_stream_contents (void *ptr);
+ 
  /* Create/open a memory based file. Can be used as a scratch buffer
     for collecting output. */
  extern struct ui_file *mem_fileopen (void);


*** /home/nick/src/gdb/ui-file.c.~1.11.~	2005-02-13 00:36:15.000000000 +1300
--- /home/nick/src/gdb/ui-file.c	2005-06-11 14:36:23.000000000 +1200
***************
*** 403,408 ****
--- 403,432 ----
        stream->length_buffer = new_length;
      }
  }
+ 
+ void
+ mem_init_stream_buffer (struct mem_file *stream)
+ {
+   stream->sizeof_buffer = 0;
+   stream->length_buffer = 0;
+   stream->buffer = xmalloc (stream->sizeof_buffer);
+ }
+ 
+ void
+ mem_free_stream_contents (void *ptr)
+ {
+   struct mem_file **stream_ptr = ptr;
+   if (stream_ptr == NULL)
+     internal_error (__FILE__, __LINE__,
+ 		    _("free_stream_contents: NULL pointer"));
+   if (*stream_ptr != NULL)
+     {
+       (*stream_ptr)->sizeof_buffer = 0;
+       (*stream_ptr)->length_buffer = 0;
+       (*stream_ptr)->buffer = xrealloc ((*stream_ptr)->buffer, 0);
+     }
+ }
+ 
  \f
  /* ``struct ui_file'' implementation that maps directly onto
     <stdio.h>'s FILE. */


*** /home/nick/src/gdb/mi/mi-main.c.~1.78.~	2005-05-27 12:27:29.000000000 +1200
--- /home/nick/src/gdb/mi/mi-main.c	2005-06-11 17:25:03.000000000 +1200
***************
*** 319,325 ****
  {
    int regnum, numregs, changed;
    int i;
!   struct cleanup *cleanup;
  
    /* Note that the test for a valid register must include checking the
       REGISTER_NAME because NUM_REGS may be allocated for the union of
--- 319,327 ----
  {
    int regnum, numregs, changed;
    int i;
!   struct cleanup *stream_cleanup, *cleanup;
!   mi_out_data *data = ui_out_data (uiout);
!   struct mem_file *stream = ui_file_data (data->buffer);
  
    /* Note that the test for a valid register must include checking the
       REGISTER_NAME because NUM_REGS may be allocated for the union of
***************
*** 329,334 ****
--- 331,338 ----
  
    numregs = NUM_REGS + NUM_PSEUDO_REGS;
  
+   mem_init_stream_buffer (stream);
+   stream_cleanup = make_cleanup (mem_free_stream_contents, &stream);
    cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changed-registers");
  
    if (argc == 0)		/* No args, just do all the regs */
***************
*** 380,385 ****
--- 384,390 ----
  	}
      }
    do_cleanups (cleanup);
+   discard_cleanups (stream_cleanup);
    return MI_CMD_DONE;
  }
  
***************
*** 388,394 ****
  {
    gdb_byte raw_buffer[MAX_REGISTER_SIZE];
  
!   if (! frame_register_read (deprecated_selected_frame, regnum, raw_buffer))
      return -1;
  
    if (memcmp (&old_regs[DEPRECATED_REGISTER_BYTE (regnum)], raw_buffer,
--- 393,399 ----
  {
    gdb_byte raw_buffer[MAX_REGISTER_SIZE];
  
!   if (! frame_register_read (get_selected_frame (NULL), regnum, raw_buffer))
      return -1;
  
    if (memcmp (&old_regs[DEPRECATED_REGISTER_BYTE (regnum)], raw_buffer,
***************
*** 415,422 ****
  {
    int regnum, numregs, format, result;
    int i;
!   struct cleanup *list_cleanup, *tuple_cleanup;
! 
    /* Note that the test for a valid register must include checking the
       REGISTER_NAME because NUM_REGS may be allocated for the union of
       the register sets within a family of related processors.  In this
--- 420,429 ----
  {
    int regnum, numregs, format, result;
    int i;
!   struct cleanup *stream_cleanup, *list_cleanup, *tuple_cleanup;
!   mi_out_data *data = ui_out_data (uiout);
!   struct mem_file *stream = ui_file_data (data->buffer);
!  
    /* Note that the test for a valid register must include checking the
       REGISTER_NAME because NUM_REGS may be allocated for the union of
       the register sets within a family of related processors.  In this
***************
*** 433,444 ****
  
    format = (int) argv[0][0];
  
!   if (!target_has_registers)
!     {
!       mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: No registers.");
!       return MI_CMD_ERROR;
!     }
! 
    list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "register-values");
  
    if (argc == 1)		/* No args, beside the format: do all the regs */
--- 440,447 ----
  
    format = (int) argv[0][0];
  
!   mem_init_stream_buffer (stream);
!   stream_cleanup = make_cleanup (mem_free_stream_contents, &stream);
    list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "register-values");
  
    if (argc == 1)		/* No args, beside the format: do all the regs */
***************
*** 490,495 ****
--- 493,499 ----
  	}
      }
    do_cleanups (list_cleanup);
+   discard_cleanups (stream_cleanup);
    return MI_CMD_DONE;
  }
  
***************
*** 509,515 ****
    if (format == 'N')
      format = 0;
  
!   frame_register (deprecated_selected_frame, regnum, &optim, &lval, &addr,
  		  &realnum, buffer);
  
    if (optim)
--- 513,519 ----
    if (format == 'N')
      format = 0;
  
!   frame_register (get_selected_frame (NULL), regnum, &optim, &lval, &addr,
  		  &realnum, buffer);
  
    if (optim)

Re: [PATCH] -data-list-changed-registers (Take 2)

[Nick Roberts]

    [Including Changelog and no duplication this time]

 > Actually this isn't quite right:
 > 
 > (gdb) 
 > -break-insert main
 > ^done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x080484d9",func="main",file="myprog.c",line="55",times="0"}
 > (gdb) 
 > -data-list-changed-registers
 > &"No registers.\n"
 > ^error,msg="No registers."
 > (gdb) 
 > -exec-run
 > ^running
 > (gdb) 
 > *stopped,changed-registers=[],reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x080484d9",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbffff794"}],file="myprog.c",fullname="/home/nick/myprog.c",line="55"}
 > (gdb) 
 > -data-list-changed-registers
 > ^done,changed-registers=["1","3","4","5","6","7","8","9","10","11","12","13","24","26","40","41"]
 > (gdb) 
 > 
 > 
 > I think the changed-registers=[] after *stopped occurs because an exception
 > is raised in register_changed_p in the _first_ call to
 > mi_cmd_data_list_changed_registers and control doesn't return to to call
 > do_cleanups (cleanup).

Well it turms out that throw_exception calls do_cleanups, but clearly the
above output isn't acceptable. With CLI everything is output immediately.
With MI it's stored in memory to be output later. This can mixed with the
output of the next MI command and break the syntax, as above.

This patch addresses this problem and is not as lightweight as my earlier
patches. If its not the right fix then its pretty damn close.

Nick

2005-06-11  Nick Roberts  <nickrob@snap.net.nz>

	* ui-file.h: Add extern declarations.
	* ui-file.c (mem_init_stream_buffer, mem_free_stream_contents):
	New functions.

	* mi/mi-main.c (mi_cmd_data_list_changed_registers)
	(mi_cmd_data_list_register_values): Cleanup pending MI output
	when there is an exception.
	(register_changed_p, get_register): Use get_selected_frame.


*** /home/nick/src/gdb/ui-file.h.~1.4.~	2003-06-11 02:37:04.000000000 +1200
--- /home/nick/src/gdb/ui-file.h	2005-06-11 14:16:15.000000000 +1200
***************
*** 83,88 ****
--- 83,93 ----
  
  extern long ui_file_read (struct ui_file *file, char *buf, long length_buf);
  
+ struct mem_file;
+ extern void mem_init_stream_buffer (struct mem_file *stream);
+ 
+ extern void mem_free_stream_contents (void *ptr);
+ 
  /* Create/open a memory based file. Can be used as a scratch buffer
     for collecting output. */
  extern struct ui_file *mem_fileopen (void);


*** /home/nick/src/gdb/ui-file.c.~1.11.~	2005-02-13 00:36:15.000000000 +1300
--- /home/nick/src/gdb/ui-file.c	2005-06-11 14:36:23.000000000 +1200
***************
*** 403,408 ****
--- 403,432 ----
        stream->length_buffer = new_length;
      }
  }
+ 
+ void
+ mem_init_stream_buffer (struct mem_file *stream)
+ {
+   stream->sizeof_buffer = 0;
+   stream->length_buffer = 0;
+   stream->buffer = xmalloc (stream->sizeof_buffer);
+ }
+ 
+ void
+ mem_free_stream_contents (void *ptr)
+ {
+   struct mem_file **stream_ptr = ptr;
+   if (stream_ptr == NULL)
+     internal_error (__FILE__, __LINE__,
+ 		    _("free_stream_contents: NULL pointer"));
+   if (*stream_ptr != NULL)
+     {
+       (*stream_ptr)->sizeof_buffer = 0;
+       (*stream_ptr)->length_buffer = 0;
+       (*stream_ptr)->buffer = xrealloc ((*stream_ptr)->buffer, 0);
+     }
+ }
+ 
  \f
  /* ``struct ui_file'' implementation that maps directly onto
     <stdio.h>'s FILE. */


*** /home/nick/src/gdb/mi/mi-main.c.~1.78.~	2005-05-27 12:27:29.000000000 +1200
--- /home/nick/src/gdb/mi/mi-main.c	2005-06-11 17:25:03.000000000 +1200
***************
*** 319,325 ****
  {
    int regnum, numregs, changed;
    int i;
!   struct cleanup *cleanup;
  
    /* Note that the test for a valid register must include checking the
       REGISTER_NAME because NUM_REGS may be allocated for the union of
--- 319,327 ----
  {
    int regnum, numregs, changed;
    int i;
!   struct cleanup *stream_cleanup, *cleanup;
!   mi_out_data *data = ui_out_data (uiout);
!   struct mem_file *stream = ui_file_data (data->buffer);
  
    /* Note that the test for a valid register must include checking the
       REGISTER_NAME because NUM_REGS may be allocated for the union of
***************
*** 329,334 ****
--- 331,338 ----
  
    numregs = NUM_REGS + NUM_PSEUDO_REGS;
  
+   mem_init_stream_buffer (stream);
+   stream_cleanup = make_cleanup (mem_free_stream_contents, &stream);
    cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changed-registers");
  
    if (argc == 0)		/* No args, just do all the regs */
***************
*** 380,385 ****
--- 384,390 ----
  	}
      }
    do_cleanups (cleanup);
+   discard_cleanups (stream_cleanup);
    return MI_CMD_DONE;
  }
  
***************
*** 388,394 ****
  {
    gdb_byte raw_buffer[MAX_REGISTER_SIZE];
  
!   if (! frame_register_read (deprecated_selected_frame, regnum, raw_buffer))
      return -1;
  
    if (memcmp (&old_regs[DEPRECATED_REGISTER_BYTE (regnum)], raw_buffer,
--- 393,399 ----
  {
    gdb_byte raw_buffer[MAX_REGISTER_SIZE];
  
!   if (! frame_register_read (get_selected_frame (NULL), regnum, raw_buffer))
      return -1;
  
    if (memcmp (&old_regs[DEPRECATED_REGISTER_BYTE (regnum)], raw_buffer,
***************
*** 415,422 ****
  {
    int regnum, numregs, format, result;
    int i;
!   struct cleanup *list_cleanup, *tuple_cleanup;
! 
    /* Note that the test for a valid register must include checking the
       REGISTER_NAME because NUM_REGS may be allocated for the union of
       the register sets within a family of related processors.  In this
--- 420,429 ----
  {
    int regnum, numregs, format, result;
    int i;
!   struct cleanup *stream_cleanup, *list_cleanup, *tuple_cleanup;
!   mi_out_data *data = ui_out_data (uiout);
!   struct mem_file *stream = ui_file_data (data->buffer);
!  
    /* Note that the test for a valid register must include checking the
       REGISTER_NAME because NUM_REGS may be allocated for the union of
       the register sets within a family of related processors.  In this
***************
*** 433,444 ****
  
    format = (int) argv[0][0];
  
!   if (!target_has_registers)
!     {
!       mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: No registers.");
!       return MI_CMD_ERROR;
!     }
! 
    list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "register-values");
  
    if (argc == 1)		/* No args, beside the format: do all the regs */
--- 440,447 ----
  
    format = (int) argv[0][0];
  
!   mem_init_stream_buffer (stream);
!   stream_cleanup = make_cleanup (mem_free_stream_contents, &stream);
    list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "register-values");
  
    if (argc == 1)		/* No args, beside the format: do all the regs */
***************
*** 490,495 ****
--- 493,499 ----
  	}
      }
    do_cleanups (list_cleanup);
+   discard_cleanups (stream_cleanup);
    return MI_CMD_DONE;
  }
  
***************
*** 509,515 ****
    if (format == 'N')
      format = 0;
  
!   frame_register (deprecated_selected_frame, regnum, &optim, &lval, &addr,
  		  &realnum, buffer);
  
    if (optim)
--- 513,519 ----
    if (format == 'N')
      format = 0;
  
!   frame_register (get_selected_frame (NULL), regnum, &optim, &lval, &addr,
  		  &realnum, buffer);
  
    if (optim)

Re: [PATCH] -data-list-changed-registers (Take 2)

[Daniel Jacobowitz]

On Sat, Jun 11, 2005 at 06:51:19PM +1200, Nick Roberts wrote:
>  > *stopped,changed-registers=[],reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x080484d9",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbffff794"}],file="myprog.c",fullname="/home/nick/myprog.c",line="55"}

> Well it turms out that throw_exception calls do_cleanups, but clearly the
> above output isn't acceptable. With CLI everything is output immediately.
> With MI it's stored in memory to be output later. This can mixed with the
> output of the next MI command and break the syntax, as above.
> 
> This patch addresses this problem and is not as lightweight as my earlier
> patches. If its not the right fix then its pretty damn close.

It's not the right fix, but we're moving in a good direction.  I
noticed this problem when working on some other patch, but never had
time to dig...

Here's my objection: There is nothing special about
mi_cmd_data_list_changed_registers that means it should have to be
aware of this problem.  Can we put this cleanup somewhere that it will
apply to all MI functions?  Can we do it without assuming things about
the internals of ui_files?  They're opaque, you shouldn't be adding
something that knows one is a mem_stream here.

It looks like you're doing basically mi_out_rewind.  There's already
several of these in captured_mi_execute_command.  They don't catch this
because throw_exception takes us past them, all the way back to
mi_execute_command.

If we add an mi_out_rewind call right here:
1176              /* The command execution failed and error() was called
1177                 somewhere */

Then the problem goes away.

So, I've committed the attached patch instead.  Let me know if you see
any problems with it.  The manual is not explicit that these two
commands reflect the state of the selected frame, but by the explicit
parallel to "info reg" it's the right choice, so get_selected_frame is
OK here.  If you feel like updating the manual for this, that would be
appreciated...

Note that I needed to correct some test cases!  There's no longer any
MI output giving the stop reason if we stop while inside a called
function (since that's an error() case).  But that's OK; the output was
showing up with the next -stack-list-frames, where it has no
intelligible meaning, so this is an improvement.

Built and tested on i386-linux and committed.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-06-12  Daniel Jacobowitz  <dan@codesourcery.com>
	    Nick Roberts  <nickrob@snap.net.nz>

	* mi/mi-main.c (register_changed_p, get_register): Use
	get_selected_frame.
	(mi_execute_command): Call mi_out_rewind after an error.

2005-06-12  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.mi/mi-syn-frame.exp, gdb.mi/mi2-syn-frame.exp: Don't expect
	excess MI output after an error.

Index: mi/mi-main.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
retrieving revision 1.78
diff -u -p -r1.78 mi-main.c
--- mi/mi-main.c	22 May 2005 14:53:35 -0000	1.78
+++ mi/mi-main.c	13 Jun 2005 02:22:19 -0000
@@ -388,7 +388,7 @@ register_changed_p (int regnum)
 {
   gdb_byte raw_buffer[MAX_REGISTER_SIZE];
 
-  if (! frame_register_read (deprecated_selected_frame, regnum, raw_buffer))
+  if (! frame_register_read (get_selected_frame (NULL), regnum, raw_buffer))
     return -1;
 
   if (memcmp (&old_regs[DEPRECATED_REGISTER_BYTE (regnum)], raw_buffer,
@@ -509,7 +509,7 @@ get_register (int regnum, int format)
   if (format == 'N')
     format = 0;
 
-  frame_register (deprecated_selected_frame, regnum, &optim, &lval, &addr,
+  frame_register (get_selected_frame (NULL), regnum, &optim, &lval, &addr,
 		  &realnum, buffer);
 
   if (optim)
@@ -1174,11 +1174,12 @@ mi_execute_command (char *cmd, int from_
       if (result.reason < 0)
 	{
 	  /* The command execution failed and error() was called
-	     somewhere */
+	     somewhere.  */
 	  fputs_unfiltered (command->token, raw_stdout);
 	  fputs_unfiltered ("^error,msg=\"", raw_stdout);
 	  fputstr_unfiltered (result.message, '"', raw_stdout);
 	  fputs_unfiltered ("\"\n", raw_stdout);
+	  mi_out_rewind (uiout);
 	}
       mi_parse_free (command);
     }
Index: testsuite/gdb.mi/mi-syn-frame.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-syn-frame.exp,v
retrieving revision 1.3
diff -u -p -r1.3 mi-syn-frame.exp
--- testsuite/gdb.mi/mi-syn-frame.exp	18 May 2005 03:41:59 -0000	1.3
+++ testsuite/gdb.mi/mi-syn-frame.exp	13 Jun 2005 02:37:16 -0000
@@ -53,7 +53,7 @@ mi_gdb_test "400-break-insert foo" "400\
 
 mi_gdb_test "401-data-evaluate-expression foo()" "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(foo\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.*\"" "call inferior's function with a breakpoint set in it"
 
-mi_gdb_test "402-stack-list-frames" "402\\^done,reason=\"breakpoint-hit\",bkptno=\"2\",thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"foo\",args=\\\[\\\],file=\".*mi-syn-frame.c\",line=\"$decimal\"\},stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
+mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
 
 #
 # Continue back to main()
@@ -83,7 +83,7 @@ mi_gdb_test "406-data-evaluate-expressio
 # We should have both a signal handler and a call dummy frame
 # in this next output.
 
-mi_gdb_test "407-stack-list-frames" "407\\^done,reason=\"breakpoint-hit\",bkptno=\"3\",thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"subroutine\",args=\\\[\{name=\"in\",value=\"$decimal\"\}\\\],file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\},stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"handler\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"<signal handler called>\"\},.*frame=\{level=\"$decimal\",addr=\"$hex\",func=\"have_a_very_merry_interrupt\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]"
+mi_gdb_test "407-stack-list-frames" "407\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"handler\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"<signal handler called>\"\},.*frame=\{level=\"$decimal\",addr=\"$hex\",func=\"have_a_very_merry_interrupt\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]"
 
 
 send_gdb "408-exec-continue\n"
@@ -104,7 +104,7 @@ mi_gdb_test "409-stack-list-frames 0 0" 
 
 mi_gdb_test "410-data-evaluate-expression bar()" "hi in bar\[\r\n\]+\\&\"The program being debugged was signaled while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"GDB remains in the frame where the signal was received.\\\\n\"\[\r\n\]+\\&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\n\"\[\r\n\]+\\&\"Evaluation of the expression containing the function \\(bar\\) will be abandoned.\\\\n\"\[\r\n\]+410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" "call inferior function which raises exception"
 
-mi_gdb_test "411-stack-list-frames" "411\\^done,reason=\"signal-received\",signal-name=\".*\",signal-meaning=\".*\",thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"bar\",args=\\\[\\\],file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\},stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
+mi_gdb_test "411-stack-list-frames" "411\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
 
 mi_gdb_exit
 
Index: testsuite/gdb.mi/mi2-syn-frame.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-syn-frame.exp,v
retrieving revision 1.2
diff -u -p -r1.2 mi2-syn-frame.exp
--- testsuite/gdb.mi/mi2-syn-frame.exp	18 May 2005 03:41:59 -0000	1.2
+++ testsuite/gdb.mi/mi2-syn-frame.exp	13 Jun 2005 02:37:16 -0000
@@ -54,7 +54,7 @@ mi_gdb_test "400-break-insert foo" "400\
 
 mi_gdb_test "401-data-evaluate-expression foo()" "\\&\"The program being debugged stopped while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"When the function \\(foo\\) is done executing, GDB will silently\\\\n\"\[\r\n\]+\\&\"stop \\(instead of continuing to evaluate the expression containing\\\\n\"\[\r\n\]+\\&\"the function call\\).\\\\n\"\[\r\n\]+401\\^error,msg=\"The program being debugged stopped while in a function called from GDB.*\"" "call inferior's function with a breakpoint set in it"
 
-mi_gdb_test "402-stack-list-frames" "402\\^done,reason=\"breakpoint-hit\",bkptno=\"2\",thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"foo\",args=\\\[\\\],file=\".*mi-syn-frame.c\",line=\"$decimal\"\},stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
+mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"foo\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" "backtrace from inferior function stopped at bp, showing gdb dummy frame"
 
 #
 # Continue back to main()
@@ -84,7 +84,7 @@ mi_gdb_test "406-data-evaluate-expressio
 # We should have both a signal handler and a call dummy frame
 # in this next output.
 
-mi_gdb_test "407-stack-list-frames" "407\\^done,reason=\"breakpoint-hit\",bkptno=\"3\",thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"subroutine\",args=\\\[\{name=\"in\",value=\"$decimal\"\}\\\],file=\".*mi-syn-frame.c\",line=\"$decimal\"\},stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"handler\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"<signal handler called>\"\},.*frame=\{level=\"$decimal\",addr=\"$hex\",func=\"have_a_very_merry_interrupt\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]"
+mi_gdb_test "407-stack-list-frames" "407\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"handler\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"<signal handler called>\"\},.*frame=\{level=\"$decimal\",addr=\"$hex\",func=\"have_a_very_merry_interrupt\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]"
 
 
 send_gdb "408-exec-continue\n"
@@ -105,7 +105,7 @@ mi_gdb_test "409-stack-list-frames 0 0" 
 
 mi_gdb_test "410-data-evaluate-expression bar()" "hi in bar\[\r\n\]+\\&\"The program being debugged was signaled while in a function called from GDB.\\\\n\"\[\r\n\]+\\&\"GDB remains in the frame where the signal was received.\\\\n\"\[\r\n\]+\\&\"To change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\n\"\[\r\n\]+\\&\"Evaluation of the expression containing the function \\(bar\\) will be abandoned.\\\\n\"\[\r\n\]+410\\^error,msg=\"The program being debugged was signaled while in a function called from GDB.\\\\nGDB remains in the frame where the signal was received.\\\\nTo change this behavior use \\\\\"set unwindonsignal on\\\\\"\\\\nEvaluation of the expression containing the function \\(bar\\) will be abandoned.\"" "call inferior function which raises exception"
 
-mi_gdb_test "411-stack-list-frames" "411\\^done,reason=\"signal-received\",signal-name=\".*\",signal-meaning=\".*\",thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"bar\",args=\\\[\\\],file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\},stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
+mi_gdb_test "411-stack-list-frames" "411\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"bar\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"},frame=\{level=\"1\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"}.*\\\]" "backtrace from inferior function at exception"
 
 mi_gdb_exit
 

Re: [PATCH] -data-list-changed-registers (Take 2)

[Nick Roberts]

 > It looks like you're doing basically mi_out_rewind.  There's already
 > several of these in captured_mi_execute_command.  They don't catch this
 > because throw_exception takes us past them, all the way back to
 > mi_execute_command.
 > 
 > If we add an mi_out_rewind call right here:
 > 1176              /* The command execution failed and error() was called
 > 1177                 somewhere */
 > 
 > Then the problem goes away.

So, basically, it just needed one line fix! Hmm...I think I'll look at the
rest of the MI code before I start re-inventing it again.

From mi_cmd_data_list_register_values, I removed:

if (!target_has_registers)
  {
    mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: No registers.");
    return MI_CMD_ERROR;
  }

Why do you think it should stay?


Nick

Re: [PATCH] -data-list-changed-registers (Take 2)

[Daniel Jacobowitz]

On Mon, Jun 13, 2005 at 04:48:20PM +1200, Nick Roberts wrote:
>  > It looks like you're doing basically mi_out_rewind.  There's already
>  > several of these in captured_mi_execute_command.  They don't catch this
>  > because throw_exception takes us past them, all the way back to
>  > mi_execute_command.
>  > 
>  > If we add an mi_out_rewind call right here:
>  > 1176              /* The command execution failed and error() was called
>  > 1177                 somewhere */
>  > 
>  > Then the problem goes away.
> 
> So, basically, it just needed one line fix! Hmm...I think I'll look at the
> rest of the MI code before I start re-inventing it again.
> 
> From mi_cmd_data_list_register_values, I removed:
> 
> if (!target_has_registers)
>   {
>     mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: No registers.");
>     return MI_CMD_ERROR;
>   }
> 
> Why do you think it should stay?

Cuz it was late and I wasn't paying a great deal of attention to your
patch, once I figured out the mi_out_rewind problem.  In the interest
of uniformity, let's remove it.  Could you (test and) commit the
obvious patch?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -data-list-changed-registers (Take 2)

[Nick Roberts]

 > > From mi_cmd_data_list_register_values, I removed:
 > > 
 > > if (!target_has_registers)
 > >   {
 > >     mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: No registers.");
 > >     return MI_CMD_ERROR;
 > >   }
 > > 
 > > Why do you think it should stay?
 > 
 > Cuz it was late and I wasn't paying a great deal of attention to your
 > patch, once I figured out the mi_out_rewind problem.  In the interest
 > of uniformity, let's remove it.  Could you (test and) commit the
 > obvious patch?

Committed.

I get many failures on the testsuite, I'll try to move to a newer PC/kernel.
mi-regs.exp seems OK though.

I would like to commit my changes to -var-update (2005-05-02) sometime.

I have also removed a large number of annotations from my working copy.
Andrew started this process, it would be nice if he can be involved but
maybe thats not possible. There are a lot of changes to the following files:

gdb/ada-valprint.c
gdb/annotate.c
gdb/annotate.h
gdb/blockframe.c
gdb/breakpoint.c
gdb/cp-valprint.c
gdb/event-top.c
gdb/frame.c
gdb/infrun.c
gdb/interps.c
gdb/jv-valprint.c
gdb/p-valprint.c
gdb/printcmd.c
gdb/stack.c
gdb/utils.c
gdb/valprint.c

Is anyone interested?  It would be good to do this now as a release doesn't
seem to be imminent.

Nick


2005-06-14  Nick Roberts  <nickrob@snap.net.nz>

	* mi/mi-main.c (mi_cmd_data_list_register_values): Remove test for
	registers now that mi_execute_command rewinds after an error.


*** /home/nick/src/gdb/mi/mi-main.c.~1.79~	2005-06-14 10:37:18.000000000 +1200
--- /home/nick/src/gdb/mi/mi-main.c	2005-06-14 09:18:08.000000000 +1200
***************
*** 433,444 ****
  
    format = (int) argv[0][0];
  
-   if (!target_has_registers)
-     {
-       mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: No registers.");
-       return MI_CMD_ERROR;
-     }
- 
    list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "register-values");
  
    if (argc == 1)		/* No args, beside the format: do all the regs */
--- 433,438 ----

Re: [PATCH] -data-list-changed-registers (Take 2)

[Daniel Jacobowitz]

On Tue, Jun 14, 2005 at 10:38:13AM +1200, Nick Roberts wrote:
>  > > From mi_cmd_data_list_register_values, I removed:
>  > > 
>  > > if (!target_has_registers)
>  > >   {
>  > >     mi_error_message = xstrprintf ("mi_cmd_data_list_register_values: No registers.");
>  > >     return MI_CMD_ERROR;
>  > >   }
>  > > 
>  > > Why do you think it should stay?
>  > 
>  > Cuz it was late and I wasn't paying a great deal of attention to your
>  > patch, once I figured out the mi_out_rewind problem.  In the interest
>  > of uniformity, let's remove it.  Could you (test and) commit the
>  > obvious patch?
> 
> Committed.

Thank you.

> 
> I get many failures on the testsuite, I'll try to move to a newer PC/kernel.
> mi-regs.exp seems OK though.

What failures, in particular?

> I would like to commit my changes to -var-update (2005-05-02) sometime.

Sorry, I need to take another look at them.  I will try to to find the
time.

> I have also removed a large number of annotations from my working copy.
> Andrew started this process, it would be nice if he can be involved but
> maybe thats not possible. There are a lot of changes to the following files:
> 
> gdb/ada-valprint.c
> gdb/annotate.c
> gdb/annotate.h
> gdb/blockframe.c
> gdb/breakpoint.c
> gdb/cp-valprint.c
> gdb/event-top.c
> gdb/frame.c
> gdb/infrun.c
> gdb/interps.c
> gdb/jv-valprint.c
> gdb/p-valprint.c
> gdb/printcmd.c
> gdb/stack.c
> gdb/utils.c
> gdb/valprint.c
> 
> Is anyone interested?  It would be good to do this now as a release doesn't
> seem to be imminent.

I can try to take a look at it.  I do not know much about the existing
annotations or how people use them; that's why it takes me a long time
to review any patches in this area.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -data-list-changed-registers (Take 2)

[Nick Roberts]

 > > I get many failures on the testsuite, I'll try to move to a newer
 > > PC/kernel.  mi-regs.exp seems OK though.
 > 
 > What failures, in particular?

I don't think you'll want to know, I think a lot of them are kernel
related (2.4.19-16mdk).  I've attached the results below of a run with a
newer kernel (2.6.9.1-1.667 Fedora Core 3)

 > > I would like to commit my changes to -var-update (2005-05-02) sometime.
 > 
 > Sorry, I need to take another look at them.  I will try to to find the
 > time.

Thanks.

 > > I have also removed a large number of annotations from my working copy.
 > > Andrew started this process, it would be nice if he can be involved but
 > > maybe thats not possible. There are a lot of changes to the following
 > > files:
 ...
 > > 
 > > Is anyone interested?  It would be good to do this now as a release doesn't
 > > seem to be imminent.
 > 
 > I can try to take a look at it.  I do not know much about the existing
 > annotations or how people use them; that's why it takes me a long time
 > to review any patches in this area.

OK.  I'll check my changes a bit more carefully and then submit them.

I've made more changes to gdb-mi.el now (it use -data-list-changed-registers
now, for a start).  Since resources do seem to be stretched, and this is
separate code that can't break GDB, could I be given authority to check in
changes to this just this one file i.e. in MAINTAINERS:

Core: Generic components used by all of GDB
...
gdb-mi.el               nickrob@snap.net.nz 

I did suggest this once to Andrew, but it never got resolved.  It might need
an emergency general meeting and you might have to dust off a copy of the
Constitution, but in reality its no big deal.  I've been doing something like
this for Emacs for a couple of years now without problems.

Nick


FAIL: gdb.base/store.exp: var longest l; print old r, expecting -2
FAIL: gdb.base/store.exp: upvar longest l; print old r, expecting -2
FAIL: gdb.cp/annota3.exp: annotate-quit (pattern 1)
FAIL: gdb.cp/anon-union.exp: print w 1
FAIL: gdb.cp/anon-union.exp: print z 1
FAIL: gdb.cp/anon-union.exp: print w 2
FAIL: gdb.cp/anon-union.exp: print z 2
FAIL: gdb.cp/anon-union.exp: print w 3
FAIL: gdb.cp/anon-union.exp: print z 3
FAIL: gdb.java/jmisc.exp: p args
FAIL: gdb.java/jmisc1.exp: p args
FAIL: gdb.java/jprint.exp: unambiguous static call
FAIL: gdb.java/jprint.exp: single argument print call
FAIL: gdb.java/jprint.exp: double argument print call
FAIL: gdb.java/jprint.exp: virtual fn call
FAIL: gdb.java/jprint.exp: inherited static call
FAIL: gdb.java/jprint.exp: inherited virtual fn call
FAIL: gdb.mi/mi-basics.exp: environment-directory arg operation
FAIL: gdb.mi/mi-basics.exp: environment-directory empty-string operation
FAIL: gdb.mi/mi-basics.exp: environment-path dir1 dir2 operation
FAIL: gdb.mi/mi-file.exp: Getting a list of source files.
FAIL: gdb.mi/mi2-basics.exp: environment-directory arg operation
FAIL: gdb.mi/mi2-basics.exp: environment-directory empty-string operation
FAIL: gdb.mi/mi2-basics.exp: environment-path dir1 dir2 operation
FAIL: gdb.threads/thread_check.exp: breakpoint at tf
FAIL: gdb.threads/watchthreads.exp: threaded watch loop
FAIL: gdb.threads/watchthreads.exp: first watchpoint on args[1] hit
FAIL: gdb.threads/watchthreads.exp: watchpoint on args[1] hit in thread
FAIL: gdb.threads/watchthreads.exp: combination of threaded watchpoints = 30

		=== gdb Summary ===

# of expected passes		10777
# of unexpected failures	29
# of unexpected successes	1
# of expected failures		42
# of unknown successes		6
# of known failures		38
# of untested testcases		1
# of unsupported tests		9

Re: [PATCH] -data-list-changed-registers (Take 2)

[Daniel Jacobowitz]

On Tue, Jun 14, 2005 at 02:46:06PM +1200, Nick Roberts wrote:
>  > > I get many failures on the testsuite, I'll try to move to a newer
>  > > PC/kernel.  mi-regs.exp seems OK though.
>  > 
>  > What failures, in particular?
> 
> I don't think you'll want to know, I think a lot of them are kernel
> related (2.4.19-16mdk).  I've attached the results below of a run with a
> newer kernel (2.6.9.1-1.667 Fedora Core 3)

These results are sane.  FYI, if you use a full path to run configure,
the mi-basics failures will go away.

> I've made more changes to gdb-mi.el now (it use -data-list-changed-registers
> now, for a start).  Since resources do seem to be stretched, and this is
> separate code that can't break GDB, could I be given authority to check in
> changes to this just this one file i.e. in MAINTAINERS:
> 
> Core: Generic components used by all of GDB
> ...
> gdb-mi.el               nickrob@snap.net.nz 
> 
> I did suggest this once to Andrew, but it never got resolved.  It might need
> an emergency general meeting and you might have to dust off a copy of the
> Constitution, but in reality its no big deal.  I've been doing something like
> this for Emacs for a couple of years now without problems.

The only reason I haven't approved the gdb-mi.el you last posted was
because it depended on the MI patch which you just reminded me to look
at (which was an interface change, and I hadn't had a chance to
digest it yet).  I do not want you to commit anything which uses MI
interfaces that don't exist yet!  Normally, I'd be perfectly willing to
rubber-stamp it in.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -data-list-changed-registers (Take 2)

[Nick Roberts]

 > The only reason I haven't approved the gdb-mi.el you last posted was
 > because it depended on the MI patch which you just reminded me to look
 > at (which was an interface change, and I hadn't had a chance to
 > digest it yet).  I do not want you to commit anything which uses MI
 > interfaces that don't exist yet!  Normally, I'd be perfectly willing to
 > rubber-stamp it in.

I'm thinking more generally.  In Emacs, I commit about one change a week.  If
I need approval, then I will group changes together and do it less often, but
someone is always available to approve those changes then there is no problem.

Nick

Re: [PATCH] -data-list-changed-registers (Take 2)

[Daniel Jacobowitz]

On Fri, Jun 17, 2005 at 03:15:34PM +1200, Nick Roberts wrote:
>  > The only reason I haven't approved the gdb-mi.el you last posted was
>  > because it depended on the MI patch which you just reminded me to look
>  > at (which was an interface change, and I hadn't had a chance to
>  > digest it yet).  I do not want you to commit anything which uses MI
>  > interfaces that don't exist yet!  Normally, I'd be perfectly willing to
>  > rubber-stamp it in.
> 
> I'm thinking more generally.  In Emacs, I commit about one change a week.  If
> I need approval, then I will group changes together and do it less often, but
> someone is always available to approve those changes then there is no problem.

Let's hold this thought for a little bit.  It may make sense for you to
commit these without approval in the future, once we've found our
stride again.

-- 
Daniel Jacobowitz
CodeSourcery, LLC