Re: ptrace single-stepping change breaks Wine

Jesse Allen
2004-12-30T19:27:14Z

On Thu, 30 Dec 2004 09:59:27 -0800 (PST), Davide Libenzi
<davidel@xmailserver.org> wrote:
> On Wed, 29 Dec 2004, Linus Torvalds wrote:
>
> > On Wed, 29 Dec 2004, Davide Libenzi wrote:
> > >
> > > I think same. My test simply let the function processing to let thru and
> > > reach the fake signal sending point.
> >
> > No, your test-case doesn't even send a signal at all, because your
> > test-program just uses a PTRACE_SINGLESTEP without any "send signal" - so
> > "exit_code" will be zero after the debuggee gets released from the
> > "ptrace_notify()", and the suspect code will never be executed..
>
> No no, my test case has nothing to do with the extra signal sent in
> do_syscall_trace(). But the test I put at the time:
>
> - if (!test_thread_flag(TIF_SYSCALL_TRACE))
> + if (!test_thread_flag(TIF_SYSCALL_TRACE) &&
> + !test_thread_flag(TIF_SINGLESTEP))
> return;
>
> will make the code to not execute the "return" (in the single step case)
> and to fall through the point where the signal is sent.

Using the latest version of the patch on do_syscall_trace(), it still
doesn't work unless I remove this test. If indeed it's supposed to
fall through to receive the proper signal, (ie to single step properly
after an int op), then removing it is wrong, and I won't consider it
anymore. I also have to use the patch shown below, with a typo-fixed,
to fix the other problem. I broke it apart from the other because we
might want to consider it seperately right now.

I spent some time speaking to my brother about this. He has done his
own kernel before for an embedded system. He was rather frustrated
with me trying to find a reason for why a rather simple change broke
my program. He told me I couldn't have it both ways. However I
believe that I don't understand the linux code well enough to make
that conclusion.

>
>
> > The problem I think I see (and which the comment alludes to) is that if
> > the controlling process continues the debuggee with a signal, we'll be
> > doing the wrong thing: the code in do_syscall_trace() will take that
> > signal (in "current->exit_code") and send it as a real signal to the
> > debuggee, but since it is debugged, it will be caught (again) by the
> > controlling process, which apparently in the case of Wine gets very
> > confused.
> >
> > So I _think_ the problem happens for the following schenario:
> > - wine for some reason does a PTRACE_SINGLESTEP over a system call
> > - after the single-step, wine ends up trying to get the sub-process to
> > enter a signal handler with ptrace( PTRACE_CONT, ... sig)
> > - the normal ptrace path (where we trap a signal - see kernel/signal.c
> > and the "ptrace_stop()" logic) handles this correctly, but
> > do_syscall_trace() will do a "send_sig()"
> > - we'll try to handle the signal when returning to the traced process
> > - now "get_signal_to_deliver()" will invoce the ptrace logic AGAIN, and
> > call ptrace_stop() for this fake signal, and we'll report a new event
> > to the controlling process.
> >
> > Does this make sense?
>
> This might explain what they were seeing, but OTOH it seems that the real
> cause of their problems is related to something else (according to other
> emails on this thread).
>
>

Actually, I don't think the vanilla kernel has the code that breaks
those other wine programs. I just learned of this yesterday and it's
not related. I believe it's only in fedora core 3 or -ac kernels and
I use vanilla kernels.

Jesse

--- linux/arch/i386/kernel/ptrace.c 2004-12-29 14:10:34.000000000 -0700
+++ linux-mod/arch/i386/kernel/ptrace.c 2004-12-29 20:50:00.000000000 -0700
@@ -142,18 +142,31 @@
{
long eflags;

+ /*
+ * Always set TIF_SINGLESTEP - this guarantees that
+ * we single-step system calls etc..
+ */
set_tsk_thread_flag(child, TIF_SINGLESTEP);
+
+ /*
+ * If TF was already set, don't do anything else
+ */
eflags = get_stack_long(child, EFL_OFFSET);
+ if (eflags & TRAP_FLAG)
+ return;
put_stack_long(child, EFL_OFFSET, eflags | TRAP_FLAG);
child->ptrace |= PT_DTRACE;
}

static void clear_singlestep(struct task_struct *child)
{
+ /* Always clear TIF_SINGLESTEP... */
+ clear_tsk_thread_flag(child, TIF_SINGLESTEP);
+
+ /* But touch TF only if it was set by us.. */
if (child->ptrace & PT_DTRACE) {
long eflags;

- clear_tsk_thread_flag(child, TIF_SINGLESTEP);
eflags = get_stack_long(child, EFL_OFFSET);
put_stack_long(child, EFL_OFFSET, eflags & ~TRAP_FLAG);
child->ptrace &= ~PT_DTRACE;