* [PATCH 4/4 v2][REPOST] Non-stop exec tests
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
@ 2014-04-30 19:18 ` Don Breazeal
2014-04-30 19:18 ` [PATCH 1/4][REPOST] Remote Linux ptrace exit events Don Breazeal
` (8 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Don Breazeal @ 2014-04-30 19:18 UTC (permalink / raw)
To: gdb-patches; +Cc: Don Breazeal
From: Don Breazeal <don_breazeal@mentor.com>
[Reposting to eliminate unexpected attachment type.]
This patch extends the non-ldr-exc-*.exp tests so that they run all their
cases in non-stop mode as well as all-stop mode. These tests cover
handling of exec events when non-leader threads call exec.
Fixed problems in the previous version of this patch to get all the tests
working correctly for native gdb, contrary to what I stated in the patch 0
email of this series. The tests also report a test error when 'runto_main'
fails. In non-stop mode with 'target extended-remote', runto_main always
fails with something like:
(gdb) run ^M
Starting program: /scratch/dbreazea/gdb-nat2/mine/gdb/testsuite/gdb.threads/non-ldr-exc-4
Unexpected vCont reply in non-stop mode: T0506:10e0ffffff7f0000;07:c8deffffff7f0000;10:c1a6abaaaa2a0000;thread:p5ee.5ee;core:0;
This happens in other tests as well (e.g. gdb.threads/thread_events.exp),
so I copied the error handling from that test so that the non-stop tests
report 'untested' for target extended-remote. I couldn't find anything
related to this in the gdb bug database, but I assume it is a known problem
since the other tests handle it.
Thanks
--Don
2014-04-03 Don Breazeal <donb@codesourcery.com>
* gdb/testsuite/gdb.threads/non-ldr-exc-1.exp: Add non-stop cases,
call 'untested' if 'runto_main' fails.
* gdb/testsuite/gdb.threads/non-ldr-exc-2.exp: Ditto.
* gdb/testsuite/gdb.threads/non-ldr-exc-3.exp: Ditto.
* gdb/testsuite/gdb.threads/non-ldr-exc-4.exp: Ditto.
---
gdb/testsuite/gdb.threads/non-ldr-exc-1.exp | 20 ++++++++++++---
gdb/testsuite/gdb.threads/non-ldr-exc-2.exp | 36 +++++++++++++++++++++++---
gdb/testsuite/gdb.threads/non-ldr-exc-3.exp | 36 +++++++++++++++++++++++---
gdb/testsuite/gdb.threads/non-ldr-exc-4.exp | 20 ++++++++++++---
4 files changed, 94 insertions(+), 18 deletions(-)
diff --git a/gdb/testsuite/gdb.threads/non-ldr-exc-1.exp b/gdb/testsuite/gdb.threads/non-ldr-exc-1.exp
index 8123a99..e35236a 100644
--- a/gdb/testsuite/gdb.threads/non-ldr-exc-1.exp
+++ b/gdb/testsuite/gdb.threads/non-ldr-exc-1.exp
@@ -28,13 +28,19 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
return -1
}
-proc do_test { lock_sched } {
- with_test_prefix "lock-sched$lock_sched" {
+proc do_test { lock_sched mode } {
+ with_test_prefix "lock-sched$lock_sched,$mode" {
global executable
clean_restart ${executable}
+ if { $mode == "non-stop" } {
+ gdb_test_no_output "set target-async on" "set async mode"
+ gdb_test_no_output "set non-stop on" "set non-stop mode"
+ }
+
if ![runto_main] {
+ untested "could not run to main"
return -1
}
@@ -48,11 +54,17 @@ proc do_test { lock_sched } {
gdb_test_no_output "set scheduler-locking on"
}
+ if { $mode == "non-stop" } {
+ gdb_test "thread 2" "Switching.*"
+ }
+
gdb_test "continue" \
".*is executing new program.*Breakpoint 1, main.* at .*" \
"continue over exec"
}
}
-do_test 0
-do_test 1
+do_test 0 "all-stop"
+do_test 1 "all-stop"
+do_test 0 "non-stop"
+do_test 1 "non-stop"
diff --git a/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp b/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp
index 857e7bc..a0281d6 100644
--- a/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp
+++ b/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp
@@ -29,18 +29,42 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
return -1
}
-proc do_test { lock_sched } {
- with_test_prefix "lock-sched$lock_sched" {
+# Test for breakpoint event among async thread events.
+# gdb_continue_to_breakpoint requires "$gdb_prompt $", but
+# here we may get a thread event message instead of EOL.
+proc gdb_continue_to_breakpoint_async { name pattern } {
+ global gdb_prompt
+
+ gdb_test_multiple "continue" $name {
+ -re ".*Breakpoint .* (at|in) ($pattern)$gdb_prompt " {
+ pass $name
+ }
+ }
+}
+
+proc do_test { lock_sched mode } {
+ with_test_prefix "lock-sched$lock_sched,$mode" {
global executable
clean_restart ${executable}
+ if { $mode == "non-stop" } {
+ gdb_test_no_output "set target-async on" "set async mode"
+ gdb_test_no_output "set non-stop on" "set non-stop mode"
+ }
+
if ![runto_main] {
+ untested "could not run to main"
return -1
}
gdb_breakpoint [gdb_get_line_number "break-here"]
- gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+ if { $mode == "non-stop" } {
+ gdb_continue_to_breakpoint_async "break-here" ".* break-here .*"
+ gdb_test "thread 2" "Switching.*"
+ } else {
+ gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+ }
gdb_test "info threads" \
"\r\n\[ \t\]*Id\[ \t\]+Target\[ \t\]+Id\[ \t\]+Frame\[ \t\]*\r\n\\* 2 *Thread \[^\r\n\]* at \[^\r\n\]*" \
@@ -59,5 +83,7 @@ proc do_test { lock_sched } {
}
}
-do_test 0
-do_test 1
+do_test 0 "all-stop"
+do_test 1 "all-stop"
+do_test 0 "non-stop"
+do_test 1 "non-stop"
diff --git a/gdb/testsuite/gdb.threads/non-ldr-exc-3.exp b/gdb/testsuite/gdb.threads/non-ldr-exc-3.exp
index 7f33d39..69c27d8 100644
--- a/gdb/testsuite/gdb.threads/non-ldr-exc-3.exp
+++ b/gdb/testsuite/gdb.threads/non-ldr-exc-3.exp
@@ -31,18 +31,42 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
return -1
}
-proc do_test { lock_sched } {
- with_test_prefix "lock-sched$lock_sched" {
+# Test for breakpoint event among async thread events.
+# gdb_continue_to_breakpoint requires "$gdb_prompt $", but
+# here we may get a thread event message instead of EOL.
+proc gdb_continue_to_breakpoint_async { name pattern } {
+ global gdb_prompt
+
+ gdb_test_multiple "continue" $name {
+ -re ".*Breakpoint .* (at|in) ($pattern)$gdb_prompt " {
+ pass $name
+ }
+ }
+}
+
+proc do_test { lock_sched mode } {
+ with_test_prefix "lock-sched$lock_sched,$mode" {
global executable
clean_restart ${executable}
+ if { $mode == "non-stop" } {
+ gdb_test_no_output "set target-async on" "set async mode"
+ gdb_test_no_output "set non-stop on" "set non-stop mode"
+ }
+
if ![runto_main] {
+ untested "could not run to main"
return -1
}
gdb_breakpoint [gdb_get_line_number "break-here"]
- gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+ if { $mode == "non-stop" } {
+ gdb_continue_to_breakpoint_async "break-here" ".* break-here .*"
+ gdb_test "thread 2" "Switching.*"
+ } else {
+ gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+ }
# Also test with sched-lock to make sure we can follow the
# non-leader thread execing even though the main thread wasn't
@@ -57,5 +81,7 @@ proc do_test { lock_sched } {
}
}
-do_test 0
-do_test 1
+do_test 0 "all-stop"
+do_test 1 "all-stop"
+do_test 0 "non-stop"
+do_test 1 "non-stop"
diff --git a/gdb/testsuite/gdb.threads/non-ldr-exc-4.exp b/gdb/testsuite/gdb.threads/non-ldr-exc-4.exp
index a5e88bb..f83577d 100644
--- a/gdb/testsuite/gdb.threads/non-ldr-exc-4.exp
+++ b/gdb/testsuite/gdb.threads/non-ldr-exc-4.exp
@@ -30,13 +30,19 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
return -1
}
-proc do_test { lock_sched } {
- with_test_prefix "lock-sched$lock_sched" {
+proc do_test { lock_sched mode } {
+ with_test_prefix "lock-sched$lock_sched,$mode" {
global executable
clean_restart ${executable}
+ if { $mode == "non-stop" } {
+ gdb_test_no_output "set target-async on" "set async mode"
+ gdb_test_no_output "set non-stop on" "set non-stop mode"
+ }
+
if ![runto_main] {
+ untested "could not run to main"
return -1
}
@@ -50,11 +56,17 @@ proc do_test { lock_sched } {
gdb_test_no_output "set scheduler-locking on"
}
+ if { $mode == "non-stop" } {
+ gdb_test "thread 2" "Switching.*"
+ }
+
gdb_test "continue" \
".*is executing new program.*Breakpoint 1, main.* at .*" \
"continue over exec"
}
}
-do_test 0
-do_test 1
+do_test 0 "all-stop"
+do_test 1 "all-stop"
+do_test 0 "non-stop"
+do_test 1 "non-stop"
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/4][REPOST] Remote Linux ptrace exec events
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
2014-04-30 19:18 ` [PATCH 4/4 v2][REPOST] Non-stop exec tests Don Breazeal
2014-04-30 19:18 ` [PATCH 1/4][REPOST] Remote Linux ptrace exit events Don Breazeal
@ 2014-04-30 19:18 ` Don Breazeal
2014-05-07 20:01 ` Luis Machado
` (2 more replies)
2014-04-30 19:18 ` [PATCH 3/4][REPOST] Document RSP support for Linux " Don Breazeal
` (6 subsequent siblings)
9 siblings, 3 replies; 33+ messages in thread
From: Don Breazeal @ 2014-04-30 19:18 UTC (permalink / raw)
To: gdb-patches; +Cc: Don Breazeal
From: Don Breazeal <don_breazeal@mentor.com>
[Reposting to eliminate unexpected attachment type.]
This patch implements support for exec events in gdbserver on linux, in
multiprocess mode (target extended-remote). Follow-exec-mode and rerun
behave as expected. Catchpoints for exec are not yet implemented since it
will be easier to implement catchpoints for fork, vfork, and exec all at
the same time.
TESTING
---------
The patch was tested on GNU/Linux x86_64 with --target_board set to
native-gdbserver and native-extended-gdbserver, as well as testing native
GDB. The test results for native-gdbserver were unchanged. Thirteen tests
that used to fail for native-extended-gdbserver on Linux pass with this
patch, and the non-ldr-exc-*.exp tests all pass in non-stop mode and
extended-remote. There are several failures in the new non-ldr-exc-*.exp
tests in non-stop mode with native GDB.
One caveat: when an exec is detected, gdbserver emits a couple of warnings:
gdbserver: unexpected r_debug version 0
gdbserver: unexpected r_debug version 0
However, debugging of shared libraries that are loaded by the exec'd
program works just fine. These messages may be caused by gdbserver making
an attempt to initialize the solib hook before the r_debug structure has
been initialized. I intend to follow up in a subsequent patch.
IMPLEMENTATION
----------------
Support for exec events in single-threaded programs was a fairly
straightforward replication of the implementation in native GDB:
1) Enable exec events via ptrace options.
2) Add support for handling the exec events to the handle_extended_wait
and linux_wait_for_event_filtered. Detect the exec event, then
find and save the pathname of the executable file being exec'd.
3) Implement an additional "stop reason", "exec", in the RSP stop reply
packet "T".
Existing GDB code takes care of handling the exec event on the host side
without modification.
Support for exec events in multi-threaded programs required some additional
work that required a couple of significant changes to existing code. In a
nutshell, the changes are to:
4) Use the PTRACE_EVENT_EXIT extended event to handle thread exit,
while not exposing any change in exit handling to the user. The
rationale for this is discussed in the "patch 0" email of this
series.
5) Recognize when the exec'ing thread has vanished (become the thread
group leader) in send_sigstop. Native GDB does this differently.
Regarding items 4 & 5: Recall that when a non-leader thread exec's, all the
other threads are terminated and the exec'ing thread changes its thread id
to that of the old leader (the process id) as part of the exec. There is
no event reported for the "exit" of the exec'ing thread; it appears to have
vanished. The original thread group leader can't be reaped until all the
other threads have been reaped, and some way of determining that it has
exited is required in order to update the lwp list (#4 above). Also, some
mechanism for deleting the lwp entry corresponding to the exec'ing thread
is needed (#5 above).
The rationale for #4 is that in my testing I ran into a race condition in
the mechanism that's intended to detect when a thread group leader has
exited, check_zombie_leaders. The race occurred when there were only two
threads in the program. In this case the leader thread passes through a
very brief zombie state before being replaced by the exec'ing thread as
the thread group leader. This state transition is asynchronous, with no
dependency on anything gdbserver does. Using PTRACE_EVENT_EXIT ensures
that the leader exit will be detected. I can provide a more detailed
explanation or the race, but I didn't want to be too long-winded here.
Regarding item #5, determining that the exec'ing thread has "vanished", In
native GDB this is done by calling waitpid(PID), and if it returns ECHILD
it means that the thread is gone. We don't want to use waitpid(PID) in
gdbserver, based on the discussion in
https://www.sourceware.org/ml/gdb-patches/2014-02/msg00828.html. An
alternative is to send a signal to each thread and look for an ESRCH (No
such process) error. In all-stop mode this can be done in the normal
course of events, since when gdbserver reports an exec event it stops all
the other threads with a SIGSTOP. In non-stop mode, when an exec event has
been detected, we can call stop_all_lwps/unstop_all_lwps to accomplish the
same thing.
gdb/
2014-04-02 Don Breazeal <donb@codesourcery.com>
* common/linux-ptrace.c (linux_test_for_tracefork)
[GDBSERVER]: Add exec tracing to ptrace options if OS supports
it.
* remote.c (remote_parse_stop_reply): Support new RSP stop
reply reason 'exec'.
gdbserver/
2014-04-02 Don Breazeal <donb@codesourcery.com>
* gdb/gdbserver/linux-low.c (linux_child_pid_to_exec_file): New
function.
(handle_extended_wait): Add support for PTRACE_EVENT_EXEC.
(check_zombie_leaders): Update comment.
(linux_low_filter_event): Add support for PTRACE_EVENT_EXEC.
(linux_wait_for_event_filtered): Update comment.
(extended_event_reported): New function.
(send_sigstop): Delete lwp on 'No such process' error and
reset current_inferior.
* gdb/gdbserver/linux-low.h (struct lwp_info): New member
'waitstatus'.
* gdb/gdbserver/remote-utils.c (prepare_resume_reply): Support
new RSP stop reply reason 'exec'.
---
gdb/common/linux-ptrace.c | 7 +-
gdb/gdbserver/linux-low.c | 175 ++++++++++++++++++++++++++++++++++++++----
gdb/gdbserver/linux-low.h | 5 +
gdb/gdbserver/remote-utils.c | 28 +++++++-
gdb/remote.c | 27 ++++++-
5 files changed, 222 insertions(+), 20 deletions(-)
diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
index e3fc705..b137df9 100644
--- a/gdb/common/linux-ptrace.c
+++ b/gdb/common/linux-ptrace.c
@@ -491,8 +491,11 @@ linux_test_for_tracefork (int child_pid)
if (ret == child_pid && WIFSTOPPED (status)
&& status >> 16 == PTRACE_EVENT_EXIT)
{
- /* PTRACE_O_TRACEEXIT is supported. */
- current_ptrace_options |= PTRACE_O_TRACEEXIT;
+ /* PTRACE_O_TRACEEXIT is supported. We use exit events to
+ implement support for exec events. Since fork events are
+ supported we know exec events are supported, so we enable
+ exec events here. */
+ current_ptrace_options |= PTRACE_O_TRACEEXIT | PTRACE_O_TRACEEXEC;
}
#endif
}
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 90e7b15..5f94490 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -281,6 +281,32 @@ static int linux_event_pipe[2] = { -1, -1 };
static void send_sigstop (struct lwp_info *lwp);
static void wait_for_sigstop (void);
+/* Accepts an integer PID. Returns a string containing the
+ name of the executable file for the child process.
+ Space for the result is malloc'd, caller must free. */
+
+static char *
+linux_child_pid_to_exec_file (int pid)
+{
+ char *name1, *name2;
+
+ name1 = xmalloc (PATH_MAX);
+ name2 = xmalloc (PATH_MAX);
+ memset (name2, 0, PATH_MAX);
+
+ sprintf (name1, "/proc/%d/exe", pid);
+ if (readlink (name1, name2, PATH_MAX) > 0)
+ {
+ free (name1);
+ return name2;
+ }
+ else
+ {
+ free (name2);
+ return name1;
+ }
+}
+
/* Return non-zero if HEADER is a 64-bit ELF file. */
static int
@@ -514,6 +540,19 @@ handle_extended_wait (struct lwp_info *event_child, int *wstatp)
(PTRACE_TYPE_ARG4) 0);
return ret;
}
+ else if (event == PTRACE_EVENT_EXEC)
+ {
+ if (debug_threads)
+ debug_printf ("LHEW: Got exec event from LWP %ld\n",
+ lwpid_of (event_thr));
+
+ event_child->waitstatus.kind = TARGET_WAITKIND_EXECD;
+ event_child->waitstatus.value.execd_pathname
+ = linux_child_pid_to_exec_file (lwpid_of (event_thr));
+
+ /* Report the event. */
+ return 0;
+ }
internal_error (__FILE__, __LINE__,
_("unknown ptrace event %d"), event);
}
@@ -1376,18 +1415,19 @@ check_zombie_leaders (void)
program). In the latter case, we can't waitpid the
leader's exit status until all other threads are gone.
- - There are 3 or more threads in the group, and a thread
+ - There are multiple threads in the group, and a thread
other than the leader exec'd. On an exec, the Linux
kernel destroys all other threads (except the execing
one) in the thread group, and resets the execing thread's
tid to the tgid. No exit notification is sent for the
execing thread -- from the ptracer's perspective, it
appears as though the execing thread just vanishes.
- Until we reap all other threads except the leader and the
- execing thread, the leader will be zombie, and the
- execing thread will be in `D (disc sleep)'. As soon as
- all other threads are reaped, the execing thread changes
- it's tid to the tgid, and the previous (zombie) leader
+ Until we reap all other threads (if any) except the
+ leader and the execing thread, the leader will be zombie,
+ and the execing thread will be in `D (disc sleep)'. As
+ soon as all other threads are reaped, or have reported
+ PTRACE_EVENT_EXIT events, the execing thread changes its
+ tid to the tgid, and the previous (zombie) leader
vanishes, giving place to the "new" leader. We could try
distinguishing the exit and exec cases, by waiting once
more, and seeing if something comes out, but it doesn't
@@ -1395,7 +1435,11 @@ check_zombie_leaders (void)
we'll re-add the new one once we see the exec event
(which is just the same as what would happen if the
previous leader did exit voluntarily before some other
- thread execs). */
+ thread execs).
+
+ Note that when PTRACE_EVENT_EXEC is supported, we use
+ that mechanism to detect thread exit, including the
+ exit of zombie leaders. */
if (debug_threads)
fprintf (stderr,
@@ -1791,6 +1835,57 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int *wstatp)
child = find_lwp_pid (pid_to_ptid (lwpid));
+ /* Check for stop events reported by a process we didn't already
+ know about - anything not already in our LWP list.
+
+ If we're expecting to receive stopped processes after
+ fork, vfork, and clone events, then we'll just add the
+ new one to our list and go back to waiting for the event
+ to be reported - the stopped process might be returned
+ from waitpid before or after the event is.
+
+ But note the case of a non-leader thread exec'ing after the
+ leader having exited, and gone from our lists. On an exec,
+ the Linux kernel destroys all other threads (except the execing
+ one) in the thread group, and resets the execing thread's tid
+ to the tgid. No exit notification is sent for the execing
+ thread -- from the ptracer's perspective, it appears as though
+ the execing thread just vanishes. When they are available, we
+ use exit events (PTRACE_EVENT_EXIT) to detect thread exit
+ reliably. As soon as all other threads (if any) are reaped or
+ have reported their PTRACE_EVENT_EXIT events, the execing
+ thread changes it's tid to the tgid, and the previous (zombie)
+ leader vanishes, giving place to the "new" leader. The lwp
+ entry for the previous leader is deleted when we handle its
+ exit event, and we re-add the new one here. */
+
+ if (WIFSTOPPED (wstat) && child == NULL
+ && (WSTOPSIG (wstat) == SIGTRAP && wstat >> 16 == PTRACE_EVENT_EXEC))
+ {
+ ptid_t child_ptid;
+
+ /* A multi-thread exec after we had seen the leader exiting. */
+ if (debug_threads)
+ debug_printf ("LLW: Re-adding thread group leader LWP %d.\n",
+ lwpid);
+
+ child_ptid = ptid_build (lwpid, lwpid, 0);
+ child = add_lwp (child_ptid);
+ child->stopped = 1;
+ current_inferior = child->thread;
+
+ if (non_stop && stopping_threads == NOT_STOPPING_THREADS)
+ {
+ /* Make sure we delete the lwp entry for the exec'ing thread,
+ which will have vanished. We do this by sending a signal
+ to all the other threads in the lwp list, deleting any
+ that are not found. Note that in all-stop mode this will
+ happen before reporting the event. */
+ stop_all_lwps (0, child);
+ unstop_all_lwps (0, child);
+ }
+ }
+
/* If we didn't find a process, one of two things presumably happened:
- A process we started and then detached from has exited. Ignore it.
- A process we are controlling has forked and the new child's stop
@@ -2122,8 +2217,7 @@ linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid,
- When a non-leader thread execs, that thread just vanishes
without reporting an exit (so we'd hang if we waited for it
explicitly in that case). The exec event is reported to
- the TGID pid (although we don't currently enable exec
- events). */
+ the TGID pid. */
errno = 0;
ret = my_waitpid (-1, wstatp, options | WNOHANG);
@@ -2520,6 +2614,21 @@ linux_stabilize_threads (void)
}
}
+/* Return non-zero if WAITSTATUS reflects an extended linux
+ event. Otherwise, return 0. Note that extended EXIT
+ events are fixed up and handled like normal events, so
+ they are not considered here. */
+
+static int
+extended_event_reported (const struct target_waitstatus *waitstatus)
+{
+
+ if (waitstatus == NULL)
+ return 0;
+
+ return waitstatus->kind == TARGET_WAITKIND_EXECD;
+}
+
/* Wait for process, returns status. */
static ptid_t
@@ -2883,7 +2992,8 @@ retry:
&& !bp_explains_trap && !trace_event)
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
- && gdb_no_commands_at_breakpoint (event_child->stop_pc)));
+ && gdb_no_commands_at_breakpoint (event_child->stop_pc))
+ || extended_event_reported (&event_child->waitstatus));
run_breakpoint_commands (event_child->stop_pc);
@@ -2905,6 +3015,15 @@ retry:
paddress (event_child->stop_pc),
paddress (event_child->step_range_start),
paddress (event_child->step_range_end));
+ if (debug_threads
+ && extended_event_reported (&event_child->waitstatus))
+ {
+ char *str = target_waitstatus_to_string (ourstatus);
+ debug_printf ("LWP %ld has forked, cloned, vforked or execd"
+ " with waitstatus %s\n",
+ lwpid_of (get_lwp_thread (event_child)), str);
+ xfree (str);
+ }
}
/* We're not reporting this breakpoint to GDB, so apply the
@@ -3003,7 +3122,19 @@ retry:
unstop_all_lwps (1, event_child);
}
- ourstatus->kind = TARGET_WAITKIND_STOPPED;
+ if (extended_event_reported (&event_child->waitstatus))
+ {
+ /* If the reported event is a fork, vfork or exec, let GDB
+ know. */
+ ourstatus->kind = event_child->waitstatus.kind;
+ ourstatus->value = event_child->waitstatus.value;
+
+ /* Reset the event child's waitstatus since we handled it
+ already. */
+ event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
+ }
+ else
+ ourstatus->kind = TARGET_WAITKIND_STOPPED;
if (current_inferior->last_resume_kind == resume_stop
&& WSTOPSIG (w) == SIGSTOP)
@@ -3014,13 +3145,14 @@ retry:
ourstatus->value.sig = GDB_SIGNAL_0;
}
else if (current_inferior->last_resume_kind == resume_stop
- && WSTOPSIG (w) != SIGSTOP)
+ && WSTOPSIG (w) != SIGSTOP
+ && !extended_event_reported (ourstatus))
{
/* A thread that has been requested to stop by GDB with vCont;t,
but, it stopped for other reasons. */
ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
}
- else
+ else if (ourstatus->kind == TARGET_WAITKIND_STOPPED)
{
ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
}
@@ -3126,6 +3258,7 @@ static void
send_sigstop (struct lwp_info *lwp)
{
int pid;
+ int ret;
pid = lwpid_of (get_lwp_thread (lwp));
@@ -3143,7 +3276,21 @@ send_sigstop (struct lwp_info *lwp)
debug_printf ("Sending sigstop to lwp %d\n", pid);
lwp->stop_expected = 1;
- kill_lwp (pid, SIGSTOP);
+ errno = 0;
+ ret = kill_lwp (pid, SIGSTOP);
+ if (ret == -1 && errno == ESRCH)
+ {
+ /* If the kill fails with "No such process", on GNU/Linux we know
+ that the LWP has vanished - it is not a zombie, it is gone.
+ This is due to a thread other than the thread group leader
+ calling exec. See comments in linux_low_filter_event regarding
+ PTRACE_EVENT_EXEC. */
+ delete_lwp (lwp);
+ set_desired_inferior (0);
+
+ if (debug_threads)
+ debug_printf ("send_sigstop: lwp %d has vanished\n", pid);
+ }
}
static int
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 7459710..7759f01 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -265,6 +265,11 @@ struct lwp_info
/* When stopped is set, the last wait status recorded for this lwp. */
int last_status;
+ /* If WAITSTATUS->KIND != TARGET_WAITKIND_IGNORE, the waitstatus for
+ this LWP's last event. This may correspond to LAST_STATUS above,
+ or to the current status during event processing. */
+ struct target_waitstatus waitstatus;
+
/* When stopped is set, this is where the lwp stopped, with
decr_pc_after_break already accounted for. */
CORE_ADDR stop_pc;
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 4fcafa0..9ce25dc 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1111,14 +1111,40 @@ prepare_resume_reply (char *buf, ptid_t ptid,
switch (status->kind)
{
case TARGET_WAITKIND_STOPPED:
+ case TARGET_WAITKIND_EXECD:
{
struct thread_info *saved_inferior;
const char **regp;
struct regcache *regcache;
+ enum gdb_signal signal;
+
+ if (status->kind == TARGET_WAITKIND_EXECD)
+ signal = GDB_SIGNAL_TRAP;
+ else
+ signal = status->value.sig;
+
+ sprintf (buf, "T%02x", signal);
- sprintf (buf, "T%02x", status->value.sig);
buf += strlen (buf);
+ if (status->kind == TARGET_WAITKIND_EXECD && multi_process)
+ {
+ const char *event = "exec";
+ char hexified_pathname[PATH_MAX];
+
+ sprintf (buf, "%s:", event);
+ buf += strlen (buf);
+
+ /* Encode pathname to hexified format. */
+ bin2hex ((const gdb_byte *) status->value.execd_pathname,
+ hexified_pathname, strlen(status->value.execd_pathname));
+
+ sprintf (buf, "%s;", hexified_pathname);
+ xfree (status->value.execd_pathname);
+ status->value.execd_pathname = NULL;
+ buf += strlen (buf);
+ }
+
saved_inferior = current_inferior;
current_inferior = find_thread_ptid (ptid);
diff --git a/gdb/remote.c b/gdb/remote.c
index be8c423..f4412d8 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5542,11 +5542,13 @@ remote_parse_stop_reply (char *buf, struct stop_reply *event)
pnum and set p1 to point to the character following it.
Otherwise p1 points to p. */
- /* If this packet is an awatch packet, don't parse the 'a'
- as a register number. */
+ /* If this packet has a stop reason string that starts
+ with a character that could be a hex digit, don't parse
+ it as a register number. */
if (strncmp (p, "awatch", strlen("awatch")) != 0
- && strncmp (p, "core", strlen ("core") != 0))
+ && strncmp (p, "core", strlen ("core") != 0)
+ && strncmp (p, "exec", strlen ("exec") != 0))
{
/* Read the ``P'' register number. */
pnum = strtol (p, &p_temp, 16);
@@ -5598,6 +5600,25 @@ Packet: '%s'\n"),
p = unpack_varlen_hex (++p1, &c);
event->core = c;
}
+ else if (strncmp (p, "exec", p1 - p) == 0)
+ {
+ ULONGEST pid;
+ char pathname[PATH_MAX];
+
+ p = unpack_varlen_hex (++p1, &pid);
+
+ /* Save the pathname for event reporting and for
+ the next run command. */
+ hex2bin (p1, (gdb_byte *) pathname, (p - p1)/2);
+ /* Add the null terminator. */
+ pathname[(p - p1)/2] = '\0';
+ /* This is freed during event handling. */
+ event->ws.value.execd_pathname = xstrdup (pathname);
+ event->ws.kind = TARGET_WAITKIND_EXECD;
+ /* Save the pathname for the next run command. */
+ xfree (remote_exec_file);
+ remote_exec_file = pathname;
+ }
else
{
/* Silently skip unknown optional info. */
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 3/4][REPOST] Document RSP support for Linux exec events
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
` (2 preceding siblings ...)
2014-04-30 19:18 ` [PATCH 2/4][REPOST] Remote Linux ptrace exec events Don Breazeal
@ 2014-04-30 19:18 ` Don Breazeal
2014-05-08 5:34 ` Yao Qi
2014-05-21 15:25 ` [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Tom Tromey
` (5 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Don Breazeal @ 2014-04-30 19:18 UTC (permalink / raw)
To: gdb-patches; +Cc: Don Breazeal
From: Don Breazeal <don_breazeal@mentor.com>
[Reposting to eliminate unexpected attachment type.]
This patch adds documentation of the new RSP support for exec events.
This patch was approved 4/4/2014 per this email:
> -----Original Message-----
> From: Eli Zaretskii [mailto:eliz@gnu.org]
> Sent: Wednesday, April 02, 2014 7:50 PM
> To: Breazeal, Don
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 3/4] Document RSP support for exec events
>
> > From: "Breazeal, Don" <donb@codesourcery.com>
> > Date: Wed, 2 Apr 2014 22:24:18 +0000
> >
> > This patch adds documentation of the new RSP support for exec events.
>
> Thanks.
>
> > doc/
> > 2014-04-02 Don Breazeal <donb@codesourcery.com>
> >
> > * gdb.texinfo: Document RSP support for exec events.
>
> Please state the name of the node where you make the change.
>
> > +@cindex exec events, remote reply
> > +@item exec
> > +The packet indicates that execve was called, and @var{r} is the
> > +absolute pathname of the file that was executed, in hex. This is
> > +only applicable to certain targets.
> > +
>
> "execve" should be in @code, as it is a C symbol.
>
> OK with that change.
doc/
2014-04-03 Don Breazeal <donb@codesourcery.com>
* gdb.texinfo (Stop Reply Packets): Document RSP support
for exec events.
---
gdb/doc/gdb.texinfo | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b218769..bb9dfb8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34582,6 +34582,12 @@ The packet indicates that the loaded libraries have changed.
@value{GDBN} should use @samp{qXfer:libraries:read} to fetch a new
list of loaded libraries. @var{r} is ignored.
+@cindex exec events, remote reply
+@item exec
+The packet indicates that @code{execve} was called, and @var{r} is the
+absolute pathname of the file that was executed, in hex. This is
+only applicable to certain targets.
+
@cindex replay log events, remote reply
@item replaylog
The packet indicates that the target cannot continue replaying
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 0/0][REPOST] Exec events in gdbserver on Linux
@ 2014-04-30 19:18 Don Breazeal
2014-04-30 19:18 ` [PATCH 4/4 v2][REPOST] Non-stop exec tests Don Breazeal
` (9 more replies)
0 siblings, 10 replies; 33+ messages in thread
From: Don Breazeal @ 2014-04-30 19:18 UTC (permalink / raw)
To: gdb-patches
[Reposting to eliminate unexpected attachment type.]
This patch implements support for exec events in gdbserver on GNU/Linux.
This is a step towards the "follow fork/exec" item in the local/remote
debugging feature parity project:
(https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity). Luis had
started work on this last year, and handed it off to me at the end of
January. (BTW, thanks to Luis for his advice on this, although any
problems with this patch are entirely mine.)
Follow-exec-mode and rerun behave as expected in multiprocess mode (target
extended-remote), where follow-exec-mode maintains the specified inferiors
and rerun runs the last executable file to be exec'd. Catchpoints for exec
are not implemented in this patch series, since this will be easier to do
once fork and vfork events are also supported.
- Patch 1/4 implements support for the extended ptrace event
PTRACE_EVENT_EXIT, which is a prerequisite for the exec event support.
- Patch 2/4 implements the exec event support.
- Patch 3/4 adds documentation for the new RSP "Stop Reply" message.
- Patch 4/4 extends the tests gdb.threads/non-ldr-exc-[1-4].exp to test in
non-stop mode as well as in all-stop mode.
There are a couple of significant aspects to this patch. First, it uses
the ptrace extension PTRACE_EVENT_EXIT to detect thread exit, in particular
the exit of a thread group leader when a non-leader calls exec. Use of
this event was necessary due to a race condition in the two-thread case.
It is only used internally; exit events detected this way are not exposed
to the user, and exit processing was changed as little as possible.
Second, it sends a signal to each lwp after an exec event in order to
identify and clean up the lwp entry for the exec'ing thread. This happens
in the normal course of things for all-stop mode, but in non-stop mode
gdbserver uses SIGSTOP to stop, then resume all of the non-leader threads
to accomplish this.
More detailed explanations of these issues are included in the patch 2/4
email.
My intent is to follow up with implementations of remote fork/vfork events
and associated catchpoints.
Thanks,
--Don
gdb/common/linux-ptrace.c | 45 ++++-
gdb/doc/gdb.texinfo | 6 +
gdb/gdbserver/linux-low.c | 274 +++++++++++++++++++++++++---
gdb/gdbserver/linux-low.h | 5 +
gdb/gdbserver/remote-utils.c | 28 ++-
gdb/remote.c | 27 ++-
gdb/testsuite/gdb.threads/non-ldr-exc-1.exp | 15 +-
gdb/testsuite/gdb.threads/non-ldr-exc-2.exp | 15 +-
gdb/testsuite/gdb.threads/non-ldr-exc-3.exp | 15 +-
gdb/testsuite/gdb.threads/non-ldr-exc-4.exp | 15 +-
10 files changed, 395 insertions(+), 50 deletions(-)
^[
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/4][REPOST] Remote Linux ptrace exit events
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
2014-04-30 19:18 ` [PATCH 4/4 v2][REPOST] Non-stop exec tests Don Breazeal
@ 2014-04-30 19:18 ` Don Breazeal
2014-05-07 19:40 ` Luis Machado
` (2 more replies)
2014-04-30 19:18 ` [PATCH 2/4][REPOST] Remote Linux ptrace exec events Don Breazeal
` (7 subsequent siblings)
9 siblings, 3 replies; 33+ messages in thread
From: Don Breazeal @ 2014-04-30 19:18 UTC (permalink / raw)
To: gdb-patches; +Cc: Don Breazeal
From: Don Breazeal <don_breazeal@mentor.com>
[Reposting to eliminate unexpected attachment type.]
This patch implements support for the extended ptrace event
PTRACE_EVENT_EXIT on Linux. This is a preparatory patch for exec event
support.
The use of this event is entirely internal to gdbserver; these events are
not reported to GDB or the user. If an existing thread is not the last
thread in a process, its lwp entry is simply deleted, since this is what
happens in the absence of exit events. If it is the last thread of a
process, the wait status is set to the actual wait status of the thread,
instead of the status that indicates the extended event, and the existing
mechanisms for handling thread exit proceed as usual.
The only purpose in using the exit events instead of the existing wait
mechanisms is to ensure that the exit of a thread group leader is detected
reliably when a non-leader thread calls exec.
gdb/
2014-04-02 Don Breazeal <donb@codesourcery.com>
* common/linux-ptrace.c (linux_test_for_tracefork)
[GDBSERVER]: Add support for PTRACE_EVENT_EXIT if the OS
supports it.
gdbserver/
2014-04-02 Don Breazeal <donb@codesourcery.com>
* linux-low.c (is_extended_waitstatus): New function.
(is_extended_exit): New function.
(handle_extended_wait): Change type from void to int, change
wait status arg to pointer, add support for PTRACE_EVENT_EXIT.
(get_stop_pc): Use is_extended_waitstatus instead of hardcoded
shift operation.
(get_detach_signal): Use is_extended_waitstatus instead of
hardcoded shift operation.
(linux_low_filter_event): Handle new return value from
handle_extended_wait. Check for extended events as needed.
Allow status arg to be modified.
(linux_wait_for_event_filtered): Allow status arg to be
modified.
---
gdb/common/linux-ptrace.c | 42 ++++++++++++++++++-
gdb/gdbserver/linux-low.c | 99 +++++++++++++++++++++++++++++++++++++++------
2 files changed, 125 insertions(+), 16 deletions(-)
diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
index 7c1b78a..e3fc705 100644
--- a/gdb/common/linux-ptrace.c
+++ b/gdb/common/linux-ptrace.c
@@ -414,7 +414,7 @@ linux_test_for_tracefork (int child_pid)
ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
(PTRACE_TYPE_ARG4) 0);
if (ret != 0)
- warning (_("linux_test_for_tracefork: failed to resume child"));
+ warning (_("linux_test_for_tracefork: failed to resume child (a)"));
ret = my_waitpid (child_pid, &status, 0);
@@ -455,10 +455,46 @@ linux_test_for_tracefork (int child_pid)
"failed to kill second child"));
my_waitpid (second_pid, &status, 0);
}
+ else
+ {
+ /* Fork events are not supported. */
+ return;
+ }
}
else
- warning (_("linux_test_for_tracefork: unexpected result from waitpid "
- "(%d, status 0x%x)"), ret, status);
+ {
+ warning (_("linux_test_for_tracefork: unexpected result from waitpid "
+ "(%d, status 0x%x)"), ret, status);
+ return;
+ }
+
+#ifdef GDBSERVER
+ /* PTRACE_O_FORK is supported, so now test for PTRACE_O_TRACEEXIT.
+ First try to set the option. If this fails, we know for sure that
+ it is not supported. */
+ ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) PTRACE_O_TRACEEXIT);
+ if (ret != 0)
+ return;
+
+ /* We don't know for sure that the feature is available; old
+ versions of PTRACE_SETOPTIONS ignored unknown options. So
+ see if the process exit will generate a PTRACE_EVENT_EXIT. */
+ ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) 0);
+ if (ret != 0)
+ warning (_("linux_test_for_tracefork: failed to resume child (b)"));
+
+ ret = my_waitpid (child_pid, &status, 0);
+
+ /* Check if we received an exit event notification. */
+ if (ret == child_pid && WIFSTOPPED (status)
+ && status >> 16 == PTRACE_EVENT_EXIT)
+ {
+ /* PTRACE_O_TRACEEXIT is supported. */
+ current_ptrace_options |= PTRACE_O_TRACEEXIT;
+ }
+#endif
}
/* Enable reporting of all currently supported ptrace events. */
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 2d8d5f5..90e7b15 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -224,6 +224,7 @@ static void proceed_all_lwps (void);
static int finish_step_over (struct lwp_info *lwp);
static CORE_ADDR get_stop_pc (struct lwp_info *lwp);
static int kill_lwp (unsigned long lwpid, int signo);
+static int num_lwps (int pid);
/* True if the low target can hardware single-step. Such targets
don't need a BREAKPOINT_REINSERT_ADDR callback. */
@@ -367,13 +368,32 @@ linux_add_process (int pid, int attached)
return proc;
}
+/* Check wait status for extended event */
+
+static int
+is_extended_waitstatus (int wstat)
+{
+ return wstat >> 16 != 0;
+}
+
+/* Check wait status for extended event */
+
+static int
+is_extended_exit (int wstat)
+{
+ return wstat >> 16 == PTRACE_EVENT_EXIT;
+}
+
/* Handle a GNU/Linux extended wait response. If we see a clone
event, we need to add the new LWP to our list (and not report the
- trap to higher layers). */
+ trap to higher layers). This function returns non-zero if the
+ event should be ignored and we should wait again. The wait status
+ in WSTATP may be modified if an exit event occurred. */
-static void
-handle_extended_wait (struct lwp_info *event_child, int wstat)
+static int
+handle_extended_wait (struct lwp_info *event_child, int *wstatp)
{
+ int wstat = *wstatp;
int event = wstat >> 16;
struct thread_info *event_thr = get_lwp_thread (event_child);
struct lwp_info *new_lwp;
@@ -448,11 +468,54 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
linux_resume_one_lwp (new_lwp, 0, WSTOPSIG (status), NULL);
}
+ /* Enable extended events for the new thread. */
+ linux_enable_event_reporting (new_pid);
+
/* Always resume the current thread. If we are stopping
threads, it will have a pending SIGSTOP; we may as well
collect it now. */
linux_resume_one_lwp (event_child, event_child->stepping, 0, NULL);
+
+ /* Don't report the event. */
+ return 1;
+ }
+ else if (event == PTRACE_EVENT_EXIT)
+ {
+ unsigned long exit_status;
+ unsigned long lwpid = lwpid_of (event_thr);
+ int ret;
+
+ if (debug_threads)
+ debug_printf ("LHEW: Got exit event from LWP %ld\n",
+ lwpid_of (event_thr));
+
+ ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
+ &exit_status);
+
+ if (num_lwps (pid_of (event_thr)) > 1)
+ {
+ /* If there is at least one more LWP, then the program still
+ exists and the exit should not be reported to GDB. */
+ delete_lwp (event_child);
+ ret = 1;
+ }
+ else
+ {
+ /* Set the exit status to the actual exit status, so normal
+ WIFEXITED/WIFSIGNALLED processing and reporting for the
+ last lwp in the process can proceed from here. */
+ *wstatp = exit_status;
+ ret = 0;
+ }
+
+ /* Resume the thread so that it actually exits. Subsequent exit
+ events for LWPs that were deleted above will be ignored. */
+ ptrace (PTRACE_CONT, lwpid, (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) 0);
+ return ret;
}
+ internal_error (__FILE__, __LINE__,
+ _("unknown ptrace event %d"), event);
}
/* Return the PC as read from the regcache of LWP, without any
@@ -516,7 +579,7 @@ get_stop_pc (struct lwp_info *lwp)
if (WSTOPSIG (lwp->last_status) == SIGTRAP
&& !lwp->stepping
&& !lwp->stopped_by_watchpoint
- && lwp->last_status >> 16 == 0)
+ && !is_extended_waitstatus (lwp->last_status))
stop_pc -= the_low_target.decr_pc_after_break;
if (debug_threads)
@@ -1031,7 +1094,7 @@ get_detach_signal (struct thread_info *thread)
}
/* Extended wait statuses aren't real SIGTRAPs. */
- if (WSTOPSIG (status) == SIGTRAP && status >> 16 != 0)
+ if (WSTOPSIG (status) == SIGTRAP && is_extended_waitstatus (status))
{
if (debug_threads)
debug_printf ("GPS: lwp %s had stopped with extended "
@@ -1716,11 +1779,13 @@ cancel_breakpoint (struct lwp_info *lwp)
/* Do low-level handling of the event, and check if we should go on
and pass it to caller code. Return the affected lwp if we are, or
- NULL otherwise. */
+ NULL otherwise. The wait status in WSTATP may be modified if an
+ exit event occurred. */
static struct lwp_info *
-linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
+linux_low_filter_event (ptid_t filter_ptid, int lwpid, int *wstatp)
{
+ int wstat = *wstatp;
struct lwp_info *child;
struct thread_info *thread;
@@ -1772,7 +1837,7 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
architecture being defined already (so that CHILD has a valid
regcache), and on LAST_STATUS being set (to check for SIGTRAP or
not). */
- if (WIFSTOPPED (wstat))
+ if (WIFSTOPPED (wstat) && !is_extended_exit (wstat))
{
if (debug_threads
&& the_low_target.get_pc != NULL)
@@ -1808,7 +1873,8 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
changes the debug registers meanwhile, we have the cached data we
can rely on. */
- if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP)
+ if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
+ && !is_extended_waitstatus (wstat))
{
if (the_low_target.stopped_by_watchpoint == NULL)
{
@@ -1844,10 +1910,17 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
}
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
- && wstat >> 16 != 0)
+ && is_extended_waitstatus (wstat))
{
- handle_extended_wait (child, wstat);
- return NULL;
+ if (handle_extended_wait (child, &wstat))
+ return NULL;
+ else
+ {
+ /* Update caller's wstat in case handle_extended_wait fixed
+ it up to reflect the actual status. */
+ *wstatp = wstat;
+ return child;
+ }
}
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGSTOP
@@ -2067,7 +2140,7 @@ linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid,
}
event_child = linux_low_filter_event (filter_ptid,
- ret, *wstatp);
+ ret, wstatp);
if (event_child != NULL)
{
/* We got an event to report to the core. */
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/4][REPOST] Remote Linux ptrace exit events
2014-04-30 19:18 ` [PATCH 1/4][REPOST] Remote Linux ptrace exit events Don Breazeal
@ 2014-05-07 19:40 ` Luis Machado
2014-05-08 5:23 ` Yao Qi
2014-05-21 15:15 ` Tom Tromey
2 siblings, 0 replies; 33+ messages in thread
From: Luis Machado @ 2014-05-07 19:40 UTC (permalink / raw)
To: Don Breazeal, gdb-patches; +Cc: Don Breazeal
Hi Don,
Thanks for moving this work forward.
The patch looks good to me. I only have a small comment below.
As for the use of the EXIT events, i think it is good that we get to
finally start using it. Maybe it will be useful for other things besides
this specific feature too.
On 04/30/2014 04:17 PM, Don Breazeal wrote:
> From: Don Breazeal <don_breazeal@mentor.com>
>
> [Reposting to eliminate unexpected attachment type.]
>
> This patch implements support for the extended ptrace event
> PTRACE_EVENT_EXIT on Linux. This is a preparatory patch for exec event
> support.
>
> The use of this event is entirely internal to gdbserver; these events are
> not reported to GDB or the user. If an existing thread is not the last
> thread in a process, its lwp entry is simply deleted, since this is what
> happens in the absence of exit events. If it is the last thread of a
> process, the wait status is set to the actual wait status of the thread,
> instead of the status that indicates the extended event, and the existing
> mechanisms for handling thread exit proceed as usual.
>
> The only purpose in using the exit events instead of the existing wait
> mechanisms is to ensure that the exit of a thread group leader is detected
> reliably when a non-leader thread calls exec.
>
> gdb/
> 2014-04-02 Don Breazeal <donb@codesourcery.com>
>
> * common/linux-ptrace.c (linux_test_for_tracefork)
> [GDBSERVER]: Add support for PTRACE_EVENT_EXIT if the OS
> supports it.
>
> gdbserver/
> 2014-04-02 Don Breazeal <donb@codesourcery.com>
>
> * linux-low.c (is_extended_waitstatus): New function.
> (is_extended_exit): New function.
> (handle_extended_wait): Change type from void to int, change
> wait status arg to pointer, add support for PTRACE_EVENT_EXIT.
> (get_stop_pc): Use is_extended_waitstatus instead of hardcoded
> shift operation.
> (get_detach_signal): Use is_extended_waitstatus instead of
> hardcoded shift operation.
> (linux_low_filter_event): Handle new return value from
> handle_extended_wait. Check for extended events as needed.
> Allow status arg to be modified.
> (linux_wait_for_event_filtered): Allow status arg to be
> modified.
>
> ---
> gdb/common/linux-ptrace.c | 42 ++++++++++++++++++-
> gdb/gdbserver/linux-low.c | 99 +++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 125 insertions(+), 16 deletions(-)
>
> diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
> index 7c1b78a..e3fc705 100644
> --- a/gdb/common/linux-ptrace.c
> +++ b/gdb/common/linux-ptrace.c
> @@ -414,7 +414,7 @@ linux_test_for_tracefork (int child_pid)
> ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
> (PTRACE_TYPE_ARG4) 0);
> if (ret != 0)
> - warning (_("linux_test_for_tracefork: failed to resume child"));
> + warning (_("linux_test_for_tracefork: failed to resume child (a)"));
>
> ret = my_waitpid (child_pid, &status, 0);
>
> @@ -455,10 +455,46 @@ linux_test_for_tracefork (int child_pid)
> "failed to kill second child"));
> my_waitpid (second_pid, &status, 0);
> }
> + else
> + {
> + /* Fork events are not supported. */
> + return;
> + }
> }
> else
> - warning (_("linux_test_for_tracefork: unexpected result from waitpid "
> - "(%d, status 0x%x)"), ret, status);
> + {
> + warning (_("linux_test_for_tracefork: unexpected result from waitpid "
> + "(%d, status 0x%x)"), ret, status);
> + return;
> + }
> +
> +#ifdef GDBSERVER
> + /* PTRACE_O_FORK is supported, so now test for PTRACE_O_TRACEEXIT.
> + First try to set the option. If this fails, we know for sure that
> + it is not supported. */
> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> + (PTRACE_TYPE_ARG4) PTRACE_O_TRACEEXIT);
> + if (ret != 0)
> + return;
> +
> + /* We don't know for sure that the feature is available; old
> + versions of PTRACE_SETOPTIONS ignored unknown options. So
> + see if the process exit will generate a PTRACE_EVENT_EXIT. */
> + ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
> + (PTRACE_TYPE_ARG4) 0);
> + if (ret != 0)
> + warning (_("linux_test_for_tracefork: failed to resume child (b)"));
> +
> + ret = my_waitpid (child_pid, &status, 0);
> +
> + /* Check if we received an exit event notification. */
> + if (ret == child_pid && WIFSTOPPED (status)
> + && status >> 16 == PTRACE_EVENT_EXIT)
> + {
> + /* PTRACE_O_TRACEEXIT is supported. */
> + current_ptrace_options |= PTRACE_O_TRACEEXIT;
> + }
> +#endif
> }
>
> /* Enable reporting of all currently supported ptrace events. */
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 2d8d5f5..90e7b15 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -224,6 +224,7 @@ static void proceed_all_lwps (void);
> static int finish_step_over (struct lwp_info *lwp);
> static CORE_ADDR get_stop_pc (struct lwp_info *lwp);
> static int kill_lwp (unsigned long lwpid, int signo);
> +static int num_lwps (int pid);
>
> /* True if the low target can hardware single-step. Such targets
> don't need a BREAKPOINT_REINSERT_ADDR callback. */
> @@ -367,13 +368,32 @@ linux_add_process (int pid, int attached)
> return proc;
> }
>
> +/* Check wait status for extended event */
> +
> +static int
> +is_extended_waitstatus (int wstat)
> +{
> + return wstat >> 16 != 0;
> +}
> +
> +/* Check wait status for extended event */
> +
> +static int
> +is_extended_exit (int wstat)
> +{
> + return wstat >> 16 == PTRACE_EVENT_EXIT;
> +}
> +
The function above needs to have its comment updated to reflect the
actual function body.
Luis
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/4][REPOST] Remote Linux ptrace exec events
2014-04-30 19:18 ` [PATCH 2/4][REPOST] Remote Linux ptrace exec events Don Breazeal
@ 2014-05-07 20:01 ` Luis Machado
2014-05-09 21:17 ` Breazeal, Don
2014-05-08 5:44 ` Yao Qi
2014-05-21 15:28 ` Tom Tromey
2 siblings, 1 reply; 33+ messages in thread
From: Luis Machado @ 2014-05-07 20:01 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
Hi,
On 04/30/2014 04:18 PM, Don Breazeal wrote:
> From: Don Breazeal <don_breazeal@mentor.com>
>
> [Reposting to eliminate unexpected attachment type.]
>
> This patch implements support for exec events in gdbserver on linux, in
> multiprocess mode (target extended-remote). Follow-exec-mode and rerun
> behave as expected. Catchpoints for exec are not yet implemented since it
> will be easier to implement catchpoints for fork, vfork, and exec all at
> the same time.
>
> TESTING
> ---------
> The patch was tested on GNU/Linux x86_64 with --target_board set to
> native-gdbserver and native-extended-gdbserver, as well as testing native
> GDB. The test results for native-gdbserver were unchanged. Thirteen tests
> that used to fail for native-extended-gdbserver on Linux pass with this
> patch, and the non-ldr-exc-*.exp tests all pass in non-stop mode and
> extended-remote. There are several failures in the new non-ldr-exc-*.exp
> tests in non-stop mode with native GDB.
>
> One caveat: when an exec is detected, gdbserver emits a couple of warnings:
> gdbserver: unexpected r_debug version 0
> gdbserver: unexpected r_debug version 0
> However, debugging of shared libraries that are loaded by the exec'd
> program works just fine. These messages may be caused by gdbserver making
> an attempt to initialize the solib hook before the r_debug structure has
> been initialized. I intend to follow up in a subsequent patch.
>
> IMPLEMENTATION
> ----------------
> Support for exec events in single-threaded programs was a fairly
> straightforward replication of the implementation in native GDB:
> 1) Enable exec events via ptrace options.
> 2) Add support for handling the exec events to the handle_extended_wait
> and linux_wait_for_event_filtered. Detect the exec event, then
> find and save the pathname of the executable file being exec'd.
> 3) Implement an additional "stop reason", "exec", in the RSP stop reply
> packet "T".
> Existing GDB code takes care of handling the exec event on the host side
> without modification.
>
> Support for exec events in multi-threaded programs required some additional
> work that required a couple of significant changes to existing code. In a
> nutshell, the changes are to:
> 4) Use the PTRACE_EVENT_EXIT extended event to handle thread exit,
> while not exposing any change in exit handling to the user. The
> rationale for this is discussed in the "patch 0" email of this
> series.
> 5) Recognize when the exec'ing thread has vanished (become the thread
> group leader) in send_sigstop. Native GDB does this differently.
>
> Regarding items 4 & 5: Recall that when a non-leader thread exec's, all the
> other threads are terminated and the exec'ing thread changes its thread id
> to that of the old leader (the process id) as part of the exec. There is
> no event reported for the "exit" of the exec'ing thread; it appears to have
> vanished. The original thread group leader can't be reaped until all the
> other threads have been reaped, and some way of determining that it has
> exited is required in order to update the lwp list (#4 above). Also, some
> mechanism for deleting the lwp entry corresponding to the exec'ing thread
> is needed (#5 above).
>
> The rationale for #4 is that in my testing I ran into a race condition in
> the mechanism that's intended to detect when a thread group leader has
> exited, check_zombie_leaders. The race occurred when there were only two
> threads in the program. In this case the leader thread passes through a
> very brief zombie state before being replaced by the exec'ing thread as
> the thread group leader. This state transition is asynchronous, with no
> dependency on anything gdbserver does. Using PTRACE_EVENT_EXIT ensures
> that the leader exit will be detected. I can provide a more detailed
> explanation or the race, but I didn't want to be too long-winded here.
>
> Regarding item #5, determining that the exec'ing thread has "vanished", In
> native GDB this is done by calling waitpid(PID), and if it returns ECHILD
> it means that the thread is gone. We don't want to use waitpid(PID) in
> gdbserver, based on the discussion in
> https://www.sourceware.org/ml/gdb-patches/2014-02/msg00828.html. An
> alternative is to send a signal to each thread and look for an ESRCH (No
> such process) error. In all-stop mode this can be done in the normal
> course of events, since when gdbserver reports an exec event it stops all
> the other threads with a SIGSTOP. In non-stop mode, when an exec event has
> been detected, we can call stop_all_lwps/unstop_all_lwps to accomplish the
> same thing.
>
> gdb/
> 2014-04-02 Don Breazeal <donb@codesourcery.com>
>
> * common/linux-ptrace.c (linux_test_for_tracefork)
> [GDBSERVER]: Add exec tracing to ptrace options if OS supports
> it.
> * remote.c (remote_parse_stop_reply): Support new RSP stop
> reply reason 'exec'.
>
> gdbserver/
> 2014-04-02 Don Breazeal <donb@codesourcery.com>
>
> * gdb/gdbserver/linux-low.c (linux_child_pid_to_exec_file): New
> function.
> (handle_extended_wait): Add support for PTRACE_EVENT_EXEC.
> (check_zombie_leaders): Update comment.
> (linux_low_filter_event): Add support for PTRACE_EVENT_EXEC.
> (linux_wait_for_event_filtered): Update comment.
> (extended_event_reported): New function.
> (send_sigstop): Delete lwp on 'No such process' error and
> reset current_inferior.
> * gdb/gdbserver/linux-low.h (struct lwp_info): New member
> 'waitstatus'.
> * gdb/gdbserver/remote-utils.c (prepare_resume_reply): Support
> new RSP stop reply reason 'exec'.
>
> ---
> gdb/common/linux-ptrace.c | 7 +-
> gdb/gdbserver/linux-low.c | 175 ++++++++++++++++++++++++++++++++++++++----
> gdb/gdbserver/linux-low.h | 5 +
> gdb/gdbserver/remote-utils.c | 28 +++++++-
> gdb/remote.c | 27 ++++++-
> 5 files changed, 222 insertions(+), 20 deletions(-)
>
> diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
> index e3fc705..b137df9 100644
> --- a/gdb/common/linux-ptrace.c
> +++ b/gdb/common/linux-ptrace.c
> @@ -491,8 +491,11 @@ linux_test_for_tracefork (int child_pid)
> if (ret == child_pid && WIFSTOPPED (status)
> && status >> 16 == PTRACE_EVENT_EXIT)
> {
> - /* PTRACE_O_TRACEEXIT is supported. */
> - current_ptrace_options |= PTRACE_O_TRACEEXIT;
> + /* PTRACE_O_TRACEEXIT is supported. We use exit events to
> + implement support for exec events. Since fork events are
> + supported we know exec events are supported, so we enable
> + exec events here. */
> + current_ptrace_options |= PTRACE_O_TRACEEXIT | PTRACE_O_TRACEEXEC;
> }
> #endif
> }
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 90e7b15..5f94490 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -281,6 +281,32 @@ static int linux_event_pipe[2] = { -1, -1 };
> static void send_sigstop (struct lwp_info *lwp);
> static void wait_for_sigstop (void);
>
> +/* Accepts an integer PID. Returns a string containing the
> + name of the executable file for the child process.
> + Space for the result is malloc'd, caller must free. */
> +
> +static char *
> +linux_child_pid_to_exec_file (int pid)
> +{
> + char *name1, *name2;
> +
> + name1 = xmalloc (PATH_MAX);
> + name2 = xmalloc (PATH_MAX);
> + memset (name2, 0, PATH_MAX);
> +
> + sprintf (name1, "/proc/%d/exe", pid);
> + if (readlink (name1, name2, PATH_MAX) > 0)
> + {
> + free (name1);
> + return name2;
> + }
> + else
> + {
> + free (name2);
> + return name1;
> + }
> +}
> +
At one point in time this function existed in gdbserver/linux-low.c but
got deleted by the following commit:
commit be07f1a20c962deb662b93209b4ca91bc8e5cbd8
Author: Pedro Alves <palves@redhat.com>
Date: Fri Jan 27 19:23:43 2012 +0000
2012-01-27 Pedro Alves <palves@redhat.com>
* linux-low.c (linux_child_pid_to_exec_file): Delete.
(elf_64_file_p): Make static.
(linux_pid_exe_is_elf_64_file): New.
* linux-low.h (linux_child_pid_to_exec_file, elf_64_file_p):
Delete declarations.
(linux_pid_exe_is_elf_64_file): Declare.
* linux-x86-low.c (x86_arch_setup): Use
linux_pid_exe_is_elf_64_file.
It still exists in linux-nat.c though. I remember i tried to unify those
and move the resulting function to common/, but there may be differences
preventing such move.
If it can be done, it would be best.
> /* Return non-zero if HEADER is a 64-bit ELF file. */
>
> static int
> @@ -514,6 +540,19 @@ handle_extended_wait (struct lwp_info *event_child, int *wstatp)
> (PTRACE_TYPE_ARG4) 0);
> return ret;
> }
> + else if (event == PTRACE_EVENT_EXEC)
> + {
> + if (debug_threads)
> + debug_printf ("LHEW: Got exec event from LWP %ld\n",
> + lwpid_of (event_thr));
> +
> + event_child->waitstatus.kind = TARGET_WAITKIND_EXECD;
> + event_child->waitstatus.value.execd_pathname
> + = linux_child_pid_to_exec_file (lwpid_of (event_thr));
> +
> + /* Report the event. */
> + return 0;
> + }
> internal_error (__FILE__, __LINE__,
> _("unknown ptrace event %d"), event);
> }
> @@ -1376,18 +1415,19 @@ check_zombie_leaders (void)
> program). In the latter case, we can't waitpid the
> leader's exit status until all other threads are gone.
>
> - - There are 3 or more threads in the group, and a thread
> + - There are multiple threads in the group, and a thread
> other than the leader exec'd. On an exec, the Linux
> kernel destroys all other threads (except the execing
> one) in the thread group, and resets the execing thread's
> tid to the tgid. No exit notification is sent for the
> execing thread -- from the ptracer's perspective, it
> appears as though the execing thread just vanishes.
> - Until we reap all other threads except the leader and the
> - execing thread, the leader will be zombie, and the
> - execing thread will be in `D (disc sleep)'. As soon as
> - all other threads are reaped, the execing thread changes
> - it's tid to the tgid, and the previous (zombie) leader
> + Until we reap all other threads (if any) except the
> + leader and the execing thread, the leader will be zombie,
> + and the execing thread will be in `D (disc sleep)'. As
> + soon as all other threads are reaped, or have reported
> + PTRACE_EVENT_EXIT events, the execing thread changes its
> + tid to the tgid, and the previous (zombie) leader
> vanishes, giving place to the "new" leader. We could try
> distinguishing the exit and exec cases, by waiting once
> more, and seeing if something comes out, but it doesn't
> @@ -1395,7 +1435,11 @@ check_zombie_leaders (void)
> we'll re-add the new one once we see the exec event
> (which is just the same as what would happen if the
> previous leader did exit voluntarily before some other
> - thread execs). */
> + thread execs).
> +
> + Note that when PTRACE_EVENT_EXEC is supported, we use
> + that mechanism to detect thread exit, including the
> + exit of zombie leaders. */
>
> if (debug_threads)
> fprintf (stderr,
> @@ -1791,6 +1835,57 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int *wstatp)
>
> child = find_lwp_pid (pid_to_ptid (lwpid));
>
> + /* Check for stop events reported by a process we didn't already
> + know about - anything not already in our LWP list.
> +
> + If we're expecting to receive stopped processes after
> + fork, vfork, and clone events, then we'll just add the
> + new one to our list and go back to waiting for the event
> + to be reported - the stopped process might be returned
> + from waitpid before or after the event is.
> +
> + But note the case of a non-leader thread exec'ing after the
> + leader having exited, and gone from our lists. On an exec,
> + the Linux kernel destroys all other threads (except the execing
> + one) in the thread group, and resets the execing thread's tid
> + to the tgid. No exit notification is sent for the execing
> + thread -- from the ptracer's perspective, it appears as though
> + the execing thread just vanishes. When they are available, we
> + use exit events (PTRACE_EVENT_EXIT) to detect thread exit
> + reliably. As soon as all other threads (if any) are reaped or
> + have reported their PTRACE_EVENT_EXIT events, the execing
> + thread changes it's tid to the tgid, and the previous (zombie)
> + leader vanishes, giving place to the "new" leader. The lwp
> + entry for the previous leader is deleted when we handle its
> + exit event, and we re-add the new one here. */
> +
> + if (WIFSTOPPED (wstat) && child == NULL
> + && (WSTOPSIG (wstat) == SIGTRAP && wstat >> 16 == PTRACE_EVENT_EXEC))
> + {
> + ptid_t child_ptid;
> +
It would be nice to replace the shift operation with a predicate to test
for PTRACE_EVENT_EXEC, just like you did for PTRACE_EVENT_EXIT.
> + /* A multi-thread exec after we had seen the leader exiting. */
> + if (debug_threads)
> + debug_printf ("LLW: Re-adding thread group leader LWP %d.\n",
> + lwpid);
> +
> + child_ptid = ptid_build (lwpid, lwpid, 0);
> + child = add_lwp (child_ptid);
> + child->stopped = 1;
> + current_inferior = child->thread;
> +
> + if (non_stop && stopping_threads == NOT_STOPPING_THREADS)
> + {
> + /* Make sure we delete the lwp entry for the exec'ing thread,
> + which will have vanished. We do this by sending a signal
> + to all the other threads in the lwp list, deleting any
> + that are not found. Note that in all-stop mode this will
> + happen before reporting the event. */
> + stop_all_lwps (0, child);
> + unstop_all_lwps (0, child);
> + }
> + }
> +
> /* If we didn't find a process, one of two things presumably happened:
> - A process we started and then detached from has exited. Ignore it.
> - A process we are controlling has forked and the new child's stop
> @@ -2122,8 +2217,7 @@ linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid,
> - When a non-leader thread execs, that thread just vanishes
> without reporting an exit (so we'd hang if we waited for it
> explicitly in that case). The exec event is reported to
> - the TGID pid (although we don't currently enable exec
> - events). */
> + the TGID pid. */
> errno = 0;
> ret = my_waitpid (-1, wstatp, options | WNOHANG);
>
> @@ -2520,6 +2614,21 @@ linux_stabilize_threads (void)
> }
> }
>
> +/* Return non-zero if WAITSTATUS reflects an extended linux
> + event. Otherwise, return 0. Note that extended EXIT
> + events are fixed up and handled like normal events, so
> + they are not considered here. */
> +
> +static int
> +extended_event_reported (const struct target_waitstatus *waitstatus)
> +{
> +
> + if (waitstatus == NULL)
> + return 0;
> +
> + return waitstatus->kind == TARGET_WAITKIND_EXECD;
> +}
> +
Someone mentioned putting ()'s around the condition for better
readability. Though not required, i also agree that it improves things.
Not a requirement, just a suggestion.
> /* Wait for process, returns status. */
>
> static ptid_t
> @@ -2883,7 +2992,8 @@ retry:
> && !bp_explains_trap && !trace_event)
> || (gdb_breakpoint_here (event_child->stop_pc)
> && gdb_condition_true_at_breakpoint (event_child->stop_pc)
> - && gdb_no_commands_at_breakpoint (event_child->stop_pc)));
> + && gdb_no_commands_at_breakpoint (event_child->stop_pc))
> + || extended_event_reported (&event_child->waitstatus));
>
> run_breakpoint_commands (event_child->stop_pc);
>
> @@ -2905,6 +3015,15 @@ retry:
> paddress (event_child->stop_pc),
> paddress (event_child->step_range_start),
> paddress (event_child->step_range_end));
> + if (debug_threads
> + && extended_event_reported (&event_child->waitstatus))
> + {
> + char *str = target_waitstatus_to_string (ourstatus);
> + debug_printf ("LWP %ld has forked, cloned, vforked or execd"
> + " with waitstatus %s\n",
> + lwpid_of (get_lwp_thread (event_child)), str);
> + xfree (str);
> + }
> }
>
Maybe we could provide more precise information about the event here
instead of something generic? It may help debugging in the future or if
we ever have a mix of these events happening very close to each other.
> /* We're not reporting this breakpoint to GDB, so apply the
> @@ -3003,7 +3122,19 @@ retry:
> unstop_all_lwps (1, event_child);
> }
>
> - ourstatus->kind = TARGET_WAITKIND_STOPPED;
> + if (extended_event_reported (&event_child->waitstatus))
> + {
> + /* If the reported event is a fork, vfork or exec, let GDB
> + know. */
> + ourstatus->kind = event_child->waitstatus.kind;
> + ourstatus->value = event_child->waitstatus.value;
> +
> + /* Reset the event child's waitstatus since we handled it
> + already. */
> + event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
> + }
> + else
> + ourstatus->kind = TARGET_WAITKIND_STOPPED;
>
This chunk of code setting waitstatus.kind to TARGET_WAITKIND_IGNORE may
be a bit confusing/hackish, but we really don't want gdbserver's event
loop to handle the extended events as TARGET_WAITKIND_STOPPED.
gdbserver's event loop could probably use a cleanup . For the time
being, this looks good to me though.
> if (current_inferior->last_resume_kind == resume_stop
> && WSTOPSIG (w) == SIGSTOP)
> @@ -3014,13 +3145,14 @@ retry:
> ourstatus->value.sig = GDB_SIGNAL_0;
> }
> else if (current_inferior->last_resume_kind == resume_stop
> - && WSTOPSIG (w) != SIGSTOP)
> + && WSTOPSIG (w) != SIGSTOP
> + && !extended_event_reported (ourstatus))
> {
> /* A thread that has been requested to stop by GDB with vCont;t,
> but, it stopped for other reasons. */
> ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
> }
> - else
> + else if (ourstatus->kind == TARGET_WAITKIND_STOPPED)
> {
> ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
> }
> @@ -3126,6 +3258,7 @@ static void
> send_sigstop (struct lwp_info *lwp)
> {
> int pid;
> + int ret;
>
> pid = lwpid_of (get_lwp_thread (lwp));
>
> @@ -3143,7 +3276,21 @@ send_sigstop (struct lwp_info *lwp)
> debug_printf ("Sending sigstop to lwp %d\n", pid);
>
> lwp->stop_expected = 1;
> - kill_lwp (pid, SIGSTOP);
> + errno = 0;
> + ret = kill_lwp (pid, SIGSTOP);
> + if (ret == -1 && errno == ESRCH)
> + {
> + /* If the kill fails with "No such process", on GNU/Linux we know
> + that the LWP has vanished - it is not a zombie, it is gone.
> + This is due to a thread other than the thread group leader
> + calling exec. See comments in linux_low_filter_event regarding
> + PTRACE_EVENT_EXEC. */
> + delete_lwp (lwp);
> + set_desired_inferior (0);
> +
> + if (debug_threads)
> + debug_printf ("send_sigstop: lwp %d has vanished\n", pid);
> + }
> }
>
> static int
> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
> index 7459710..7759f01 100644
> --- a/gdb/gdbserver/linux-low.h
> +++ b/gdb/gdbserver/linux-low.h
> @@ -265,6 +265,11 @@ struct lwp_info
> /* When stopped is set, the last wait status recorded for this lwp. */
> int last_status;
>
> + /* If WAITSTATUS->KIND != TARGET_WAITKIND_IGNORE, the waitstatus for
> + this LWP's last event. This may correspond to LAST_STATUS above,
> + or to the current status during event processing. */
> + struct target_waitstatus waitstatus;
> +
> /* When stopped is set, this is where the lwp stopped, with
> decr_pc_after_break already accounted for. */
> CORE_ADDR stop_pc;
> diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
> index 4fcafa0..9ce25dc 100644
> --- a/gdb/gdbserver/remote-utils.c
> +++ b/gdb/gdbserver/remote-utils.c
> @@ -1111,14 +1111,40 @@ prepare_resume_reply (char *buf, ptid_t ptid,
> switch (status->kind)
> {
> case TARGET_WAITKIND_STOPPED:
> + case TARGET_WAITKIND_EXECD:
> {
> struct thread_info *saved_inferior;
> const char **regp;
> struct regcache *regcache;
> + enum gdb_signal signal;
> +
> + if (status->kind == TARGET_WAITKIND_EXECD)
> + signal = GDB_SIGNAL_TRAP;
> + else
> + signal = status->value.sig;
> +
> + sprintf (buf, "T%02x", signal);
>
> - sprintf (buf, "T%02x", status->value.sig);
> buf += strlen (buf);
>
> + if (status->kind == TARGET_WAITKIND_EXECD && multi_process)
> + {
> + const char *event = "exec";
> + char hexified_pathname[PATH_MAX];
> +
> + sprintf (buf, "%s:", event);
> + buf += strlen (buf);
> +
> + /* Encode pathname to hexified format. */
> + bin2hex ((const gdb_byte *) status->value.execd_pathname,
> + hexified_pathname, strlen(status->value.execd_pathname));
> +
> + sprintf (buf, "%s;", hexified_pathname);
> + xfree (status->value.execd_pathname);
> + status->value.execd_pathname = NULL;
> + buf += strlen (buf);
> + }
> +
> saved_inferior = current_inferior;
>
> current_inferior = find_thread_ptid (ptid);
> diff --git a/gdb/remote.c b/gdb/remote.c
> index be8c423..f4412d8 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -5542,11 +5542,13 @@ remote_parse_stop_reply (char *buf, struct stop_reply *event)
> pnum and set p1 to point to the character following it.
> Otherwise p1 points to p. */
>
> - /* If this packet is an awatch packet, don't parse the 'a'
> - as a register number. */
> + /* If this packet has a stop reason string that starts
> + with a character that could be a hex digit, don't parse
> + it as a register number. */
>
> if (strncmp (p, "awatch", strlen("awatch")) != 0
> - && strncmp (p, "core", strlen ("core") != 0))
> + && strncmp (p, "core", strlen ("core") != 0)
> + && strncmp (p, "exec", strlen ("exec") != 0))
> {
> /* Read the ``P'' register number. */
> pnum = strtol (p, &p_temp, 16);
> @@ -5598,6 +5600,25 @@ Packet: '%s'\n"),
> p = unpack_varlen_hex (++p1, &c);
> event->core = c;
> }
> + else if (strncmp (p, "exec", p1 - p) == 0)
> + {
> + ULONGEST pid;
> + char pathname[PATH_MAX];
> +
> + p = unpack_varlen_hex (++p1, &pid);
> +
> + /* Save the pathname for event reporting and for
> + the next run command. */
> + hex2bin (p1, (gdb_byte *) pathname, (p - p1)/2);
> + /* Add the null terminator. */
> + pathname[(p - p1)/2] = '\0';
> + /* This is freed during event handling. */
> + event->ws.value.execd_pathname = xstrdup (pathname);
> + event->ws.kind = TARGET_WAITKIND_EXECD;
> + /* Save the pathname for the next run command. */
> + xfree (remote_exec_file);
> + remote_exec_file = pathname;
> + }
> else
> {
> /* Silently skip unknown optional info. */
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/4][REPOST] Remote Linux ptrace exit events
2014-04-30 19:18 ` [PATCH 1/4][REPOST] Remote Linux ptrace exit events Don Breazeal
2014-05-07 19:40 ` Luis Machado
@ 2014-05-08 5:23 ` Yao Qi
2014-05-09 21:03 ` Breazeal, Don
2014-05-21 15:15 ` Tom Tromey
2 siblings, 1 reply; 33+ messages in thread
From: Yao Qi @ 2014-05-08 5:23 UTC (permalink / raw)
To: Don Breazeal, gdb-patches; +Cc: Don Breazeal
Don,
I am not the right person to review this patch series and approve them,
but I can do a primary review. Hope it helps to relieve feeling of the
long time review :)
On 05/01/2014 03:17 AM, Don Breazeal wrote:
> This patch implements support for the extended ptrace event
> PTRACE_EVENT_EXIT on Linux. This is a preparatory patch for exec event
> support.
>
> The use of this event is entirely internal to gdbserver; these events are
> not reported to GDB or the user. If an existing thread is not the last
> thread in a process, its lwp entry is simply deleted, since this is what
> happens in the absence of exit events. If it is the last thread of a
> process, the wait status is set to the actual wait status of the thread,
> instead of the status that indicates the extended event, and the existing
> mechanisms for handling thread exit proceed as usual.
>
> The only purpose in using the exit events instead of the existing wait
> mechanisms is to ensure that the exit of a thread group leader is detected
> reliably when a non-leader thread calls exec.
Can you elaborate a little please? why existing wait mechanism is not
reliable?
> gdb/common/linux-ptrace.c | 42 ++++++++++++++++++-
> gdb/gdbserver/linux-low.c | 99 +++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 125 insertions(+), 16 deletions(-)
>
> diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
> index 7c1b78a..e3fc705 100644
> --- a/gdb/common/linux-ptrace.c
> +++ b/gdb/common/linux-ptrace.c
> @@ -414,7 +414,7 @@ linux_test_for_tracefork (int child_pid)
> ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
> (PTRACE_TYPE_ARG4) 0);
> if (ret != 0)
> - warning (_("linux_test_for_tracefork: failed to resume child"));
> + warning (_("linux_test_for_tracefork: failed to resume child (a)"));
>
> ret = my_waitpid (child_pid, &status, 0);
>
> @@ -455,10 +455,46 @@ linux_test_for_tracefork (int child_pid)
> "failed to kill second child"));
> my_waitpid (second_pid, &status, 0);
> }
> + else
> + {
> + /* Fork events are not supported. */
> + return;
> + }
> }
> else
> - warning (_("linux_test_for_tracefork: unexpected result from waitpid "
> - "(%d, status 0x%x)"), ret, status);
> + {
> + warning (_("linux_test_for_tracefork: unexpected result from waitpid "
> + "(%d, status 0x%x)"), ret, status);
> + return;
> + }
> +
> +#ifdef GDBSERVER
> + /* PTRACE_O_FORK is supported, so now test for PTRACE_O_TRACEEXIT.
> + First try to set the option. If this fails, we know for sure that
> + it is not supported. */
> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> + (PTRACE_TYPE_ARG4) PTRACE_O_TRACEEXIT);
> + if (ret != 0)
> + return;
> +
> + /* We don't know for sure that the feature is available; old
> + versions of PTRACE_SETOPTIONS ignored unknown options. So
> + see if the process exit will generate a PTRACE_EVENT_EXIT. */
> + ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
> + (PTRACE_TYPE_ARG4) 0);
> + if (ret != 0)
> + warning (_("linux_test_for_tracefork: failed to resume child (b)"));
> +
> + ret = my_waitpid (child_pid, &status, 0);
> +
> + /* Check if we received an exit event notification. */
> + if (ret == child_pid && WIFSTOPPED (status)
> + && status >> 16 == PTRACE_EVENT_EXIT)
> + {
> + /* PTRACE_O_TRACEEXIT is supported. */
> + current_ptrace_options |= PTRACE_O_TRACEEXIT;
> + }
> +#endif
> }
>
The code looks clear to me, but it is strange to put them in function
linux_test_for_tracefork. IWBN to move them to a new function.
> /* Enable reporting of all currently supported ptrace events. */
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 2d8d5f5..90e7b15 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -224,6 +224,7 @@ static void proceed_all_lwps (void);
> static int finish_step_over (struct lwp_info *lwp);
> static CORE_ADDR get_stop_pc (struct lwp_info *lwp);
> static int kill_lwp (unsigned long lwpid, int signo);
> +static int num_lwps (int pid);
>
> /* True if the low target can hardware single-step. Such targets
> don't need a BREAKPOINT_REINSERT_ADDR callback. */
> @@ -367,13 +368,32 @@ linux_add_process (int pid, int attached)
> return proc;
> }
>
> +/* Check wait status for extended event */
Period and one space is missing at the end of the comment.
> +
> +static int
> +is_extended_waitstatus (int wstat)
> +{
> + return wstat >> 16 != 0;
^^^^
Two spaces instead of four.
> +}
> +
> +/* Check wait status for extended event */
Likewise.
> +
> +static int
> +is_extended_exit (int wstat)
> +{
> + return wstat >> 16 == PTRACE_EVENT_EXIT;
^^^^
Likewise.
> +}
> +
> /* Handle a GNU/Linux extended wait response. If we see a clone
> event, we need to add the new LWP to our list (and not report the
I know it is not about your change, but better to replace LWP with
EVENT_CHILD. We also need to update it for the exit event.
> - trap to higher layers). */
> + trap to higher layers). This function returns non-zero if the
> + event should be ignored and we should wait again. The wait status
> + in WSTATP may be modified if an exit event occurred. */
>
> -static void
> -handle_extended_wait (struct lwp_info *event_child, int wstat)
> +static int
> +handle_extended_wait (struct lwp_info *event_child, int *wstatp)
> {
> + int wstat = *wstatp;
> int event = wstat >> 16;
> struct thread_info *event_thr = get_lwp_thread (event_child);
> struct lwp_info *new_lwp;
> @@ -448,11 +468,54 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
> linux_resume_one_lwp (new_lwp, 0, WSTOPSIG (status), NULL);
> }
>
> + /* Enable extended events for the new thread. */
> + linux_enable_event_reporting (new_pid);
> +
This change isn't mentioned in ChangeLog entry. I also wonder why is it
needed here.
> /* Always resume the current thread. If we are stopping
> threads, it will have a pending SIGSTOP; we may as well
> collect it now. */
> linux_resume_one_lwp (event_child, event_child->stepping, 0, NULL);
> +
> + /* Don't report the event. */
> + return 1;
> + }
> + else if (event == PTRACE_EVENT_EXIT)
> + {
> + unsigned long exit_status;
> + unsigned long lwpid = lwpid_of (event_thr);
> + int ret;
> +
> + if (debug_threads)
> + debug_printf ("LHEW: Got exit event from LWP %ld\n",
s/LHEW/HEW/
> + lwpid_of (event_thr));
> +
> + ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
> + &exit_status);
> +
> + if (num_lwps (pid_of (event_thr)) > 1)
> + {
> + /* If there is at least one more LWP, then the program still
> + exists and the exit should not be reported to GDB. */
> + delete_lwp (event_child);
> + ret = 1;
> + }
> + else
> + {
> + /* Set the exit status to the actual exit status, so normal
> + WIFEXITED/WIFSIGNALLED processing and reporting for the
> + last lwp in the process can proceed from here. */
> + *wstatp = exit_status;
> + ret = 0;
> + }
> +
> + /* Resume the thread so that it actually exits. Subsequent exit
> + events for LWPs that were deleted above will be ignored. */
> + ptrace (PTRACE_CONT, lwpid, (PTRACE_TYPE_ARG3) 0,
> + (PTRACE_TYPE_ARG4) 0);
> + return ret;
> }
> + internal_error (__FILE__, __LINE__,
> + _("unknown ptrace event %d"), event);
This should be mentioned in ChangeLog too.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/4][REPOST] Document RSP support for Linux exec events
2014-04-30 19:18 ` [PATCH 3/4][REPOST] Document RSP support for Linux " Don Breazeal
@ 2014-05-08 5:34 ` Yao Qi
0 siblings, 0 replies; 33+ messages in thread
From: Yao Qi @ 2014-05-08 5:34 UTC (permalink / raw)
To: Don Breazeal, gdb-patches; +Cc: Don Breazeal
On 05/01/2014 03:18 AM, Don Breazeal wrote:
> This patch adds documentation of the new RSP support for exec events.
The RSP change is user-visible, so we need an entry in NEWS for it, like
* New remote packets
Extend stop reasons in stop rely packets for exec event.
This patch series implements the support for exec events in gdbserver
on GNU/Linux, which should be mentioned in NEWS too, IMO.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/4][REPOST] Remote Linux ptrace exec events
2014-04-30 19:18 ` [PATCH 2/4][REPOST] Remote Linux ptrace exec events Don Breazeal
2014-05-07 20:01 ` Luis Machado
@ 2014-05-08 5:44 ` Yao Qi
2014-05-21 15:28 ` Tom Tromey
2 siblings, 0 replies; 33+ messages in thread
From: Yao Qi @ 2014-05-08 5:44 UTC (permalink / raw)
To: Don Breazeal, gdb-patches; +Cc: Don Breazeal
I don't have much based on Luis's comments. Two nits on changelog:
On 05/01/2014 03:18 AM, Don Breazeal wrote:
> gdbserver/
> 2014-04-02 Don Breazeal <donb@codesourcery.com>
>
> * gdb/gdbserver/linux-low.c (linux_child_pid_to_exec_file): New
^^^^^^^^^^^^^
There is a ChangeLog file in gdb/gdbserver directory, so don't need to
have "gdb/gdbserver/" in file name. "* linux-low.c (...." should be fine.
> function.
> (handle_extended_wait): Add support for PTRACE_EVENT_EXEC.
> (check_zombie_leaders): Update comment.
> (linux_low_filter_event): Add support for PTRACE_EVENT_EXEC.
> (linux_wait_for_event_filtered): Update comment.
> (extended_event_reported): New function.
> (send_sigstop): Delete lwp on 'No such process' error and
> reset current_inferior.
> * gdb/gdbserver/linux-low.h (struct lwp_info): New member
> 'waitstatus'.
We document new member with angle brackets, like
* linux-low.h (struct lwp_info) <waitstatus>: New member.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/4][REPOST] Remote Linux ptrace exit events
2014-05-08 5:23 ` Yao Qi
@ 2014-05-09 21:03 ` Breazeal, Don
0 siblings, 0 replies; 33+ messages in thread
From: Breazeal, Don @ 2014-05-09 21:03 UTC (permalink / raw)
To: Yao Qi, gdb-patches; +Cc: Don Breazeal
Hi Yao,
Thanks for the review.
On 5/7/2014 10:20 PM, Yao Qi wrote:
> Don,
> I am not the right person to review this patch series and approve them,
> but I can do a primary review. Hope it helps to relieve feeling of the
> long time review :)
>
> On 05/01/2014 03:17 AM, Don Breazeal wrote:
>> This patch implements support for the extended ptrace event
>> PTRACE_EVENT_EXIT on Linux. This is a preparatory patch for exec event
>> support.
>>
>> The use of this event is entirely internal to gdbserver; these events are
>> not reported to GDB or the user. If an existing thread is not the last
>> thread in a process, its lwp entry is simply deleted, since this is what
>> happens in the absence of exit events. If it is the last thread of a
>> process, the wait status is set to the actual wait status of the thread,
>> instead of the status that indicates the extended event, and the existing
>> mechanisms for handling thread exit proceed as usual.
>>
>> The only purpose in using the exit events instead of the existing wait
>> mechanisms is to ensure that the exit of a thread group leader is detected
>> reliably when a non-leader thread calls exec.
>
> Can you elaborate a little please? why existing wait mechanism is not
> reliable?
I'm working on a write-up that I will include in the next version of the
patch. The short version is that there is a race condition in the
current implementation that can leave a dangling entry in the lwp list
(an entry that doesn't have a corresponding actual lwp). In this case
gdbserver will hang waiting for the non-existent lwp to stop. Using the
exit events eliminates this race condition.
>
>> gdb/common/linux-ptrace.c | 42 ++++++++++++++++++-
>> gdb/gdbserver/linux-low.c | 99 +++++++++++++++++++++++++++++++++++++++------
>> 2 files changed, 125 insertions(+), 16 deletions(-)
>>
>> diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
>> index 7c1b78a..e3fc705 100644
>> --- a/gdb/common/linux-ptrace.c
>> +++ b/gdb/common/linux-ptrace.c
>> @@ -414,7 +414,7 @@ linux_test_for_tracefork (int child_pid)
>> ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
>> (PTRACE_TYPE_ARG4) 0);
>> if (ret != 0)
>> - warning (_("linux_test_for_tracefork: failed to resume child"));
>> + warning (_("linux_test_for_tracefork: failed to resume child (a)"));
>>
>> ret = my_waitpid (child_pid, &status, 0);
>>
>> @@ -455,10 +455,46 @@ linux_test_for_tracefork (int child_pid)
>> "failed to kill second child"));
>> my_waitpid (second_pid, &status, 0);
>> }
>> + else
>> + {
>> + /* Fork events are not supported. */
>> + return;
>> + }
>> }
>> else
>> - warning (_("linux_test_for_tracefork: unexpected result from waitpid "
>> - "(%d, status 0x%x)"), ret, status);
>> + {
>> + warning (_("linux_test_for_tracefork: unexpected result from waitpid "
>> + "(%d, status 0x%x)"), ret, status);
>> + return;
>> + }
>> +
>> +#ifdef GDBSERVER
>> + /* PTRACE_O_FORK is supported, so now test for PTRACE_O_TRACEEXIT.
>> + First try to set the option. If this fails, we know for sure that
>> + it is not supported. */
>> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
>> + (PTRACE_TYPE_ARG4) PTRACE_O_TRACEEXIT);
>> + if (ret != 0)
>> + return;
>> +
>> + /* We don't know for sure that the feature is available; old
>> + versions of PTRACE_SETOPTIONS ignored unknown options. So
>> + see if the process exit will generate a PTRACE_EVENT_EXIT. */
>> + ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
>> + (PTRACE_TYPE_ARG4) 0);
>> + if (ret != 0)
>> + warning (_("linux_test_for_tracefork: failed to resume child (b)"));
>> +
>> + ret = my_waitpid (child_pid, &status, 0);
>> +
>> + /* Check if we received an exit event notification. */
>> + if (ret == child_pid && WIFSTOPPED (status)
>> + && status >> 16 == PTRACE_EVENT_EXIT)
>> + {
>> + /* PTRACE_O_TRACEEXIT is supported. */
>> + current_ptrace_options |= PTRACE_O_TRACEEXIT;
>> + }
>> +#endif
>> }
>>
>
> The code looks clear to me, but it is strange to put them in function
> linux_test_for_tracefork. IWBN to move them to a new function.
I had gone back and forth on this issue. I will make a new function for
this.
>
>> /* Enable reporting of all currently supported ptrace events. */
>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
>> index 2d8d5f5..90e7b15 100644
>> --- a/gdb/gdbserver/linux-low.c
>> +++ b/gdb/gdbserver/linux-low.c
>> @@ -224,6 +224,7 @@ static void proceed_all_lwps (void);
>> static int finish_step_over (struct lwp_info *lwp);
>> static CORE_ADDR get_stop_pc (struct lwp_info *lwp);
>> static int kill_lwp (unsigned long lwpid, int signo);
>> +static int num_lwps (int pid);
>>
>> /* True if the low target can hardware single-step. Such targets
>> don't need a BREAKPOINT_REINSERT_ADDR callback. */
>> @@ -367,13 +368,32 @@ linux_add_process (int pid, int attached)
>> return proc;
>> }
>>
>> +/* Check wait status for extended event */
>
> Period and one space is missing at the end of the comment.
I will clean up everything you mentioned from here to the bottom.
thanks
--Don
>
>> +
>> +static int
>> +is_extended_waitstatus (int wstat)
>> +{
>> + return wstat >> 16 != 0;
> ^^^^
> Two spaces instead of four.
>
>> +}
>> +
>> +/* Check wait status for extended event */
>
> Likewise.
>
>> +
>> +static int
>> +is_extended_exit (int wstat)
>> +{
>> + return wstat >> 16 == PTRACE_EVENT_EXIT;
> ^^^^
> Likewise.
>
>> +}
>> +
>> /* Handle a GNU/Linux extended wait response. If we see a clone
>> event, we need to add the new LWP to our list (and not report the
>
> I know it is not about your change, but better to replace LWP with
> EVENT_CHILD. We also need to update it for the exit event.
>
>> - trap to higher layers). */
>> + trap to higher layers). This function returns non-zero if the
>> + event should be ignored and we should wait again. The wait status
>> + in WSTATP may be modified if an exit event occurred. */
>>
>> -static void
>> -handle_extended_wait (struct lwp_info *event_child, int wstat)
>> +static int
>> +handle_extended_wait (struct lwp_info *event_child, int *wstatp)
>> {
>> + int wstat = *wstatp;
>> int event = wstat >> 16;
>> struct thread_info *event_thr = get_lwp_thread (event_child);
>> struct lwp_info *new_lwp;
>> @@ -448,11 +468,54 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
>> linux_resume_one_lwp (new_lwp, 0, WSTOPSIG (status), NULL);
>> }
>>
>> + /* Enable extended events for the new thread. */
>> + linux_enable_event_reporting (new_pid);
>> +
>
> This change isn't mentioned in ChangeLog entry. I also wonder why is it
> needed here.
>
>> /* Always resume the current thread. If we are stopping
>> threads, it will have a pending SIGSTOP; we may as well
>> collect it now. */
>> linux_resume_one_lwp (event_child, event_child->stepping, 0, NULL);
>> +
>> + /* Don't report the event. */
>> + return 1;
>> + }
>> + else if (event == PTRACE_EVENT_EXIT)
>> + {
>> + unsigned long exit_status;
>> + unsigned long lwpid = lwpid_of (event_thr);
>> + int ret;
>> +
>> + if (debug_threads)
>> + debug_printf ("LHEW: Got exit event from LWP %ld\n",
>
> s/LHEW/HEW/
>
>> + lwpid_of (event_thr));
>> +
>> + ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
>> + &exit_status);
>> +
>> + if (num_lwps (pid_of (event_thr)) > 1)
>> + {
>> + /* If there is at least one more LWP, then the program still
>> + exists and the exit should not be reported to GDB. */
>> + delete_lwp (event_child);
>> + ret = 1;
>> + }
>> + else
>> + {
>> + /* Set the exit status to the actual exit status, so normal
>> + WIFEXITED/WIFSIGNALLED processing and reporting for the
>> + last lwp in the process can proceed from here. */
>> + *wstatp = exit_status;
>> + ret = 0;
>> + }
>> +
>> + /* Resume the thread so that it actually exits. Subsequent exit
>> + events for LWPs that were deleted above will be ignored. */
>> + ptrace (PTRACE_CONT, lwpid, (PTRACE_TYPE_ARG3) 0,
>> + (PTRACE_TYPE_ARG4) 0);
>> + return ret;
>> }
>> + internal_error (__FILE__, __LINE__,
>> + _("unknown ptrace event %d"), event);
>
> This should be mentioned in ChangeLog too.
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/4][REPOST] Remote Linux ptrace exec events
2014-05-07 20:01 ` Luis Machado
@ 2014-05-09 21:17 ` Breazeal, Don
0 siblings, 0 replies; 33+ messages in thread
From: Breazeal, Don @ 2014-05-09 21:17 UTC (permalink / raw)
To: lgustavo, gdb-patches
Hi Luis,
Thanks for the review.
On 5/7/2014 1:01 PM, Luis Machado wrote:
> Hi,
>
> On 04/30/2014 04:18 PM, Don Breazeal wrote:
>> From: Don Breazeal <don_breazeal@mentor.com>
>>
>> [Reposting to eliminate unexpected attachment type.]
>>
>> This patch implements support for exec events in gdbserver on linux, in
>> multiprocess mode (target extended-remote). Follow-exec-mode and rerun
>> behave as expected. Catchpoints for exec are not yet implemented
>> since it
>> will be easier to implement catchpoints for fork, vfork, and exec all at
>> the same time.
>>
>> TESTING
>> ---------
>> The patch was tested on GNU/Linux x86_64 with --target_board set to
>> native-gdbserver and native-extended-gdbserver, as well as testing native
>> GDB. The test results for native-gdbserver were unchanged. Thirteen
>> tests
>> that used to fail for native-extended-gdbserver on Linux pass with this
>> patch, and the non-ldr-exc-*.exp tests all pass in non-stop mode and
>> extended-remote. There are several failures in the new non-ldr-exc-*.exp
>> tests in non-stop mode with native GDB.
>>
>> One caveat: when an exec is detected, gdbserver emits a couple of
>> warnings:
>> gdbserver: unexpected r_debug version 0
>> gdbserver: unexpected r_debug version 0
>> However, debugging of shared libraries that are loaded by the exec'd
>> program works just fine. These messages may be caused by gdbserver
>> making
>> an attempt to initialize the solib hook before the r_debug structure has
>> been initialized. I intend to follow up in a subsequent patch.
>>
>> IMPLEMENTATION
>> ----------------
>> Support for exec events in single-threaded programs was a fairly
>> straightforward replication of the implementation in native GDB:
>> 1) Enable exec events via ptrace options.
>> 2) Add support for handling the exec events to the
>> handle_extended_wait
>> and linux_wait_for_event_filtered. Detect the exec event, then
>> find and save the pathname of the executable file being exec'd.
>> 3) Implement an additional "stop reason", "exec", in the RSP stop
>> reply
>> packet "T".
>> Existing GDB code takes care of handling the exec event on the host side
>> without modification.
>>
>> Support for exec events in multi-threaded programs required some
>> additional
>> work that required a couple of significant changes to existing code.
>> In a
>> nutshell, the changes are to:
>> 4) Use the PTRACE_EVENT_EXIT extended event to handle thread exit,
>> while not exposing any change in exit handling to the user. The
>> rationale for this is discussed in the "patch 0" email of this
>> series.
>> 5) Recognize when the exec'ing thread has vanished (become the thread
>> group leader) in send_sigstop. Native GDB does this differently.
>>
>> Regarding items 4 & 5: Recall that when a non-leader thread exec's,
>> all the
>> other threads are terminated and the exec'ing thread changes its
>> thread id
>> to that of the old leader (the process id) as part of the exec. There is
>> no event reported for the "exit" of the exec'ing thread; it appears to
>> have
>> vanished. The original thread group leader can't be reaped until all the
>> other threads have been reaped, and some way of determining that it has
>> exited is required in order to update the lwp list (#4 above). Also, some
>> mechanism for deleting the lwp entry corresponding to the exec'ing thread
>> is needed (#5 above).
>>
>> The rationale for #4 is that in my testing I ran into a race condition in
>> the mechanism that's intended to detect when a thread group leader has
>> exited, check_zombie_leaders. The race occurred when there were only two
>> threads in the program. In this case the leader thread passes through a
>> very brief zombie state before being replaced by the exec'ing thread as
>> the thread group leader. This state transition is asynchronous, with no
>> dependency on anything gdbserver does. Using PTRACE_EVENT_EXIT ensures
>> that the leader exit will be detected. I can provide a more detailed
>> explanation or the race, but I didn't want to be too long-winded here.
>>
>> Regarding item #5, determining that the exec'ing thread has
>> "vanished", In
>> native GDB this is done by calling waitpid(PID), and if it returns ECHILD
>> it means that the thread is gone. We don't want to use waitpid(PID) in
>> gdbserver, based on the discussion in
>> https://www.sourceware.org/ml/gdb-patches/2014-02/msg00828.html. An
>> alternative is to send a signal to each thread and look for an ESRCH (No
>> such process) error. In all-stop mode this can be done in the normal
>> course of events, since when gdbserver reports an exec event it stops all
>> the other threads with a SIGSTOP. In non-stop mode, when an exec
>> event has
>> been detected, we can call stop_all_lwps/unstop_all_lwps to accomplish
>> the
>> same thing.
>>
>> gdb/
>> 2014-04-02 Don Breazeal <donb@codesourcery.com>
>>
>> * common/linux-ptrace.c (linux_test_for_tracefork)
>> [GDBSERVER]: Add exec tracing to ptrace options if OS supports
>> it.
>> * remote.c (remote_parse_stop_reply): Support new RSP stop
>> reply reason 'exec'.
>>
>> gdbserver/
>> 2014-04-02 Don Breazeal <donb@codesourcery.com>
>>
>> * gdb/gdbserver/linux-low.c (linux_child_pid_to_exec_file): New
>> function.
>> (handle_extended_wait): Add support for PTRACE_EVENT_EXEC.
>> (check_zombie_leaders): Update comment.
>> (linux_low_filter_event): Add support for PTRACE_EVENT_EXEC.
>> (linux_wait_for_event_filtered): Update comment.
>> (extended_event_reported): New function.
>> (send_sigstop): Delete lwp on 'No such process' error and
>> reset current_inferior.
>> * gdb/gdbserver/linux-low.h (struct lwp_info): New member
>> 'waitstatus'.
>> * gdb/gdbserver/remote-utils.c (prepare_resume_reply): Support
>> new RSP stop reply reason 'exec'.
>>
>> ---
>> gdb/common/linux-ptrace.c | 7 +-
>> gdb/gdbserver/linux-low.c | 175
>> ++++++++++++++++++++++++++++++++++++++----
>> gdb/gdbserver/linux-low.h | 5 +
>> gdb/gdbserver/remote-utils.c | 28 +++++++-
>> gdb/remote.c | 27 ++++++-
>> 5 files changed, 222 insertions(+), 20 deletions(-)
>>
>> diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
>> index e3fc705..b137df9 100644
>> --- a/gdb/common/linux-ptrace.c
>> +++ b/gdb/common/linux-ptrace.c
>> @@ -491,8 +491,11 @@ linux_test_for_tracefork (int child_pid)
>> if (ret == child_pid && WIFSTOPPED (status)
>> && status >> 16 == PTRACE_EVENT_EXIT)
>> {
>> - /* PTRACE_O_TRACEEXIT is supported. */
>> - current_ptrace_options |= PTRACE_O_TRACEEXIT;
>> + /* PTRACE_O_TRACEEXIT is supported. We use exit events to
>> + implement support for exec events. Since fork events are
>> + supported we know exec events are supported, so we enable
>> + exec events here. */
>> + current_ptrace_options |= PTRACE_O_TRACEEXIT | PTRACE_O_TRACEEXEC;
>> }
>> #endif
>> }
>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
>> index 90e7b15..5f94490 100644
>> --- a/gdb/gdbserver/linux-low.c
>> +++ b/gdb/gdbserver/linux-low.c
>> @@ -281,6 +281,32 @@ static int linux_event_pipe[2] = { -1, -1 };
>> static void send_sigstop (struct lwp_info *lwp);
>> static void wait_for_sigstop (void);
>>
>> +/* Accepts an integer PID. Returns a string containing the
>> + name of the executable file for the child process.
>> + Space for the result is malloc'd, caller must free. */
>> +
>> +static char *
>> +linux_child_pid_to_exec_file (int pid)
>> +{
>> + char *name1, *name2;
>> +
>> + name1 = xmalloc (PATH_MAX);
>> + name2 = xmalloc (PATH_MAX);
>> + memset (name2, 0, PATH_MAX);
>> +
>> + sprintf (name1, "/proc/%d/exe", pid);
>> + if (readlink (name1, name2, PATH_MAX) > 0)
>> + {
>> + free (name1);
>> + return name2;
>> + }
>> + else
>> + {
>> + free (name2);
>> + return name1;
>> + }
>> +}
>> +
>
> At one point in time this function existed in gdbserver/linux-low.c but
> got deleted by the following commit:
>
> commit be07f1a20c962deb662b93209b4ca91bc8e5cbd8
> Author: Pedro Alves <palves@redhat.com>
> Date: Fri Jan 27 19:23:43 2012 +0000
>
> 2012-01-27 Pedro Alves <palves@redhat.com>
>
> * linux-low.c (linux_child_pid_to_exec_file): Delete.
> (elf_64_file_p): Make static.
> (linux_pid_exe_is_elf_64_file): New.
> * linux-low.h (linux_child_pid_to_exec_file, elf_64_file_p):
> Delete declarations.
> (linux_pid_exe_is_elf_64_file): Declare.
> * linux-x86-low.c (x86_arch_setup): Use
> linux_pid_exe_is_elf_64_file.
>
> It still exists in linux-nat.c though. I remember i tried to unify those
> and move the resulting function to common/, but there may be differences
> preventing such move.
>
> If it can be done, it would be best.
I am working on doing this. It seems like it should be possible. One
question: the version of the function in linux-nat.c takes a "struct
target_ops *" argument, I assume because it is used in the target_ops
function vector. Is it acceptable to include "target.h" in a common
file? It will pick up different files for GDB and GDBserver. I'm not
sure if that would violate some aspect of the design. common/btrace.c
includes it, but it's the only common file that does.
>
>> /* Return non-zero if HEADER is a 64-bit ELF file. */
>>
>> static int
>> @@ -514,6 +540,19 @@ handle_extended_wait (struct lwp_info
>> *event_child, int *wstatp)
>> (PTRACE_TYPE_ARG4) 0);
>> return ret;
>> }
>> + else if (event == PTRACE_EVENT_EXEC)
>> + {
>> + if (debug_threads)
>> + debug_printf ("LHEW: Got exec event from LWP %ld\n",
>> + lwpid_of (event_thr));
>> +
>> + event_child->waitstatus.kind = TARGET_WAITKIND_EXECD;
>> + event_child->waitstatus.value.execd_pathname
>> + = linux_child_pid_to_exec_file (lwpid_of (event_thr));
>> +
>> + /* Report the event. */
>> + return 0;
>> + }
>> internal_error (__FILE__, __LINE__,
>> _("unknown ptrace event %d"), event);
>> }
>> @@ -1376,18 +1415,19 @@ check_zombie_leaders (void)
>> program). In the latter case, we can't waitpid the
>> leader's exit status until all other threads are gone.
>>
>> - - There are 3 or more threads in the group, and a thread
>> + - There are multiple threads in the group, and a thread
>> other than the leader exec'd. On an exec, the Linux
>> kernel destroys all other threads (except the execing
>> one) in the thread group, and resets the execing thread's
>> tid to the tgid. No exit notification is sent for the
>> execing thread -- from the ptracer's perspective, it
>> appears as though the execing thread just vanishes.
>> - Until we reap all other threads except the leader and the
>> - execing thread, the leader will be zombie, and the
>> - execing thread will be in `D (disc sleep)'. As soon as
>> - all other threads are reaped, the execing thread changes
>> - it's tid to the tgid, and the previous (zombie) leader
>> + Until we reap all other threads (if any) except the
>> + leader and the execing thread, the leader will be zombie,
>> + and the execing thread will be in `D (disc sleep)'. As
>> + soon as all other threads are reaped, or have reported
>> + PTRACE_EVENT_EXIT events, the execing thread changes its
>> + tid to the tgid, and the previous (zombie) leader
>> vanishes, giving place to the "new" leader. We could try
>> distinguishing the exit and exec cases, by waiting once
>> more, and seeing if something comes out, but it doesn't
>> @@ -1395,7 +1435,11 @@ check_zombie_leaders (void)
>> we'll re-add the new one once we see the exec event
>> (which is just the same as what would happen if the
>> previous leader did exit voluntarily before some other
>> - thread execs). */
>> + thread execs).
>> +
>> + Note that when PTRACE_EVENT_EXEC is supported, we use
>> + that mechanism to detect thread exit, including the
>> + exit of zombie leaders. */
>>
>> if (debug_threads)
>> fprintf (stderr,
>> @@ -1791,6 +1835,57 @@ linux_low_filter_event (ptid_t filter_ptid, int
>> lwpid, int *wstatp)
>>
>> child = find_lwp_pid (pid_to_ptid (lwpid));
>>
>> + /* Check for stop events reported by a process we didn't already
>> + know about - anything not already in our LWP list.
>> +
>> + If we're expecting to receive stopped processes after
>> + fork, vfork, and clone events, then we'll just add the
>> + new one to our list and go back to waiting for the event
>> + to be reported - the stopped process might be returned
>> + from waitpid before or after the event is.
>> +
>> + But note the case of a non-leader thread exec'ing after the
>> + leader having exited, and gone from our lists. On an exec,
>> + the Linux kernel destroys all other threads (except the execing
>> + one) in the thread group, and resets the execing thread's tid
>> + to the tgid. No exit notification is sent for the execing
>> + thread -- from the ptracer's perspective, it appears as though
>> + the execing thread just vanishes. When they are available, we
>> + use exit events (PTRACE_EVENT_EXIT) to detect thread exit
>> + reliably. As soon as all other threads (if any) are reaped or
>> + have reported their PTRACE_EVENT_EXIT events, the execing
>> + thread changes it's tid to the tgid, and the previous (zombie)
>> + leader vanishes, giving place to the "new" leader. The lwp
>> + entry for the previous leader is deleted when we handle its
>> + exit event, and we re-add the new one here. */
>> +
>> + if (WIFSTOPPED (wstat) && child == NULL
>> + && (WSTOPSIG (wstat) == SIGTRAP && wstat >> 16 ==
>> PTRACE_EVENT_EXEC))
>> + {
>> + ptid_t child_ptid;
>> +
>
> It would be nice to replace the shift operation with a predicate to test
> for PTRACE_EVENT_EXEC, just like you did for PTRACE_EVENT_EXIT.
Will do. Actually, given your previous comment, I intend to move all of
these predicates into common/linux-ptrace.[ch].
>
>> + /* A multi-thread exec after we had seen the leader exiting. */
>> + if (debug_threads)
>> + debug_printf ("LLW: Re-adding thread group leader LWP %d.\n",
>> + lwpid);
>> +
>> + child_ptid = ptid_build (lwpid, lwpid, 0);
>> + child = add_lwp (child_ptid);
>> + child->stopped = 1;
>> + current_inferior = child->thread;
>> +
>> + if (non_stop && stopping_threads == NOT_STOPPING_THREADS)
>> + {
>> + /* Make sure we delete the lwp entry for the exec'ing thread,
>> + which will have vanished. We do this by sending a signal
>> + to all the other threads in the lwp list, deleting any
>> + that are not found. Note that in all-stop mode this will
>> + happen before reporting the event. */
>> + stop_all_lwps (0, child);
>> + unstop_all_lwps (0, child);
>> + }
>> + }
>> +
>> /* If we didn't find a process, one of two things presumably
>> happened:
>> - A process we started and then detached from has exited.
>> Ignore it.
>> - A process we are controlling has forked and the new child's stop
>> @@ -2122,8 +2217,7 @@ linux_wait_for_event_filtered (ptid_t wait_ptid,
>> ptid_t filter_ptid,
>> - When a non-leader thread execs, that thread just vanishes
>> without reporting an exit (so we'd hang if we waited for it
>> explicitly in that case). The exec event is reported to
>> - the TGID pid (although we don't currently enable exec
>> - events). */
>> + the TGID pid. */
>> errno = 0;
>> ret = my_waitpid (-1, wstatp, options | WNOHANG);
>>
>> @@ -2520,6 +2614,21 @@ linux_stabilize_threads (void)
>> }
>> }
>>
>> +/* Return non-zero if WAITSTATUS reflects an extended linux
>> + event. Otherwise, return 0. Note that extended EXIT
>> + events are fixed up and handled like normal events, so
>> + they are not considered here. */
>> +
>> +static int
>> +extended_event_reported (const struct target_waitstatus *waitstatus)
>> +{
>> +
>> + if (waitstatus == NULL)
>> + return 0;
>> +
>> + return waitstatus->kind == TARGET_WAITKIND_EXECD;
>> +}
>> +
>
> Someone mentioned putting ()'s around the condition for better
> readability. Though not required, i also agree that it improves things.
>
> Not a requirement, just a suggestion.
I'll do it.
>
>> /* Wait for process, returns status. */
>>
>> static ptid_t
>> @@ -2883,7 +2992,8 @@ retry:
>> && !bp_explains_trap && !trace_event)
>> || (gdb_breakpoint_here (event_child->stop_pc)
>> && gdb_condition_true_at_breakpoint
>> (event_child->stop_pc)
>> - && gdb_no_commands_at_breakpoint
>> (event_child->stop_pc)));
>> + && gdb_no_commands_at_breakpoint (event_child->stop_pc))
>> + || extended_event_reported (&event_child->waitstatus));
>>
>> run_breakpoint_commands (event_child->stop_pc);
>>
>> @@ -2905,6 +3015,15 @@ retry:
>> paddress (event_child->stop_pc),
>> paddress (event_child->step_range_start),
>> paddress (event_child->step_range_end));
>> + if (debug_threads
>> + && extended_event_reported (&event_child->waitstatus))
>> + {
>> + char *str = target_waitstatus_to_string (ourstatus);
>> + debug_printf ("LWP %ld has forked, cloned, vforked or execd"
>> + " with waitstatus %s\n",
>> + lwpid_of (get_lwp_thread (event_child)), str);
>> + xfree (str);
>> + }
>> }
>>
>
> Maybe we could provide more precise information about the event here
> instead of something generic? It may help debugging in the future or if
> we ever have a mix of these events happening very close to each other.
It turns out the string from target_waitstatus_to_string contains the
precise information. I'll clean this up.
>
>> /* We're not reporting this breakpoint to GDB, so apply the
>> @@ -3003,7 +3122,19 @@ retry:
>> unstop_all_lwps (1, event_child);
>> }
>>
>> - ourstatus->kind = TARGET_WAITKIND_STOPPED;
>> + if (extended_event_reported (&event_child->waitstatus))
>> + {
>> + /* If the reported event is a fork, vfork or exec, let GDB
>> + know. */
>> + ourstatus->kind = event_child->waitstatus.kind;
>> + ourstatus->value = event_child->waitstatus.value;
>> +
>> + /* Reset the event child's waitstatus since we handled it
>> + already. */
>> + event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
>> + }
>> + else
>> + ourstatus->kind = TARGET_WAITKIND_STOPPED;
>>
>
> This chunk of code setting waitstatus.kind to TARGET_WAITKIND_IGNORE may
> be a bit confusing/hackish, but we really don't want gdbserver's event
> loop to handle the extended events as TARGET_WAITKIND_STOPPED.
>
> gdbserver's event loop could probably use a cleanup . For the time
> being, this looks good to me though.
>
>> if (current_inferior->last_resume_kind == resume_stop
>> && WSTOPSIG (w) == SIGSTOP)
>> @@ -3014,13 +3145,14 @@ retry:
>> ourstatus->value.sig = GDB_SIGNAL_0;
>> }
>> else if (current_inferior->last_resume_kind == resume_stop
>> - && WSTOPSIG (w) != SIGSTOP)
>> + && WSTOPSIG (w) != SIGSTOP
>> + && !extended_event_reported (ourstatus))
>> {
>> /* A thread that has been requested to stop by GDB with vCont;t,
>> but, it stopped for other reasons. */
>> ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
>> }
>> - else
>> + else if (ourstatus->kind == TARGET_WAITKIND_STOPPED)
>> {
>> ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
>> }
>> @@ -3126,6 +3258,7 @@ static void
>> send_sigstop (struct lwp_info *lwp)
>> {
>> int pid;
>> + int ret;
>>
>> pid = lwpid_of (get_lwp_thread (lwp));
>>
>> @@ -3143,7 +3276,21 @@ send_sigstop (struct lwp_info *lwp)
>> debug_printf ("Sending sigstop to lwp %d\n", pid);
>>
>> lwp->stop_expected = 1;
>> - kill_lwp (pid, SIGSTOP);
>> + errno = 0;
>> + ret = kill_lwp (pid, SIGSTOP);
>> + if (ret == -1 && errno == ESRCH)
>> + {
>> + /* If the kill fails with "No such process", on GNU/Linux we know
>> + that the LWP has vanished - it is not a zombie, it is gone.
>> + This is due to a thread other than the thread group leader
>> + calling exec. See comments in linux_low_filter_event regarding
>> + PTRACE_EVENT_EXEC. */
>> + delete_lwp (lwp);
>> + set_desired_inferior (0);
>> +
>> + if (debug_threads)
>> + debug_printf ("send_sigstop: lwp %d has vanished\n", pid);
>> + }
>> }
>>
>> static int
>> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
>> index 7459710..7759f01 100644
>> --- a/gdb/gdbserver/linux-low.h
>> +++ b/gdb/gdbserver/linux-low.h
>> @@ -265,6 +265,11 @@ struct lwp_info
>> /* When stopped is set, the last wait status recorded for this
>> lwp. */
>> int last_status;
>>
>> + /* If WAITSTATUS->KIND != TARGET_WAITKIND_IGNORE, the waitstatus for
>> + this LWP's last event. This may correspond to LAST_STATUS above,
>> + or to the current status during event processing. */
>> + struct target_waitstatus waitstatus;
>> +
>> /* When stopped is set, this is where the lwp stopped, with
>> decr_pc_after_break already accounted for. */
>> CORE_ADDR stop_pc;
>> diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
>> index 4fcafa0..9ce25dc 100644
>> --- a/gdb/gdbserver/remote-utils.c
>> +++ b/gdb/gdbserver/remote-utils.c
>> @@ -1111,14 +1111,40 @@ prepare_resume_reply (char *buf, ptid_t ptid,
>> switch (status->kind)
>> {
>> case TARGET_WAITKIND_STOPPED:
>> + case TARGET_WAITKIND_EXECD:
>> {
>> struct thread_info *saved_inferior;
>> const char **regp;
>> struct regcache *regcache;
>> + enum gdb_signal signal;
>> +
>> + if (status->kind == TARGET_WAITKIND_EXECD)
>> + signal = GDB_SIGNAL_TRAP;
>> + else
>> + signal = status->value.sig;
>> +
>> + sprintf (buf, "T%02x", signal);
>>
>> - sprintf (buf, "T%02x", status->value.sig);
>> buf += strlen (buf);
>>
>> + if (status->kind == TARGET_WAITKIND_EXECD && multi_process)
>> + {
>> + const char *event = "exec";
>> + char hexified_pathname[PATH_MAX];
>> +
>> + sprintf (buf, "%s:", event);
>> + buf += strlen (buf);
>> +
>> + /* Encode pathname to hexified format. */
>> + bin2hex ((const gdb_byte *) status->value.execd_pathname,
>> + hexified_pathname, strlen(status->value.execd_pathname));
>> +
>> + sprintf (buf, "%s;", hexified_pathname);
>> + xfree (status->value.execd_pathname);
>> + status->value.execd_pathname = NULL;
>> + buf += strlen (buf);
>> + }
>> +
>> saved_inferior = current_inferior;
>>
>> current_inferior = find_thread_ptid (ptid);
>> diff --git a/gdb/remote.c b/gdb/remote.c
>> index be8c423..f4412d8 100644
>> --- a/gdb/remote.c
>> +++ b/gdb/remote.c
>> @@ -5542,11 +5542,13 @@ remote_parse_stop_reply (char *buf, struct
>> stop_reply *event)
>> pnum and set p1 to point to the character following it.
>> Otherwise p1 points to p. */
>>
>> - /* If this packet is an awatch packet, don't parse the 'a'
>> - as a register number. */
>> + /* If this packet has a stop reason string that starts
>> + with a character that could be a hex digit, don't parse
>> + it as a register number. */
>>
>> if (strncmp (p, "awatch", strlen("awatch")) != 0
>> - && strncmp (p, "core", strlen ("core") != 0))
>> + && strncmp (p, "core", strlen ("core") != 0)
>> + && strncmp (p, "exec", strlen ("exec") != 0))
>> {
>> /* Read the ``P'' register number. */
>> pnum = strtol (p, &p_temp, 16);
>> @@ -5598,6 +5600,25 @@ Packet: '%s'\n"),
>> p = unpack_varlen_hex (++p1, &c);
>> event->core = c;
>> }
>> + else if (strncmp (p, "exec", p1 - p) == 0)
>> + {
>> + ULONGEST pid;
>> + char pathname[PATH_MAX];
>> +
>> + p = unpack_varlen_hex (++p1, &pid);
>> +
>> + /* Save the pathname for event reporting and for
>> + the next run command. */
>> + hex2bin (p1, (gdb_byte *) pathname, (p - p1)/2);
>> + /* Add the null terminator. */
>> + pathname[(p - p1)/2] = '\0';
>> + /* This is freed during event handling. */
>> + event->ws.value.execd_pathname = xstrdup (pathname);
>> + event->ws.kind = TARGET_WAITKIND_EXECD;
>> + /* Save the pathname for the next run command. */
>> + xfree (remote_exec_file);
>> + remote_exec_file = pathname;
>> + }
>> else
>> {
>> /* Silently skip unknown optional info. */
>>
>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/4][REPOST] Remote Linux ptrace exit events
2014-04-30 19:18 ` [PATCH 1/4][REPOST] Remote Linux ptrace exit events Don Breazeal
2014-05-07 19:40 ` Luis Machado
2014-05-08 5:23 ` Yao Qi
@ 2014-05-21 15:15 ` Tom Tromey
2014-05-22 17:42 ` Breazeal, Don
2 siblings, 1 reply; 33+ messages in thread
From: Tom Tromey @ 2014-05-21 15:15 UTC (permalink / raw)
To: Don Breazeal; +Cc: gdb-patches, Don Breazeal
>>>>> "Don" == Don Breazeal <donb@codesourcery.com> writes:
Don> This patch implements support for the extended ptrace event
Don> PTRACE_EVENT_EXIT on Linux. This is a preparatory patch for exec event
Don> support.
Thanks.
I had a few questions about this patch.
Don> * common/linux-ptrace.c (linux_test_for_tracefork)
Don> [GDBSERVER]: Add support for PTRACE_EVENT_EXIT if the OS
Don> supports it.
I'm curious why PTRACE_O_TRACEEXIT is needed by gdbserver to implement
this feature. It isn't needed by gdb. And, I think it's preferable to
try to push the two back ends closer together -- it's a long-term goal
-- so new divergences are subject to special scrutiny.
Don> +#ifdef GDBSERVER
Don> + /* PTRACE_O_FORK is supported, so now test for PTRACE_O_TRACEEXIT.
Don> + First try to set the option. If this fails, we know for sure that
Don> + it is not supported. */
Don> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
Don> + (PTRACE_TYPE_ARG4) PTRACE_O_TRACEEXIT);
Don> + if (ret != 0)
Don> + return;
[...]
It would be much nicer to reduce the use of #ifdef GDBSERVER, rather
than to add to it.
I think this could be done a different way, say by having the clients of
this interface specify which flags they're interested in. Then
gdbserver could ask for PTRACE_O_TRACEEXIT and gdb could refrain.
This would be just as simple but still be a good fit for the long-term
goal.
I've appended a patch I wrote along these lines -- perhaps hacky,
definitely untested -- from my experiment with moving gdbserver to
top-level. Feel free to use or ignore it.
Don> + else if (event == PTRACE_EVENT_EXIT)
Don> + {
Don> + unsigned long exit_status;
Don> + unsigned long lwpid = lwpid_of (event_thr);
Don> + int ret;
Don> +
Don> + if (debug_threads)
Don> + debug_printf ("LHEW: Got exit event from LWP %ld\n",
Don> + lwpid_of (event_thr));
Don> +
Don> + ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
Don> + &exit_status);
Don> +
Don> + if (num_lwps (pid_of (event_thr)) > 1)
Don> + {
Don> + /* If there is at least one more LWP, then the program still
Don> + exists and the exit should not be reported to GDB. */
I thought, from the man page, that PTRACE_EVENT_EXIT was for a process
exit event only. So checking num_lwps doesn't seem correct here. But
after seeing your patch I am not sure; and I would like to know the
answer.
Tom
commit a1b6a7417e0e192c8f925ac94491a819c384c7e9
Author: Tom Tromey <tromey@redhat.com>
Date: Fri Jan 3 10:55:52 2014 -0700
remove some GDBSERVER checks from linux-ptrace
diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
index 7c1b78a..d1659e0 100644
--- a/gdb/common/linux-ptrace.c
+++ b/gdb/common/linux-ptrace.c
@@ -37,6 +37,10 @@
there are no supported features. */
static int current_ptrace_options = -1;
+/* Additional flags to test. */
+
+static int additional_flags;
+
/* Find all possible reasons we could fail to attach PID and append these
newline terminated reason strings to initialized BUFFER. '\0' termination
of BUFFER must be done by the caller. */
@@ -359,16 +363,15 @@ linux_check_ptrace_features (void)
static void
linux_test_for_tracesysgood (int child_pid)
{
-#ifdef GDBSERVER
- /* gdbserver does not support PTRACE_O_TRACESYSGOOD. */
-#else
- int ret;
+ if ((additional_flags & PTRACE_O_TRACESYSGOOD) != 0)
+ {
+ int ret;
- ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
- (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
- if (ret == 0)
- current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
-#endif
+ ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
+ if (ret != 0)
+ additional_flags &= ~PTRACE_O_TRACESYSGOOD;
+ }
}
/* Determine if PTRACE_O_TRACEFORK can be used to follow fork
@@ -388,16 +391,15 @@ linux_test_for_tracefork (int child_pid)
if (ret != 0)
return;
-#ifdef GDBSERVER
- /* gdbserver does not support PTRACE_O_TRACEVFORKDONE yet. */
-#else
- /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */
- ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
- (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
- | PTRACE_O_TRACEVFORKDONE));
- if (ret == 0)
- current_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
-#endif
+ if ((additional_flags & PTRACE_O_TRACEVFORKDONE) != 0)
+ {
+ /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */
+ ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
+ | PTRACE_O_TRACEVFORKDONE));
+ if (ret != 0)
+ additional_flags &= ~PTRACE_O_TRACEVFORKDONE;
+ }
/* Setting PTRACE_O_TRACEFORK did not cause an error, however we
don't know for sure that the feature is available; old
@@ -433,18 +435,7 @@ linux_test_for_tracefork (int child_pid)
/* We got the PID from the grandchild, which means fork
tracing is supported. */
-#ifdef GDBSERVER
- /* Do not enable all the options for now since gdbserver does not
- properly support them. This restriction will be lifted when
- gdbserver is augmented to support them. */
- current_ptrace_options |= PTRACE_O_TRACECLONE;
-#else
- current_ptrace_options |= PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK
- | PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC;
-
- /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to
- support read-only process state. */
-#endif
+ current_ptrace_options |= PTRACE_O_TRACECLONE | additional_flags;
/* Do some cleanup and kill the grandchild. */
my_waitpid (second_pid, &second_status, 0);
@@ -542,3 +533,9 @@ linux_ptrace_init_warnings (void)
linux_ptrace_test_ret_to_nx ();
}
+
+void
+linux_ptrace_set_additional_flags (int flags)
+{
+ additional_flags = flags;
+}
diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h
index 38bb9ea..e5094df 100644
--- a/gdb/common/linux-ptrace.h
+++ b/gdb/common/linux-ptrace.h
@@ -90,5 +90,6 @@ extern int linux_supports_tracefork (void);
extern int linux_supports_traceclone (void);
extern int linux_supports_tracevforkdone (void);
extern int linux_supports_tracesysgood (void);
+extern void linux_ptrace_set_additional_flags (int);
#endif /* COMMON_LINUX_PTRACE_H */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index bf6f586..289ae41 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4980,6 +4980,12 @@ Enables printf debugging output."),
sigdelset (&suspend_mask, SIGCHLD);
sigemptyset (&blocked_mask);
+
+ linux_ptrace_set_additional_flags (PTRACE_O_TRACESYSGOOD
+ | PTRACE_O_TRACEVFORKDONE
+ | PTRACE_O_TRACEVFORK
+ | PTRACE_O_TRACEFORK
+ | PTRACE_O_TRACEEXEC);
}
\f
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/0][REPOST] Exec events in gdbserver on Linux
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
` (3 preceding siblings ...)
2014-04-30 19:18 ` [PATCH 3/4][REPOST] Document RSP support for Linux " Don Breazeal
@ 2014-05-21 15:25 ` Tom Tromey
2014-05-23 22:49 ` [PATCH 2/4 v3] Remote Linux ptrace exec events Don Breazeal
` (4 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Tom Tromey @ 2014-05-21 15:25 UTC (permalink / raw)
To: Don Breazeal; +Cc: gdb-patches
>>>>> "Don" == Don Breazeal <donb@codesourcery.com> writes:
Don> There are a couple of significant aspects to this patch. First, it uses
Don> the ptrace extension PTRACE_EVENT_EXIT to detect thread exit, in particular
Don> the exit of a thread group leader when a non-leader calls exec. Use of
Don> this event was necessary due to a race condition in the two-thread case.
Don> It is only used internally; exit events detected this way are not exposed
Don> to the user, and exit processing was changed as little as possible.
Ok, sorry, I missed this note earlier.
I still don't understand why this is needed in gdbserver but not in gdb,
though.
And I'm still curious about PTRACE_EVENT_EXIT notifying process rather
than thread exit.
Tom
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/4][REPOST] Remote Linux ptrace exec events
2014-04-30 19:18 ` [PATCH 2/4][REPOST] Remote Linux ptrace exec events Don Breazeal
2014-05-07 20:01 ` Luis Machado
2014-05-08 5:44 ` Yao Qi
@ 2014-05-21 15:28 ` Tom Tromey
2 siblings, 0 replies; 33+ messages in thread
From: Tom Tromey @ 2014-05-21 15:28 UTC (permalink / raw)
To: Don Breazeal; +Cc: gdb-patches, Don Breazeal
>>>>> "Don" == Don Breazeal <donb@codesourcery.com> writes:
Don> One caveat: when an exec is detected, gdbserver emits a couple of warnings:
Don> gdbserver: unexpected r_debug version 0
Don> gdbserver: unexpected r_debug version 0
Don> However, debugging of shared libraries that are loaded by the exec'd
Don> program works just fine. These messages may be caused by gdbserver making
Don> an attempt to initialize the solib hook before the r_debug structure has
Don> been initialized. I intend to follow up in a subsequent patch.
I think it would be better to have that fix in the initial patch.
I am curious what others think though.
Don> diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
Don> index e3fc705..b137df9 100644
Don> --- a/gdb/common/linux-ptrace.c
Don> +++ b/gdb/common/linux-ptrace.c
Don> @@ -491,8 +491,11 @@ linux_test_for_tracefork (int child_pid)
Don> if (ret == child_pid && WIFSTOPPED (status)
Don> && status >> 16 == PTRACE_EVENT_EXIT)
Don> {
Don> - /* PTRACE_O_TRACEEXIT is supported. */
Don> - current_ptrace_options |= PTRACE_O_TRACEEXIT;
Don> + /* PTRACE_O_TRACEEXIT is supported. We use exit events to
Don> + implement support for exec events. Since fork events are
Don> + supported we know exec events are supported, so we enable
Don> + exec events here. */
Don> + current_ptrace_options |= PTRACE_O_TRACEEXIT | PTRACE_O_TRACEEXEC;
Don> }
There's code earlier in the function that is #ifdef GDBSERVER
and that checks PTRACE_O_TRACEEXEC for gdb. It seems like more
unification there would be a better approach.
Don> +static int
Don> +extended_event_reported (const struct target_waitstatus *waitstatus)
Don> +{
Don> +
Don> + if (waitstatus == NULL)
Don> + return 0;
Don> +
Don> + return waitstatus->kind == TARGET_WAITKIND_EXECD;
Extra newline after the "{".
I can't really comment on the rest, I'm afraid.
Tom
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/4][REPOST] Remote Linux ptrace exit events
2014-05-21 15:15 ` Tom Tromey
@ 2014-05-22 17:42 ` Breazeal, Don
0 siblings, 0 replies; 33+ messages in thread
From: Breazeal, Don @ 2014-05-22 17:42 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches, Don Breazeal
Hi Tom,
Thank you for looking at this. Several responses below...
On 5/21/2014 8:15 AM, Tom Tromey wrote:
>>>>>> "Don" == Don Breazeal <donb@codesourcery.com> writes:
>
> Don> This patch implements support for the extended ptrace event
> Don> PTRACE_EVENT_EXIT on Linux. This is a preparatory patch for exec event
> Don> support.
>
> Thanks.
>
> I had a few questions about this patch.
>
> Don> * common/linux-ptrace.c (linux_test_for_tracefork)
> Don> [GDBSERVER]: Add support for PTRACE_EVENT_EXIT if the OS
> Don> supports it.
>
> I'm curious why PTRACE_O_TRACEEXIT is needed by gdbserver to implement
> this feature. It isn't needed by gdb. And, I think it's preferable to
> try to push the two back ends closer together -- it's a long-term goal
> -- so new divergences are subject to special scrutiny.
As you saw later in the "Patch 0" email:
Don> Use of
Don> this event was necessary due to a race condition in the two-thread
case.
Don> It is only used internally; exit events detected this way are not
exposed
Don> to the user, and exit processing was changed as little as possible.
I believe exit events are needed in GDB. I'm fairly confident that the
same race condition I found in gdbserver exists in GDB. It is a really
small window, though, so it might be difficult to concoct a test case
that demonstrates it reliably.
I totally agree that moving the two implementations closer together
should be the goal, but my position is that GDB needs to use this new
approach, as opposed to vice versa. Since I started this work on the
gdbserver side, I'd like to get buy-in on the exit event approach before
spending effort implementing the same thing on the native GDB side.
>
> Don> +#ifdef GDBSERVER
> Don> + /* PTRACE_O_FORK is supported, so now test for PTRACE_O_TRACEEXIT.
> Don> + First try to set the option. If this fails, we know for sure that
> Don> + it is not supported. */
> Don> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> Don> + (PTRACE_TYPE_ARG4) PTRACE_O_TRACEEXIT);
> Don> + if (ret != 0)
> Don> + return;
> [...]
>
> It would be much nicer to reduce the use of #ifdef GDBSERVER, rather
> than to add to it.
>
> I think this could be done a different way, say by having the clients of
> this interface specify which flags they're interested in. Then
> gdbserver could ask for PTRACE_O_TRACEEXIT and gdb could refrain.
>
> This would be just as simple but still be a good fit for the long-term
> goal.
Agreed, but I'm going to hold off making this change for now. If we go
with the exit event approach GDB and gdbserver will converge anyway.
>
> I've appended a patch I wrote along these lines -- perhaps hacky,
> definitely untested -- from my experiment with moving gdbserver to
> top-level. Feel free to use or ignore it.
>
> Don> + else if (event == PTRACE_EVENT_EXIT)
> Don> + {
> Don> + unsigned long exit_status;
> Don> + unsigned long lwpid = lwpid_of (event_thr);
> Don> + int ret;
> Don> +
> Don> + if (debug_threads)
> Don> + debug_printf ("LHEW: Got exit event from LWP %ld\n",
> Don> + lwpid_of (event_thr));
> Don> +
> Don> + ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
> Don> + &exit_status);
> Don> +
> Don> + if (num_lwps (pid_of (event_thr)) > 1)
> Don> + {
> Don> + /* If there is at least one more LWP, then the program still
> Don> + exists and the exit should not be reported to GDB. */
>
> I thought, from the man page, that PTRACE_EVENT_EXIT was for a process
> exit event only. So checking num_lwps doesn't seem correct here. But
> after seeing your patch I am not sure; and I would like to know the
> answer.
PTRACE_EVENT_EXIT is reported for each LWP. Here is a snippet of
--debug output from debugging (with exit events) a test program that
creates four pthreads that each do some busywork and exit:
LHEW: Got exit event from LWP 10650
--snip--
LHEW: Got exit event from LWP 10648
--snip--
LHEW: Got exit event from LWP 10649
--snip--
LHEW: Got exit event from LWP 10647
--snip--
LHEW: Got exit event from LWP 10634
LWP 10634 is the last lwp of process. Process 10634 exiting.
linux_wait_1 ret = LWP 10634.10634, exited with retcode 0
I agree that the ptrace man page is not really clear on this point. In
the section on execve it does say that all non-execing threads stop in
PTRACE_EVENT_EXIT stop, which is ambiguous about reporting the event.
I'm working on a revised version of the patch series, and I'll include a
detailed description of the race condition. Yao also asked about this.
Thanks!
--Don
>
> Tom
>
>
> commit a1b6a7417e0e192c8f925ac94491a819c384c7e9
> Author: Tom Tromey <tromey@redhat.com>
> Date: Fri Jan 3 10:55:52 2014 -0700
>
> remove some GDBSERVER checks from linux-ptrace
>
> diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
> index 7c1b78a..d1659e0 100644
> --- a/gdb/common/linux-ptrace.c
> +++ b/gdb/common/linux-ptrace.c
> @@ -37,6 +37,10 @@
> there are no supported features. */
> static int current_ptrace_options = -1;
>
> +/* Additional flags to test. */
> +
> +static int additional_flags;
> +
> /* Find all possible reasons we could fail to attach PID and append these
> newline terminated reason strings to initialized BUFFER. '\0' termination
> of BUFFER must be done by the caller. */
> @@ -359,16 +363,15 @@ linux_check_ptrace_features (void)
> static void
> linux_test_for_tracesysgood (int child_pid)
> {
> -#ifdef GDBSERVER
> - /* gdbserver does not support PTRACE_O_TRACESYSGOOD. */
> -#else
> - int ret;
> + if ((additional_flags & PTRACE_O_TRACESYSGOOD) != 0)
> + {
> + int ret;
>
> - ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> - (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
> - if (ret == 0)
> - current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
> -#endif
> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> + (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
> + if (ret != 0)
> + additional_flags &= ~PTRACE_O_TRACESYSGOOD;
> + }
> }
>
> /* Determine if PTRACE_O_TRACEFORK can be used to follow fork
> @@ -388,16 +391,15 @@ linux_test_for_tracefork (int child_pid)
> if (ret != 0)
> return;
>
> -#ifdef GDBSERVER
> - /* gdbserver does not support PTRACE_O_TRACEVFORKDONE yet. */
> -#else
> - /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */
> - ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> - (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
> - | PTRACE_O_TRACEVFORKDONE));
> - if (ret == 0)
> - current_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
> -#endif
> + if ((additional_flags & PTRACE_O_TRACEVFORKDONE) != 0)
> + {
> + /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */
> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> + (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
> + | PTRACE_O_TRACEVFORKDONE));
> + if (ret != 0)
> + additional_flags &= ~PTRACE_O_TRACEVFORKDONE;
> + }
>
> /* Setting PTRACE_O_TRACEFORK did not cause an error, however we
> don't know for sure that the feature is available; old
> @@ -433,18 +435,7 @@ linux_test_for_tracefork (int child_pid)
>
> /* We got the PID from the grandchild, which means fork
> tracing is supported. */
> -#ifdef GDBSERVER
> - /* Do not enable all the options for now since gdbserver does not
> - properly support them. This restriction will be lifted when
> - gdbserver is augmented to support them. */
> - current_ptrace_options |= PTRACE_O_TRACECLONE;
> -#else
> - current_ptrace_options |= PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK
> - | PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC;
> -
> - /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to
> - support read-only process state. */
> -#endif
> + current_ptrace_options |= PTRACE_O_TRACECLONE | additional_flags;
>
> /* Do some cleanup and kill the grandchild. */
> my_waitpid (second_pid, &second_status, 0);
> @@ -542,3 +533,9 @@ linux_ptrace_init_warnings (void)
>
> linux_ptrace_test_ret_to_nx ();
> }
> +
> +void
> +linux_ptrace_set_additional_flags (int flags)
> +{
> + additional_flags = flags;
> +}
> diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h
> index 38bb9ea..e5094df 100644
> --- a/gdb/common/linux-ptrace.h
> +++ b/gdb/common/linux-ptrace.h
> @@ -90,5 +90,6 @@ extern int linux_supports_tracefork (void);
> extern int linux_supports_traceclone (void);
> extern int linux_supports_tracevforkdone (void);
> extern int linux_supports_tracesysgood (void);
> +extern void linux_ptrace_set_additional_flags (int);
>
> #endif /* COMMON_LINUX_PTRACE_H */
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index bf6f586..289ae41 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -4980,6 +4980,12 @@ Enables printf debugging output."),
> sigdelset (&suspend_mask, SIGCHLD);
>
> sigemptyset (&blocked_mask);
> +
> + linux_ptrace_set_additional_flags (PTRACE_O_TRACESYSGOOD
> + | PTRACE_O_TRACEVFORKDONE
> + | PTRACE_O_TRACEVFORK
> + | PTRACE_O_TRACEFORK
> + | PTRACE_O_TRACEEXEC);
> }
> \f
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 0/4 v3] Exec events in gdbserver on Linux
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
` (8 preceding siblings ...)
2014-05-23 22:49 ` [PATCH 1/4 v3] Remote Linux ptrace exit events Don Breazeal
@ 2014-05-23 22:49 ` Don Breazeal
2014-05-26 4:55 ` Doug Evans
2014-06-05 22:06 ` Breazeal, Don
9 siblings, 2 replies; 33+ messages in thread
From: Don Breazeal @ 2014-05-23 22:49 UTC (permalink / raw)
To: gdb-patches
This patch series is an update to the gdbserver Linux exec event patches based on review comments for the previous version. The changes from the previous version are summarized below.
Patch 1 (gdbserver exit events):
Fixed up comments and whitespace
Moved test for extended exit events from linux_test_for_tracefork
into new function, linux_test_for_traceexit, called from
linux_check_ptrace_features.
Moved predicate functions is_extended_* into common/linux-ptrace.c
renamed to linux_is_extended_* and made new function
linux_is_extended_fork. Replaced shift operations with calls to
these functions in gdbserver.
Eliminated the call to linux_enable_event_reporting that I had
added to gdbserver's CLONE event handling.
Wrote explanation of why extended exit events are required.
Patch 2 (gdbserver exec events):
Moved linux_child_pid_to_exec_file from linux-nat.c and linux-low.c
to common/linux-procfs.c and renamed it linux_proc_pid_to_exec_file.
Gdbserver has to take "struct target_ops *" argument now.
Implemented new predicate linux_is_extended_exec in common file.
Parenthesize condition in return stmt in extended_event_reported.
Delete extra text from debug_printf.
Patch 3 (gdbserver exec event documentation):
Added NEWS entries for gdbserver exec event support.
Patch 4 (exec event tests):
No changes from the previous version.
This patch series implements support for exec events in gdbserver on GNU/Linux. This is a step towards the "follow fork/exec" item in the local/remote debugging feature parity project: (https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity). Luis had started work on this last year, and handed it off to me at the end of January. (BTW, thanks to Luis for his advice on this, although any problems with this patch are entirely mine.)
Follow-exec-mode and rerun behave as expected in multiprocess mode (target extended-remote), where follow-exec-mode maintains the specified inferiors and rerun runs the last executable file to be exec'd. Catchpoints for exec are not implemented in this patch series, since this will be easier to do once fork and vfork events are also supported.
- Patch 1/4 implements support for the extended ptrace event PTRACE_EVENT_EXIT, which is a prerequisite for the exec event support.
- Patch 2/4 implements the exec event support.
- Patch 3/4 adds documentation for the new RSP "Stop Reply" message.
- Patch 4/4 extends the tests gdb.threads/non-ldr-exc-[1-4].exp to test in non-stop mode as well as in all-stop mode.
There are a couple of significant aspects to this patch. First, it uses the ptrace extension PTRACE_EVENT_EXIT to detect thread exit, in particular the exit of a thread group leader when a non-leader calls exec. Use of this event was necessary due to a race condition in the two-thread case. A detailed description of the race condition can be found at the bottom of this message. Exit events are only used internally and are not exposed to the user, and exit processing was changed as little as possible.
Second, it sends a signal to each lwp after an exec event in order to identify and clean up the lwp entry for the exec'ing thread. This happens in the normal course of things for all-stop mode, but in non-stop mode gdbserver uses SIGSTOP to stop, then resume all of the non-leader threads to accomplish this.
My intent is to follow up with implementations of remote fork/vfork events
and associated catchpoints.
RACE CONDITION
This section explains why the existing techniques for detecting thread exit weren't sufficient for gdbserver exec events, necessitating the use of PTRACE_EVENT_EXIT. The short answer is that there is a race condition in the current implementation that can leave a dangling entry in the lwp list (an entry that doesn't have a corresponding actual lwp). In this case gdbserver will hang waiting for the non-existent lwp to stop. Using the exit events eliminates this race condition.
The same race may exist in the native implementation, since the two implementations are similar, but I haven't verified that. It may be difficult to concoct a test case that demonstrates the race since the window is so small.
Now for the long answer: in my testing I ran into a race condition in check_zombie_leaders, which detects when a thread group leader has exited and other threads still exist. On the Linux kernel, ptrace/waitpid don't allow reaping the leader thread until all other threads in the group are reaped. When the leader exits, it goes zombie, but waitpid will not return an exit status until the other threads are gone. When a non-leader thread calls exec, all other non-leader threads are destroyed, the leader becomes a zombie, and once the "other" threads have been reaped, the execing thread takes over the leader's pid (tgid) and appears to vanish. In order to handle this situation, check_zombie_leaders polls the process state in /proc and deletes thread group leaders that are in a zombie state. The replacement is added to the lwp list when the exec event is reported.
See https://sourceware.org/ml/gdb-patches/2011-10/msg00704.html for a more detailed explanation of how this works.
Here is the relevant part of check_zombie_leaders:
if (leader_lp != NULL
/* Check if there are other threads in the group, as we may
have raced with the inferior simply exiting. */
&& !last_thread_of_process_p (leader_pid)
&& linux_proc_pid_is_zombie (leader_pid))
{
/* ...large informative comment block... */
delete_lwp (leader_lp);
The race occurred when there were two threads in the program, and the non-leader thread called exec. In this case the leader thread passed through a very brief zombie state before being replaced by the exec'ing thread as the thread group leader. This state transition was asynchronous, with no dependency on anything gdbserver did. Because there were no other threads, there were no thread exit events, and thus there was no synchronization with the leader passing through the zombie state and the exec completing. If there had been more threads, the leader would remain in the zombie state until they were waited for. In the two-thread case, sometimes the leader exit was detected and sometimes it wasn't. (Recall that check_zombie_leaders is polling the state, via linux_proc_pid_is_zombie. The race is between the leader thread passing through the zombie state and check_zombie_leaders testing for zombie state.) If leader exit wasn't detected, gdbserver would end up with a dangl
ing lwp entry that didn't correspond to any real lwp, and would hang waiting for that lwp to stop. Using PTRACE_EVENT_EXIT guarantees that the leader exit will be detected.
Note that check_zombie_leaders works just fine for the scenarios where the leader thread exits and the other threads continue to run, with no exec calls. It is required for systems that don't support the extended ptrace events.
The sequence of events resulting in the race condition was this:
1) In the program, a CLONE event for a new thread occurs.
2) In the program, both threads are resumed once gdbserver has
completed the new thread processing.
3) In gdbserver, the function linux_wait_for_event_filtered loops until
waitpid returns "no more events" for the SIGCHLD generated by the
CLONE event. Then linux_wait_for_event_filtered calls
check_zombie_leaders.
4) In the program, the new thread is doing the exec. During the exec
the leader thread will pass through a transitory zombie state. If
there were more than two threads, the leader thread would remain a
zombie until all the non-leader, non-exec'ing threads were reaped by
gdbserver. Since there are no such threads to reap, the leader just
becomes a zombie and is replaced by the exec'ing thread on-the-fly.
(Note that it appears that the leader thread is a zombie just for a
very brief instant.)
5) In gdbserver, check_zombie_leaders checks whether an lwp entry
corresponds to a zombie leader thread, and if so, deletes it. Here
is the race: in (4) above, the leader may or may not be in the
transitory zombie state. In the case where a zombie isn't detected,
delete_lwp is not called.
6) In gdbserver, an EXEC event is detected and processed. When it gets
ready to report the event to GDB, it calls stop_all_lwps, which sends
a SIGSTOP to each lwp in the list and the waits until all the lwps in
the list have reported a stop event. If the zombie leader wasn't
detected and processed in step (5), gdbserver blocks forever in
linux_wait_for_event_filtered, waiting for the undeleted lwp to be
stopped, which will never happen.
Regards,
--Don
gdb/NEWS | 6 +
gdb/common/linux-procfs.c | 18 ++
gdb/common/linux-procfs.h | 4 +
gdb/common/linux-ptrace.c | 75 +++++++++
gdb/common/linux-ptrace.h | 4 +
gdb/doc/gdb.texinfo | 6 +
gdb/gdbserver/linux-low.c | 229 +++++++++++++++++++++++----
gdb/gdbserver/linux-low.h | 5 +
gdb/gdbserver/remote-utils.c | 28 +++-
gdb/linux-nat.c | 22 +---
gdb/remote.c | 27 +++-
gdb/testsuite/gdb.threads/non-ldr-exc-1.exp | 20 ++-
gdb/testsuite/gdb.threads/non-ldr-exc-2.exp | 36 ++++-
gdb/testsuite/gdb.threads/non-ldr-exc-3.exp | 36 ++++-
gdb/testsuite/gdb.threads/non-ldr-exc-4.exp | 20 ++-
15 files changed, 466 insertions(+), 70 deletions(-)
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 4/4] Non-stop exec tests
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
` (6 preceding siblings ...)
2014-05-23 22:49 ` [PATCH 3/4] Document RSP support for Linux " Don Breazeal
@ 2014-05-23 22:49 ` Don Breazeal
2014-05-26 3:29 ` Doug Evans
2014-05-23 22:49 ` [PATCH 1/4 v3] Remote Linux ptrace exit events Don Breazeal
2014-05-23 22:49 ` [PATCH 0/4 v3] Exec events in gdbserver on Linux Don Breazeal
9 siblings, 1 reply; 33+ messages in thread
From: Don Breazeal @ 2014-05-23 22:49 UTC (permalink / raw)
To: gdb-patches
This patch extends the non-ldr-exc-*.exp tests so that they run all their cases in non-stop mode as well as all-stop mode. These tests cover handling of exec events when non-leader threads call exec.
The tests now report 'untested when 'runto_main' fails. In non-stop mode with 'target extended-remote', runto_main always fails with something like:
(gdb) run
Starting program: /home/me/gdb/testsuite/gdb.threads/non-ldr-exc-4
Unexpected vCont reply in non-stop mode: T0506:10e0ffffff7f0000;07:c8deffffff7f0000;10:c1a6abaaaa2a0000;thread:p5ee.5ee;core:0;
This happens in other tests as well (e.g. gdb.threads/thread_events.exp), so I copied the error handling from that test so that the non-stop tests report 'untested' for target extended-remote. I couldn't find anything related to this in the gdb bug database, but I assume it is a known problem since the other tests handle it.
Regards,
--Don
testsuite/
2014-05-22 Don Breazeal <donb@codesourcery.com>
* gdb.threads/non-ldr-exc-1.exp: Add non-stop cases
* gdb.threads/non-ldr-exc-2.exp: Ditto.
* gdb.threads/non-ldr-exc-3.exp: Ditto.
* gdb.threads/non-ldr-exc-4.exp: Ditto.
---
gdb/testsuite/gdb.threads/non-ldr-exc-1.exp | 20 ++++++++++++---
gdb/testsuite/gdb.threads/non-ldr-exc-2.exp | 36 +++++++++++++++++++++++---
gdb/testsuite/gdb.threads/non-ldr-exc-3.exp | 36 +++++++++++++++++++++++---
gdb/testsuite/gdb.threads/non-ldr-exc-4.exp | 20 ++++++++++++---
4 files changed, 94 insertions(+), 18 deletions(-)
diff --git a/gdb/testsuite/gdb.threads/non-ldr-exc-1.exp b/gdb/testsuite/gdb.threads/non-ldr-exc-1.exp
index 8123a99..e35236a 100644
--- a/gdb/testsuite/gdb.threads/non-ldr-exc-1.exp
+++ b/gdb/testsuite/gdb.threads/non-ldr-exc-1.exp
@@ -28,13 +28,19 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
return -1
}
-proc do_test { lock_sched } {
- with_test_prefix "lock-sched$lock_sched" {
+proc do_test { lock_sched mode } {
+ with_test_prefix "lock-sched$lock_sched,$mode" {
global executable
clean_restart ${executable}
+ if { $mode == "non-stop" } {
+ gdb_test_no_output "set target-async on" "set async mode"
+ gdb_test_no_output "set non-stop on" "set non-stop mode"
+ }
+
if ![runto_main] {
+ untested "could not run to main"
return -1
}
@@ -48,11 +54,17 @@ proc do_test { lock_sched } {
gdb_test_no_output "set scheduler-locking on"
}
+ if { $mode == "non-stop" } {
+ gdb_test "thread 2" "Switching.*"
+ }
+
gdb_test "continue" \
".*is executing new program.*Breakpoint 1, main.* at .*" \
"continue over exec"
}
}
-do_test 0
-do_test 1
+do_test 0 "all-stop"
+do_test 1 "all-stop"
+do_test 0 "non-stop"
+do_test 1 "non-stop"
diff --git a/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp b/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp
index 857e7bc..a0281d6 100644
--- a/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp
+++ b/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp
@@ -29,18 +29,42 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
return -1
}
-proc do_test { lock_sched } {
- with_test_prefix "lock-sched$lock_sched" {
+# Test for breakpoint event among async thread events.
+# gdb_continue_to_breakpoint requires "$gdb_prompt $", but
+# here we may get a thread event message instead of EOL.
+proc gdb_continue_to_breakpoint_async { name pattern } {
+ global gdb_prompt
+
+ gdb_test_multiple "continue" $name {
+ -re ".*Breakpoint .* (at|in) ($pattern)$gdb_prompt " {
+ pass $name
+ }
+ }
+}
+
+proc do_test { lock_sched mode } {
+ with_test_prefix "lock-sched$lock_sched,$mode" {
global executable
clean_restart ${executable}
+ if { $mode == "non-stop" } {
+ gdb_test_no_output "set target-async on" "set async mode"
+ gdb_test_no_output "set non-stop on" "set non-stop mode"
+ }
+
if ![runto_main] {
+ untested "could not run to main"
return -1
}
gdb_breakpoint [gdb_get_line_number "break-here"]
- gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+ if { $mode == "non-stop" } {
+ gdb_continue_to_breakpoint_async "break-here" ".* break-here .*"
+ gdb_test "thread 2" "Switching.*"
+ } else {
+ gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+ }
gdb_test "info threads" \
"\r\n\[ \t\]*Id\[ \t\]+Target\[ \t\]+Id\[ \t\]+Frame\[ \t\]*\r\n\\* 2 *Thread \[^\r\n\]* at \[^\r\n\]*" \
@@ -59,5 +83,7 @@ proc do_test { lock_sched } {
}
}
-do_test 0
-do_test 1
+do_test 0 "all-stop"
+do_test 1 "all-stop"
+do_test 0 "non-stop"
+do_test 1 "non-stop"
diff --git a/gdb/testsuite/gdb.threads/non-ldr-exc-3.exp b/gdb/testsuite/gdb.threads/non-ldr-exc-3.exp
index 7f33d39..69c27d8 100644
--- a/gdb/testsuite/gdb.threads/non-ldr-exc-3.exp
+++ b/gdb/testsuite/gdb.threads/non-ldr-exc-3.exp
@@ -31,18 +31,42 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
return -1
}
-proc do_test { lock_sched } {
- with_test_prefix "lock-sched$lock_sched" {
+# Test for breakpoint event among async thread events.
+# gdb_continue_to_breakpoint requires "$gdb_prompt $", but
+# here we may get a thread event message instead of EOL.
+proc gdb_continue_to_breakpoint_async { name pattern } {
+ global gdb_prompt
+
+ gdb_test_multiple "continue" $name {
+ -re ".*Breakpoint .* (at|in) ($pattern)$gdb_prompt " {
+ pass $name
+ }
+ }
+}
+
+proc do_test { lock_sched mode } {
+ with_test_prefix "lock-sched$lock_sched,$mode" {
global executable
clean_restart ${executable}
+ if { $mode == "non-stop" } {
+ gdb_test_no_output "set target-async on" "set async mode"
+ gdb_test_no_output "set non-stop on" "set non-stop mode"
+ }
+
if ![runto_main] {
+ untested "could not run to main"
return -1
}
gdb_breakpoint [gdb_get_line_number "break-here"]
- gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+ if { $mode == "non-stop" } {
+ gdb_continue_to_breakpoint_async "break-here" ".* break-here .*"
+ gdb_test "thread 2" "Switching.*"
+ } else {
+ gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+ }
# Also test with sched-lock to make sure we can follow the
# non-leader thread execing even though the main thread wasn't
@@ -57,5 +81,7 @@ proc do_test { lock_sched } {
}
}
-do_test 0
-do_test 1
+do_test 0 "all-stop"
+do_test 1 "all-stop"
+do_test 0 "non-stop"
+do_test 1 "non-stop"
diff --git a/gdb/testsuite/gdb.threads/non-ldr-exc-4.exp b/gdb/testsuite/gdb.threads/non-ldr-exc-4.exp
index a5e88bb..f83577d 100644
--- a/gdb/testsuite/gdb.threads/non-ldr-exc-4.exp
+++ b/gdb/testsuite/gdb.threads/non-ldr-exc-4.exp
@@ -30,13 +30,19 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab
return -1
}
-proc do_test { lock_sched } {
- with_test_prefix "lock-sched$lock_sched" {
+proc do_test { lock_sched mode } {
+ with_test_prefix "lock-sched$lock_sched,$mode" {
global executable
clean_restart ${executable}
+ if { $mode == "non-stop" } {
+ gdb_test_no_output "set target-async on" "set async mode"
+ gdb_test_no_output "set non-stop on" "set non-stop mode"
+ }
+
if ![runto_main] {
+ untested "could not run to main"
return -1
}
@@ -50,11 +56,17 @@ proc do_test { lock_sched } {
gdb_test_no_output "set scheduler-locking on"
}
+ if { $mode == "non-stop" } {
+ gdb_test "thread 2" "Switching.*"
+ }
+
gdb_test "continue" \
".*is executing new program.*Breakpoint 1, main.* at .*" \
"continue over exec"
}
}
-do_test 0
-do_test 1
+do_test 0 "all-stop"
+do_test 1 "all-stop"
+do_test 0 "non-stop"
+do_test 1 "non-stop"
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/4 v3] Remote Linux ptrace exit events
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
` (7 preceding siblings ...)
2014-05-23 22:49 ` [PATCH 4/4] Non-stop exec tests Don Breazeal
@ 2014-05-23 22:49 ` Don Breazeal
2014-05-23 22:49 ` [PATCH 0/4 v3] Exec events in gdbserver on Linux Don Breazeal
9 siblings, 0 replies; 33+ messages in thread
From: Don Breazeal @ 2014-05-23 22:49 UTC (permalink / raw)
To: gdb-patches
This patch implements support for the extended ptrace event PTRACE_EVENT_EXIT on Linux. This is a preparatory patch for exec event support.
The use of this event is entirely internal to gdbserver; these events are not reported to GDB or the user. When this event occurs, if the reporting thread is not the last thread in a process, its lwp entry is simply deleted, since this is what happens in the absence of exit events. If it is the last thread of a process, the wait status is set to the actual wait status of the thread, retrieved by PTRACE_O_GETMESSAGE, instead of the status that indicates the extended event, and the existing mechanisms for handling thread exit proceed as usual.
The only purpose in using the exit events instead of the existing wait mechanisms is to ensure that the exit of a thread group leader is detected reliably when a non-leader thread calls exec. The "Patch 0" part of this patch contains a detailed explanation of this.
Regards,
--Don
gdb/
2014-05-22 Don Breazeal <donb@codesourcery.com>
* common/linux-ptrace.c (linux_test_for_tracefork)
[GDBSERVER]: Add support for PTRACE_EVENT_EXIT if the OS
supports it.
(linux_test_for_traceexit): New function.
(linux_check_ptrace_features): Call linux_test_for_traceexit.
(linux_is_extended_waitstatus): New function.
(linux_is_extended_exit): New function.
(linux_is_extended_fork): New function.
* common/linux-ptrace.h (linux_is_extended_waitstatus): New
declaration.
(linux_is_extended_exit): New declaration.
(linux_is_extended_fork): New declaration.
gdbserver/
2014-05-22 Don Breazeal <donb@codesourcery.com>
* linux-low.c (handle_extended_wait): Improve comment block.
Change return type from void to int, change wait status arg
to pointer, add support for PTRACE_EVENT_EXIT. Call
internal_error for unknown event types.
(get_stop_pc): Use linux_is_extended_waitstatus instead of
hardcoded shift operation.
(get_detach_signal): Use linux_is_extended_waitstatus instead
of hardcoded shift operation.
(linux_low_filter_event): Handle new return value from
handle_extended_wait. Check for extended events as needed.
Allow status arg to be modified.
(linux_wait_for_event_filtered): Allow status arg to be
modified by call to linux_low_filter_event.
---
gdb/common/linux-ptrace.c | 64 +++++++++++++++++++++++++++++++++++
gdb/common/linux-ptrace.h | 3 ++
gdb/gdbserver/linux-low.c | 82 +++++++++++++++++++++++++++++++++++++--------
3 files changed, 135 insertions(+), 14 deletions(-)
diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
index efbd1ea..af1d306 100644
--- a/gdb/common/linux-ptrace.c
+++ b/gdb/common/linux-ptrace.c
@@ -310,6 +310,7 @@ linux_child_function (gdb_byte *child_stack)
static void linux_test_for_tracesysgood (int child_pid);
static void linux_test_for_tracefork (int child_pid);
+static void linux_test_for_traceexit (int child_pid);
/* Determine ptrace features available on this target. */
@@ -341,6 +342,8 @@ linux_check_ptrace_features (void)
linux_test_for_tracefork (child_pid);
+ linux_test_for_traceexit (child_pid);
+
/* Clean things up and kill any pending children. */
do
{
@@ -461,6 +464,43 @@ linux_test_for_tracefork (int child_pid)
"(%d, status 0x%x)"), ret, status);
}
+/* Determine if PTRACE_O_TRACEEXIT can be used to follow exit
+ events. */
+
+static void
+linux_test_for_traceexit (int child_pid)
+{
+ int ret, status;
+
+#ifdef GDBSERVER
+ /* Test for PTRACE_O_TRACEEXIT. First try to set the option.
+ If this fails, we know for sure that it is not supported. */
+ ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) PTRACE_O_TRACEEXIT);
+ if (ret != 0)
+ return;
+
+ /* We don't know for sure that the feature is available; old
+ versions of PTRACE_SETOPTIONS ignored unknown options. So
+ see if the process exit will generate a PTRACE_EVENT_EXIT. */
+ ret = ptrace (PTRACE_CONT, child_pid, (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) 0);
+ if (ret != 0)
+ warning (_("linux_test_for_traceexit: failed to resume child"));
+
+ ret = my_waitpid (child_pid, &status, 0);
+
+ /* Check if we received an exit event notification. */
+ if (ret == child_pid && WIFSTOPPED (status)
+ && linux_is_extended_exit (status))
+ {
+ /* PTRACE_O_TRACEEXIT is supported. */
+ current_ptrace_options |= PTRACE_O_TRACEEXIT;
+ }
+#endif
+}
+
+
/* Enable reporting of all currently supported ptrace events. */
void
@@ -542,3 +582,27 @@ linux_ptrace_init_warnings (void)
linux_ptrace_test_ret_to_nx ();
}
+
+/* Check wait status for any extended event. */
+
+int
+linux_is_extended_waitstatus (int wstat)
+{
+ return (wstat >> 16 != 0);
+}
+
+/* Check wait status for extended exit event. */
+
+int
+linux_is_extended_exit (int wstat)
+{
+ return (wstat >> 16 == PTRACE_EVENT_EXIT);
+}
+
+/* Check wait status for extended fork event. */
+
+int
+linux_is_extended_fork (int wstat)
+{
+ return (wstat >> 16 == PTRACE_EVENT_FORK);
+}
diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h
index 881d9c9..eb93862 100644
--- a/gdb/common/linux-ptrace.h
+++ b/gdb/common/linux-ptrace.h
@@ -90,5 +90,8 @@ extern int linux_supports_tracefork (void);
extern int linux_supports_traceclone (void);
extern int linux_supports_tracevforkdone (void);
extern int linux_supports_tracesysgood (void);
+extern int linux_is_extended_waitstatus (int wstat);
+extern int linux_is_extended_exit (int wstat);
+extern int linux_is_extended_fork (int wstat);
#endif /* COMMON_LINUX_PTRACE_H */
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 1932ff2..5404404 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -224,6 +224,7 @@ static void proceed_all_lwps (void);
static int finish_step_over (struct lwp_info *lwp);
static CORE_ADDR get_stop_pc (struct lwp_info *lwp);
static int kill_lwp (unsigned long lwpid, int signo);
+static int num_lwps (int pid);
/* True if the low target can hardware single-step. Such targets
don't need a BREAKPOINT_REINSERT_ADDR callback. */
@@ -368,12 +369,15 @@ linux_add_process (int pid, int attached)
}
/* Handle a GNU/Linux extended wait response. If we see a clone
- event, we need to add the new LWP to our list (and not report the
- trap to higher layers). */
+ event, we need to add the new lwp EVENT_CHILD to our list (and not
+ report the trap to higher layers). This function returns non-zero
+ if the event should be ignored and we should wait again. The wait
+ status in WSTATP may be modified if an exit event occurred. */
-static void
-handle_extended_wait (struct lwp_info *event_child, int wstat)
+static int
+handle_extended_wait (struct lwp_info *event_child, int *wstatp)
{
+ int wstat = *wstatp;
int event = wstat >> 16;
struct thread_info *event_thr = get_lwp_thread (event_child);
struct lwp_info *new_lwp;
@@ -452,7 +456,47 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
threads, it will have a pending SIGSTOP; we may as well
collect it now. */
linux_resume_one_lwp (event_child, event_child->stepping, 0, NULL);
+
+ /* Don't report the event. */
+ return 1;
+ }
+ else if (event == PTRACE_EVENT_EXIT)
+ {
+ unsigned long exit_status;
+ unsigned long lwpid = lwpid_of (event_thr);
+ int ret;
+
+ if (debug_threads)
+ debug_printf ("HEW: Got exit event from LWP %ld\n",
+ lwpid_of (event_thr));
+
+ ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
+ &exit_status);
+
+ if (num_lwps (pid_of (event_thr)) > 1)
+ {
+ /* If there is at least one more LWP, then the program still
+ exists and the exit should not be reported to GDB. */
+ delete_lwp (event_child);
+ ret = 1;
+ }
+ else
+ {
+ /* Set the exit status to the actual exit status, so normal
+ WIFEXITED/WIFSIGNALLED processing and reporting for the
+ last lwp in the process can proceed from here. */
+ *wstatp = exit_status;
+ ret = 0;
+ }
+
+ /* Resume the thread so that it actually exits. Subsequent exit
+ events for LWPs that were deleted above will be ignored. */
+ ptrace (PTRACE_CONT, lwpid, (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) 0);
+ return ret;
}
+ internal_error (__FILE__, __LINE__,
+ _("unknown ptrace event %d"), event);
}
/* Return the PC as read from the regcache of LWP, without any
@@ -516,7 +560,7 @@ get_stop_pc (struct lwp_info *lwp)
if (WSTOPSIG (lwp->last_status) == SIGTRAP
&& !lwp->stepping
&& !lwp->stopped_by_watchpoint
- && lwp->last_status >> 16 == 0)
+ && !linux_is_extended_waitstatus (lwp->last_status))
stop_pc -= the_low_target.decr_pc_after_break;
if (debug_threads)
@@ -1036,7 +1080,7 @@ get_detach_signal (struct thread_info *thread)
}
/* Extended wait statuses aren't real SIGTRAPs. */
- if (WSTOPSIG (status) == SIGTRAP && status >> 16 != 0)
+ if (WSTOPSIG (status) == SIGTRAP && linux_is_extended_waitstatus (status))
{
if (debug_threads)
debug_printf ("GPS: lwp %s had stopped with extended "
@@ -1721,11 +1765,13 @@ cancel_breakpoint (struct lwp_info *lwp)
/* Do low-level handling of the event, and check if we should go on
and pass it to caller code. Return the affected lwp if we are, or
- NULL otherwise. */
+ NULL otherwise. The wait status in WSTATP may be modified if an
+ exit event occurred. */
static struct lwp_info *
-linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
+linux_low_filter_event (ptid_t filter_ptid, int lwpid, int *wstatp)
{
+ int wstat = *wstatp;
struct lwp_info *child;
struct thread_info *thread;
@@ -1777,7 +1823,7 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
architecture being defined already (so that CHILD has a valid
regcache), and on LAST_STATUS being set (to check for SIGTRAP or
not). */
- if (WIFSTOPPED (wstat))
+ if (WIFSTOPPED (wstat) && !linux_is_extended_exit (wstat))
{
if (debug_threads
&& the_low_target.get_pc != NULL)
@@ -1813,7 +1859,8 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
changes the debug registers meanwhile, we have the cached data we
can rely on. */
- if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP)
+ if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
+ && !linux_is_extended_waitstatus (wstat))
{
if (the_low_target.stopped_by_watchpoint == NULL)
{
@@ -1849,10 +1896,17 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int wstat)
}
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
- && wstat >> 16 != 0)
+ && linux_is_extended_waitstatus (wstat))
{
- handle_extended_wait (child, wstat);
- return NULL;
+ if (handle_extended_wait (child, &wstat))
+ return NULL;
+ else
+ {
+ /* Update caller's wstat in case handle_extended_wait fixed
+ it up to reflect the actual status. */
+ *wstatp = wstat;
+ return child;
+ }
}
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGSTOP
@@ -2072,7 +2126,7 @@ linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid,
}
event_child = linux_low_filter_event (filter_ptid,
- ret, *wstatp);
+ ret, wstatp);
if (event_child != NULL)
{
/* We got an event to report to the core. */
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/4 v3] Remote Linux ptrace exec events
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
` (4 preceding siblings ...)
2014-05-21 15:25 ` [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Tom Tromey
@ 2014-05-23 22:49 ` Don Breazeal
2014-05-23 22:49 ` [PATCH 3/4] Document RSP support for Linux " Don Breazeal
` (3 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Don Breazeal @ 2014-05-23 22:49 UTC (permalink / raw)
To: gdb-patches
This patch implements support for exec events in gdbserver on linux, in multiprocess mode (target extended-remote). Follow-exec-mode and rerun behave as expected. Catchpoints for exec are not yet implemented since it will be easier to implement catchpoints for fork, vfork, and exec all at the same time.
TESTING
---------
The patch was tested on GNU/Linux x86_64 with --target_board set to native-gdbserver and native-extended-gdbserver, as well as testing native GDB. The test results for native-gdbserver were unchanged. Thirteen tests that used to fail for native-extended-gdbserver on Linux pass with this patch, and the non-ldr-exc-*.exp tests all pass in all-stop mode and extended-remote.
One caveat: when an exec is detected, gdbserver emits a couple of warnings:
gdbserver: unexpected r_debug version 0
gdbserver: unexpected r_debug version 0
However, debugging of shared libraries that are loaded by the exec'd
program works just fine. These messages may be caused by gdbserver making
an attempt to initialize the solib hook before the r_debug structure has
been initialized. I intend to follow up in a subsequent patch, unless the
consensus is that the fix needs to be part of this patch.
IMPLEMENTATION
----------------
Support for exec events in single-threaded programs was a fairly straightforward replication of the implementation in native GDB:
1) Enable exec events via ptrace options.
2) Add support for handling the exec events to the handle_extended_wait and linux_wait_for_event_filtered. Detect the exec event, then find and save the pathname of the executable file being exec'd.
3) Implement an additional "stop reason", "exec", in the RSP stop reply packet "T".
Existing GDB code takes care of handling the exec event on the host side without modification.
Support for exec events in multi-threaded programs required some additional work that required a couple of significant changes to existing code. In a nutshell, the changes are to:
4) Use the PTRACE_EVENT_EXIT extended event to handle thread exit, while not exposing any change in exit handling to the user.
5) Recognize when the exec'ing thread has vanished (become the thread group leader) in send_sigstop. Native GDB does this differently.
The rationale for #4 is discussed in the "Patch 0" email of this series.
Regarding items 4 & 5: Recall that when a non-leader thread exec's, all the other threads are terminated and the exec'ing thread changes its thread id to that of the old leader (the process id) as part of the exec. There is no event reported for the "exit" of the exec'ing thread; it appears to have vanished.
Determining that the exec'ing thread has "vanished" in native GDB is done by calling waitpid(PID), and if it returns ECHILD it means that the thread is gone. We don't want to use waitpid(PID) in gdbserver, based on the discussion in https://www.sourceware.org/ml/gdb-patches/2014-02/msg00828.html. An alternative is to send a signal to each thread and look for an ESRCH (No such process) error. In all-stop mode this can be done in the normal course of events, since when gdbserver reports an exec event it stops all the other threads with a SIGSTOP. In non-stop mode, when an exec event has been detected, we can call stop_all_lwps/unstop_all_lwps to accomplish the same thing.
Regards,
--Don
gdb/
2014-05-22 Don Breazeal <donb@codesourcery.com>
* common/linux-procfs.c (linux_proc_pid_to_exec_file): New function.
* common/linux-procfs.h (linux_proc_pid_to_exec_file): New
declaration.
* common/linux-ptrace.c (linux_test_for_tracefork)
[GDBSERVER]: Add exec tracing to ptrace options if OS supports
exit tracing.
(linux_is_extended_exec): New function.
* common/linux-ptrace.h (linux_is_extended_exec): New declaration.
* linux-nat.c (linux_child_pid_to_exec_file): Move to common code,
renamed linux_proc_pid_to_exec_file.
(linux_handle_extended_wait): Use linux_proc_pid_to_exec_file
instead of linux_child_pid_to_exec_file.
(linux_target_install_ops): Ditto.
* remote.c (remote_parse_stop_reply): Support new RSP stop
reply reason 'exec'.
gdbserver/
2014-05-22 Don Breazeal <donb@codesourcery.com>
* linux-low.c (handle_extended_wait): Add support for
PTRACE_EVENT_EXEC.
(check_zombie_leaders): Update comment.
(linux_low_filter_event): Add support for PTRACE_EVENT_EXEC.
(linux_wait_for_event_filtered): Update comment.
(extended_event_reported): New function.
(linux_wait_1): Set status to report extended event to GDB.
(send_sigstop): Delete lwp on 'No such process' error and
reset current_inferior.
* linux-low.h (struct lwp_info) <waitstatus>: New member.
* remote-utils.c (prepare_resume_reply): Support
new RSP stop reply reason 'exec'.
---
gdb/common/linux-procfs.c | 18 +++++
gdb/common/linux-procfs.h | 4 +
gdb/common/linux-ptrace.c | 15 ++++-
gdb/common/linux-ptrace.h | 1 +
gdb/gdbserver/linux-low.c | 147 ++++++++++++++++++++++++++++++++++++++----
gdb/gdbserver/linux-low.h | 5 ++
gdb/gdbserver/remote-utils.c | 28 ++++++++-
gdb/linux-nat.c | 22 +------
gdb/remote.c | 27 +++++++-
9 files changed, 227 insertions(+), 40 deletions(-)
diff --git a/gdb/common/linux-procfs.c b/gdb/common/linux-procfs.c
index 1443a88..6333b0b 100644
--- a/gdb/common/linux-procfs.c
+++ b/gdb/common/linux-procfs.c
@@ -25,6 +25,7 @@
#include "linux-procfs.h"
#include "filestuff.h"
+#include "target.h"
/* Return the TGID of LWPID from /proc/pid/status. Returns -1 if not
found. */
@@ -119,3 +120,20 @@ linux_proc_pid_is_zombie (pid_t pid)
{
return linux_proc_pid_has_state (pid, "Z (zombie)");
}
+
+/* Accepts an integer PID; Returns a string representing a file that
+ can be opened to get the symbols for the child process. */
+
+char *
+linux_proc_pid_to_exec_file (struct target_ops *self, int pid)
+{
+ static char buf[PATH_MAX];
+ char name[PATH_MAX];
+
+ xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+ memset (buf, 0, PATH_MAX);
+ if (readlink (name, buf, PATH_MAX - 1) <= 0)
+ strcpy (buf, name);
+
+ return buf;
+}
diff --git a/gdb/common/linux-procfs.h b/gdb/common/linux-procfs.h
index d13fff7..d3581de 100644
--- a/gdb/common/linux-procfs.h
+++ b/gdb/common/linux-procfs.h
@@ -40,4 +40,8 @@ extern int linux_proc_pid_is_stopped (pid_t pid);
extern int linux_proc_pid_is_zombie (pid_t pid);
+/* Return pathname of exec file for process with PID. */
+
+extern char *linux_proc_pid_to_exec_file (struct target_ops *self, int pid);
+
#endif /* COMMON_LINUX_PROCFS_H */
diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
index af1d306..9fac55a 100644
--- a/gdb/common/linux-ptrace.c
+++ b/gdb/common/linux-ptrace.c
@@ -494,8 +494,11 @@ linux_test_for_traceexit (int child_pid)
if (ret == child_pid && WIFSTOPPED (status)
&& linux_is_extended_exit (status))
{
- /* PTRACE_O_TRACEEXIT is supported. */
- current_ptrace_options |= PTRACE_O_TRACEEXIT;
+ /* PTRACE_O_TRACEEXIT is supported. We use exit events to
+ implement support for exec events. Since fork events are
+ supported we know exec events are supported, so we enable
+ exec events here. */
+ current_ptrace_options |= PTRACE_O_TRACEEXIT | PTRACE_O_TRACEEXEC;
}
#endif
}
@@ -606,3 +609,11 @@ linux_is_extended_fork (int wstat)
{
return (wstat >> 16 == PTRACE_EVENT_FORK);
}
+
+/* Check wait status for extended exec event. */
+
+int
+linux_is_extended_exec (int wstat)
+{
+ return (wstat >> 16 == PTRACE_EVENT_EXEC);
+}
diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h
index eb93862..461cb5d 100644
--- a/gdb/common/linux-ptrace.h
+++ b/gdb/common/linux-ptrace.h
@@ -93,5 +93,6 @@ extern int linux_supports_tracesysgood (void);
extern int linux_is_extended_waitstatus (int wstat);
extern int linux_is_extended_exit (int wstat);
extern int linux_is_extended_fork (int wstat);
+extern int linux_is_extended_exec (int wstat);
#endif /* COMMON_LINUX_PTRACE_H */
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 5404404..99b0360 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -495,6 +495,19 @@ handle_extended_wait (struct lwp_info *event_child, int *wstatp)
(PTRACE_TYPE_ARG4) 0);
return ret;
}
+ else if (event == PTRACE_EVENT_EXEC)
+ {
+ if (debug_threads)
+ debug_printf ("HEW: Got exec event from LWP %ld\n",
+ lwpid_of (event_thr));
+
+ event_child->waitstatus.kind = TARGET_WAITKIND_EXECD;
+ event_child->waitstatus.value.execd_pathname
+ = xstrdup (linux_proc_pid_to_exec_file (NULL, lwpid_of (event_thr)));
+
+ /* Report the event. */
+ return 0;
+ }
internal_error (__FILE__, __LINE__,
_("unknown ptrace event %d"), event);
}
@@ -1362,18 +1375,19 @@ check_zombie_leaders (void)
program). In the latter case, we can't waitpid the
leader's exit status until all other threads are gone.
- - There are 3 or more threads in the group, and a thread
+ - There are multiple threads in the group, and a thread
other than the leader exec'd. On an exec, the Linux
kernel destroys all other threads (except the execing
one) in the thread group, and resets the execing thread's
tid to the tgid. No exit notification is sent for the
execing thread -- from the ptracer's perspective, it
appears as though the execing thread just vanishes.
- Until we reap all other threads except the leader and the
- execing thread, the leader will be zombie, and the
- execing thread will be in `D (disc sleep)'. As soon as
- all other threads are reaped, the execing thread changes
- it's tid to the tgid, and the previous (zombie) leader
+ Until we reap all other threads (if any) except the
+ leader and the execing thread, the leader will be zombie,
+ and the execing thread will be in `D (disc sleep)'. As
+ soon as all other threads are reaped, or have reported
+ PTRACE_EVENT_EXIT events, the execing thread changes its
+ tid to the tgid, and the previous (zombie) leader
vanishes, giving place to the "new" leader. We could try
distinguishing the exit and exec cases, by waiting once
more, and seeing if something comes out, but it doesn't
@@ -1381,7 +1395,11 @@ check_zombie_leaders (void)
we'll re-add the new one once we see the exec event
(which is just the same as what would happen if the
previous leader did exit voluntarily before some other
- thread execs). */
+ thread execs).
+
+ Note that when PTRACE_EVENT_EXEC is supported, we use
+ that mechanism to detect thread exit, including the
+ exit of zombie leaders. */
if (debug_threads)
fprintf (stderr,
@@ -1777,6 +1795,57 @@ linux_low_filter_event (ptid_t filter_ptid, int lwpid, int *wstatp)
child = find_lwp_pid (pid_to_ptid (lwpid));
+ /* Check for stop events reported by a process we didn't already
+ know about - anything not already in our LWP list.
+
+ If we're expecting to receive stopped processes after
+ fork, vfork, and clone events, then we'll just add the
+ new one to our list and go back to waiting for the event
+ to be reported - the stopped process might be returned
+ from waitpid before or after the event is.
+
+ But note the case of a non-leader thread exec'ing after the
+ leader having exited, and gone from our lists. On an exec,
+ the Linux kernel destroys all other threads (except the execing
+ one) in the thread group, and resets the execing thread's tid
+ to the tgid. No exit notification is sent for the execing
+ thread -- from the ptracer's perspective, it appears as though
+ the execing thread just vanishes. When they are available, we
+ use exit events (PTRACE_EVENT_EXIT) to detect thread exit
+ reliably. As soon as all other threads (if any) are reaped or
+ have reported their PTRACE_EVENT_EXIT events, the execing
+ thread changes it's tid to the tgid, and the previous (zombie)
+ leader vanishes, giving place to the "new" leader. The lwp
+ entry for the previous leader is deleted when we handle its
+ exit event, and we re-add the new one here. */
+
+ if (WIFSTOPPED (wstat) && child == NULL
+ && (WSTOPSIG (wstat) == SIGTRAP && linux_is_extended_exec (wstat)))
+ {
+ ptid_t child_ptid;
+
+ /* A multi-thread exec after we had seen the leader exiting. */
+ if (debug_threads)
+ debug_printf ("LLW: Re-adding thread group leader LWP %d.\n",
+ lwpid);
+
+ child_ptid = ptid_build (lwpid, lwpid, 0);
+ child = add_lwp (child_ptid);
+ child->stopped = 1;
+ current_inferior = child->thread;
+
+ if (non_stop && stopping_threads == NOT_STOPPING_THREADS)
+ {
+ /* Make sure we delete the lwp entry for the exec'ing thread,
+ which will have vanished. We do this by sending a signal
+ to all the other threads in the lwp list, deleting any
+ that are not found. Note that in all-stop mode this will
+ happen before reporting the event. */
+ stop_all_lwps (0, child);
+ unstop_all_lwps (0, child);
+ }
+ }
+
/* If we didn't find a process, one of two things presumably happened:
- A process we started and then detached from has exited. Ignore it.
- A process we are controlling has forked and the new child's stop
@@ -2108,8 +2177,7 @@ linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid,
- When a non-leader thread execs, that thread just vanishes
without reporting an exit (so we'd hang if we waited for it
explicitly in that case). The exec event is reported to
- the TGID pid (although we don't currently enable exec
- events). */
+ the TGID pid. */
errno = 0;
ret = my_waitpid (-1, wstatp, options | WNOHANG);
@@ -2506,6 +2574,20 @@ linux_stabilize_threads (void)
}
}
+/* Return non-zero if WAITSTATUS reflects an extended linux
+ event. Otherwise, return 0. Note that extended EXIT
+ events are fixed up and handled like normal events, so
+ they are not considered here. */
+
+static int
+extended_event_reported (const struct target_waitstatus *waitstatus)
+{
+ if (waitstatus == NULL)
+ return 0;
+
+ return (waitstatus->kind == TARGET_WAITKIND_EXECD);
+}
+
/* Wait for process, returns status. */
static ptid_t
@@ -2869,7 +2951,8 @@ retry:
&& !bp_explains_trap && !trace_event)
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
- && gdb_no_commands_at_breakpoint (event_child->stop_pc)));
+ && gdb_no_commands_at_breakpoint (event_child->stop_pc))
+ || extended_event_reported (&event_child->waitstatus));
run_breakpoint_commands (event_child->stop_pc);
@@ -2891,6 +2974,14 @@ retry:
paddress (event_child->stop_pc),
paddress (event_child->step_range_start),
paddress (event_child->step_range_end));
+ if (debug_threads
+ && extended_event_reported (&event_child->waitstatus))
+ {
+ char *str = target_waitstatus_to_string (ourstatus);
+ debug_printf ("LWP %ld: extended event with waitstatus %s\n",
+ lwpid_of (get_lwp_thread (event_child)), str);
+ xfree (str);
+ }
}
/* We're not reporting this breakpoint to GDB, so apply the
@@ -2989,7 +3080,19 @@ retry:
unstop_all_lwps (1, event_child);
}
- ourstatus->kind = TARGET_WAITKIND_STOPPED;
+ if (extended_event_reported (&event_child->waitstatus))
+ {
+ /* If the reported event is a fork, vfork or exec, let GDB
+ know. */
+ ourstatus->kind = event_child->waitstatus.kind;
+ ourstatus->value = event_child->waitstatus.value;
+
+ /* Reset the event child's waitstatus since we handled it
+ already. */
+ event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
+ }
+ else
+ ourstatus->kind = TARGET_WAITKIND_STOPPED;
if (current_inferior->last_resume_kind == resume_stop
&& WSTOPSIG (w) == SIGSTOP)
@@ -3000,13 +3103,14 @@ retry:
ourstatus->value.sig = GDB_SIGNAL_0;
}
else if (current_inferior->last_resume_kind == resume_stop
- && WSTOPSIG (w) != SIGSTOP)
+ && WSTOPSIG (w) != SIGSTOP
+ && !extended_event_reported (ourstatus))
{
/* A thread that has been requested to stop by GDB with vCont;t,
but, it stopped for other reasons. */
ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
}
- else
+ else if (ourstatus->kind == TARGET_WAITKIND_STOPPED)
{
ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
}
@@ -3112,6 +3216,7 @@ static void
send_sigstop (struct lwp_info *lwp)
{
int pid;
+ int ret;
pid = lwpid_of (get_lwp_thread (lwp));
@@ -3129,7 +3234,21 @@ send_sigstop (struct lwp_info *lwp)
debug_printf ("Sending sigstop to lwp %d\n", pid);
lwp->stop_expected = 1;
- kill_lwp (pid, SIGSTOP);
+ errno = 0;
+ ret = kill_lwp (pid, SIGSTOP);
+ if (ret == -1 && errno == ESRCH)
+ {
+ /* If the kill fails with "No such process", on GNU/Linux we know
+ that the LWP has vanished - it is not a zombie, it is gone.
+ This is due to a thread other than the thread group leader
+ calling exec. See comments in linux_low_filter_event regarding
+ PTRACE_EVENT_EXEC. */
+ delete_lwp (lwp);
+ set_desired_inferior (0);
+
+ if (debug_threads)
+ debug_printf ("send_sigstop: lwp %d has vanished\n", pid);
+ }
}
static int
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 498b221..fea6deb 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -269,6 +269,11 @@ struct lwp_info
/* When stopped is set, the last wait status recorded for this lwp. */
int last_status;
+ /* If WAITSTATUS->KIND != TARGET_WAITKIND_IGNORE, the waitstatus for
+ this LWP's last event. This may correspond to LAST_STATUS above,
+ or to the current status during event processing. */
+ struct target_waitstatus waitstatus;
+
/* When stopped is set, this is where the lwp stopped, with
decr_pc_after_break already accounted for. */
CORE_ADDR stop_pc;
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 4fcafa0..9ce25dc 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1111,14 +1111,40 @@ prepare_resume_reply (char *buf, ptid_t ptid,
switch (status->kind)
{
case TARGET_WAITKIND_STOPPED:
+ case TARGET_WAITKIND_EXECD:
{
struct thread_info *saved_inferior;
const char **regp;
struct regcache *regcache;
+ enum gdb_signal signal;
+
+ if (status->kind == TARGET_WAITKIND_EXECD)
+ signal = GDB_SIGNAL_TRAP;
+ else
+ signal = status->value.sig;
+
+ sprintf (buf, "T%02x", signal);
- sprintf (buf, "T%02x", status->value.sig);
buf += strlen (buf);
+ if (status->kind == TARGET_WAITKIND_EXECD && multi_process)
+ {
+ const char *event = "exec";
+ char hexified_pathname[PATH_MAX];
+
+ sprintf (buf, "%s:", event);
+ buf += strlen (buf);
+
+ /* Encode pathname to hexified format. */
+ bin2hex ((const gdb_byte *) status->value.execd_pathname,
+ hexified_pathname, strlen(status->value.execd_pathname));
+
+ sprintf (buf, "%s;", hexified_pathname);
+ xfree (status->value.execd_pathname);
+ status->value.execd_pathname = NULL;
+ buf += strlen (buf);
+ }
+
saved_inferior = current_inferior;
current_inferior = find_thread_ptid (ptid);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index e84ee95..762811c 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -845,7 +845,6 @@ linux_nat_pass_signals (struct target_ops *self,
/* Prototypes for local functions. */
static int stop_wait_callback (struct lwp_info *lp, void *data);
static int linux_thread_alive (ptid_t ptid);
-static char *linux_child_pid_to_exec_file (struct target_ops *self, int pid);
\f
@@ -2172,7 +2171,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status,
ourstatus->kind = TARGET_WAITKIND_EXECD;
ourstatus->value.execd_pathname
- = xstrdup (linux_child_pid_to_exec_file (NULL, pid));
+ = xstrdup (linux_proc_pid_to_exec_file (NULL, pid));
return 0;
}
@@ -4008,23 +4007,6 @@ linux_nat_thread_name (struct target_ops *self, struct thread_info *thr)
return result;
}
-/* Accepts an integer PID; Returns a string representing a file that
- can be opened to get the symbols for the child process. */
-
-static char *
-linux_child_pid_to_exec_file (struct target_ops *self, int pid)
-{
- static char buf[PATH_MAX];
- char name[PATH_MAX];
-
- xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
- memset (buf, 0, PATH_MAX);
- if (readlink (name, buf, PATH_MAX - 1) <= 0)
- strcpy (buf, name);
-
- return buf;
-}
-
/* Records the thread's register state for the corefile note
section. */
@@ -4473,7 +4455,7 @@ linux_target_install_ops (struct target_ops *t)
t->to_insert_exec_catchpoint = linux_child_insert_exec_catchpoint;
t->to_remove_exec_catchpoint = linux_child_remove_exec_catchpoint;
t->to_set_syscall_catchpoint = linux_child_set_syscall_catchpoint;
- t->to_pid_to_exec_file = linux_child_pid_to_exec_file;
+ t->to_pid_to_exec_file = linux_proc_pid_to_exec_file;
t->to_post_startup_inferior = linux_child_post_startup_inferior;
t->to_post_attach = linux_child_post_attach;
t->to_follow_fork = linux_child_follow_fork;
diff --git a/gdb/remote.c b/gdb/remote.c
index 964bd41..ffe9df4 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5415,11 +5415,13 @@ remote_parse_stop_reply (char *buf, struct stop_reply *event)
pnum and set p1 to point to the character following it.
Otherwise p1 points to p. */
- /* If this packet is an awatch packet, don't parse the 'a'
- as a register number. */
+ /* If this packet has a stop reason string that starts
+ with a character that could be a hex digit, don't parse
+ it as a register number. */
if (strncmp (p, "awatch", strlen("awatch")) != 0
- && strncmp (p, "core", strlen ("core") != 0))
+ && strncmp (p, "core", strlen ("core") != 0)
+ && strncmp (p, "exec", strlen ("exec") != 0))
{
/* Read the ``P'' register number. */
pnum = strtol (p, &p_temp, 16);
@@ -5471,6 +5473,25 @@ Packet: '%s'\n"),
p = unpack_varlen_hex (++p1, &c);
event->core = c;
}
+ else if (strncmp (p, "exec", p1 - p) == 0)
+ {
+ ULONGEST pid;
+ char pathname[PATH_MAX];
+
+ p = unpack_varlen_hex (++p1, &pid);
+
+ /* Save the pathname for event reporting and for
+ the next run command. */
+ hex2bin (p1, (gdb_byte *) pathname, (p - p1)/2);
+ /* Add the null terminator. */
+ pathname[(p - p1)/2] = '\0';
+ /* This is freed during event handling. */
+ event->ws.value.execd_pathname = xstrdup (pathname);
+ event->ws.kind = TARGET_WAITKIND_EXECD;
+ /* Save the pathname for the next run command. */
+ xfree (remote_exec_file);
+ remote_exec_file = pathname;
+ }
else
{
/* Silently skip unknown optional info. */
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 3/4] Document RSP support for Linux exec events
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
` (5 preceding siblings ...)
2014-05-23 22:49 ` [PATCH 2/4 v3] Remote Linux ptrace exec events Don Breazeal
@ 2014-05-23 22:49 ` Don Breazeal
2014-05-24 7:20 ` Eli Zaretskii
2014-05-23 22:49 ` [PATCH 4/4] Non-stop exec tests Don Breazeal
` (2 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Don Breazeal @ 2014-05-23 22:49 UTC (permalink / raw)
To: gdb-patches
This patch adds documentation of the new RSP support for exec events.
Regards,
--Don
gdb/
2014-05-22 Don Breazeal <donb@codesourcery.com>
* NEWS: Mention RSP Stop Reply Packet, new stop reason 'exec'.
Mention gdbserver support for exec events on Linux.
doc/
2014-05-22 Don Breazeal <donb@codesourcery.com>
* gdb.texinfo (Stop Reply Packets): Document RSP support
for exec events.
---
gdb/NEWS | 6 ++++++
gdb/doc/gdb.texinfo | 6 ++++++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index b23c8a0..ffe3520 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -69,6 +69,8 @@ maint ada show ignore-descriptive-types
Timestamps can also be turned on with the
"monitor set debug-format timestamps" command from GDB.
+ ** GDBserver on GNU/Linux now supports exec events (follow-exec-mode).
+
* The 'record instruction-history' command now starts counting instructions
at one. This also affects the instruction ranges reported by the
'record function-call-history' command when given the /i modifier.
@@ -103,6 +105,10 @@ qXfer:btrace:read's annex
The qXfer:btrace:read packet supports a new annex 'delta' to read
branch trace incrementally.
+T Stop Reply Packet's reason
+ The T stop reply packet supports a new stop reason, 'exec', signifying
+ that the specified inferior executed a call to execve.
+
* Python Scripting
** Valid Python operations on gdb.Value objects representing
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6092ff4..55f98c3 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34589,6 +34589,12 @@ The packet indicates that the loaded libraries have changed.
@value{GDBN} should use @samp{qXfer:libraries:read} to fetch a new
list of loaded libraries. @var{r} is ignored.
+@cindex exec events, remote reply
+@item exec
+The packet indicates that @code{execve} was called, and @var{r} is the
+absolute pathname of the file that was executed, in hex. This is
+only applicable to certain targets.
+
@cindex replay log events, remote reply
@item replaylog
The packet indicates that the target cannot continue replaying
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/4] Document RSP support for Linux exec events
2014-05-23 22:49 ` [PATCH 3/4] Document RSP support for Linux " Don Breazeal
@ 2014-05-24 7:20 ` Eli Zaretskii
2014-05-27 21:28 ` Breazeal, Don
0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2014-05-24 7:20 UTC (permalink / raw)
To: Don Breazeal; +Cc: gdb-patches
> From: Don Breazeal <donb@codesourcery.com>
> Date: Fri, 23 May 2014 15:49:33 -0700
>
> This patch adds documentation of the new RSP support for exec events.
Thanks.
> +@cindex exec events, remote reply
> +@item exec
> +The packet indicates that @code{execve} was called, and @var{r} is the
> +absolute pathname of the file that was executed, in hex.
Please use "file name". GNU coding standards frown on using
"pathname" in this context.
> This is
> +only applicable to certain targets.
What is "only applicable to certain targets", the packet or the fact
that R is the file name of the exec'ed process?
I also think you should mention the fact that gdbserver supports
follow-exec-mode in the "Forks" node, as the text there does sound as
if this is only supported in native debugging.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/4] Non-stop exec tests
2014-05-23 22:49 ` [PATCH 4/4] Non-stop exec tests Don Breazeal
@ 2014-05-26 3:29 ` Doug Evans
2014-05-27 18:31 ` Breazeal, Don
0 siblings, 1 reply; 33+ messages in thread
From: Doug Evans @ 2014-05-26 3:29 UTC (permalink / raw)
To: Don Breazeal; +Cc: gdb-patches
On Fri, May 23, 2014 at 3:49 PM, Don Breazeal <donb@codesourcery.com> wrote:
> This patch extends the non-ldr-exc-*.exp tests so that they run all their cases in non-stop mode as well as all-stop mode. These tests cover handling of exec events when non-leader threads call exec.
>
> The tests now report 'untested when 'runto_main' fails. In non-stop mode with 'target extended-remote', runto_main always fails with something like:
>
> (gdb) run
> Starting program: /home/me/gdb/testsuite/gdb.threads/non-ldr-exc-4
> Unexpected vCont reply in non-stop mode: T0506:10e0ffffff7f0000;07:c8deffffff7f0000;10:c1a6abaaaa2a0000;thread:p5ee.5ee;core:0;
Did you try --target_board=native-extended-gdbserver ?
> This happens in other tests as well (e.g. gdb.threads/thread_events.exp), so I copied the error handling from that test so that the non-stop tests report 'untested' for target extended-remote. I couldn't find anything related to this in the gdb bug database, but I assume it is a known problem since the other tests handle it.
>
> Regards,
> --Don
>
> testsuite/
> 2014-05-22 Don Breazeal <donb@codesourcery.com>
>
> * gdb.threads/non-ldr-exc-1.exp: Add non-stop cases
> * gdb.threads/non-ldr-exc-2.exp: Ditto.
> * gdb.threads/non-ldr-exc-3.exp: Ditto.
> * gdb.threads/non-ldr-exc-4.exp: Ditto.
Hi.
Still going through the patch series, but have an initial comment.
The following test at the top of each .exp file could be changed to
something like !linux && is_remote, right?
# No exec event support in the remote protocol.
if { [is_remote target] } then {
continue
}
These tests now pass with native-gdbserver (and
native-extended-gdbserver) so we'll want to at least run these tests
with those, I presume.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/4 v3] Exec events in gdbserver on Linux
2014-05-23 22:49 ` [PATCH 0/4 v3] Exec events in gdbserver on Linux Don Breazeal
@ 2014-05-26 4:55 ` Doug Evans
2014-05-27 18:49 ` Breazeal, Don
2014-06-05 22:06 ` Breazeal, Don
1 sibling, 1 reply; 33+ messages in thread
From: Doug Evans @ 2014-05-26 4:55 UTC (permalink / raw)
To: Don Breazeal; +Cc: gdb-patches
On Fri, May 23, 2014 at 3:49 PM, Don Breazeal <donb@codesourcery.com> wrote:
> This patch series is an update to the gdbserver Linux exec event patches based on review comments for the previous version. The changes from the previous version are summarized below.
>
> [...]
>
> RACE CONDITION
>
> This section explains why the existing techniques for detecting thread exit weren't sufficient for gdbserver exec events, necessitating the use of PTRACE_EVENT_EXIT. The short answer is that there is a race condition in the current implementation that can leave a dangling entry in the lwp list (an entry that doesn't have a corresponding actual lwp). In this case gdbserver will hang waiting for the non-existent lwp to stop. Using the exit events eliminates this race condition.
>
> The same race may exist in the native implementation, since the two implementations are similar, but I haven't verified that. It may be difficult to concoct a test case that demonstrates the race since the window is so small.
>
> Now for the long answer: in my testing I ran into a race condition in check_zombie_leaders, which detects when a thread group leader has exited and other threads still exist. On the Linux kernel, ptrace/waitpid don't allow reaping the leader thread until all other threads in the group are reaped. When the leader exits, it goes zombie, but waitpid will not return an exit status until the other threads are gone. When a non-leader thread calls exec, all other non-leader threads are destroyed, the leader becomes a zombie, and once the "other" threads have been reaped, the execing thread takes over the leader's pid (tgid) and appears to vanish. In order to handle this situation, check_zombie_leaders polls the process state in /proc and deletes thread group leaders that are in a zombie state. The replacement is added to the lwp list when the exec event is reported.
>
> See https://sourceware.org/ml/gdb-patches/2011-10/msg00704.html for a more detailed explanation of how this works.
>
> Here is the relevant part of check_zombie_leaders:
>
> if (leader_lp != NULL
> /* Check if there are other threads in the group, as we may
> have raced with the inferior simply exiting. */
> && !last_thread_of_process_p (leader_pid)
> && linux_proc_pid_is_zombie (leader_pid))
> {
> /* ...large informative comment block... */
> delete_lwp (leader_lp);
>
> The race occurred when there were two threads in the program, and the non-leader thread called exec. In this case the leader thread passed through a very brief zombie state before being replaced by the exec'ing thread as the thread group leader. This state transition was asynchronous, with no dependency on anything gdbserver did. Because there were no other threads, there were no thread exit events, and thus there was no synchronization with the leader passing through the zombie state and the exec completing. If there had been more threads, the leader would remain in the zombie state until they were waited for. In the two-thread case, sometimes the leader exit was detected and sometimes it wasn't. (Recall that check_zombie_leaders is polling the state, via linux_proc_pid_is_zombie. The race is between the leader thread passing through the zombie state and check_zombie_leaders testing for zombie state.) If leader exit wasn't detected, gdbserver would end up with a dangl
> ing lwp entry that didn't correspond to any real lwp, and would hang waiting for that lwp to stop. Using PTRACE_EVENT_EXIT guarantees that the leader exit will be detected.
>
> Note that check_zombie_leaders works just fine for the scenarios where the leader thread exits and the other threads continue to run, with no exec calls. It is required for systems that don't support the extended ptrace events.
>
> The sequence of events resulting in the race condition was this:
>
> 1) In the program, a CLONE event for a new thread occurs.
>
> 2) In the program, both threads are resumed once gdbserver has
> completed the new thread processing.
>
> 3) In gdbserver, the function linux_wait_for_event_filtered loops until
> waitpid returns "no more events" for the SIGCHLD generated by the
> CLONE event. Then linux_wait_for_event_filtered calls
> check_zombie_leaders.
>
> 4) In the program, the new thread is doing the exec. During the exec
> the leader thread will pass through a transitory zombie state. If
> there were more than two threads, the leader thread would remain a
> zombie until all the non-leader, non-exec'ing threads were reaped by
> gdbserver. Since there are no such threads to reap, the leader just
> becomes a zombie and is replaced by the exec'ing thread on-the-fly.
> (Note that it appears that the leader thread is a zombie just for a
> very brief instant.)
>
> 5) In gdbserver, check_zombie_leaders checks whether an lwp entry
> corresponds to a zombie leader thread, and if so, deletes it. Here
> is the race: in (4) above, the leader may or may not be in the
> transitory zombie state. In the case where a zombie isn't detected,
> delete_lwp is not called.
>
> 6) In gdbserver, an EXEC event is detected and processed. When it gets
> ready to report the event to GDB, it calls stop_all_lwps, which sends
> a SIGSTOP to each lwp in the list and the waits until all the lwps in
> the list have reported a stop event. If the zombie leader wasn't
> detected and processed in step (5), gdbserver blocks forever in
> linux_wait_for_event_filtered, waiting for the undeleted lwp to be
> stopped, which will never happen.
Hi.
How do I tweak your patch so that I can see the race condition for myself?
[I realize the window is small ... I'd just like to play with it.]
I tried just disabling PTRACE_O_TRACEEXIT but that causes segvs in the
inferior (using non-ldr-exc-1 testcase).
Still digging into that.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/4] Non-stop exec tests
2014-05-26 3:29 ` Doug Evans
@ 2014-05-27 18:31 ` Breazeal, Don
0 siblings, 0 replies; 33+ messages in thread
From: Breazeal, Don @ 2014-05-27 18:31 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
Thank you for looking at this.
On 5/25/2014 8:29 PM, Doug Evans wrote:
> On Fri, May 23, 2014 at 3:49 PM, Don Breazeal <donb@codesourcery.com> wrote:
>> This patch extends the non-ldr-exc-*.exp tests so that they run all their cases in non-stop mode as well as all-stop mode. These tests cover handling of exec events when non-leader threads call exec.
>>
>> The tests now report 'untested when 'runto_main' fails. In non-stop mode with 'target extended-remote', runto_main always fails with something like:
>>
>> (gdb) run
>> Starting program: /home/me/gdb/testsuite/gdb.threads/non-ldr-exc-4
>> Unexpected vCont reply in non-stop mode: T0506:10e0ffffff7f0000;07:c8deffffff7f0000;10:c1a6abaaaa2a0000;thread:p5ee.5ee;core:0;
>
> Did you try --target_board=native-extended-gdbserver ?
Yes. To be clear, the exec events are only implemented for multiprocess
/ extended-remote. Before posting the initial patch I tested native
GDB, --target_board=native-gdbserver, and
--target_board=native-extended-gdbserver.
>
>> This happens in other tests as well (e.g. gdb.threads/thread_events.exp), so I copied the error handling from that test so that the non-stop tests report 'untested' for target extended-remote. I couldn't find anything related to this in the gdb bug database, but I assume it is a known problem since the other tests handle it.
>>
>> Regards,
>> --Don
>>
>> testsuite/
>> 2014-05-22 Don Breazeal <donb@codesourcery.com>
>>
>> * gdb.threads/non-ldr-exc-1.exp: Add non-stop cases
>> * gdb.threads/non-ldr-exc-2.exp: Ditto.
>> * gdb.threads/non-ldr-exc-3.exp: Ditto.
>> * gdb.threads/non-ldr-exc-4.exp: Ditto.
>
> Hi.
> Still going through the patch series, but have an initial comment.
>
> The following test at the top of each .exp file could be changed to
> something like !linux && is_remote, right?
>
> # No exec event support in the remote protocol.
> if { [is_remote target] } then {
> continue
> }
>
> These tests now pass with native-gdbserver (and
> native-extended-gdbserver) so we'll want to at least run these tests
> with those, I presume.
>
I don't expect the tests to pass with native-gdbserver, since the
implementation is only for multiprocess/extended-remote. I should have
made that more apparent.
[Note: the rationale for restricting exec events to multiprocess was
that follow-exec-mode can allow multiple inferiors to exist, even if
they aren't running simultaneously. Also, follow-exec-mode and
follow-fork-mode are closely related, and follow-fork-mode definitely
requires multiprocess.]
The native-extended-gdbserver tests aren't affected by the test for
remote at the top of each .exp file. The GDB manual says that these
events (or at least fork events) are supported for native GDB on newer
versions of Linux and HP-UX, and any change I make for
native-extended-gdbserver will affect the native tests.
Given that the feature is only available with extended-remote, do you
still feel I need to change something? I'm not sure how to restrict a
test to "Linux or HP-UX", or to newer versions of them.
Thanks
--Don
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/4 v3] Exec events in gdbserver on Linux
2014-05-26 4:55 ` Doug Evans
@ 2014-05-27 18:49 ` Breazeal, Don
2014-05-27 21:41 ` Breazeal, Don
0 siblings, 1 reply; 33+ messages in thread
From: Breazeal, Don @ 2014-05-27 18:49 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
On 5/25/2014 9:55 PM, Doug Evans wrote:
> On Fri, May 23, 2014 at 3:49 PM, Don Breazeal <donb@codesourcery.com> wrote:
>> This patch series is an update to the gdbserver Linux exec event patches based on review comments for the previous version. The changes from the previous version are summarized below.
>>
>> [...]
>>
>> RACE CONDITION
>>
>> This section explains why the existing techniques for detecting thread exit weren't sufficient for gdbserver exec events, necessitating the use of PTRACE_EVENT_EXIT. The short answer is that there is a race condition in the current implementation that can leave a dangling entry in the lwp list (an entry that doesn't have a corresponding actual lwp). In this case gdbserver will hang waiting for the non-existent lwp to stop. Using the exit events eliminates this race condition.
>>
>> The same race may exist in the native implementation, since the two implementations are similar, but I haven't verified that. It may be difficult to concoct a test case that demonstrates the race since the window is so small.
>>
>> Now for the long answer: in my testing I ran into a race condition in check_zombie_leaders, which detects when a thread group leader has exited and other threads still exist. On the Linux kernel, ptrace/waitpid don't allow reaping the leader thread until all other threads in the group are reaped. When the leader exits, it goes zombie, but waitpid will not return an exit status until the other threads are gone. When a non-leader thread calls exec, all other non-leader threads are destroyed, the leader becomes a zombie, and once the "other" threads have been reaped, the execing thread takes over the leader's pid (tgid) and appears to vanish. In order to handle this situation, check_zombie_leaders polls the process state in /proc and deletes thread group leaders that are in a zombie state. The replacement is added to the lwp list when the exec event is reported.
>>
>> See https://sourceware.org/ml/gdb-patches/2011-10/msg00704.html for a more detailed explanation of how this works.
>>
>> Here is the relevant part of check_zombie_leaders:
>>
>> if (leader_lp != NULL
>> /* Check if there are other threads in the group, as we may
>> have raced with the inferior simply exiting. */
>> && !last_thread_of_process_p (leader_pid)
>> && linux_proc_pid_is_zombie (leader_pid))
>> {
>> /* ...large informative comment block... */
>> delete_lwp (leader_lp);
>>
>> The race occurred when there were two threads in the program, and the non-leader thread called exec. In this case the leader thread passed through a very brief zombie state before being replaced by the exec'ing thread as the thread group leader. This state transition was asynchronous, with no dependency on anything gdbserver did. Because there were no other threads, there were no thread exit events, and thus there was no synchronization with the leader passing through the zombie state and the exec completing. If there had been more threads, the leader would remain in the zombie state until they were waited for. In the two-thread case, sometimes the leader exit was detected and sometimes it wasn't. (Recall that check_zombie_leaders is polling the state, via linux_proc_pid_is_zombie. The race is between the leader thread passing through the zombie state and check_zombie_leaders testing for zombie state.) If leader exit wasn't detected, gdbserver would end up with a dangl
>> ing lwp entry that didn't correspond to any real lwp, and would hang waiting for that lwp to stop. Using PTRACE_EVENT_EXIT guarantees that the leader exit will be detected.
>>
>> Note that check_zombie_leaders works just fine for the scenarios where the leader thread exits and the other threads continue to run, with no exec calls. It is required for systems that don't support the extended ptrace events.
>>
>> The sequence of events resulting in the race condition was this:
>>
>> 1) In the program, a CLONE event for a new thread occurs.
>>
>> 2) In the program, both threads are resumed once gdbserver has
>> completed the new thread processing.
>>
>> 3) In gdbserver, the function linux_wait_for_event_filtered loops until
>> waitpid returns "no more events" for the SIGCHLD generated by the
>> CLONE event. Then linux_wait_for_event_filtered calls
>> check_zombie_leaders.
>>
>> 4) In the program, the new thread is doing the exec. During the exec
>> the leader thread will pass through a transitory zombie state. If
>> there were more than two threads, the leader thread would remain a
>> zombie until all the non-leader, non-exec'ing threads were reaped by
>> gdbserver. Since there are no such threads to reap, the leader just
>> becomes a zombie and is replaced by the exec'ing thread on-the-fly.
>> (Note that it appears that the leader thread is a zombie just for a
>> very brief instant.)
>>
>> 5) In gdbserver, check_zombie_leaders checks whether an lwp entry
>> corresponds to a zombie leader thread, and if so, deletes it. Here
>> is the race: in (4) above, the leader may or may not be in the
>> transitory zombie state. In the case where a zombie isn't detected,
>> delete_lwp is not called.
>>
>> 6) In gdbserver, an EXEC event is detected and processed. When it gets
>> ready to report the event to GDB, it calls stop_all_lwps, which sends
>> a SIGSTOP to each lwp in the list and the waits until all the lwps in
>> the list have reported a stop event. If the zombie leader wasn't
>> detected and processed in step (5), gdbserver blocks forever in
>> linux_wait_for_event_filtered, waiting for the undeleted lwp to be
>> stopped, which will never happen.
>
> Hi.
>
> How do I tweak your patch so that I can see the race condition for myself?
> [I realize the window is small ... I'd just like to play with it.]
Hi Doug, thanks for looking at this. I will have to look back through
my notes to see if I can come up with a reasonable way of doing this. I
was using the same test case that you were, running the basic test
manually. I saw the race condition by inserting fprintf(stderr,.. into
linux_proc_pid_has_state and check_zombie_leaders, running with no debug
output.
From my notes:
----------------------------------------------
# result of if stmt condition in check_zombie_leaders
# original leader is in zombie state
# linux_proc_pid_has_state: procfile:
# State: Z (zombie)
#
# result inside 'if' stmt in check_zombie_leaders - execing
# thread has replaced original leader since we evaluated
# the 'if' condition
# linux_proc_pid_has_state: procfile:
# State: R (running)
#
# printed inside if stmt, required zombie=1 to get here
# we still think we have 2 lwps, but after the exec there
# is only one. zombie=0 came from call to linux_proc_pid_has_state
# above.
# check_zombie_leaders: leader_pid=30981, leader_lp!=NULL=1, num_lwps=2,
# zombie=0
# ----------------------------------------------
The race could manifest in ways other than the one demonstrated here, of
course.
>
> I tried just disabling PTRACE_O_TRACEEXIT but that causes segvs in the
> inferior (using non-ldr-exc-1 testcase).
> Still digging into that.
>
I made the EXEC event support depend on the EXIT events. I have no idea
what would happen if the EXIT events were disabled.
Thanks
--Don
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/4] Document RSP support for Linux exec events
2014-05-24 7:20 ` Eli Zaretskii
@ 2014-05-27 21:28 ` Breazeal, Don
2014-05-28 14:22 ` Eli Zaretskii
0 siblings, 1 reply; 33+ messages in thread
From: Breazeal, Don @ 2014-05-27 21:28 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 5/24/2014 12:20 AM, Eli Zaretskii wrote:
>> From: Don Breazeal <donb@codesourcery.com>
>> Date: Fri, 23 May 2014 15:49:33 -0700
>>
>> This patch adds documentation of the new RSP support for exec events.
>
> Thanks.
>
>> +@cindex exec events, remote reply
>> +@item exec
>> +The packet indicates that @code{execve} was called, and @var{r} is the
>> +absolute pathname of the file that was executed, in hex.
>
> Please use "file name". GNU coding standards frown on using
> "pathname" in this context.
OK.
>
>> This is
>> +only applicable to certain targets.
>
> What is "only applicable to certain targets", the packet or the fact
> that R is the file name of the exec'ed process?
Point taken. It's the packet.
>
> I also think you should mention the fact that gdbserver supports
> follow-exec-mode in the "Forks" node, as the text there does sound as
> if this is only supported in native debugging.
OK.
Updated patch follows below.
Thanks for checking this over,
--Don
gdb/
2014-05-27 Don Breazeal <donb@codesourcery.com>
* NEWS: Mention RSP Stop Reply Packet, new stop reason 'exec'.
Mention gdbserver support for exec events on Linux.
doc/
2014-05-27 Don Breazeal <donb@codesourcery.com>
* gdb.texinfo (Stop Reply Packets): Document RSP support
and gdbserver support for remote Linux exec events.
---
gdb/NEWS | 6 ++++++
gdb/doc/gdb.texinfo | 9 ++++++++-
2 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index b23c8a0..ffe3520 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -69,6 +69,8 @@ maint ada show ignore-descriptive-types
Timestamps can also be turned on with the
"monitor set debug-format timestamps" command from GDB.
+ ** GDBserver on GNU/Linux now supports exec events (follow-exec-mode).
+
* The 'record instruction-history' command now starts counting instructions
at one. This also affects the instruction ranges reported by the
'record function-call-history' command when given the /i modifier.
@@ -103,6 +105,10 @@ qXfer:btrace:read's annex
The qXfer:btrace:read packet supports a new annex 'delta' to read
branch trace incrementally.
+T Stop Reply Packet's reason
+ The T stop reply packet supports a new stop reason, 'exec', signifying
+ that the specified inferior executed a call to execve.
+
* Python Scripting
** Valid Python operations on gdb.Value objects representing
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6092ff4..efe7603 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3141,7 +3141,8 @@ process, use the @code{file} command with the
parent executable name
as its argument. By default, after an @code{exec} call executes,
@value{GDBN} discards the symbols of the previous executable image.
You can change this behaviour with the @w{@code{set follow-exec-mode}}
-command.
+command. This command is supported when connected to @code{gdbserver}
+using @kbd{target extended-remote} as well as in native mode.
@table @code
@kindex set follow-exec-mode
@@ -34589,6 +34590,12 @@ The packet indicates that the loaded libraries
have changed.
@value{GDBN} should use @samp{qXfer:libraries:read} to fetch a new
list of loaded libraries. @var{r} is ignored.
+@cindex exec events, remote reply
+@item exec
+The packet indicates that @code{execve} was called, and @var{r} is the
+absolute file name of the file that was executed, in hex. This packet
+is only applicable to certain targets.
+
@cindex replay log events, remote reply
@item replaylog
The packet indicates that the target cannot continue replaying
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/4 v3] Exec events in gdbserver on Linux
2014-05-27 18:49 ` Breazeal, Don
@ 2014-05-27 21:41 ` Breazeal, Don
2014-05-28 18:02 ` Breazeal, Don
0 siblings, 1 reply; 33+ messages in thread
From: Breazeal, Don @ 2014-05-27 21:41 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
On 5/27/2014 11:49 AM, Breazeal, Don wrote:
> On 5/25/2014 9:55 PM, Doug Evans wrote:
>> On Fri, May 23, 2014 at 3:49 PM, Don Breazeal <donb@codesourcery.com> wrote:
>>> This patch series is an update to the gdbserver Linux exec event patches based on review comments for the previous version. The changes from the previous version are summarized below.
>>>
>>> [...]
>>>
>>> RACE CONDITION
>>>
>>> This section explains why the existing techniques for detecting thread exit weren't sufficient for gdbserver exec events, necessitating the use of PTRACE_EVENT_EXIT. The short answer is that there is a race condition in the current implementation that can leave a dangling entry in the lwp list (an entry that doesn't have a corresponding actual lwp). In this case gdbserver will hang waiting for the non-existent lwp to stop. Using the exit events eliminates this race condition.
>>>
>>> The same race may exist in the native implementation, since the two implementations are similar, but I haven't verified that. It may be difficult to concoct a test case that demonstrates the race since the window is so small.
>>>
>>> Now for the long answer: in my testing I ran into a race condition in check_zombie_leaders, which detects when a thread group leader has exited and other threads still exist. On the Linux kernel, ptrace/waitpid don't allow reaping the leader thread until all other threads in the group are reaped. When the leader exits, it goes zombie, but waitpid will not return an exit status until the other threads are gone. When a non-leader thread calls exec, all other non-leader threads are destroyed, the leader becomes a zombie, and once the "other" threads have been reaped, the execing thread takes over the leader's pid (tgid) and appears to vanish. In order to handle this situation, check_zombie_leaders polls the process state in /proc and deletes thread group leaders that are in a zombie state. The replacement is added to the lwp list when the exec event is reported.
>>>
>>> See https://sourceware.org/ml/gdb-patches/2011-10/msg00704.html for a more detailed explanation of how this works.
>>>
>>> Here is the relevant part of check_zombie_leaders:
>>>
>>> if (leader_lp != NULL
>>> /* Check if there are other threads in the group, as we may
>>> have raced with the inferior simply exiting. */
>>> && !last_thread_of_process_p (leader_pid)
>>> && linux_proc_pid_is_zombie (leader_pid))
>>> {
>>> /* ...large informative comment block... */
>>> delete_lwp (leader_lp);
>>>
>>> The race occurred when there were two threads in the program, and the non-leader thread called exec. In this case the leader thread passed through a very brief zombie state before being replaced by the exec'ing thread as the thread group leader. This state transition was asynchronous, with no dependency on anything gdbserver did. Because there were no other threads, there were no thread exit events, and thus there was no synchronization with the leader passing through the zombie state and the exec completing. If there had been more threads, the leader would remain in the zombie state until they were waited for. In the two-thread case, sometimes the leader exit was detected and sometimes it wasn't. (Recall that check_zombie_leaders is polling the state, via linux_proc_pid_is_zombie. The race is between the leader thread passing through the zombie state and check_zombie_leaders testing for zombie state.) If leader exit wasn't detected, gdbserver would end up with a dangl
>>> ing lwp entry that didn't correspond to any real lwp, and would hang waiting for that lwp to stop. Using PTRACE_EVENT_EXIT guarantees that the leader exit will be detected.
>>>
>>> Note that check_zombie_leaders works just fine for the scenarios where the leader thread exits and the other threads continue to run, with no exec calls. It is required for systems that don't support the extended ptrace events.
>>>
>>> The sequence of events resulting in the race condition was this:
>>>
>>> 1) In the program, a CLONE event for a new thread occurs.
>>>
>>> 2) In the program, both threads are resumed once gdbserver has
>>> completed the new thread processing.
>>>
>>> 3) In gdbserver, the function linux_wait_for_event_filtered loops until
>>> waitpid returns "no more events" for the SIGCHLD generated by the
>>> CLONE event. Then linux_wait_for_event_filtered calls
>>> check_zombie_leaders.
>>>
>>> 4) In the program, the new thread is doing the exec. During the exec
>>> the leader thread will pass through a transitory zombie state. If
>>> there were more than two threads, the leader thread would remain a
>>> zombie until all the non-leader, non-exec'ing threads were reaped by
>>> gdbserver. Since there are no such threads to reap, the leader just
>>> becomes a zombie and is replaced by the exec'ing thread on-the-fly.
>>> (Note that it appears that the leader thread is a zombie just for a
>>> very brief instant.)
>>>
>>> 5) In gdbserver, check_zombie_leaders checks whether an lwp entry
>>> corresponds to a zombie leader thread, and if so, deletes it. Here
>>> is the race: in (4) above, the leader may or may not be in the
>>> transitory zombie state. In the case where a zombie isn't detected,
>>> delete_lwp is not called.
>>>
>>> 6) In gdbserver, an EXEC event is detected and processed. When it gets
>>> ready to report the event to GDB, it calls stop_all_lwps, which sends
>>> a SIGSTOP to each lwp in the list and the waits until all the lwps in
>>> the list have reported a stop event. If the zombie leader wasn't
>>> detected and processed in step (5), gdbserver blocks forever in
>>> linux_wait_for_event_filtered, waiting for the undeleted lwp to be
>>> stopped, which will never happen.
>>
>> Hi.
>>
>> How do I tweak your patch so that I can see the race condition for myself?
>> [I realize the window is small ... I'd just like to play with it.]
>
> Hi Doug, thanks for looking at this. I will have to look back through
> my notes to see if I can come up with a reasonable way of doing this. I
> was using the same test case that you were, running the basic test
> manually. I saw the race condition by inserting fprintf(stderr,.. into
> linux_proc_pid_has_state and check_zombie_leaders, running with no debug
> output.
>
> From my notes:
> ----------------------------------------------
> # result of if stmt condition in check_zombie_leaders
> # original leader is in zombie state
> # linux_proc_pid_has_state: procfile:
> # State: Z (zombie)
> #
> # result inside 'if' stmt in check_zombie_leaders - execing
> # thread has replaced original leader since we evaluated
> # the 'if' condition
> # linux_proc_pid_has_state: procfile:
> # State: R (running)
> #
> # printed inside if stmt, required zombie=1 to get here
> # we still think we have 2 lwps, but after the exec there
> # is only one. zombie=0 came from call to linux_proc_pid_has_state
> # above.
> # check_zombie_leaders: leader_pid=30981, leader_lp!=NULL=1, num_lwps=2,
> # zombie=0
> # ----------------------------------------------
>
My text editor "helped" me with extra comment characters. Here's the
corrected trace log from my notes. The '#' lines are my annotations,
the other lines were actual trace log output.
----------------------------------------------
# result of if stmt condition in check_zombie_leaders
# original leader is in zombie state
linux_proc_pid_has_state: procfile:
State: Z (zombie)
# result inside 'if' stmt in check_zombie_leaders - execing
# thread has replaced original leader since we evaluated
# the 'if' condition
linux_proc_pid_has_state: procfile:
State: R (running)
# printed inside if stmt, required zombie=1 to get here
# we still think we have 2 lwps, but after the exec there
# is only one. zombie=0 came from call to linux_proc_pid_has_state
# above.
check_zombie_leaders: leader_pid=30981, leader_lp!=NULL=1, num_lwps=2,
zombie=0
----------------------------------------------
> The race could manifest in ways other than the one demonstrated here, of
> course.
>
>>
>> I tried just disabling PTRACE_O_TRACEEXIT but that causes segvs in the
>> inferior (using non-ldr-exc-1 testcase).
>> Still digging into that.
>>
> I made the EXEC event support depend on the EXIT events. I have no idea
> what would happen if the EXIT events were disabled.
>
> Thanks
> --Don
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/4] Document RSP support for Linux exec events
2014-05-27 21:28 ` Breazeal, Don
@ 2014-05-28 14:22 ` Eli Zaretskii
0 siblings, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2014-05-28 14:22 UTC (permalink / raw)
To: Breazeal, Don; +Cc: gdb-patches
> Date: Tue, 27 May 2014 14:28:38 -0700
> From: "Breazeal, Don" <donb@codesourcery.com>
> CC: gdb-patches@sourceware.org
>
> On 5/24/2014 12:20 AM, Eli Zaretskii wrote:
> >> From: Don Breazeal <donb@codesourcery.com>
> >> Date: Fri, 23 May 2014 15:49:33 -0700
> >>
> Updated patch follows below.
Thanks, this is good to go in.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/4 v3] Exec events in gdbserver on Linux
2014-05-27 21:41 ` Breazeal, Don
@ 2014-05-28 18:02 ` Breazeal, Don
0 siblings, 0 replies; 33+ messages in thread
From: Breazeal, Don @ 2014-05-28 18:02 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
On 5/27/2014 2:41 PM, Breazeal, Don wrote:
> On 5/27/2014 11:49 AM, Breazeal, Don wrote:
>> On 5/25/2014 9:55 PM, Doug Evans wrote:
--snip snip--
>>> Hi.
>>>
>>> How do I tweak your patch so that I can see the race condition for myself?
>>> [I realize the window is small ... I'd just like to play with it.]
>>
>> Hi Doug, thanks for looking at this. I will have to look back through
>> my notes to see if I can come up with a reasonable way of doing this. I
>> was using the same test case that you were, running the basic test
>> manually. I saw the race condition by inserting fprintf(stderr,.. into
>> linux_proc_pid_has_state and check_zombie_leaders, running with no debug
>> output.
Now that I have gone back to look at this, I recall that I wasn't
running the test manually, but using a script that ran it repeatedly
until it failed. At different times the failure would occur at
different frequencies. I attributed that to system load, which I think
was relatively high.
>>
>> From my notes:
--snip snip--
> The '#' lines are my annotations,
> the other lines were actual trace log output.
> ----------------------------------------------
> # result of if stmt condition in check_zombie_leaders
> # original leader is in zombie state
> linux_proc_pid_has_state: procfile:
> State: Z (zombie)
>
> # result inside 'if' stmt in check_zombie_leaders - execing
> # thread has replaced original leader since we evaluated
> # the 'if' condition
> linux_proc_pid_has_state: procfile:
> State: R (running)
>
> # printed inside if stmt, required zombie=1 to get here
> # we still think we have 2 lwps, but after the exec there
> # is only one. zombie=0 came from call to linux_proc_pid_has_state
> # above.
> check_zombie_leaders: leader_pid=30981, leader_lp!=NULL=1, num_lwps=2,
> zombie=0
> ----------------------------------------------
I don't think there is a way to (reasonably) tweak my existing patch to
use something other than exit events. Here is a patch for an earlier
implementation of remote exec events that contains the race condition.
If I hadn't run into the race condition in my testing I would have
submitted something similar to this.
I spent yesterday afternoon and part of this morning trying to reproduce
the failure, without success. I ran gdb.threads/non-ldr-exc-1.exp
several thousand times, with/without optimization, on local/NFS drives,
inserted random sleeps, etc.
Print statements similar to those that generated the logs above are in
check_zombie_leaders and linux_proc_pid_has_state, commented out.
I hope this is useful.
--Don
Subject: Exec patch w/race condition
---
gdb/common/linux-procfs.c | 1 +
gdb/common/linux-ptrace.c | 2 +-
gdb/gdbserver/linux-low.c | 98
++++++++++++++++++++++++++++++++++++++---
gdb/gdbserver/linux-low.h | 5 ++
gdb/gdbserver/remote-utils.c | 27 +++++++++++-
gdb/remote.c | 19 ++++++++
6 files changed, 142 insertions(+), 10 deletions(-)
diff --git a/gdb/common/linux-procfs.c b/gdb/common/linux-procfs.c
index 1443a88..7233ce4 100644
--- a/gdb/common/linux-procfs.c
+++ b/gdb/common/linux-procfs.c
@@ -95,6 +95,7 @@ linux_proc_pid_has_state (pid_t pid, const char *state)
while (fgets (buffer, sizeof (buffer), procfile) != NULL)
if (strncmp (buffer, "State:", 6) == 0)
{
+//fprintf (stderr, "%s: %s\n", __func__, buffer);
have_state = 1;
break;
}
diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c
index efbd1ea..e38e266 100644
--- a/gdb/common/linux-ptrace.c
+++ b/gdb/common/linux-ptrace.c
@@ -437,7 +437,7 @@ linux_test_for_tracefork (int child_pid)
/* Do not enable all the options for now since gdbserver does not
properly support them. This restriction will be lifted when
gdbserver is augmented to support them. */
- current_ptrace_options |= PTRACE_O_TRACECLONE;
+ current_ptrace_options |= PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC;
#else
current_ptrace_options |= PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK
| PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC;
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 1932ff2..b8a808f 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -280,6 +280,32 @@ static int linux_event_pipe[2] = { -1, -1 };
static void send_sigstop (struct lwp_info *lwp);
static void wait_for_sigstop (void);
+/* Accepts an integer PID; Returns a string representing a file that
+ can be opened to get info for the child process.
+ Space for the result is malloc'd, caller must free. */
+
+static char *
+linux_child_pid_to_exec_file (int pid)
+{
+ char *name1, *name2;
+
+ name1 = xmalloc (PATH_MAX);
+ name2 = xmalloc (PATH_MAX);
+ memset (name2, 0, PATH_MAX);
+
+ sprintf (name1, "/proc/%d/exe", pid);
+ if (readlink (name1, name2, PATH_MAX) > 0)
+ {
+ free (name1);
+ return name2;
+ }
+ else
+ {
+ free (name2);
+ return name1;
+ }
+}
+
/* Return non-zero if HEADER is a 64-bit ELF file. */
static int
@@ -369,9 +395,10 @@ linux_add_process (int pid, int attached)
/* Handle a GNU/Linux extended wait response. If we see a clone
event, we need to add the new LWP to our list (and not report the
- trap to higher layers). */
+ trap to higher layers). This function returns non-zero if the
+ event should be ignored and we should wait again. */
-static void
+static int
handle_extended_wait (struct lwp_info *event_child, int wstat)
{
int event = wstat >> 16;
@@ -452,7 +479,27 @@ handle_extended_wait (struct lwp_info *event_child,
int wstat)
threads, it will have a pending SIGSTOP; we may as well
collect it now. */
linux_resume_one_lwp (event_child, event_child->stepping, 0, NULL);
+
+ /* Don't report the event. */
+ return 1;
}
+
+ if (event == PTRACE_EVENT_EXEC)
+ {
+ if (debug_threads)
+ debug_printf ("LHEW: Got exec event from LWP %ld\n",
+ lwpid_of (event_thr));
+
+ event_child->waitstatus.kind = TARGET_WAITKIND_EXECD;
+ event_child->waitstatus.value.execd_pathname
+ = linux_child_pid_to_exec_file (lwpid_of (event_thr));
+
+ /* Report the event. */
+ return 0;
+ }
+
+ internal_error (__FILE__, __LINE__,
+ _("unknown ptrace event %d"), event);
}
/* Return the PC as read from the regcache of LWP, without any
@@ -1345,6 +1392,7 @@ check_zombie_leaders (void)
"(it exited, or another thread execd).\n",
leader_pid);
+//fprintf (stderr, "%s: ldr lwp %d, zombie %d\n", __func__, leader_pid,
linux_proc_pid_is_zombie (leader_pid));
delete_lwp (leader_lp);
}
}
@@ -1851,8 +1899,8 @@ linux_low_filter_event (ptid_t filter_ptid, int
lwpid, int wstat)
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGTRAP
&& wstat >> 16 != 0)
{
- handle_extended_wait (child, wstat);
- return NULL;
+ if (handle_extended_wait (child, wstat))
+ return NULL;
}
if (WIFSTOPPED (wstat) && WSTOPSIG (wstat) == SIGSTOP
@@ -2452,6 +2500,19 @@ linux_stabilize_threads (void)
}
}
+/* Return non-zero if WAITSTATUS reflects an extended linux
+ event. Otherwise, return 0. */
+
+static int
+extended_event_reported (const struct target_waitstatus *waitstatus)
+{
+
+ if (waitstatus == NULL)
+ return 0;
+
+ return waitstatus->kind == TARGET_WAITKIND_EXECD;
+}
+
/* Wait for process, returns status. */
static ptid_t
@@ -2815,7 +2876,8 @@ retry:
&& !bp_explains_trap && !trace_event)
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
- && gdb_no_commands_at_breakpoint (event_child->stop_pc)));
+ && gdb_no_commands_at_breakpoint (event_child->stop_pc))
+ || extended_event_reported (&event_child->waitstatus));
run_breakpoint_commands (event_child->stop_pc);
@@ -2837,7 +2899,16 @@ retry:
paddress (event_child->stop_pc),
paddress (event_child->step_range_start),
paddress (event_child->step_range_end));
- }
+ if (debug_threads
+ && extended_event_reported (&event_child->waitstatus))
+ {
+ char *str = target_waitstatus_to_string (ourstatus);
+ debug_printf ("LWP %ld has forked, cloned, vforked or
execd"
+ " with waitstatus %s\n",
+ lwpid_of (current_inferior), str);
+ xfree (str);
+ }
+ }
/* We're not reporting this breakpoint to GDB, so apply the
decr_pc_after_break adjustment to the inferior's regcache
@@ -2935,7 +3006,18 @@ retry:
unstop_all_lwps (1, event_child);
}
- ourstatus->kind = TARGET_WAITKIND_STOPPED;
+ /* If the reported event is a fork, vfork or exec, let GDB know. */
+ if (extended_event_reported (&event_child->waitstatus))
+ {
+ ourstatus->kind = event_child->waitstatus.kind;
+ ourstatus->value = event_child->waitstatus.value;
+
+ /* Reset the event child's waitstatus since we handled it
+ already. */
+ event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
+ }
+ else
+ ourstatus->kind = TARGET_WAITKIND_STOPPED;
if (current_inferior->last_resume_kind == resume_stop
&& WSTOPSIG (w) == SIGSTOP)
@@ -2952,7 +3034,7 @@ retry:
but, it stopped for other reasons. */
ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
}
- else
+ else if (ourstatus->kind == TARGET_WAITKIND_STOPPED)
{
ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
}
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 498b221..2534ad1 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -269,6 +269,11 @@ struct lwp_info
/* When stopped is set, the last wait status recorded for this lwp. */
int last_status;
+ /* If WAITSTATUS->KIND != TARGET_WAITKIND_SPURIOUS, the waitstatus
+ for this LWP's last event. This may correspond to LAST_STATUS above,
+ or to a local variable in lin_lwp_wait. */
+ struct target_waitstatus waitstatus;
+
/* When stopped is set, this is where the lwp stopped, with
decr_pc_after_break already accounted for. */
CORE_ADDR stop_pc;
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 4fcafa0..6b8c10b 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1111,14 +1111,39 @@ prepare_resume_reply (char *buf, ptid_t ptid,
switch (status->kind)
{
case TARGET_WAITKIND_STOPPED:
+ case TARGET_WAITKIND_EXECD:
{
struct thread_info *saved_inferior;
const char **regp;
struct regcache *regcache;
+ enum gdb_signal signal;
- sprintf (buf, "T%02x", status->value.sig);
+ if (status->kind == TARGET_WAITKIND_EXECD)
+ signal = GDB_SIGNAL_TRAP;
+ else
+ signal = status->value.sig;
+
+ sprintf (buf, "T%02x", signal);
buf += strlen (buf);
+ if (status->kind == TARGET_WAITKIND_EXECD && multi_process)
+ {
+ const char *event = "target_exec";
+ char hexified_pathname[PATH_MAX];
+
+ sprintf (buf, "%s:", event);
+ buf += strlen (buf);
+
+ /* Encode pathname to hexified format. */
+ bin2hex ((const gdb_byte *) status->value.execd_pathname,
+ hexified_pathname,
strlen(status->value.execd_pathname));
+
+ sprintf (buf, "%s;", hexified_pathname);
+ xfree (status->value.execd_pathname);
+ status->value.execd_pathname = NULL;
+ buf += strlen (buf);
+ }
+
saved_inferior = current_inferior;
current_inferior = find_thread_ptid (ptid);
diff --git a/gdb/remote.c b/gdb/remote.c
index 964bd41..55bafc2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5471,6 +5471,25 @@ Packet: '%s'\n"),
p = unpack_varlen_hex (++p1, &c);
event->core = c;
}
+ else if (strncmp (p, "target_exec", p1 - p) == 0)
+ {
+ ULONGEST pid;
+ char pathname[PATH_MAX];
+
+ p = unpack_varlen_hex (++p1, &pid);
+
+ /* Save the pathname for event reporting and for
+ the next run command. */
+ hex2bin (p1, (gdb_byte *) pathname, (p - p1)/2);
+ /* Add the null terminator. */
+ pathname[(p - p1)/2] = '\0';
+ /* This is freed during event handling. */
+ event->ws.value.execd_pathname = xstrdup (pathname);
+ event->ws.kind = TARGET_WAITKIND_EXECD;
+ /* Save the pathname for the next run command. */
+ xfree (remote_exec_file);
+ remote_exec_file = pathname;
+ }
else
{
/* Silently skip unknown optional info. */
--
1.7.0.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/4 v3] Exec events in gdbserver on Linux
2014-05-23 22:49 ` [PATCH 0/4 v3] Exec events in gdbserver on Linux Don Breazeal
2014-05-26 4:55 ` Doug Evans
@ 2014-06-05 22:06 ` Breazeal, Don
2014-06-06 12:29 ` Luis Machado
2014-06-19 15:56 ` Breazeal, Don
1 sibling, 2 replies; 33+ messages in thread
From: Breazeal, Don @ 2014-06-05 22:06 UTC (permalink / raw)
To: gdb-patches
Ping...I know people are busy wrapping up gdb 7.8, but I just want to
check that the reviewers who raised questions or issues (Doug, Luis,
Tom, Yao) all feel that I have responded adequately (i.e. aren't waiting
for something more from me). I'm not planning any further responses to
the feedback so far, so please let me know if you are waiting for me on
issues regarding this patch series.
0/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00605.html
1/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00607.html
2/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00608.html
3/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00604.html
4/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00606.html
Thanks,
--Don
On 5/23/2014 3:49 PM, Don Breazeal wrote:
> This patch series is an update to the gdbserver Linux exec event patches based on review comments for the previous version. The changes from the previous version are summarized below.
>
> Patch 1 (gdbserver exit events):
> Fixed up comments and whitespace
> Moved test for extended exit events from linux_test_for_tracefork
> into new function, linux_test_for_traceexit, called from
> linux_check_ptrace_features.
> Moved predicate functions is_extended_* into common/linux-ptrace.c
> renamed to linux_is_extended_* and made new function
> linux_is_extended_fork. Replaced shift operations with calls to
> these functions in gdbserver.
> Eliminated the call to linux_enable_event_reporting that I had
> added to gdbserver's CLONE event handling.
> Wrote explanation of why extended exit events are required.
>
> Patch 2 (gdbserver exec events):
> Moved linux_child_pid_to_exec_file from linux-nat.c and linux-low.c
> to common/linux-procfs.c and renamed it linux_proc_pid_to_exec_file.
> Gdbserver has to take "struct target_ops *" argument now.
> Implemented new predicate linux_is_extended_exec in common file.
> Parenthesize condition in return stmt in extended_event_reported.
> Delete extra text from debug_printf.
>
> Patch 3 (gdbserver exec event documentation):
> Added NEWS entries for gdbserver exec event support.
>
> Patch 4 (exec event tests):
> No changes from the previous version.
>
> This patch series implements support for exec events in gdbserver on GNU/Linux. This is a step towards the "follow fork/exec" item in the local/remote debugging feature parity project: (https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity). Luis had started work on this last year, and handed it off to me at the end of January. (BTW, thanks to Luis for his advice on this, although any problems with this patch are entirely mine.)
>
> Follow-exec-mode and rerun behave as expected in multiprocess mode (target extended-remote), where follow-exec-mode maintains the specified inferiors and rerun runs the last executable file to be exec'd. Catchpoints for exec are not implemented in this patch series, since this will be easier to do once fork and vfork events are also supported.
>
> - Patch 1/4 implements support for the extended ptrace event PTRACE_EVENT_EXIT, which is a prerequisite for the exec event support.
> - Patch 2/4 implements the exec event support.
> - Patch 3/4 adds documentation for the new RSP "Stop Reply" message.
> - Patch 4/4 extends the tests gdb.threads/non-ldr-exc-[1-4].exp to test in non-stop mode as well as in all-stop mode.
>
> There are a couple of significant aspects to this patch. First, it uses the ptrace extension PTRACE_EVENT_EXIT to detect thread exit, in particular the exit of a thread group leader when a non-leader calls exec. Use of this event was necessary due to a race condition in the two-thread case. A detailed description of the race condition can be found at the bottom of this message. Exit events are only used internally and are not exposed to the user, and exit processing was changed as little as possible.
>
> Second, it sends a signal to each lwp after an exec event in order to identify and clean up the lwp entry for the exec'ing thread. This happens in the normal course of things for all-stop mode, but in non-stop mode gdbserver uses SIGSTOP to stop, then resume all of the non-leader threads to accomplish this.
>
> My intent is to follow up with implementations of remote fork/vfork events
> and associated catchpoints.
>
> RACE CONDITION
>
> This section explains why the existing techniques for detecting thread exit weren't sufficient for gdbserver exec events, necessitating the use of PTRACE_EVENT_EXIT. The short answer is that there is a race condition in the current implementation that can leave a dangling entry in the lwp list (an entry that doesn't have a corresponding actual lwp). In this case gdbserver will hang waiting for the non-existent lwp to stop. Using the exit events eliminates this race condition.
>
> The same race may exist in the native implementation, since the two implementations are similar, but I haven't verified that. It may be difficult to concoct a test case that demonstrates the race since the window is so small.
>
> Now for the long answer: in my testing I ran into a race condition in check_zombie_leaders, which detects when a thread group leader has exited and other threads still exist. On the Linux kernel, ptrace/waitpid don't allow reaping the leader thread until all other threads in the group are reaped. When the leader exits, it goes zombie, but waitpid will not return an exit status until the other threads are gone. When a non-leader thread calls exec, all other non-leader threads are destroyed, the leader becomes a zombie, and once the "other" threads have been reaped, the execing thread takes over the leader's pid (tgid) and appears to vanish. In order to handle this situation, check_zombie_leaders polls the process state in /proc and deletes thread group leaders that are in a zombie state. The replacement is added to the lwp list when the exec event is reported.
>
> See https://sourceware.org/ml/gdb-patches/2011-10/msg00704.html for a more detailed explanation of how this works.
>
> Here is the relevant part of check_zombie_leaders:
>
> if (leader_lp != NULL
> /* Check if there are other threads in the group, as we may
> have raced with the inferior simply exiting. */
> && !last_thread_of_process_p (leader_pid)
> && linux_proc_pid_is_zombie (leader_pid))
> {
> /* ...large informative comment block... */
> delete_lwp (leader_lp);
>
> The race occurred when there were two threads in the program, and the non-leader thread called exec. In this case the leader thread passed through a very brief zombie state before being replaced by the exec'ing thread as the thread group leader. This state transition was asynchronous, with no dependency on anything gdbserver did. Because there were no other threads, there were no thread exit events, and thus there was no synchronization with the leader passing through the zombie state and the exec completing. If there had been more threads, the leader would remain in the zombie state until they were waited for. In the two-thread case, sometimes the leader exit was detected and sometimes it wasn't. (Recall that check_zombie_leaders is polling the state, via linux_proc_pid_is_zombie. The race is between the leader thread passing through the zombie state and check_zombie_leaders testing for zombie state.) If leader exit wasn't detected, gdbserver would end up with a dangl
> ing lwp entry that didn't correspond to any real lwp, and would hang waiting for that lwp to stop. Using PTRACE_EVENT_EXIT guarantees that the leader exit will be detected.
>
> Note that check_zombie_leaders works just fine for the scenarios where the leader thread exits and the other threads continue to run, with no exec calls. It is required for systems that don't support the extended ptrace events.
>
> The sequence of events resulting in the race condition was this:
>
> 1) In the program, a CLONE event for a new thread occurs.
>
> 2) In the program, both threads are resumed once gdbserver has
> completed the new thread processing.
>
> 3) In gdbserver, the function linux_wait_for_event_filtered loops until
> waitpid returns "no more events" for the SIGCHLD generated by the
> CLONE event. Then linux_wait_for_event_filtered calls
> check_zombie_leaders.
>
> 4) In the program, the new thread is doing the exec. During the exec
> the leader thread will pass through a transitory zombie state. If
> there were more than two threads, the leader thread would remain a
> zombie until all the non-leader, non-exec'ing threads were reaped by
> gdbserver. Since there are no such threads to reap, the leader just
> becomes a zombie and is replaced by the exec'ing thread on-the-fly.
> (Note that it appears that the leader thread is a zombie just for a
> very brief instant.)
>
> 5) In gdbserver, check_zombie_leaders checks whether an lwp entry
> corresponds to a zombie leader thread, and if so, deletes it. Here
> is the race: in (4) above, the leader may or may not be in the
> transitory zombie state. In the case where a zombie isn't detected,
> delete_lwp is not called.
>
> 6) In gdbserver, an EXEC event is detected and processed. When it gets
> ready to report the event to GDB, it calls stop_all_lwps, which sends
> a SIGSTOP to each lwp in the list and the waits until all the lwps in
> the list have reported a stop event. If the zombie leader wasn't
> detected and processed in step (5), gdbserver blocks forever in
> linux_wait_for_event_filtered, waiting for the undeleted lwp to be
> stopped, which will never happen.
>
> Regards,
> --Don
>
> gdb/NEWS | 6 +
> gdb/common/linux-procfs.c | 18 ++
> gdb/common/linux-procfs.h | 4 +
> gdb/common/linux-ptrace.c | 75 +++++++++
> gdb/common/linux-ptrace.h | 4 +
> gdb/doc/gdb.texinfo | 6 +
> gdb/gdbserver/linux-low.c | 229 +++++++++++++++++++++++----
> gdb/gdbserver/linux-low.h | 5 +
> gdb/gdbserver/remote-utils.c | 28 +++-
> gdb/linux-nat.c | 22 +---
> gdb/remote.c | 27 +++-
> gdb/testsuite/gdb.threads/non-ldr-exc-1.exp | 20 ++-
> gdb/testsuite/gdb.threads/non-ldr-exc-2.exp | 36 ++++-
> gdb/testsuite/gdb.threads/non-ldr-exc-3.exp | 36 ++++-
> gdb/testsuite/gdb.threads/non-ldr-exc-4.exp | 20 ++-
> 15 files changed, 466 insertions(+), 70 deletions(-)
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/4 v3] Exec events in gdbserver on Linux
2014-06-05 22:06 ` Breazeal, Don
@ 2014-06-06 12:29 ` Luis Machado
2014-06-19 15:56 ` Breazeal, Don
1 sibling, 0 replies; 33+ messages in thread
From: Luis Machado @ 2014-06-06 12:29 UTC (permalink / raw)
To: Breazeal, Don, gdb-patches
Hi Don,
On 06/05/2014 11:06 PM, Breazeal, Don wrote:
> Ping...I know people are busy wrapping up gdb 7.8, but I just want to
> check that the reviewers who raised questions or issues (Doug, Luis,
> Tom, Yao) all feel that I have responded adequately (i.e. aren't waiting
> for something more from me). I'm not planning any further responses to
> the feedback so far, so please let me know if you are waiting for me on
> issues regarding this patch series.
>
> 0/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00605.html
> 1/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00607.html
> 2/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00608.html
> 3/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00604.html
> 4/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00606.html
>
> Thanks,
> --Don
>
I don't have anything else to add to the discussion at this point, but i
can't officially approve it.
Luis
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/4 v3] Exec events in gdbserver on Linux
2014-06-05 22:06 ` Breazeal, Don
2014-06-06 12:29 ` Luis Machado
@ 2014-06-19 15:56 ` Breazeal, Don
1 sibling, 0 replies; 33+ messages in thread
From: Breazeal, Don @ 2014-06-19 15:56 UTC (permalink / raw)
To: gdb-patches
Ping. (Eli approved part 3 on May 28.)
thanks
--Don
On 6/5/2014 3:06 PM, Breazeal, Don wrote:
> Ping...I know people are busy wrapping up gdb 7.8, but I just want to
> check that the reviewers who raised questions or issues (Doug, Luis,
> Tom, Yao) all feel that I have responded adequately (i.e. aren't waiting
> for something more from me). I'm not planning any further responses to
> the feedback so far, so please let me know if you are waiting for me on
> issues regarding this patch series.
>
> 0/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00605.html
> 1/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00607.html
> 2/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00608.html
> 3/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00604.html
> 4/4: https://sourceware.org/ml/gdb-patches/2014-05/msg00606.html
>
> Thanks,
> --Don
>
> On 5/23/2014 3:49 PM, Don Breazeal wrote:
>> This patch series is an update to the gdbserver Linux exec event patches based on review comments for the previous version. The changes from the previous version are summarized below.
>>
>> Patch 1 (gdbserver exit events):
>> Fixed up comments and whitespace
>> Moved test for extended exit events from linux_test_for_tracefork
>> into new function, linux_test_for_traceexit, called from
>> linux_check_ptrace_features.
>> Moved predicate functions is_extended_* into common/linux-ptrace.c
>> renamed to linux_is_extended_* and made new function
>> linux_is_extended_fork. Replaced shift operations with calls to
>> these functions in gdbserver.
>> Eliminated the call to linux_enable_event_reporting that I had
>> added to gdbserver's CLONE event handling.
>> Wrote explanation of why extended exit events are required.
>>
>> Patch 2 (gdbserver exec events):
>> Moved linux_child_pid_to_exec_file from linux-nat.c and linux-low.c
>> to common/linux-procfs.c and renamed it linux_proc_pid_to_exec_file.
>> Gdbserver has to take "struct target_ops *" argument now.
>> Implemented new predicate linux_is_extended_exec in common file.
>> Parenthesize condition in return stmt in extended_event_reported.
>> Delete extra text from debug_printf.
>>
>> Patch 3 (gdbserver exec event documentation):
>> Added NEWS entries for gdbserver exec event support.
>>
>> Patch 4 (exec event tests):
>> No changes from the previous version.
>>
>> This patch series implements support for exec events in gdbserver on GNU/Linux. This is a step towards the "follow fork/exec" item in the local/remote debugging feature parity project: (https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity). Luis had started work on this last year, and handed it off to me at the end of January. (BTW, thanks to Luis for his advice on this, although any problems with this patch are entirely mine.)
>>
>> Follow-exec-mode and rerun behave as expected in multiprocess mode (target extended-remote), where follow-exec-mode maintains the specified inferiors and rerun runs the last executable file to be exec'd. Catchpoints for exec are not implemented in this patch series, since this will be easier to do once fork and vfork events are also supported.
>>
>> - Patch 1/4 implements support for the extended ptrace event PTRACE_EVENT_EXIT, which is a prerequisite for the exec event support.
>> - Patch 2/4 implements the exec event support.
>> - Patch 3/4 adds documentation for the new RSP "Stop Reply" message.
>> - Patch 4/4 extends the tests gdb.threads/non-ldr-exc-[1-4].exp to test in non-stop mode as well as in all-stop mode.
>>
>> There are a couple of significant aspects to this patch. First, it uses the ptrace extension PTRACE_EVENT_EXIT to detect thread exit, in particular the exit of a thread group leader when a non-leader calls exec. Use of this event was necessary due to a race condition in the two-thread case. A detailed description of the race condition can be found at the bottom of this message. Exit events are only used internally and are not exposed to the user, and exit processing was changed as little as possible.
>>
>> Second, it sends a signal to each lwp after an exec event in order to identify and clean up the lwp entry for the exec'ing thread. This happens in the normal course of things for all-stop mode, but in non-stop mode gdbserver uses SIGSTOP to stop, then resume all of the non-leader threads to accomplish this.
>>
>> My intent is to follow up with implementations of remote fork/vfork events
>> and associated catchpoints.
>>
>> RACE CONDITION
>>
>> This section explains why the existing techniques for detecting thread exit weren't sufficient for gdbserver exec events, necessitating the use of PTRACE_EVENT_EXIT. The short answer is that there is a race condition in the current implementation that can leave a dangling entry in the lwp list (an entry that doesn't have a corresponding actual lwp). In this case gdbserver will hang waiting for the non-existent lwp to stop. Using the exit events eliminates this race condition.
>>
>> The same race may exist in the native implementation, since the two implementations are similar, but I haven't verified that. It may be difficult to concoct a test case that demonstrates the race since the window is so small.
>>
>> Now for the long answer: in my testing I ran into a race condition in check_zombie_leaders, which detects when a thread group leader has exited and other threads still exist. On the Linux kernel, ptrace/waitpid don't allow reaping the leader thread until all other threads in the group are reaped. When the leader exits, it goes zombie, but waitpid will not return an exit status until the other threads are gone. When a non-leader thread calls exec, all other non-leader threads are destroyed, the leader becomes a zombie, and once the "other" threads have been reaped, the execing thread takes over the leader's pid (tgid) and appears to vanish. In order to handle this situation, check_zombie_leaders polls the process state in /proc and deletes thread group leaders that are in a zombie state. The replacement is added to the lwp list when the exec event is reported.
>>
>> See https://sourceware.org/ml/gdb-patches/2011-10/msg00704.html for a more detailed explanation of how this works.
>>
>> Here is the relevant part of check_zombie_leaders:
>>
>> if (leader_lp != NULL
>> /* Check if there are other threads in the group, as we may
>> have raced with the inferior simply exiting. */
>> && !last_thread_of_process_p (leader_pid)
>> && linux_proc_pid_is_zombie (leader_pid))
>> {
>> /* ...large informative comment block... */
>> delete_lwp (leader_lp);
>>
>> The race occurred when there were two threads in the program, and the non-leader thread called exec. In this case the leader thread passed through a very brief zombie state before being replaced by the exec'ing thread as the thread group leader. This state transition was asynchronous, with no dependency on anything gdbserver did. Because there were no other threads, there were no thread exit events, and thus there was no synchronization with the leader passing through the zombie state and the exec completing. If there had been more threads, the leader would remain in the zombie state until they were waited for. In the two-thread case, sometimes the leader exit was detected and sometimes it wasn't. (Recall that check_zombie_leaders is polling the state, via linux_proc_pid_is_zombie. The race is between the leader thread passing through the zombie state and check_zombie_leaders testing for zombie state.) If leader exit wasn't detected, gdbserver would end up with a dangl
>> ing lwp entry that didn't correspond to any real lwp, and would hang waiting for that lwp to stop. Using PTRACE_EVENT_EXIT guarantees that the leader exit will be detected.
>>
>> Note that check_zombie_leaders works just fine for the scenarios where the leader thread exits and the other threads continue to run, with no exec calls. It is required for systems that don't support the extended ptrace events.
>>
>> The sequence of events resulting in the race condition was this:
>>
>> 1) In the program, a CLONE event for a new thread occurs.
>>
>> 2) In the program, both threads are resumed once gdbserver has
>> completed the new thread processing.
>>
>> 3) In gdbserver, the function linux_wait_for_event_filtered loops until
>> waitpid returns "no more events" for the SIGCHLD generated by the
>> CLONE event. Then linux_wait_for_event_filtered calls
>> check_zombie_leaders.
>>
>> 4) In the program, the new thread is doing the exec. During the exec
>> the leader thread will pass through a transitory zombie state. If
>> there were more than two threads, the leader thread would remain a
>> zombie until all the non-leader, non-exec'ing threads were reaped by
>> gdbserver. Since there are no such threads to reap, the leader just
>> becomes a zombie and is replaced by the exec'ing thread on-the-fly.
>> (Note that it appears that the leader thread is a zombie just for a
>> very brief instant.)
>>
>> 5) In gdbserver, check_zombie_leaders checks whether an lwp entry
>> corresponds to a zombie leader thread, and if so, deletes it. Here
>> is the race: in (4) above, the leader may or may not be in the
>> transitory zombie state. In the case where a zombie isn't detected,
>> delete_lwp is not called.
>>
>> 6) In gdbserver, an EXEC event is detected and processed. When it gets
>> ready to report the event to GDB, it calls stop_all_lwps, which sends
>> a SIGSTOP to each lwp in the list and the waits until all the lwps in
>> the list have reported a stop event. If the zombie leader wasn't
>> detected and processed in step (5), gdbserver blocks forever in
>> linux_wait_for_event_filtered, waiting for the undeleted lwp to be
>> stopped, which will never happen.
>>
>> Regards,
>> --Don
>>
>> gdb/NEWS | 6 +
>> gdb/common/linux-procfs.c | 18 ++
>> gdb/common/linux-procfs.h | 4 +
>> gdb/common/linux-ptrace.c | 75 +++++++++
>> gdb/common/linux-ptrace.h | 4 +
>> gdb/doc/gdb.texinfo | 6 +
>> gdb/gdbserver/linux-low.c | 229 +++++++++++++++++++++++----
>> gdb/gdbserver/linux-low.h | 5 +
>> gdb/gdbserver/remote-utils.c | 28 +++-
>> gdb/linux-nat.c | 22 +---
>> gdb/remote.c | 27 +++-
>> gdb/testsuite/gdb.threads/non-ldr-exc-1.exp | 20 ++-
>> gdb/testsuite/gdb.threads/non-ldr-exc-2.exp | 36 ++++-
>> gdb/testsuite/gdb.threads/non-ldr-exc-3.exp | 36 ++++-
>> gdb/testsuite/gdb.threads/non-ldr-exc-4.exp | 20 ++-
>> 15 files changed, 466 insertions(+), 70 deletions(-)
>>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2014-06-19 15:56 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-30 19:18 [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Don Breazeal
2014-04-30 19:18 ` [PATCH 4/4 v2][REPOST] Non-stop exec tests Don Breazeal
2014-04-30 19:18 ` [PATCH 1/4][REPOST] Remote Linux ptrace exit events Don Breazeal
2014-05-07 19:40 ` Luis Machado
2014-05-08 5:23 ` Yao Qi
2014-05-09 21:03 ` Breazeal, Don
2014-05-21 15:15 ` Tom Tromey
2014-05-22 17:42 ` Breazeal, Don
2014-04-30 19:18 ` [PATCH 2/4][REPOST] Remote Linux ptrace exec events Don Breazeal
2014-05-07 20:01 ` Luis Machado
2014-05-09 21:17 ` Breazeal, Don
2014-05-08 5:44 ` Yao Qi
2014-05-21 15:28 ` Tom Tromey
2014-04-30 19:18 ` [PATCH 3/4][REPOST] Document RSP support for Linux " Don Breazeal
2014-05-08 5:34 ` Yao Qi
2014-05-21 15:25 ` [PATCH 0/0][REPOST] Exec events in gdbserver on Linux Tom Tromey
2014-05-23 22:49 ` [PATCH 2/4 v3] Remote Linux ptrace exec events Don Breazeal
2014-05-23 22:49 ` [PATCH 3/4] Document RSP support for Linux " Don Breazeal
2014-05-24 7:20 ` Eli Zaretskii
2014-05-27 21:28 ` Breazeal, Don
2014-05-28 14:22 ` Eli Zaretskii
2014-05-23 22:49 ` [PATCH 4/4] Non-stop exec tests Don Breazeal
2014-05-26 3:29 ` Doug Evans
2014-05-27 18:31 ` Breazeal, Don
2014-05-23 22:49 ` [PATCH 1/4 v3] Remote Linux ptrace exit events Don Breazeal
2014-05-23 22:49 ` [PATCH 0/4 v3] Exec events in gdbserver on Linux Don Breazeal
2014-05-26 4:55 ` Doug Evans
2014-05-27 18:49 ` Breazeal, Don
2014-05-27 21:41 ` Breazeal, Don
2014-05-28 18:02 ` Breazeal, Don
2014-06-05 22:06 ` Breazeal, Don
2014-06-06 12:29 ` Luis Machado
2014-06-19 15:56 ` Breazeal, Don
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).