* [PATCH] Fix typo in i386 single step changes
@ 2005-01-02 23:32 Andi Kleen
2005-01-02 23:41 ` Daniel Jacobowitz
2005-01-03 3:30 ` Jesse Allen
0 siblings, 2 replies; 6+ messages in thread
From: Andi Kleen @ 2005-01-02 23:32 UTC (permalink / raw)
To: torvalds; +Cc: linux-kernel, davidel, mh, dan, the3dfxdude
Fix an obvious typo in the recent i386 single stepping changes.
I would recommend to redo all the Wine etc. testing that lead to this patch
since it probably never worked.
Signed-off-by: Andi Kleen <ak@muc.de>
--- linux-2.6.10-bk5/arch/i386/kernel/traps.c-o Mon Jan 3 01:02:06 2005
+++ linux-2.6.10-bk5/arch/i386/kernel/traps.c Mon Jan 3 01:03:05 2005
@@ -725,7 +725,7 @@
if (tsk->ptrace & PT_DTRACE) {
regs->eflags &= ~TF_MASK;
tsk->ptrace &= ~PT_DTRACE;
- if (!tsk->ptrace & PT_DTRACE)
+ if (!(tsk->ptrace & PT_DTRACE))
goto clear_TF;
}
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix typo in i386 single step changes 2005-01-02 23:32 [PATCH] Fix typo in i386 single step changes Andi Kleen @ 2005-01-02 23:41 ` Daniel Jacobowitz 2005-01-03 0:01 ` Andi Kleen 2005-01-03 1:11 ` Linus Torvalds 2005-01-03 3:30 ` Jesse Allen 1 sibling, 2 replies; 6+ messages in thread From: Daniel Jacobowitz @ 2005-01-02 23:41 UTC (permalink / raw) To: Andi Kleen; +Cc: torvalds, linux-kernel, davidel, mh, the3dfxdude On Mon, Jan 03, 2005 at 12:32:19AM +0100, Andi Kleen wrote: > > Fix an obvious typo in the recent i386 single stepping changes. > > I would recommend to redo all the Wine etc. testing that lead to this patch > since it probably never worked. > > Signed-off-by: Andi Kleen <ak@muc.de> > > --- linux-2.6.10-bk5/arch/i386/kernel/traps.c-o Mon Jan 3 01:02:06 2005 > +++ linux-2.6.10-bk5/arch/i386/kernel/traps.c Mon Jan 3 01:03:05 2005 > @@ -725,7 +725,7 @@ > if (tsk->ptrace & PT_DTRACE) { > regs->eflags &= ~TF_MASK; > tsk->ptrace &= ~PT_DTRACE; > - if (!tsk->ptrace & PT_DTRACE) > + if (!(tsk->ptrace & PT_DTRACE)) > goto clear_TF; > } > } That test is still wrong... the bit is cleared on the previous line. Is it supposed to be testing something else entirely? -- Daniel Jacobowitz ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix typo in i386 single step changes 2005-01-02 23:41 ` Daniel Jacobowitz @ 2005-01-03 0:01 ` Andi Kleen 2005-01-03 1:41 ` Linus Torvalds 2005-01-03 1:11 ` Linus Torvalds 1 sibling, 1 reply; 6+ messages in thread From: Andi Kleen @ 2005-01-03 0:01 UTC (permalink / raw) To: torvalds, linux-kernel, davidel, mh, the3dfxdude On Sun, Jan 02, 2005 at 06:41:55PM -0500, Daniel Jacobowitz wrote: > On Mon, Jan 03, 2005 at 12:32:19AM +0100, Andi Kleen wrote: > > > > Fix an obvious typo in the recent i386 single stepping changes. > > > > I would recommend to redo all the Wine etc. testing that lead to this patch > > since it probably never worked. > > > > Signed-off-by: Andi Kleen <ak@muc.de> > > > > --- linux-2.6.10-bk5/arch/i386/kernel/traps.c-o Mon Jan 3 01:02:06 2005 > > +++ linux-2.6.10-bk5/arch/i386/kernel/traps.c Mon Jan 3 01:03:05 2005 > > @@ -725,7 +725,7 @@ > > if (tsk->ptrace & PT_DTRACE) { > > regs->eflags &= ~TF_MASK; > > tsk->ptrace &= ~PT_DTRACE; > > - if (!tsk->ptrace & PT_DTRACE) > > + if (!(tsk->ptrace & PT_DTRACE)) > > goto clear_TF; > > } > > } > > That test is still wrong... the bit is cleared on the previous line. > Is it supposed to be testing something else entirely? Davide also pointed out that the typo was already in the old code. The goto was never executed because neither 0 nor 1 gave true when anded with 2 (PT_DTRACE) And we need to deliver the signal anyways, otherwise the debugger cannot see it. The early return is definitely wrong. Also looking at the code more closely the comment above doesn't match what the code does. I fixed that too. So a better patch is probably: Fix logic that worked only by accident in ptrace path. Fix wrong comment. Signed-off-by: Andi Kleen <ak@muc.de> --- linux-2.6.10-bk5/arch/i386/kernel/traps.c-o Mon Jan 3 01:02:06 2005 +++ linux-2.6.10-bk5/arch/i386/kernel/traps.c Mon Jan 3 01:41:17 2005 @@ -706,27 +706,23 @@ /* Mask out spurious TF errors due to lazy TF clearing */ if (condition & DR_STEP) { - /* - * The TF error should be masked out only if the current - * process is not traced and if the TRAP flag has been set - * previously by a tracing process (condition detected by - * the PT_DTRACE flag); remember that the i386 TRAP flag - * can be modified by the process itself in user mode, - * allowing programs to debug themselves without the ptrace() - * interface. + /* + * Ignore steps comming from kernel space. + * Apparently they can happen due to lazy TF clearing + * (where exactly? -AK) */ if ((regs->xcs & 3) == 0) goto clear_TF_reenable; /* * Was the TF flag set by a debugger? If so, clear it now, - * so that register information is correct. + * so that register information is correct and then + * deliver it as signal so that the debugger sees the + * step. */ if (tsk->ptrace & PT_DTRACE) { regs->eflags &= ~TF_MASK; tsk->ptrace &= ~PT_DTRACE; - if (!tsk->ptrace & PT_DTRACE) - goto clear_TF; } } @@ -748,7 +744,6 @@ clear_TF_reenable: set_tsk_thread_flag(tsk, TIF_SINGLESTEP); -clear_TF: regs->eflags &= ~TF_MASK; return; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix typo in i386 single step changes 2005-01-03 0:01 ` Andi Kleen @ 2005-01-03 1:41 ` Linus Torvalds 0 siblings, 0 replies; 6+ messages in thread From: Linus Torvalds @ 2005-01-03 1:41 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-kernel, davidel, mh, the3dfxdude On Sun, 3 Jan 2005, Andi Kleen wrote: > > Also looking at the code more closely the comment above doesn't > match what the code does. I fixed that too. The comment is slightly stale, but yours perpetuates the staleness, and doesn't fix the first comment which also talks about staleness. Back when we were really broken with TF handling, we also used to set TF when we started single-stepping, but we never cleared it until we got a stale DR_STEP event. In fact, we could have the debugger detach from the process, and leave TF set. That's not true any more, and I don't think it was true even before the latest changes - the code (and comment) has been stale for quite a while. So the whole "lazy TF" thing is really incorrect, and these days it's about the user setting TF on its own. And the _code_ was corrupted too (and working, but only because it could never trigger anyway). I'll remove it and fix up the comments. Linus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix typo in i386 single step changes 2005-01-02 23:41 ` Daniel Jacobowitz 2005-01-03 0:01 ` Andi Kleen @ 2005-01-03 1:11 ` Linus Torvalds 1 sibling, 0 replies; 6+ messages in thread From: Linus Torvalds @ 2005-01-03 1:11 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Andi Kleen, linux-kernel, davidel, mh, the3dfxdude On Sun, 2 Jan 2005, Daniel Jacobowitz wrote: > > That test is still wrong... the bit is cleared on the previous line. > Is it supposed to be testing something else entirely? Yeah, it's supposed to be removed. It was old (and clearly broken) cut-and-paste for a condition that can't happen any more, namely that "ptrace & PT_TRACED" is not set any more. That "stale ptrace bit set" condition hasn't been valid for a long time, it just never got removed. Oh, well. Linus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix typo in i386 single step changes 2005-01-02 23:32 [PATCH] Fix typo in i386 single step changes Andi Kleen 2005-01-02 23:41 ` Daniel Jacobowitz @ 2005-01-03 3:30 ` Jesse Allen 1 sibling, 0 replies; 6+ messages in thread From: Jesse Allen @ 2005-01-03 3:30 UTC (permalink / raw) To: Andi Kleen; +Cc: torvalds, linux-kernel, davidel, mh, dan On Mon, 03 Jan 2005 00:32:19 +0100, Andi Kleen <ak@muc.de> wrote: > > Fix an obvious typo in the recent i386 single stepping changes. > > I would recommend to redo all the Wine etc. testing that lead to this patch > since it probably never worked. > Well, even though it's unlikely it does anything, I tried the later patch and everything is still OK. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-01-03 3:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2005-01-02 23:32 [PATCH] Fix typo in i386 single step changes Andi Kleen 2005-01-02 23:41 ` Daniel Jacobowitz 2005-01-03 0:01 ` Andi Kleen 2005-01-03 1:41 ` Linus Torvalds 2005-01-03 1:11 ` Linus Torvalds 2005-01-03 3:30 ` Jesse Allen
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 NNTP newsgroup(s).