Re: ptrace single-stepping change breaks Wine

Linus Torvalds
2004-12-30T00:50:09Z

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2539 bytes --]

On Wed, 29 Dec 2004, Jesse Allen wrote:
> >
> > So instead of removing the setting of TIF_SINGLESTEP in set_singlestep(),
> > can you test whether removing the _testing_ of it in do_syscall_trace()
> > makes things happier for you? Hmm?
> >
>
> Yes, doing that does work. But I still have to remove the conditional
> TF clear. Here's the diff now to show you.

Ok. That's a good piece of information.

I don't actually understand why do_syscall_trace() does that
TIF_SINGLESTEP test in the first place, since the ptrace_notify() should
happen as part of the _normal_ TIF_SINGLESTEP handling, afaiks. No
apparent need to do it in syscall tracing, and do_syscall_trace() does
that bogus fake signal sending.

Hmm.. That TF_SINGLESTEP test was added by Davide Libenzi in August, and I
think Davide had some test for exactly this. Davide? Can you recall why
you did this in the system call tracing code, rather than depend on the
regular TIF_SINGLESTEP handling in arch/i386/kernel/signal.c:
do_notify_resume()?

(Jesse's patch re-appended here in-line for Davide's "reading pleasure").

That still leaves the problem of the clearing of TF_MASK. I _appears_ that
the problem is that TF was set both by Wine doing a PTRACE_SINGLESTEP
(since PT_DTRACE is set) _and_ the application having TF enabled in eflags
from before (since it seems to want to keep it enabled).

How about something like the attachment for the TF_MASK issue (replacing
your "don't clear" code)? The comments should make the intention pretty
obvious, but I have equally obviously not actually _tested_ any of this..

Linus

---
--- linux/arch/i386/kernel/ptrace.c 2004-12-29 14:10:34.000000000 -0700
+++ linux-mod/arch/i386/kernel/ptrace.c 2004-12-29 14:22:33.000000000 -0700
@@ -568,8 +568,7 @@
audit_syscall_exit(current, regs->eax);
}

- if (!test_thread_flag(TIF_SYSCALL_TRACE) &&
- !test_thread_flag(TIF_SINGLESTEP))
+ if (!test_thread_flag(TIF_SYSCALL_TRACE))
return;
if (!(current->ptrace & PT_PTRACED))
return;
--- linux/arch/i386/kernel/signal.c 2004-12-29 14:10:34.000000000 -0700
+++ linux-mod/arch/i386/kernel/signal.c 2004-12-29 14:23:04.000000000 -0700
@@ -299,8 +299,8 @@
* don't want debugging to change state.
*/
eflags = regs->eflags;
- if (current->ptrace & PT_DTRACE)
- eflags &= ~TF_MASK;
+// if (current->ptrace & PT_DTRACE)
+// eflags &= ~TF_MASK;
err |= __put_user(eflags, &sc->eflags);
err |= __put_user(regs->esp, &sc->esp_at_signal);
err |= __put_user(regs->xss, (unsigned int __user *)&sc->ss);

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1084 bytes --]

===== arch/i386/kernel/ptrace.c 1.28 vs edited =====
--- 1.28/arch/i386/kernel/ptrace.c 2004-11-22 09:44:52 -08:00
+++ edited/arch/i386/kernel/ptrace.c 2004-12-29 16:42:04 -08:00
@@ -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 (flags & 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;