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);
===== 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;

Reply via email to