[v3,1/2] gdbserver: set ptrace flags after creating inferiors
Commit Message
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
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
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.
@@ -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,
@@ -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,
@@ -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,
@@ -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)
@@ -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,
@@ -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) \
@@ -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,
new file mode 100644
@@ -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"