[patch] Linux MAY_FOLLOW_EXEC #2 [Re: RFC: Fix crash on i386 (%gs-)threaded programs using execve(2)]

[Jan Kratochvil <jan.kratochvil@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]

Hi Daniel,

On Mon, 24 Jul 2006 21:03:32 +0200, Daniel Jacobowitz wrote:
...
> The reason I find it so disorienting is this:
> 
> % gdb file1
> 
> (gdb) run
> [starts file1]
> [file1 execs file2]
> [file2 exits]
> 
> (gdb) run
> [file2 starts instead of file1!]
> 
> I don't know if it should do that or not.  I tend to use "run" a lot
> and want to get back to the beginning of my debug session.

Included restoration patch using make_run_cleanup(), session log below.

That waitpid(3) WNOHANG smells there for me but it does not hurt the testsuite.
Should I investigate it more?

2006-07-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* inf-ptrace.c (inf_ptrace_mourn_inferior): waitpid(3) with WNOHANG
	to avoid lockup on the introduced TARGET_WAITKIND_EXECD handling.
	* infrun.c (follow_exec): Unconditionally enabled by MAY_FOLLOW_EXEC.
	Provide restoration of exec_bfd and symfile_objfile for any new "run".
	* linux-thread-db.c (thread_db_wait): Handle TARGET_WAITKIND_EXECD.
	* linux-thread-db.c (thread_db_mourn_inferior): Turn off threading.
	* foll-exec.exp: Uncoditionally enabled for all platforms.
	Relaxed regex to apply besides HP-UX also for GNU/Linux backtrace.


Thanks,
Jan

------------------------------------------------------------------------------

(gdb) file execve
Reading symbols from /tmp/execve...done.
Using host libthread_db library "/lib/libthread_db.so.1".
(gdb) catch exec
Catchpoint 1 (exec)
(gdb) set args gdb-entered parameters
(gdb) run
Starting program: /tmp/execve gdb-entered parameters
Executing new program: /tmp/hello
[Switching to process 16524]
Catchpoint 1 (exec'd /tmp/hello), 0x44031840 in ?? ()
(gdb) show args
Argument list to give program being debugged when it is started is "gdb-entered parameters".
(gdb) info files
Symbols from "/tmp/hello".
Unix child process:
	Using the running image of child process 16524.
	While running this, GDB does not access memory from...
Local exec file:
	`/tmp/hello', file type elf32-i386.
[...]
(gdb) c
Continuing.
[New process 16524]
hello world

Program exited normally.
(gdb) info files
Symbols from "/tmp/hello".
Local exec file:
	`/tmp/hello', file type elf32-i386.
[...]
(gdb) run
Starting program: /tmp/execve gdb-entered parameters
Executing new program: /tmp/hello
[Switching to process 16827]

Catchpoint 1 (exec'd /tmp/hello), 0x44031840 in ?? ()
(gdb)

[-- Attachment #2: gdb-6.5-follow-exec.patch --]
[-- Type: text/plain, Size: 6552 bytes --]

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=182116


diff -rup gdb-6.5-orig/gdb/inf-ptrace.c gdb-6.5-follow_exec/gdb/inf-ptrace.c
--- gdb-6.5-orig/gdb/inf-ptrace.c	2006-01-24 23:34:34.000000000 +0100
+++ gdb-6.5-follow_exec/gdb/inf-ptrace.c	2006-07-29 11:26:32.000000000 +0200
@@ -166,8 +166,10 @@ inf_ptrace_mourn_inferior (void)
   /* Wait just one more time to collect the inferior's exit status.
      Do not check whether this succeeds though, since we may be
      dealing with a process that we attached to.  Such a process will
-     only report its exit status to its original parent.  */
-  waitpid (ptid_get_pid (inferior_ptid), &status, 0);
+     only report its exit status to its original parent.
+     WNOHANG is required as on follow_exec() (due to TARGET_WAITKIND_EXECD)
+     resulted from waitpid(2) and it called us by target_mourn_inferior().  */
+  waitpid (ptid_get_pid (inferior_ptid), &status, WNOHANG);
 
   unpush_target (ptrace_ops_hack);
   generic_mourn_inferior ();
diff -rup gdb-6.5-orig/gdb/infrun.c gdb-6.5-follow_exec/gdb/infrun.c
--- gdb-6.5-orig/gdb/infrun.c	2006-07-28 23:30:22.000000000 +0200
+++ gdb-6.5-follow_exec/gdb/infrun.c	2006-07-29 19:06:06.000000000 +0200
@@ -47,6 +47,7 @@
 #include "language.h"
 #include "solib.h"
 #include "main.h"
+#include "objfiles.h"
 
 #include "gdb_assert.h"
 #include "mi/mi-common.h"
@@ -109,10 +110,10 @@ int sync_execution = 0;
 static ptid_t previous_inferior_ptid;
 
 /* This is true for configurations that may follow through execl() and
-   similar functions.  At present this is only true for HP-UX native.  */
+   similar functions.  */
 
 #ifndef MAY_FOLLOW_EXEC
-#define MAY_FOLLOW_EXEC (0)
+#define MAY_FOLLOW_EXEC (1)
 #endif
 
 static int may_follow_exec = MAY_FOLLOW_EXEC;
@@ -375,6 +376,37 @@ follow_inferior_reset_breakpoints (void)
   insert_breakpoints ();
 }
 
+static void
+follow_exec_restore_execfile (void *filename_new_untyped)
+{
+  char *filename_new = filename_new_untyped;
+
+  /* filename_new == NULL is not expected.  */
+  if (filename_new == NULL && exec_bfd != NULL)
+    exec_file_clear (0);
+  /* exec_bfd == NULL is not expected.  */
+  if (filename_new != NULL && 
+      (exec_bfd == NULL || strcmp (get_exec_file (0), filename_new)))
+    exec_file_attach (filename_new, 0);
+
+  free (filename_new);
+}
+
+static void
+follow_exec_restore_symfile (void *filename_new_untyped)
+{
+  char *filename_new = filename_new_untyped;
+
+  /* symfile_objfile == NULL is not expected.  */
+  if (filename_new == NULL && symfile_objfile != NULL)
+    symbol_file_clear (0);
+  if (filename_new != NULL && (symfile_objfile == NULL
+      || strcmp (symfile_objfile->name, filename_new)))
+    symbol_file_add_main (filename_new, 0);
+
+  free (filename_new);
+}
+
 /* EXECD_PATHNAME is assumed to be non-NULL. */
 
 static void
@@ -382,6 +414,7 @@ follow_exec (int pid, char *execd_pathna
 {
   int saved_pid = pid;
   struct target_ops *tgt;
+  struct objfile *objfile, *objfile_temp;
 
   if (!may_follow_exec)
     return;
@@ -427,6 +460,30 @@ follow_exec (int pid, char *execd_pathna
     error (_("Could find run target to save before following exec"));
 
   gdb_flush (gdb_stdout);
+
+  /* During the common "run" bare command we should run again the original
+   * program spawning us.  Stacking ordering is correct this way.  */
+  make_run_cleanup (follow_exec_restore_execfile,
+                    (!exec_bfd ? NULL : xstrdup (exec_bfd->filename)));
+  make_run_cleanup (follow_exec_restore_symfile, (!symfile_objfile ? NULL :
+                    xstrdup (symfile_objfile->name)));
+
+  /* As symbol_file_add_main()->thread_db_new_objfile()->check_for_thread_db()
+   * would fine already loaded libpthread.so while the threading structures
+   * would not be yet initialized for this early inferior.
+   * Call before target_mourn_inferior() as it will breakpoint_re_set().  */
+#ifdef CLEAR_SOLIB
+  CLEAR_SOLIB ();
+#else
+  clear_solib ();
+#endif
+  /* Do not: symbol_file_clear()->clear_symtab_users()->breakpoint_re_set().  */
+  ALL_OBJFILES_SAFE (objfile, objfile_temp)
+  {
+    free_objfile (objfile);
+  }
+  symfile_objfile = NULL;
+
   target_mourn_inferior ();
   inferior_ptid = pid_to_ptid (saved_pid);
   /* Because mourn_inferior resets inferior_ptid. */
diff -rup gdb-6.5-orig/gdb/linux-thread-db.c gdb-6.5-follow_exec/gdb/linux-thread-db.c
--- gdb-6.5-orig/gdb/linux-thread-db.c	2006-07-28 23:30:22.000000000 +0200
+++ gdb-6.5-follow_exec/gdb/linux-thread-db.c	2006-07-29 11:26:32.000000000 +0200
@@ -939,6 +939,10 @@ thread_db_wait (ptid_t ptid, struct targ
       || ourstatus->kind == TARGET_WAITKIND_SIGNALLED)
     return pid_to_ptid (-1);
 
+  /* Threading structures got reset.  Return as nonthreaded.  */
+  if (ourstatus->kind == TARGET_WAITKIND_EXECD)
+    return pid_to_ptid (GET_PID (ptid));
+
   if (ourstatus->kind == TARGET_WAITKIND_STOPPED
       && (ourstatus->value.sig == TARGET_SIGNAL_TRAP
           || ourstatus->value.sig == TARGET_SIGNAL_ILL))
@@ -1106,6 +1110,9 @@ thread_db_mourn_inferior (void)
      the inferior, so that we don't try to uninsert them.  */
   remove_thread_event_breakpoints ();
 
+  /* Destroy thread info; it's no longer valid.  */
+  init_thread_list ();
+
   /* Detach thread_db target ops.  */
   unpush_target (&thread_db_ops);
   using_thread_db = 0;
diff -rup gdb-6.5-orig/gdb/testsuite/gdb.base/foll-exec.exp gdb-6.5-follow_exec/gdb/testsuite/gdb.base/foll-exec.exp
--- gdb-6.5-orig/gdb/testsuite/gdb.base/foll-exec.exp	2001-03-06 09:21:50.000000000 +0100
+++ gdb-6.5-follow_exec/gdb/testsuite/gdb.base/foll-exec.exp	2006-07-29 11:26:32.000000000 +0200
@@ -45,12 +45,6 @@ if  { [gdb_compile "${srcdir}/${subdir}/
 }
 
 
-# Until "catch exec" is implemented on other targets...
-#
-if ![istarget "hppa*-hp-hpux*"] then {
-    continue
-}
-
 proc zap_session {} {
    global gdb_prompt
    global binfile
@@ -212,7 +206,9 @@ proc do_exec_tests {} {
    setup_xfail hppa2.0w-hp-hpux* CLLbs16760
    send_gdb "continue\n"
    gdb_expect {
-     -re ".*Executing new program:.*${testfile2}.*Catchpoint .*(exec\'d .*${testfile2}).*in .START..*$gdb_prompt $"\
+     # It is OS dependent and no symbols may be found, GNU/Linux has "_start"
+     # while HP-UX has " in .START..*$gdb_prompt" etc.
+     -re ".*Executing new program:.*${testfile2}.*Catchpoint .*(exec\'d .*${testfile2}).*in .*$gdb_prompt $"\
                      {pass "hit catch exec"}
      -re "$gdb_prompt $" {fail "hit catch exec"}
      timeout         {fail "(timeout) hit catch exec"}