[RFC] mask off is-syscall bit for TRAP_IS_SYSCALL

Doug Evans
2009-09-29T12:45:07Z

Pedro wrote:
> In case I wasn't clear, I was alluding at the fact that I think
> the filtering is done too late..

Ya, it was clear.

There are (basically) two callers of my_waitpid that care:
wait_lwp and linux_nat_wait_1.
I was looking at linux_nat_wait_1, leaving wait_lwp for another pass.
But I see some problems alright.

I'm not sure the filtering is done too late though.
[One *could* do it earlier, but it seems like it could be done
later too.]

One could keep bit 0x80 with the recording pending status,
and process it immediately before linux_nat_wait_1 returns.
Something like the appended patch
(included for illustration's sake, it has a few hacks).

btw, I'm seeing lots of "syscall 0" (presumably restarted system calls).
I hacked another patch to save the the syscall number so that the user
wouldn't see syscall 0, but maybe the thing to do is record the
fact that the syscall got restarted and report that to the user?
[I'm assuming the 0's I see are indeed restarted syscalls.]

E.g.

(gdb)
Continuing.
[Switching to Thread 0xf6561b90 (LWP 23241)]

Catchpoint 1 (call to syscall 0), 0xffffe410 in __kernel_vsyscall ()
(gdb)
Continuing.

Catchpoint 1 (returned from syscall 0), 0xffffe410 in __kernel_vsyscall ()
(gdb)

---

Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.149
diff -u -p -r1.149 linux-nat.c
--- linux-nat.c 28 Sep 2009 21:09:15 -0000 1.149
+++ linux-nat.c 29 Sep 2009 12:39:06 -0000
@@ -1719,8 +1719,8 @@ resume_callback (struct lwp_info *lp, vo
memset (&lp->siginfo, 0, sizeof (lp->siginfo));
}
else if (lp->stopped && debug_linux_nat)
- fprintf_unfiltered (gdb_stdlog, "RC: Not resuming sibling %s (has pending)\n",
- target_pid_to_str (lp->ptid));
+ fprintf_unfiltered (gdb_stdlog, "RC: Not resuming sibling %s (has pending 0x%x)\n",
+ target_pid_to_str (lp->ptid), lp->status);
else if (debug_linux_nat)
fprintf_unfiltered (gdb_stdlog, "RC: Not resuming sibling %s (not stopped)\n",
target_pid_to_str (lp->ptid));
@@ -2070,47 +2070,6 @@ linux_handle_extended_wait (struct lwp_i
return 0;
}

- /* Used for 'catch syscall' feature. */
- if (WSTOPSIG (status) == TRAP_IS_SYSCALL)
- {
- if (catch_syscall_enabled () == 0)
- ourstatus->kind = TARGET_WAITKIND_IGNORE;
- else
- {
- struct regcache *regcache = get_thread_regcache (lp->ptid);
- struct gdbarch *gdbarch = get_regcache_arch (regcache);
-
- ourstatus->value.syscall_number =
- (int) gdbarch_get_syscall_number (gdbarch, lp->ptid);
-
- /* If we are catching this specific syscall number, then we
- should update the target_status to reflect which event
- has occurred. But if this syscall is not to be caught,
- then we can safely mark the event as a SYSCALL_RETURN.
-
- This is particularly needed if:
-
- - We are catching any syscalls, or
- - We are catching the syscall "exit"
-
- In this case, as the syscall "exit" *doesn't* return,
- then GDB would be confused because it would mark the last
- syscall event as a SYSCALL_ENTRY. After that, if we re-ran the
- inferior GDB will think that the first syscall event is
- the opposite of a SYSCALL_ENTRY, which is the SYSCALL_RETURN.
- Therefore, GDB would report inverted syscall events. */
- if (catching_syscall_number (ourstatus->value.syscall_number))
- ourstatus->kind =
- (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY) ?
- TARGET_WAITKIND_SYSCALL_RETURN : TARGET_WAITKIND_SYSCALL_ENTRY;
- else
- ourstatus->kind = TARGET_WAITKIND_SYSCALL_RETURN;
-
- lp->syscall_state = ourstatus->kind;
- }
- return 0;
- }
-
internal_error (__FILE__, __LINE__,
_("unknown ptrace event %d"), event);
}
@@ -2326,7 +2285,8 @@ stop_wait_callback (struct lwp_info *lp,

if (WSTOPSIG (status) != SIGSTOP)
{
- if (WSTOPSIG (status) == SIGTRAP)
+ if (WSTOPSIG (status) == SIGTRAP
+ || WSTOPSIG (status) == TRAP_IS_SYSCALL)
{
/* If a LWP other than the LWP that we're reporting an
event for has hit a GDB breakpoint (as opposed to
@@ -2344,6 +2304,14 @@ stop_wait_callback (struct lwp_info *lp,
/* Save the trap's siginfo in case we need it later. */
save_siginfo (lp);

+ if (WSTOPSIG (status) == TRAP_IS_SYSCALL)
+ {
+ /* Simulate the case of a signal < SIGTRAP and
+ SIGSTOP being delivered before the SIGSTOP. */
+ //xxx
+ //kill_lwp (GET_LWP (lp->ptid), SIGINT);
+ }
+
/* Now resume this LWP and get the SIGSTOP event. */
errno = 0;
ptrace (PTRACE_CONT, GET_LWP (lp->ptid), 0, 0);
@@ -2367,11 +2335,12 @@ stop_wait_callback (struct lwp_info *lp,
queue. */
if (lp->status)
{
+ gdb_assert (WSTOPSIG (lp->status) != TRAP_IS_SYSCALL);
if (debug_linux_nat)
fprintf_unfiltered (gdb_stdlog,
"SWC: kill %s, %s\n",
target_pid_to_str (lp->ptid),
- status_to_str ((int) status));
+ status_to_str ((int) lp->status));
kill_lwp (GET_LWP (lp->ptid), WSTOPSIG (lp->status));
}

@@ -2497,7 +2466,9 @@ select_event_lwp_callback (struct lwp_in

/* Select only resumed LWPs that have a SIGTRAP event pending. */
if (lp->status != 0 && lp->resumed
- && WIFSTOPPED (lp->status) && WSTOPSIG (lp->status) == SIGTRAP)
+ && WIFSTOPPED (lp->status)
+ && (WSTOPSIG (lp->status) == SIGTRAP
+ || WSTOPSIG (lp->status) == TRAP_IS_SYSCALL))
if ((*selector)-- == 0)
return 1;

@@ -2613,6 +2584,17 @@ select_event_lwp (ptid_t filter, struct
if (event_lp != NULL)
{
/* Switch the event LWP. */
+ if (debug_linux_nat && event_lp != *orig_lp)
+ {
+ /* This has to be done in two printf's.
+ target_pid_to_str can only have one in-use result. */
+ fprintf_unfiltered (gdb_stdlog,
+ "SEL: orig_lp = %s",
+ target_pid_to_str ((*orig_lp)->ptid));
+ fprintf_unfiltered (gdb_stdlog,
+ ", switching to %s\n",
+ target_pid_to_str (event_lp->ptid));
+ }
*orig_lp = event_lp;
*status = event_lp->status;
}
@@ -2713,12 +2695,8 @@ linux_nat_filter_event (int lwpid, int s
&& (WSTOPSIG (status) == SIGTRAP || WSTOPSIG (status) == TRAP_IS_SYSCALL))
save_siginfo (lp);

- /* Handle GNU/Linux's extended waitstatus for trace events.
- It is necessary to check if WSTOPSIG is signaling that
- the inferior is entering/exiting a system call. */
- if (WIFSTOPPED (status)
- && ((WSTOPSIG (status) == TRAP_IS_SYSCALL)
- || (WSTOPSIG (status) == SIGTRAP && status >> 16 != 0)))
+ /* Handle GNU/Linux's extended waitstatus for trace events. */
+ if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP && status >> 16 != 0)
{
if (debug_linux_nat)
fprintf_unfiltered (gdb_stdlog,
@@ -2863,7 +2841,7 @@ linux_nat_wait_1 (struct target_ops *ops
int status = 0;
pid_t pid;

- if (debug_linux_nat_async)
+ if (debug_linux_nat || debug_linux_nat_async)
fprintf_unfiltered (gdb_stdlog, "LLW: enter\n");

/* The first time we get here after starting a new inferior, we may
@@ -3013,12 +2991,6 @@ retry:

lp = linux_nat_filter_event (lwpid, status, options);

- /* If this was a syscall trap, we no longer need or want
- the 0x80 flag, remove it. */
- if (WIFSTOPPED (status)
- && WSTOPSIG (status) == TRAP_IS_SYSCALL)
- status = TRAP_REMOVE_SYSCALL_FLAG (status);
-
if (lp
&& ptid_is_pid (ptid)
&& ptid_get_pid (lp->ptid) != ptid_get_pid (ptid))
@@ -3108,7 +3080,7 @@ retry:
/* No interesting event. */
ourstatus->kind = TARGET_WAITKIND_IGNORE;

- if (debug_linux_nat_async)
+ if (debug_linux_nat || debug_linux_nat_async)
fprintf_unfiltered (gdb_stdlog, "LLW: exit (ignore)\n");

restore_child_signals_mask (&prev_mask);
@@ -3137,7 +3109,10 @@ retry:

if (WIFSTOPPED (status))
{
- int signo = target_signal_from_host (WSTOPSIG (status));
+ int stopsig = (WSTOPSIG (status) == TRAP_IS_SYSCALL
+ ? WSTOPSIG (TRAP_REMOVE_SYSCALL_FLAG (status))
+ : WSTOPSIG (status));
+ int signo = target_signal_from_host (stopsig);
struct inferior *inf;

inf = find_inferior_pid (ptid_get_pid (lp->ptid));
@@ -3222,6 +3197,50 @@ retry:
the comment in cancel_breakpoints_callback to find out why. */
iterate_over_lwps (minus_one_ptid, cancel_breakpoints_callback, lp);

+ /* Check for caught syscalls (via "catch syscall"). */
+ if (WIFSTOPPED (status)
+ && WSTOPSIG (status) == TRAP_IS_SYSCALL)
+ {
+ if (catch_syscall_enabled () == 0)
+ lp->waitstatus.kind = TARGET_WAITKIND_IGNORE;
+ else
+ {
+ struct regcache *regcache = get_thread_regcache (lp->ptid);
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+ lp->waitstatus.value.syscall_number =
+ (int) gdbarch_get_syscall_number (gdbarch, lp->ptid);
+
+ /* If we are catching this specific syscall number, then we
+ should update the target_status to reflect which event
+ has occurred. But if this syscall is not to be caught,
+ then we can safely mark the event as a SYSCALL_RETURN.
+
+ This is particularly needed if:
+
+ - We are catching any syscalls, or
+ - We are catching the syscall "exit"
+
+ In this case, as the syscall "exit" *doesn't* return,
+ then GDB would be confused because it would mark the last
+ syscall event as a SYSCALL_ENTRY. After that, if we re-ran the
+ inferior GDB will think that the first syscall event is
+ the opposite of a SYSCALL_ENTRY, which is the SYSCALL_RETURN.
+ Therefore, GDB would report inverted syscall events. */
+ if (catching_syscall_number (lp->waitstatus.value.syscall_number))
+ lp->waitstatus.kind =
+ (lp->syscall_state == TARGET_WAITKIND_SYSCALL_ENTRY) ?
+ TARGET_WAITKIND_SYSCALL_RETURN : TARGET_WAITKIND_SYSCALL_ENTRY;
+ else
+ lp->waitstatus.kind = TARGET_WAITKIND_SYSCALL_RETURN;
+
+ lp->syscall_state = lp->waitstatus.kind;
+ }
+
+ /* We no longer need or want the 0x80 flag, remove it. */
+ status = TRAP_REMOVE_SYSCALL_FLAG (status);
+ }
+
if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP)
{
if (debug_linux_nat)
@@ -3238,7 +3257,7 @@ retry:
else
store_waitstatus (ourstatus, status);

- if (debug_linux_nat_async)
+ if (debug_linux_nat || debug_linux_nat_async)
fprintf_unfiltered (gdb_stdlog, "LLW: exit\n");

restore_child_signals_mask (&prev_mask);