[RFA] infcmd.c: Fix UI problem in attach_command

[RFA] infcmd.c: Fix UI problem in attach_command

[Corinna Vinschen]

Hi,

this is the infcmd.c fix which is necessary to run the to_pid_to_exec_file
functionality in win32-nat.c smoothly.  But I'm not sure if this patch is
the right way to solve the situation.  Sigh, so short a patch, so longish
to explain.

What happens is this:  Usually, when the user attaches to a running
process, the executable (determined by the to_pid_to_exec_file function)
is opened and the symbols are loaded.  Since the executable is the first
file opened, it's symbols are loaded first.

On Cygwin the situation is a bit different.  Attaching to a process
results in a bunch of debug messages from the Windows kernel.  One of
these messages is a LOAD_DLL_DEBUG_EVENT per DLL which is in use by the
attached process.  Each of these messages results in loading the symbol
table of the DLL into GDB.  This happens *before* the call to
to_pid_to_exec_file and so naturally also *before* the symbols from
the executable itself are loaded into GDB.

The result of this change in load order is, that GDB has already the
partial symbol table filled with symbols when it's asked to load the
executable's symbol table.  This in turn changes the UI behaviour.
Instead of just printing "Reading symbols from...", the function
symbol_file_add_with_addrs_or_offsets in symfile.c asks the user
"Load new symbol table from... (y or n)" using the query function.

That wouldn't be an actual problem if there wouldn't be this interesting
behaviour in the attach_command function.  Before trying to load the
executable's symbol table, attach_command calls target_terminal_inferior().
The effect is, that when the above query function is called, GDB is
not the foreground process.  When running under a shell with job control,
something funny happens.  After printing the "Load new..." message, fgetc
is called and at that point, the parent shell (here tcsh) takes over and
prints a message "[1]  +  3784 Suspended (tty input)         gdb"
Entering "fg" works, but it's not exactly helpful, right?

However, now I'm finally at the point where my patch kicks in.  When
attach_command loads the symbol table of the executable, my patch
temporarily returns the terminal to GDB.  After that, it returns terminal
ownership to the inferior process again.

I'm not sure if that's the right way to fix it for two reasons.  The
first is, it might be the same, just to move the original call to
target_terminal_inferior behinf the "if (!exec_file)" expression, this
way avoiding to bounce terminal ownership between GDB and the inferior.
The second reason is, if it might be better to avoid asking the user for
loading the symbol table at all, even on Cygwin.  In that case, I'm not
sure how to do it.

Anyway, below is my patch.


Corinna

	* infcmd.c (attach_command): Return terminal control to GDB
	before loading executable's file symbols.

Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.117
diff -u -p -r1.117 infcmd.c
--- infcmd.c	20 Jun 2004 18:10:14 -0000	1.117
+++ infcmd.c	26 Jun 2004 08:18:41 -0000
@@ -1829,8 +1829,10 @@ attach_command (char *args, int from_tty
 	  if (!source_full_path_of (exec_file, &full_exec_path))
 	    full_exec_path = savestring (exec_file, strlen (exec_file));
 
+	  target_terminal_ours ();
 	  exec_file_attach (full_exec_path, from_tty);
 	  symbol_file_add_main (full_exec_path, from_tty);
+	  target_terminal_inferior ();
 	}
     }
   else


-- 
Corinna Vinschen
Cygwin Co-Project Leader
Red Hat, Inc.

Re: [RFA] infcmd.c: Fix UI problem in attach_command

[Eli Zaretskii]

> Date: Sat, 26 Jun 2004 14:11:21 +0200
> From: Corinna Vinschen <vinschen@redhat.com>
> 
> Before trying to load the executable's symbol table, attach_command
> calls target_terminal_inferior().

Do you (or anyone else, like Elena) know why do we relinquish the
terminal to the inferior while loading the symbol table?  It sounds
like a strange thing to do at this point.

Re: [RFA] infcmd.c: Fix UI problem in attach_command

[Corinna Vinschen]

On Jun 27 20:11, Eli Zaretskii wrote:
> > Date: Sat, 26 Jun 2004 14:11:21 +0200
> > From: Corinna Vinschen <vinschen@redhat.com>
> > 
> > Before trying to load the executable's symbol table, attach_command
> > calls target_terminal_inferior().
> 
> Do you (or anyone else, like Elena) know why do we relinquish the
> terminal to the inferior while loading the symbol table?  It sounds
> like a strange thing to do at this point.

I don't know and it sounds strange to me as well.  I've tested a simlified
patch which just moves the call to target_terminal_inferior right before
the normal_stop call.  It works as good as my original patch, but I'm not
sure if there's a specific situation which requires an early switch to
the inferior.

Corinna


	* infcmd.c (attach_command): Move call to target_terminal_inferior
	behind loading symbol table.

Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.117
diff -u -p -r1.117 infcmd.c
--- infcmd.c	20 Jun 2004 18:10:14 -0000	1.117
+++ infcmd.c	28 Jun 2004 11:11:15 -0000
@@ -1788,9 +1788,6 @@ attach_command (char *args, int from_tty
      based on what modes we are starting it with.  */
   target_terminal_init ();
 
-  /* Install inferior's terminal modes.  */
-  target_terminal_inferior ();
-
   /* Set up execution context to know that we should return from
      wait_for_inferior as soon as the target reports a stop.  */
   init_wait_for_inferior ();
@@ -1849,6 +1846,9 @@ attach_command (char *args, int from_tty
    */
   target_post_attach (PIDGET (inferior_ptid));
 
+  /* Install inferior's terminal modes.  */
+  target_terminal_inferior ();
+
   normal_stop ();
 
   if (deprecated_attach_hook)


-- 
Corinna Vinschen
Cygwin Co-Project Leader
Red Hat, Inc.

Re: [RFA] infcmd.c: Fix UI problem in attach_command

[Eli Zaretskii]

> Date: Mon, 28 Jun 2004 13:14:18 +0200
> From: Corinna Vinschen <vinschen@redhat.com>
> > 
> > Do you (or anyone else, like Elena) know why do we relinquish the
> > terminal to the inferior while loading the symbol table?  It sounds
> > like a strange thing to do at this point.
> 
> I don't know and it sounds strange to me as well.  I've tested a simlified
> patch which just moves the call to target_terminal_inferior right before
> the normal_stop call.  It works as good as my original patch, but I'm not
> sure if there's a specific situation which requires an early switch to
> the inferior.

I tend to suggest that we commit this simplified patch and see if
anybody screams.

Hmm... I'm not sure whether there's a designated maintainer for
infcmd.c.  Andrew?

Re: [RFA] infcmd.c: Fix UI problem in attach_command

[Daniel Jacobowitz]

On Mon, Jun 28, 2004 at 07:58:57PM +0200, Eli Zaretskii wrote:
> > Date: Mon, 28 Jun 2004 13:14:18 +0200
> > From: Corinna Vinschen <vinschen@redhat.com>
> > > 
> > > Do you (or anyone else, like Elena) know why do we relinquish the
> > > terminal to the inferior while loading the symbol table?  It sounds
> > > like a strange thing to do at this point.
> > 
> > I don't know and it sounds strange to me as well.  I've tested a simlified
> > patch which just moves the call to target_terminal_inferior right before
> > the normal_stop call.  It works as good as my original patch, but I'm not
> > sure if there's a specific situation which requires an early switch to
> > the inferior.
> 
> I tend to suggest that we commit this simplified patch and see if
> anybody screams.

This seems reasonable to me; if the patch tested OK on one platform
with job control I don't think there are major terminal-handling
gotchas it might trigger.

-- 
Daniel Jacobowitz

Re: [RFA] infcmd.c: Fix UI problem in attach_command

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:

> On Mon, Jun 28, 2004 at 07:58:57PM +0200, Eli Zaretskii wrote:
> > > Date: Mon, 28 Jun 2004 13:14:18 +0200
> > > From: Corinna Vinschen <vinschen@redhat.com>
> > > > 
> > > > Do you (or anyone else, like Elena) know why do we relinquish the
> > > > terminal to the inferior while loading the symbol table?  It sounds
> > > > like a strange thing to do at this point.
> > > 
> > > I don't know and it sounds strange to me as well.  I've tested a simlified
> > > patch which just moves the call to target_terminal_inferior right before
> > > the normal_stop call.  It works as good as my original patch, but I'm not
> > > sure if there's a specific situation which requires an early switch to
> > > the inferior.
> > 
> > I tend to suggest that we commit this simplified patch and see if
> > anybody screams.
> 
> This seems reasonable to me; if the patch tested OK on one platform
> with job control I don't think there are major terminal-handling
> gotchas it might trigger.

I don't know of any reason the inferior could possibly need to own the
terminal while it's not running.  If GDB needs it, it might as well
own it.

Re: [RFA] infcmd.c: Fix UI problem in attach_command

[Corinna Vinschen]

On Jun 28 17:13, Jim Blandy wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> 
> > On Mon, Jun 28, 2004 at 07:58:57PM +0200, Eli Zaretskii wrote:
> > > > Date: Mon, 28 Jun 2004 13:14:18 +0200
> > > > From: Corinna Vinschen <vinschen@redhat.com>
> > > > > 
> > > > > Do you (or anyone else, like Elena) know why do we relinquish the
> > > > > terminal to the inferior while loading the symbol table?  It sounds
> > > > > like a strange thing to do at this point.
> > > > 
> > > > I don't know and it sounds strange to me as well.  I've tested a simlified
> > > > patch which just moves the call to target_terminal_inferior right before
> > > > the normal_stop call.  It works as good as my original patch, but I'm not
> > > > sure if there's a specific situation which requires an early switch to
> > > > the inferior.
> > > 
> > > I tend to suggest that we commit this simplified patch and see if
> > > anybody screams.
> > 
> > This seems reasonable to me; if the patch tested OK on one platform
> > with job control I don't think there are major terminal-handling
> > gotchas it might trigger.
> 
> I don't know of any reason the inferior could possibly need to own the
> terminal while it's not running.  If GDB needs it, it might as well
> own it.

Thanks, folks.

I've just applied the simplified variation of my patch.  I hope that's ok.


Corinna

-- 
Corinna Vinschen
Cygwin Co-Project Leader
Red Hat, Inc.