TUI + gdbserver broken?

[Pedro Alves <pedro_alves at portugalmail dot pt>]

Hi all,

TUI mode when debugging a remote target got broken after the recent
selected frame changes.  After connecting to the target (target remote ...)
Issuing a continue results in:

(gdb) c
Continuing.
Remote communication error: Software caused connection abort.

And a snippet of a set debug remote session:

(...)
Packet vCont (verbose-resume) is supported
Sending packet: $vCont;c#a8...Ack
Packet received: W00
Sending packet: $g#67...Remote communication error: Software caused connection
abort.
(gdb) c
Continuing.

gdb is sending a g packet after a W.  Something in gdb didn't
realize that the the target already exited.

The problem is that we now call deprecated_safe_get_selected_frame
in tui_selected_frame_level_changed_hook... :

static void
tui_selected_frame_level_changed_hook (int level)
{
    struct frame_info *fi;

    fi = deprecated_safe_get_selected_frame ();
    /* Ensure that symbols for this frame are read in.  Also, determine the
       source language of this frame, and switch to it if desired.  */
    if (fi)
      {
        struct symtab *s;

        s = find_pc_symtab (get_frame_pc (fi));

(...)

... and the deprecated_safe_get_selected_frame is not realizing the
target is not running anymore, so calls get_selected_frame, which
ensures that there is always a frame selected.  Then in
tui_selected_frame_level_changed_hook, the get_frame_pc is called,
which ends up trying to talk to the remote target, but at this point
the stub is not reachable anymore, hence the Remote
communication error.

The way deprecated_safe_get_selected_frame checks to see if there
is a selectable frame is:

struct frame_info *
deprecated_safe_get_selected_frame (void)
{
    if (!target_has_registers || !target_has_stack || !target_has_memory)
      return NULL;
    return get_selected_frame (NULL);
}

When using a remote target the following test is not true on when the 'W'
packet arrives:
    if (!target_has_registers || !target_has_stack || !target_has_memory)

The attached patch fixes it by calling target_mark_running, and
target_mark_exited in remote.c.  These functions set those
target_has_* to 0 and 1 appropriately.  They are currently only
used on remote-sim.c.  Let me know if there is a better way to
know if the target is running.

tui_registers_changed_hook then has the problem that is is calling
get_selected_frame, when target_has_registers is false.  Fixed by
using deprecated_safe_get_selected_frame here too.

Note that I still see a TUI slow repainting problem introduced by
the selected frame changes.  I haven't investigated that yet.

Cheers,
Pedro Alves



gdb/ChangeLog

	* remote.c (remote_open_1): Call target_mark_running.
	(remote_wait): Call target_mark_exited.
	(remote_async_wait): Likewise.
	(remote_mourn_1): Likewise.
	(extended_remote_create_inferior): Call target_mark_running.
	(extended_remote_async_create_inferior): Likewise.
	* tui/tui-hooks.c (tui_registers_changed_hook): Get selected
	frame using deprecated_safe_get_selected_frame.

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2007-03-18 21:43:06.000000000 +0000
+++ src/gdb/remote.c	2007-03-18 22:18:46.000000000 +0000
@@ -2583,6 +2583,8 @@ remote_open_1 (char *name, int from_tty,
 
   if (exec_bfd) 	/* No use without an exec file.  */
     remote_check_symbols (symfile_objfile);
+
+  target_mark_running (target);
 }
 
 /* This takes a program previously attached to and detaches it.  After
@@ -3240,6 +3242,7 @@ Packet: '%s'\n"),
 	case 'W':		/* Target exited.  */
 	  {
 	    /* The remote process exited.  */
+	    target_mark_exited (&remote_ops);
 	    status->kind = TARGET_WAITKIND_EXITED;
 	    status->value.integer = (fromhex (buf[1]) << 4) + fromhex (buf[2]);
 	    goto got_status;
@@ -3436,6 +3439,7 @@ Packet: '%s'\n"),
 	case 'W':		/* Target exited.  */
 	  {
 	    /* The remote process exited.  */
+	    target_mark_exited (&async_remote_ops);
 	    status->kind = TARGET_WAITKIND_EXITED;
 	    status->value.integer = (fromhex (buf[1]) << 4) + fromhex (buf[2]);
 	    goto got_status;
@@ -5036,6 +5040,7 @@ extended_remote_mourn (void)
 static void
 remote_mourn_1 (struct target_ops *target)
 {
+  target_mark_exited (target);
   unpush_target (target);
   generic_mourn_inferior ();
 }
@@ -5066,6 +5071,8 @@ extended_remote_create_inferior (char *e
      will have a legacy FPA coprocessor emulated and others may have
      access to a hardware VFP unit.  */
 
+  target_mark_running (&extended_remote_ops);
+
   /* Now put the breakpoints back in.  This way we're safe if the
      restart function works via a unix fork on the remote side.  */
   insert_breakpoints ();
@@ -5091,6 +5098,8 @@ extended_remote_async_create_inferior (c
   /* Now restart the remote server.  */
   extended_remote_restart ();
 
+  target_mark_running (&extended_async_remote_ops);
+
   /* NOTE: We don't need to recheck for a target description here; but
      if we gain the ability to switch the remote executable we may
      need to, if for instance we are running a process which requested
Index: src/gdb/tui/tui-hooks.c
===================================================================
--- src.orig/gdb/tui/tui-hooks.c	2007-03-18 21:42:30.000000000 +0000
+++ src/gdb/tui/tui-hooks.c	2007-03-18 21:43:36.000000000 +0000
@@ -132,7 +132,7 @@ tui_registers_changed_hook (void)
 {
   struct frame_info *fi;
 
-  fi = get_selected_frame (NULL);
+  fi = deprecated_safe_get_selected_frame ();
   if (tui_refreshing_registers == 0)
     {
       tui_refreshing_registers = 1;