[v3,1/2] gdbserver: set ptrace flags after creating inferiors

Message ID 1449196006-13759-1-git-send-email-jistone@redhat.com
State New, archived
Headers

Commit Message

Josh Stone Dec. 4, 2015, 2:26 a.m. UTC
  Rename target_ops.arch_setup to .post_create_inferior.  In the Linux
hook, continue calling the low arch setup, then also set ptrace flags.
This corrects the possibility of running without flags, demonstrated by
a new test that would fail to catch a fork before.

gdb/gdbserver/ChangeLog:

2015-12-03  Josh Stone  <jistone@redhat.com>

	* target.h (struct target_ops) <arch_setup>: Rename to ...
	(struct target_ops) <post_create_inferior>: ... this.
	(target_arch_setup): Rename to ...
	(target_post_create_inferior): ... this, calling post_create_inferior.
	* server.c (start_inferior): Update target_arch_setup calls to
	target_post_create_inferior.
	* linux-low.c (linux_low_ptrace_options): Forward declare.
	(linux_arch_setup): Update its comment for general use.
	(linux_post_create_inferior): New, run arch_setup and setup ptrace.
	(struct linux_target_ops): Use linux_post_create_inferior.
	* lynx-low.c (struct lynx_target_ops): Update arch_setup stub comment
	to post_create_inferior.
	* nto-low.c (struct nto_target_ops): Likewise.
	* spu-low.c (struct spu_target_ops): Likewise.
	* win32-low.c (struct win32_target_ops): Likewise.

gdb/testsuite/ChangeLog:

2015-12-03  Josh Stone  <jistone@redhat.com>

	* gdb.base/catch-static-fork.exp: New.
---
 gdb/gdbserver/linux-low.c                    | 24 +++++++++++++--
 gdb/gdbserver/lynx-low.c                     |  2 +-
 gdb/gdbserver/nto-low.c                      |  2 +-
 gdb/gdbserver/server.c                       |  4 +--
 gdb/gdbserver/spu-low.c                      |  2 +-
 gdb/gdbserver/target.h                       | 15 ++++-----
 gdb/gdbserver/win32-low.c                    |  2 +-
 gdb/testsuite/gdb.base/catch-static-fork.exp | 46 ++++++++++++++++++++++++++++
 8 files changed, 82 insertions(+), 15 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/catch-static-fork.exp
  

Comments

Pedro Alves Dec. 4, 2015, 12:16 p.m. UTC | #1
Hi Josh,

Thanks for the update, and thanks for the new test.

On 12/04/2015 02:26 AM, Josh Stone wrote:

> 2015-12-03  Josh Stone  <jistone@redhat.com>
> 
> 	* gdb.base/catch-static-fork.exp: New.

Nit, to me, "catch-fork-static.exp" would be more natural.
As it reads as something about either "catch static" or
about a "static fork".

> +gdb_test "run" "Catchpoint \[0-9\]* \\(forked process \[0-9\]*\\),.*" \
> +    "run to fork"
> 

Please avoid hardcoding the "run" command:

gdb_run_cmd
gdb_test "" \
    "Catchpoint \[0-9\]* \\(forked process \[0-9\]*\\),.*" \
    "run to fork"

Even though "catch fork" currently doesn't work with plain
"target remote", which doesn't do "run", there are patches pending
to make it work.  With this change, this test should then work
with --target_board=native-gdbserver too.

OK with that change.

Thanks,
Pedro Alves
  
Josh Stone Dec. 5, 2015, 2:14 a.m. UTC | #2
On 12/04/2015 04:16 AM, Pedro Alves wrote:
> Hi Josh,
> 
> Thanks for the update, and thanks for the new test.
> 
> On 12/04/2015 02:26 AM, Josh Stone wrote:
> 
>> 2015-12-03  Josh Stone  <jistone@redhat.com>
>>
>> 	* gdb.base/catch-static-fork.exp: New.
> 
> Nit, to me, "catch-fork-static.exp" would be more natural.
> As it reads as something about either "catch static" or
> about a "static fork".

Sure, I'll rename it.

>> +gdb_test "run" "Catchpoint \[0-9\]* \\(forked process \[0-9\]*\\),.*" \
>> +    "run to fork"
>>
> 
> Please avoid hardcoding the "run" command:
> 
> gdb_run_cmd
> gdb_test "" \
>     "Catchpoint \[0-9\]* \\(forked process \[0-9\]*\\),.*" \
>     "run to fork"
> 
> Even though "catch fork" currently doesn't work with plain
> "target remote", which doesn't do "run", there are patches pending
> to make it work.  With this change, this test should then work
> with --target_board=native-gdbserver too.

Great, I'll make this change too.

> OK with that change.

Thanks, I'll push it out with those tweaks.
  

Patch

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index ec5337ae418b..5e2dc5857a8d 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -264,6 +264,7 @@  static int finish_step_over (struct lwp_info *lwp);
 static int kill_lwp (unsigned long lwpid, int signo);
 static void enqueue_pending_signal (struct lwp_info *lwp, int signal, siginfo_t *info);
 static void complete_ongoing_step_over (void);
+static int linux_low_ptrace_options (int attached);
 
 /* When the event-loop is doing a step-over, this points at the thread
    being stepped.  */
@@ -421,7 +422,7 @@  linux_add_process (int pid, int attached)
 
 static CORE_ADDR get_pc (struct lwp_info *lwp);
 
-/* Implement the arch_setup target_ops method.  */
+/* Call the target arch_setup function on the current thread.  */
 
 static void
 linux_arch_setup (void)
@@ -919,6 +920,25 @@  linux_create_inferior (char *program, char **allargs)
   return pid;
 }
 
+/* Implement the post_create_inferior target_ops method.  */
+
+static void
+linux_post_create_inferior (void)
+{
+  struct lwp_info *lwp = get_thread_lwp (current_thread);
+
+  linux_arch_setup ();
+
+  if (lwp->must_set_ptrace_flags)
+    {
+      struct process_info *proc = current_process ();
+      int options = linux_low_ptrace_options (proc->attached);
+
+      linux_enable_event_reporting (lwpid_of (current_thread), options);
+      lwp->must_set_ptrace_flags = 0;
+    }
+}
+
 /* Attach to an inferior process.  Returns 0 on success, ERRNO on
    error.  */
 
@@ -7077,7 +7097,7 @@  linux_breakpoint_kind_from_current_state (CORE_ADDR *pcptr)
 
 static struct target_ops linux_target_ops = {
   linux_create_inferior,
-  linux_arch_setup,
+  linux_post_create_inferior,
   linux_attach,
   linux_kill,
   linux_detach,
diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c
index 36ab0a3b2d8e..2940ce516ad6 100644
--- a/gdb/gdbserver/lynx-low.c
+++ b/gdb/gdbserver/lynx-low.c
@@ -722,7 +722,7 @@  lynx_request_interrupt (void)
 
 static struct target_ops lynx_target_ops = {
   lynx_create_inferior,
-  NULL,  /* arch_setup */
+  NULL,  /* post_create_inferior */
   lynx_attach,
   lynx_kill,
   lynx_detach,
diff --git a/gdb/gdbserver/nto-low.c b/gdb/gdbserver/nto-low.c
index d72c46515d13..ee043b120830 100644
--- a/gdb/gdbserver/nto-low.c
+++ b/gdb/gdbserver/nto-low.c
@@ -933,7 +933,7 @@  nto_sw_breakpoint_from_kind (int kind, int *size)
 
 static struct target_ops nto_target_ops = {
   nto_create_inferior,
-  NULL,  /* arch_setup */
+  NULL,  /* post_create_inferior */
   nto_attach,
   nto_kill,
   nto_detach,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 5e053b39b10b..6d151ee35d39 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -282,7 +282,7 @@  start_inferior (char **argv)
 	    }
 	  while (last_status.value.sig != GDB_SIGNAL_TRAP);
 	}
-      target_arch_setup ();
+      target_post_create_inferior ();
       return signal_pid;
     }
 
@@ -290,7 +290,7 @@  start_inferior (char **argv)
      (assuming success).  */
   last_ptid = mywait (pid_to_ptid (signal_pid), &last_status, 0, 0);
 
-  target_arch_setup ();
+  target_post_create_inferior ();
 
   if (last_status.kind != TARGET_WAITKIND_EXITED
       && last_status.kind != TARGET_WAITKIND_SIGNALLED)
diff --git a/gdb/gdbserver/spu-low.c b/gdb/gdbserver/spu-low.c
index 89bed7a02dd1..64a440cf7ea2 100644
--- a/gdb/gdbserver/spu-low.c
+++ b/gdb/gdbserver/spu-low.c
@@ -653,7 +653,7 @@  spu_sw_breakpoint_from_kind (int kind, int *size)
 
 static struct target_ops spu_target_ops = {
   spu_create_inferior,
-  NULL,  /* arch_setup */
+  NULL,  /* post_create_inferior */
   spu_attach,
   spu_kill,
   spu_detach,
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 9cd07bc2d945..1a38b2052c2c 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -75,8 +75,9 @@  struct target_ops
 
   int (*create_inferior) (char *program, char **args);
 
-  /* Architecture-specific setup.  */
-  void (*arch_setup) (void);
+  /* Do additional setup after a new process is created, including
+     exec-wrapper completion.  */
+  void (*post_create_inferior) (void);
 
   /* Attach to a running process.
 
@@ -475,11 +476,11 @@  void set_target_ops (struct target_ops *);
 #define create_inferior(program, args) \
   (*the_target->create_inferior) (program, args)
 
-#define target_arch_setup()			 \
-  do						 \
-    {						 \
-      if (the_target->arch_setup != NULL)	 \
-	(*the_target->arch_setup) ();		 \
+#define target_post_create_inferior()			 \
+  do							 \
+    {							 \
+      if (the_target->post_create_inferior != NULL)	 \
+	(*the_target->post_create_inferior) ();		 \
     } while (0)
 
 #define myattach(pid) \
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index b1de139cac70..ba20aea35a71 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -1794,7 +1794,7 @@  win32_sw_breakpoint_from_kind (int kind, int *size)
 
 static struct target_ops win32_target_ops = {
   win32_create_inferior,
-  NULL,  /* arch_setup */
+  NULL,  /* post_create_inferior */
   win32_attach,
   win32_kill,
   win32_detach,
diff --git a/gdb/testsuite/gdb.base/catch-static-fork.exp b/gdb/testsuite/gdb.base/catch-static-fork.exp
new file mode 100644
index 000000000000..6961346b0204
--- /dev/null
+++ b/gdb/testsuite/gdb.base/catch-static-fork.exp
@@ -0,0 +1,46 @@ 
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that "catch fork" works on static executables.
+
+# For instance, on Linux we need PTRACE_O_TRACEFORK set before the program
+# reaches the fork.  GDBserver was only setting flags when it reached the first
+# stop *after* arch_setup.  In a dynamic executable there is often a swbreak in
+# ld.so probes before reaching main, and ptrace flags were set then.  But a
+# static executable would just keep running and never catch the fork.
+
+if { [is_remote target] || ![isnative] } then {
+    continue
+}
+
+# Until "catch fork" is implemented on other targets...
+#
+if { ![istarget "hppa*-hp-hpux*"] && ![istarget "*-*-linux*"]
+     && ![istarget "*-*-openbsd*"] } then {
+    continue
+}
+
+# Reusing foll-fork.c since it's a simple forking program.
+standard_testfile foll-fork.c
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile \
+      {additional_flags=-static}] } {
+    return -1
+}
+
+gdb_test "catch fork" "Catchpoint \[0-9\]* \\(fork\\)"
+
+gdb_test "run" "Catchpoint \[0-9\]* \\(forked process \[0-9\]*\\),.*" \
+    "run to fork"