[patch, rfc] Re: target_find_description question

Ulrich Weigand uweigand@de.ibm.com
Fri Sep 5 00:00:00 GMT 2008


Pedro Alves wrote.
> On Thursday 04 September 2008 23:12:10, Pedro Alves wrote:
> > On Thursday 04 September 2008 22:51:32, Daniel Jacobowitz wrote:
> > > I don't think that will work, e.g. solib-irix.c and various other
> > > solib modules run expecting to hit a breakpoint.
> >
> > Isn't that a breakpoint in the real inferior, not the shell, and
> > expected *after* stop_soon is reset (after create_inferior), in
> > post_create_inferior/solib_create_inferior_hook?
> 
> I meant to suggest un-overload STOP_QUIETLY, and split it in two,
> at the "and" below:
> 
>  "STOP_QUIETLY is used when running in the shell before the child
>  program has been exec'd and when running through shared
>  library loading."


At this point it seems a mistake to try to re-use wait_for_inferior for the
startup sequence in the first place.  Most of what this cares about cannot
ever happen during startup, and some of the remaining actions are actively
harmful (accessing registers / memory).

I've tried to strip down the wait_for_inferior and resume calls done by
startup_inferior to the minimum that is actually needed for startup, and
the result looks pretty straigtforward, and boils down to mostly using
target_wait and target_resume directly.

In addition, this has some further benefits:
- We can now properly handle cases where the inferior dies while in the
  shell (the FIXME in the old startup_inferior code)
- The treat_exec_as_sigtrap argument to handle_inferior_event is no longer
  used and could be removed (not part of this patch)

As startup_inferior no longer calls init_wait_for_inferior, I've moved this
to run_command_1 (which used to call init_wait_for_inferior indirectly via
kill_if_already_running, but not if the program was already killed).  This
means that kill_if_already_running no longer needs to call it (nor does it
need to call no_shared_libraries, which is now always called from
target_pre_inferior.)

Tested on amd64-linux with no regressions.

What do you think?

Bye,
Ulrich


ChangeLog:

	* fork-child.c (startup_inferior): Use target_wait and target_resume
	directly instead of calling wait_for_inferior / resume.

	* infcmd.c (kill_if_already_running): Do not call no_shared_libraries
	or init_wait_for_inferior.
	(run_command_1): Call init_wait_for_inferior.


diff -urNp gdb-orig/gdb/fork-child.c gdb-head/gdb/fork-child.c
--- gdb-orig/gdb/fork-child.c	2008-05-25 18:19:22.000000000 +0200
+++ gdb-head/gdb/fork-child.c	2008-09-05 01:35:03.000000000 +0200
@@ -422,21 +422,67 @@ startup_inferior (int ntraps)
   if (exec_wrapper)
     pending_execs++;
 
-  clear_proceed_status ();
-
-  init_wait_for_inferior ();
-
   while (1)
     {
-      /* Make wait_for_inferior be quiet. */
-      stop_soon = STOP_QUIETLY;
-      wait_for_inferior (1);
-      if (stop_signal != TARGET_SIGNAL_TRAP)
+      int resume_signal = TARGET_SIGNAL_0;
+      ptid_t resume_ptid;
+
+      struct target_waitstatus ws;
+      memset (&ws, 0, sizeof (ws));
+      resume_ptid = target_wait (pid_to_ptid (-1), &ws);
+
+      /* Mark all threads non-executing.  */
+      set_executing (pid_to_ptid (-1), 0);
+
+      /* In all-stop mode, resume all threads.  */
+      if (!non_stop)
+	resume_ptid = pid_to_ptid (-1);
+
+      switch (ws.kind)
+	{
+	  case TARGET_WAITKIND_IGNORE:
+	  case TARGET_WAITKIND_SPURIOUS:
+	  case TARGET_WAITKIND_LOADED:
+	  case TARGET_WAITKIND_FORKED:
+	  case TARGET_WAITKIND_VFORKED:
+	  case TARGET_WAITKIND_SYSCALL_ENTRY:
+	  case TARGET_WAITKIND_SYSCALL_RETURN:
+	    /* Ignore gracefully during startup of the inferior.  */
+	    break;
+
+	  case TARGET_WAITKIND_SIGNALLED:
+	    target_terminal_ours ();
+	    target_mourn_inferior ();
+	    error (_("During startup program terminated with signal %s, %s."),
+		   target_signal_to_name (ws.value.sig),
+		   target_signal_to_string (ws.value.sig));
+	    return;
+
+	  case TARGET_WAITKIND_EXITED:
+	    target_terminal_ours ();
+	    target_mourn_inferior ();
+	    if (ws.value.integer)
+	      error (_("During startup program exited with code %d."),
+		     ws.value.integer);
+	    else
+	      error (_("During startup program exited normally."));
+	    return;
+
+	  case TARGET_WAITKIND_EXECD:
+	    /* Handle EXEC signals as if they were SIGTRAP signals.  */
+	    xfree (ws.value.execd_pathname);
+	    resume_signal = TARGET_SIGNAL_TRAP;
+	    break;
+
+	  case TARGET_WAITKIND_STOPPED:
+	    resume_signal = ws.value.sig;
+	    break;
+	}
+
+      if (resume_signal != TARGET_SIGNAL_TRAP)
 	{
-	  /* Let shell child handle its own signals in its own way.
-	     FIXME: what if child has exited?  Must exit loop
-	     somehow.  */
-	  resume (0, stop_signal);
+	  /* Let shell child handle its own signals in its own way.  */
+	  target_resume (resume_ptid, 0, resume_signal);
 	}
       else
 	{
@@ -461,10 +507,10 @@ startup_inferior (int ntraps)
 	  if (--pending_execs == 0)
 	    break;
 
-	  resume (0, TARGET_SIGNAL_0);	/* Just make it go on.  */
+	  /* Just make it go on.  */
+	  target_resume (resume_ptid, 0, TARGET_SIGNAL_0);
 	}
     }
-  stop_soon = NO_STOP_QUIETLY;
 }
 
 /* Implement the "unset exec-wrapper" command.  */
diff -urNp gdb-orig/gdb/infcmd.c gdb-head/gdb/infcmd.c
--- gdb-orig/gdb/infcmd.c	2008-08-22 03:25:28.000000000 +0200
+++ gdb-head/gdb/infcmd.c	2008-09-05 01:02:02.000000000 +0200
@@ -464,8 +464,6 @@ kill_if_already_running (int from_tty)
 Start it from the beginning? "))
 	error (_("Program not restarted."));
       target_kill ();
-      no_shared_libraries (NULL, from_tty);
-      init_wait_for_inferior ();
     }
 }
 
@@ -481,6 +479,8 @@ run_command_1 (char *args, int from_tty,
   dont_repeat ();
 
   kill_if_already_running (from_tty);
+
+  init_wait_for_inferior ();
   clear_breakpoint_hit_counts ();
 
   /* Clean up any leftovers from other runs.  Some other things from


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com



More information about the Gdb-patches mailing list