[PATCH] ptrace(2) single-stepping into signal handlers

[PATCH] ptrace(2) single-stepping into signal handlers

[John Blackwood]

Hi all,

The intent of this posting is to generate some discussion concerning
ptrace(2) single-step operations into user signal handlers.

When a ptraced process has done a debugger stop due to a pending signal
delivery, and this ptraced process has a corresponding signal handler,
recent linux kernels have changed the way that a subsequent single-step
operation is treated/processed.

Specifically, when a PTRACE_SINGLESTEP operation is executed at this
point, the kernel will now single-step to the first instruction in the
target process's signal handler.  Additional single-step calls will
let us single-step through the ptraced process's signal handler, and a
PTRACE_CONT call will cause the ptraced process to continue onward through
the entire signal handler and stop with a SIGTRAP signal once again
back at the original instruction where the original PTRACE_SINGLESTEP
call was made (this last stop point seems to exhibit a bug for the i386
architecture - see below for an explanation).

While being able to single-step through a ptraced process's signal
handler may be desirable sometimes, I would like to submit the notion
that usually, a user would like simply to single-step to the next
instruction in the main stream of execution.  That is to say, when the
single-step operation is issued, the user would like to execute the
signal handler, return back from the signal handler, and then stop at
the next instruction in the normal (non-signal) stream of execution.
Note that this described behavior is how previous linux kernels
handled single-step operations when the user had a signal pending with
a corresponding user signal handler.  The kernel did NOT previously step
into the signal handler, but would instead step to the next instruction
in the normal, main stream of execution.

As support for this preference, consider attempting to debug a program
that has setup a SIGALRM signal handler, and while you are attempting
to debug a stream of instructions by single-stepping through the code,
you constantly instead end up in the SIGALRM signal handler, and have
to set a breakpoint at the instruction that you actually wanted to
single-step to, and then 'continue' out of the SIGALARM signal handler.

Since some of us believe that single-stepping into a signal handler is
more of an extended feature as opposed to the usual behavior of stepping
to the next instruction in the normal stream of execution, I would like
to suggest/submit the following ideas for enhancing this particular area
of ptrace(2) support.

The enhancement ideas are:

1. Make the default behavior that we single-step to the next instruction
    in the main (non-signal handler) stream of execution, instead of
    single-stepping into the user's signal handler.

2. Add a new ptrace PTRACE_SETOPTIONS command flag,
    PTRACE_O_TRACESSTEPSIG.  When this new flag is set by the debugger,
    then execute the new behavior of single-stepping into the user's
    signal handler.  (Yes, this means that debuggers such as gdb will need
    to be taught about this new PTRACE_SETOPTIONS flag if the user wishes
    to single-step into their signal handler.)

3. When the PTRACE_O_TRACESSTEPSIG flag is set and the ptraced process
    single-steps to the first instruction in the signal handler and executes
    a SIGTRAP debugger stop, make it easier for the debugger to determine
    why the ptraced process stopped, by passing a new extended result code,
    PTRACE_EVENT_SSTEPSIG.

    This extended code will be returned in the status parameter location
    of a wait(2) style call, and the debugger can extract this additional
    information with the following pseudo code snippet:

        waitpid(-1, &wait_status, ...);
        if (WIFSTOPPED(wait_status)) {
            ret_sig = WSTOPSIG(wait_status);
            if ((wait_status >> 16) & 0xff) == PTRACE_EVENT_SSTEPSIG) {
                /* single stepped into a user signal handler */
            }
        }

In addition to the above suggestions for making the "single-step into
signal handlers" optional and easier to detect, I have also noticed a
bug in the current i386 implementation that I would like to mention.

The following sequence will cause the debugger to forever single step
the target process, even though the ptrace(2) PTRACE_CONT command is
being issued to the target process:

- The ptraced process has a pending signal and
   it stops to notify the debugger.

- The debugger then single-steps into the ptraced process's signal handler.

- On the next eventual continue (PTRACE_CONT) command, we run through the
   signal handler, and we stop once again at the next instruction before
   we changed our execution stream and single-stepped into the user's
   signal handler.

- At this point, we can no longer continue the ptraced process
   with a PTRACE_CONT command.  Instead, all ptrace(2) PTRACE_CONT calls
   are treated as if we had made a ptrace(2) PTRACE_SINGLESTEP call.


The reason for this behavior is due to the fact that:

- We saved off the eflags (with the TF bit set) in setup_sigcontext()
   before we single stepped into the user's signal handler.

- When we exit the user's signal handler via a PTRACE_CONT call,
   we restore the original eflags value (with the TF bit set) in the
   restore_sigcontext() routine.  However, at this point, the
   PT_DTRACE flag is no longer set.

- Subsequent ptrace(2) PTRACE_CONT calls will call
   clear_singlestep() to make sure that any single-step state is
   cleared out before we continue the ptrace process.

   However, since the PT_DTRACE flag is not set, we do NOT clear
   the TF bit in the eflags register stack location, and we
   instead end up doing a single-step operation.

   Also, if we do a PTRACE_SINGLESTEP operation at this point, to
   attempt to get out of this stuck state, the set_singlestep()
   routine will not set the PT_DTRACE flag since the TF flag
   is already set!

============================================================================

Below are some diffs for the changes described above.  These diffs are
only for i386 and x86_64 architectures, since these are the only ones
that I have access to, and have actually tested on.

The changes are:

- include/linux/ptrace.h
   Add the PTRACE_O_TRACESSTEPSIG, PTRACE_EVENT_SSTEPSIG, PT_TRACE_SSTEPSIG
   defines, and modify the appropriate masks.

- kernel/ptrace.c
   Process the new PTRACE_O_TRACESSTEPSIG flag in ptrace_setoptions().

- arch/i386/kernel/ptrace.c
   In clear_singlestep(), always clear TRAP_FLAG out of the eflags field,
   since PT_DTRACE will not be set if we just came out of a signal handler
   that we single-stepped into.   This particuar change is admittedly
   less than optimal, but it does work.

- arch/i386/kernel/signal.c
   In both setup_frame() and setup_rt_frame(), notify the debugger with
   a ptrace_notify() call only if the PT_TRACE_SSTEPSIG flag is set.
   Also, pass the PTRACE_EVENT_SSTEPSIG extended event/result code on
   the ptrace_notify() SIGTRAP call.  If PT_TRACE_SSTEPSIG is not set,
   then clear the TIF_SINGLESTEP flag since we do NOT want to single step
   anywhere within the signal handler.

- arch/x86_64/ia32/ia32_signal.c
   In ia32_setup_sigcontext(), do not clear the TF_MASK bit in eflags when
   saving eflags off to sigcontext.  This change lets us stop again at
   the next instruction in the normal execution stream after we continue
   out of single-stepping into the signal handler.  This change matches
   the i386 version.

   In ia32_setup_frame() and ia32_setup_rt_frame(), process the
   PT_TRACE_SSTEPSIG flag in the same fashion as the i386 version.

- arch/x86_64/kernel/signal.c
   In setup_sigcontext(), preserve the TF_MASK bit in eflags.

   In setup_rt_frame() process the PT_TRACE_SSTEPSIG flag just
   as the i386 version does.

============================================================================

diff -ru linux-2.6.11.10/include/linux/ptrace.h new/include/linux/ptrace.h
--- linux-2.6.11.10/include/linux/ptrace.h	2005-05-16 13:51:47.000000000 
-0400
+++ new/include/linux/ptrace.h	2005-06-02 16:47:46.000000000 -0400
@@ -35,8 +35,9 @@
  #define PTRACE_O_TRACEEXEC	0x00000010
  #define PTRACE_O_TRACEVFORKDONE	0x00000020
  #define PTRACE_O_TRACEEXIT	0x00000040
+#define PTRACE_O_TRACESSTEPSIG	0x00000080

-#define PTRACE_O_MASK		0x0000007f
+#define PTRACE_O_MASK		0x000000ff

  /* Wait extended result codes for the above trace options.  */
  #define PTRACE_EVENT_FORK	1
@@ -45,6 +46,7 @@
  #define PTRACE_EVENT_EXEC	4
  #define PTRACE_EVENT_VFORK_DONE	5
  #define PTRACE_EVENT_EXIT	6
+#define PTRACE_EVENT_SSTEPSIG	7

  #include <asm/ptrace.h>

@@ -64,8 +66,9 @@
  #define PT_TRACE_VFORK_DONE	0x00000100
  #define PT_TRACE_EXIT	0x00000200
  #define PT_ATTACHED	0x00000400	/* parent != real_parent */
+#define PT_TRACE_SSTEPSIG	0x00000800

-#define PT_TRACE_MASK	0x000003f4
+#define PT_TRACE_MASK	0x00000bf4

  /* single stepping state bits (used on ARM and PA-RISC) */
  #define PT_SINGLESTEP_BIT	31
diff -ru linux-2.6.11.10/kernel/ptrace.c new/kernel/ptrace.c
--- linux-2.6.11.10/kernel/ptrace.c	2005-05-16 13:51:53.000000000 -0400
+++ new/kernel/ptrace.c	2005-06-02 16:48:01.000000000 -0400
@@ -314,6 +314,9 @@
  	if (data & PTRACE_O_TRACEEXIT)
  		child->ptrace |= PT_TRACE_EXIT;

+	if (data & PTRACE_O_TRACESSTEPSIG)
+		child->ptrace |= PT_TRACE_SSTEPSIG;
+
  	return (data & ~PTRACE_O_MASK) ? -EINVAL : 0;
  }

diff -ru linux-2.6.11.10/arch/i386/kernel/ptrace.c 
new/arch/i386/kernel/ptrace.c
--- linux-2.6.11.10/arch/i386/kernel/ptrace.c	2005-05-16 
13:50:30.000000000 -0400
+++ new/arch/i386/kernel/ptrace.c	2005-06-02 16:49:12.000000000 -0400
@@ -250,15 +250,18 @@

  static void clear_singlestep(struct task_struct *child)
  {
+	struct pt_regs *regs = get_child_regs(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) {
-		struct pt_regs *regs = get_child_regs(child);
-		regs->eflags &= ~TRAP_FLAG;
+	/* Clear PT_DTRACE if it is set.  Always clear TF
+	 * since it could be set from a restore_sigcontext()
+	 * signal handler return with PT_DTRACE not set.
+	 */
+	if (child->ptrace & PT_DTRACE)
  		child->ptrace &= ~PT_DTRACE;
-	}
+	regs->eflags &= ~TRAP_FLAG;
  }

  /*
diff -ru linux-2.6.11.10/arch/i386/kernel/signal.c 
new/arch/i386/kernel/signal.c
--- linux-2.6.11.10/arch/i386/kernel/signal.c	2005-05-16 
13:50:30.000000000 -0400
+++ new/arch/i386/kernel/signal.c	2005-06-03 11:14:40.293133119 -0400
@@ -414,15 +414,18 @@
  	regs->xss = __USER_DS;
  	regs->xcs = __USER_CS;

-	/*
-	 * Clear TF when entering the signal handler, but
-	 * notify any tracer that was single-stepping it.
-	 * The tracer may want to single-step inside the
-	 * handler too.
+	/* Clear TF when entering the signal handler, but notify any tracer
+	 * that was single-stepping if the PT_TRACE_SSTEPSIG flag is set.
+	 * Clear TIF_SINGLESTEP if PT_TRACE_SSTEPSIG is not set, since we
+	 * don't want to single-step through the signal handler in that case.
  	 */
  	regs->eflags &= ~TF_MASK;
-	if (test_thread_flag(TIF_SINGLESTEP))
-		ptrace_notify(SIGTRAP);
+	if (test_thread_flag(TIF_SINGLESTEP)) {
+	    if (current->ptrace & PT_TRACE_SSTEPSIG)
+		ptrace_notify((PTRACE_EVENT_SSTEPSIG << 8) | SIGTRAP);
+	    else
+		clear_thread_flag(TIF_SINGLESTEP);
+	}

  #if DEBUG_SIG
  	printk("SIG deliver (%s:%d): sp=%p pc=%p ra=%p\n",
@@ -507,15 +510,18 @@
  	regs->xss = __USER_DS;
  	regs->xcs = __USER_CS;

-	/*
-	 * Clear TF when entering the signal handler, but
-	 * notify any tracer that was single-stepping it.
-	 * The tracer may want to single-step inside the
-	 * handler too.
+	/* Clear TF when entering the signal handler, but notify any tracer
+	 * that was single-stepping if the PT_TRACE_SSTEPSIG flag is set.
+	 * Clear TIF_SINGLESTEP if PT_TRACE_SSTEPSIG is not set, since we
+	 * don't want to single-step through the signal handler in that case.
  	 */
  	regs->eflags &= ~TF_MASK;
-	if (test_thread_flag(TIF_SINGLESTEP))
-		ptrace_notify(SIGTRAP);
+	if (test_thread_flag(TIF_SINGLESTEP)) {
+	    if (current->ptrace & PT_TRACE_SSTEPSIG)
+		ptrace_notify((PTRACE_EVENT_SSTEPSIG << 8) | SIGTRAP);
+	    else
+		clear_thread_flag(TIF_SINGLESTEP);
+	}

  #if DEBUG_SIG
  	printk("SIG deliver (%s:%d): sp=%p pc=%p ra=%p\n",
diff -ru linux-2.6.11.10/arch/x86_64/ia32/ia32_signal.c 
new/arch/x86_64/ia32/ia32_signal.c
--- linux-2.6.11.10/arch/x86_64/ia32/ia32_signal.c	2005-05-16 
13:50:31.000000000 -0400
+++ new/arch/x86_64/ia32/ia32_signal.c	2005-06-03 11:08:33.728120866 -0400
@@ -351,7 +351,6 @@
  		 struct pt_regs *regs, unsigned int mask)
  {
  	int tmp, err = 0;
-	u32 eflags;

  	tmp = 0;
  	__asm__("movl %%gs,%0" : "=r"(tmp): "0"(tmp));
@@ -376,11 +375,7 @@
  	err |= __put_user(current->thread.trap_no, &sc->trapno);
  	err |= __put_user(current->thread.error_code, &sc->err);
  	err |= __put_user((u32)regs->rip, &sc->eip);
-	eflags = regs->eflags;
-	if (current->ptrace & PT_PTRACED) {
-		eflags &= ~TF_MASK;
-	}
-	err |= __put_user((u32)eflags, &sc->eflags);
+	err |= __put_user((u32)regs->eflags, &sc->eflags);
  	err |= __put_user((u32)regs->rsp, &sc->esp_at_signal);

  	tmp = save_i387_ia32(current, fpstate, regs, 0);
@@ -498,12 +493,18 @@
  	regs->ss = __USER32_DS;

  	set_fs(USER_DS);
-	if (regs->eflags & TF_MASK) {
-		if (current->ptrace & PT_PTRACED) {
-			ptrace_notify(SIGTRAP);
-		} else {
-			regs->eflags &= ~TF_MASK;
-		}
+
+	/* Clear TF when entering the signal handler, but notify any tracer
+	 * that was single-stepping if the PT_TRACE_SSTEPSIG flag is set.
+	 * Clear TIF_SINGLESTEP if PT_TRACE_SSTEPSIG is not set, since we
+	 * don't want to single-step through the signal handler in that case.
+	 */
+	regs->eflags &= ~TF_MASK;
+	if (test_thread_flag(TIF_SINGLESTEP)) {
+	    if (current->ptrace & PT_TRACE_SSTEPSIG)
+		ptrace_notify((PTRACE_EVENT_SSTEPSIG << 8) | SIGTRAP);
+	    else
+		clear_thread_flag(TIF_SINGLESTEP);
  	}

  #if DEBUG_SIG
@@ -598,12 +599,18 @@
  	regs->ss = __USER32_DS;

  	set_fs(USER_DS);
-	if (regs->eflags & TF_MASK) {
-		if (current->ptrace & PT_PTRACED) {
-			ptrace_notify(SIGTRAP);
-		} else {
-			regs->eflags &= ~TF_MASK;
-		}
+
+	/* Clear TF when entering the signal handler, but notify any tracer
+	 * that was single-stepping if the PT_TRACE_SSTEPSIG flag is set.
+	 * Clear TIF_SINGLESTEP if PT_TRACE_SSTEPSIG is not set, since we
+	 * don't want to single-step through the signal handler in that case.
+	 */
+	regs->eflags &= ~TF_MASK;
+	if (test_thread_flag(TIF_SINGLESTEP)) {
+	    if (current->ptrace & PT_TRACE_SSTEPSIG)
+		ptrace_notify((PTRACE_EVENT_SSTEPSIG << 8) | SIGTRAP);
+	    else
+		clear_thread_flag(TIF_SINGLESTEP);
  	}

  #if DEBUG_SIG
diff -ru linux-2.6.11.10/arch/x86_64/kernel/signal.c 
new/arch/x86_64/kernel/signal.c
--- linux-2.6.11.10/arch/x86_64/kernel/signal.c	2005-05-16 
13:50:31.000000000 -0400
+++ new/arch/x86_64/kernel/signal.c	2005-06-03 11:09:03.963991909 -0400
@@ -186,7 +186,6 @@
  setup_sigcontext(struct sigcontext __user *sc, struct pt_regs *regs, 
unsigned long mask, struct task_struct *me)
  {
  	int err = 0;
-	unsigned long eflags;

  	err |= __put_user(0, &sc->gs);
  	err |= __put_user(0, &sc->fs);
@@ -210,11 +209,7 @@
  	err |= __put_user(me->thread.trap_no, &sc->trapno);
  	err |= __put_user(me->thread.error_code, &sc->err);
  	err |= __put_user(regs->rip, &sc->rip);
-	eflags = regs->eflags;
-	if (current->ptrace & PT_PTRACED) {
-		eflags &= ~TF_MASK;
-	}
-	err |= __put_user(eflags, &sc->eflags);
+	err |= __put_user(regs->eflags, &sc->eflags);
  	err |= __put_user(mask, &sc->oldmask);
  	err |= __put_user(me->thread.cr2, &sc->cr2);

@@ -329,12 +324,18 @@
  	regs->rsp = (unsigned long)frame;

  	set_fs(USER_DS);
-	if (regs->eflags & TF_MASK) {
-		if ((current->ptrace & (PT_PTRACED | PT_DTRACE)) == (PT_PTRACED | 
PT_DTRACE)) {
-			ptrace_notify(SIGTRAP);
-		} else {
-			regs->eflags &= ~TF_MASK;
-		}
+
+	/* Clear TF when entering the signal handler, but notify any tracer
+	 * that was single-stepping if the PT_TRACE_SSTEPSIG flag is set.
+	 * Clear TIF_SINGLESTEP if PT_TRACE_SSTEPSIG is not set, since we
+	 * don't want to single-step through the signal handler in that case.
+	 */
+	regs->eflags &= ~TF_MASK;
+	if (test_thread_flag(TIF_SINGLESTEP)) {
+	    if (current->ptrace & PT_TRACE_SSTEPSIG)
+		ptrace_notify((PTRACE_EVENT_SSTEPSIG << 8) | SIGTRAP);
+	    else
+		clear_thread_flag(TIF_SINGLESTEP);
  	}

  #ifdef DEBUG_SIG

Re: [PATCH] ptrace(2) single-stepping into signal handlers

[Daniel Jacobowitz]

On Fri, Jun 03, 2005 at 01:21:20PM -0400, John Blackwood wrote:
> While being able to single-step through a ptraced process's signal
> handler may be desirable sometimes, I would like to submit the notion
> that usually, a user would like simply to single-step to the next
> instruction in the main stream of execution.  That is to say, when the
> single-step operation is issued, the user would like to execute the
> signal handler, return back from the signal handler, and then stop at
> the next instruction in the normal (non-signal) stream of execution.
> Note that this described behavior is how previous linux kernels
> handled single-step operations when the user had a signal pending with
> a corresponding user signal handler.  The kernel did NOT previously step
> into the signal handler, but would instead step to the next instruction
> in the normal, main stream of execution.

No, in fact, the user will almost always want to see the signal
handler.  The GDB user experience is dramatically worse when signal
handlers execute without any sign to the user; single steps can appear
to leave you absolutely anywhere, esp. if the signal handler longjumps
or hits a breakpoint.

You can see this by using GDB on MIPS or ARM, where it does not use
PTRACE_SINGLESTEP.

If your application wants to skip the signal handler, you will have to
use a breakpoint to do so.

I don't know what previous version of the kernel you are referring to. 
This behavior is not new.

> As support for this preference, consider attempting to debug a program
> that has setup a SIGALRM signal handler, and while you are attempting
> to debug a stream of instructions by single-stepping through the code,
> you constantly instead end up in the SIGALRM signal handler, and have
> to set a breakpoint at the instruction that you actually wanted to
> single-step to, and then 'continue' out of the SIGALARM signal handler.

Yes.  That's three context switches instead of one; if you are using a
local debug agent, then this is not a big deal.  If you are using a
simplistic remote debug agent, then you need to teach the agent when to
do this automatically.

> 1. Make the default behavior that we single-step to the next instruction
>    in the main (non-signal handler) stream of execution, instead of
>    single-stepping into the user's signal handler.

An incompatible change.  No.

> 2. Add a new ptrace PTRACE_SETOPTIONS command flag,
>    PTRACE_O_TRACESSTEPSIG.  When this new flag is set by the debugger,
>    then execute the new behavior of single-stepping into the user's
>    signal handler.  (Yes, this means that debuggers such as gdb will need
>    to be taught about this new PTRACE_SETOPTIONS flag if the user wishes
>    to single-step into their signal handler.)

ptrace options are for asynchronous behaviors.  This is a synchronous
behavior; if it is worth implementing at all it should be an action,
not an option.

> 3. When the PTRACE_O_TRACESSTEPSIG flag is set and the ptraced process
>    single-steps to the first instruction in the signal handler and executes
>    a SIGTRAP debugger stop, make it easier for the debugger to determine
>    why the ptraced process stopped, by passing a new extended result code,
>    PTRACE_EVENT_SSTEPSIG.
> 
>    This extended code will be returned in the status parameter location
>    of a wait(2) style call, and the debugger can extract this additional
>    information with the following pseudo code snippet:

First of all we already have an event message facility,
PTRACE_GETEVENTMSG.  Secondly the only way we will step into a signal
handler after PTRACE_SINGLESTEP is if the PTRACE_SINGLESTEP requests
a signal.  That was your application; keep track of it in your
application.

> The reason for this behavior is due to the fact that:
> 
> - We saved off the eflags (with the TF bit set) in setup_sigcontext()
>   before we single stepped into the user's signal handler.

You didn't say what kernel you were using.  I believe this was fixed
some time ago.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] ptrace(2) single-stepping into signal handlers

[Andi Kleen]

On Fri, Jun 03, 2005 at 01:21:20PM -0400, John Blackwood wrote:
> 1. Make the default behavior that we single-step to the next instruction
>    in the main (non-signal handler) stream of execution, instead of
>    single-stepping into the user's signal handler.

I think it is better to not change the default behaviour. You risk
breaking programs and there are much more that use ptrace than just gdb.

> 
> 2. Add a new ptrace PTRACE_SETOPTIONS command flag,

I think it would be better to just define a new ptrace singlestep command
with the new semantics instead of adding options.

> - The ptraced process has a pending signal and
>   it stops to notify the debugger.
> 
> - The debugger then single-steps into the ptraced process's signal handler.
> 
> - On the next eventual continue (PTRACE_CONT) command, we run through the
>   signal handler, and we stop once again at the next instruction before
>   we changed our execution stream and single-stepped into the user's
>   signal handler.
> 
> - At this point, we can no longer continue the ptraced process
>   with a PTRACE_CONT command.  Instead, all ptrace(2) PTRACE_CONT calls
>   are treated as if we had made a ptrace(2) PTRACE_SINGLESTEP call.

Have you actually seen this in practice?
> 
> - We saved off the eflags (with the TF bit set) in setup_sigcontext()
>   before we single stepped into the user's signal handler.
> 
> - When we exit the user's signal handler via a PTRACE_CONT call,
>   we restore the original eflags value (with the TF bit set) in the
>   restore_sigcontext() routine.  However, at this point, the
>   PT_DTRACE flag is no longer set.
> 
> - Subsequent ptrace(2) PTRACE_CONT calls will call
>   clear_singlestep() to make sure that any single-step state is
>   cleared out before we continue the ptrace process.
> 
>   However, since the PT_DTRACE flag is not set, we do NOT clear
>   the TF bit in the eflags register stack location, and we
>   instead end up doing a single-step operation.
> 
>   Also, if we do a PTRACE_SINGLESTEP operation at this point, to
>   attempt to get out of this stuck state, the set_singlestep()
>   routine will not set the PT_DTRACE flag since the TF flag
>   is already set!

Sounds like a bug. Can you submit a change for that separately?

-Andi

Re: [PATCH] ptrace(2) single-stepping into signal handlers

[cutaway]

----- Original Message ----- 
From: "Andi Kleen" <ak@suse.de>
To: "John Blackwood" <john.blackwood@ccur.com>
Cc: <linux-kernel@vger.kernel.org>; <roland@redhat.com>; <ak@suse.de>;
<akpm@osdl.org>; <bugsy@ccur.com>
Sent: Sunday, June 05, 2005 07:21
Subject: Re: [PATCH] ptrace(2) single-stepping into signal handlers


> On Fri, Jun 03, 2005 at 01:21:20PM -0400, John Blackwood wrote:
> > 1. Make the default behavior that we single-step to the next instruction
> >    in the main (non-signal handler) stream of execution, instead of
> >    single-stepping into the user's signal handler.
>
> I think it is better to not change the default behaviour. You risk
> breaking programs and there are much more that use ptrace than just gdb.

A performance trace profiler would want to step through signal handlers.
Sometimes they're an architected norm for a program rather than an abnormal
condition handler (ex. Lisp systems routinely handling GP faults on null
pointers rather than doing null pointer checks in critical loops)

Re: [PATCH] ptrace(2) single-stepping into signal handlers

[John Blackwood]

 > Subject: Re: [PATCH] ptrace(2) single-stepping into signal handlers
 > From: Andi Kleen <ak@suse.de>
 > Date: Sun, 5 Jun 2005 13:21:19 +0200
 > To: John Blackwood <john.blackwood@ccur.com>
 > CC: linux-kernel@vger.kernel.org, roland@redhat.com, ak@suse.de, 
akpm@osdl.org, bugsy@ccur.com
 >
 > On Fri, Jun 03, 2005 at 01:21:20PM -0400, John Blackwood wrote:
 >
 > >> 1. Make the default behavior that we single-step to the next 
instruction
 > >>    in the main (non-signal handler) stream of execution, instead of
 > >>    single-stepping into the user's signal handler.
 >
 >
 > I think it is better to not change the default behaviour. You risk
 > breaking programs and there are much more that use ptrace than just gdb.

O.K.  Yes, it seems that this is definitely not a popular idea.   :-)


 > >> 2. Add a new ptrace PTRACE_SETOPTIONS command flag,
 >
 >
 > I think it would be better to just define a new ptrace singlestep command
 > with the new semantics instead of adding options.

Yes, this is a good idea...  I just might re-post this approach
as a suggestion.


 > >> - The ptraced process has a pending signal and
 > >>   it stops to notify the debugger.
 > >>
 > >> - The debugger then single-steps into the ptraced process's signal 
handler.
 > >>
 > >> - On the next eventual continue (PTRACE_CONT) command, we run 
through the
 > >>   signal handler, and we stop once again at the next instruction 
before
 > >>   we changed our execution stream and single-stepped into the user's
 > >>   signal handler.
 > >>
 > >> - At this point, we can no longer continue the ptraced process
 > >>   with a PTRACE_CONT command.  Instead, all ptrace(2) PTRACE_CONT 
calls
 > >>   are treated as if we had made a ptrace(2) PTRACE_SINGLESTEP call.
 >
 >
 > Have you actually seen this in practice?

Yes, but Andrew Morton suggested I try out the above test scenario on:

 
ftp://ftp.kernel.org/pub/linux/kernel/v2.6/testing/linux-2.6.12-rc5.tar.bz2

And the problem has indeed been fixed - there is some new code in the
i386 version of handle_signal() that fixes this problem.

(I saw the problem on earlier 2.6.11.10 and 2.6.11.11 kernels.)


Thanks for the feedback, Andrew.

Re: [PATCH] ptrace(2) single-stepping into signal handlers

[John Blackwood]

 > Subject: Re: [PATCH] ptrace(2) single-stepping into signal handlers
 > From: Andrew Morton <akpm@osdl.org>
 > Date: Fri, 3 Jun 2005 16:31:29 -0700
 > To: Daniel Jacobowitz <dan@debian.org>
 > CC: john.blackwood@ccur.com
 >
 > >>> >
 > >>> > Hi Dan,
 > >>> >
 > >>> > I observed this behavior in a 2.6.11.10 kernel.  The code in 
2.6.11.11
 > >>> > looks the same in this area... this is the i386 code that I am 
speaking of.
 > >>> >
 > >>> > I guess that 'some time ago' is more recent than that?
 > >>> >
 > >>> >
 > >>> > If so, then please excuse me... and it's great that this is fixed.
 > >
 > >>
 > >> I'm not sure of the timeline, but could you check that in a current
 > >> 2.6.12 GIT snapshot?
 > >>
 >
 >
 > 2.6.12-rc5 should be sufficient.
 >
 > 
ftp://ftp.kernel.org/pub/linux/kernel/v2.6/testing/linux-2.6.12-rc5.tar.bz2
 >
 > or, even better, the above plus:
 >
 > 
ftp://ftp.kernel.org/pub/linux/kernel/v2.6/snapshots/patch-2.6.12-rc5-git8.bz2

Andrew and Dan,

Yes, I tried the above combination, and the stuck-in-single-step mode
after continuing out of the signal handler is indeed FIXED.

Some additional code was added to the i386 version of handle_signal()
(even before the git8 additions) so that the TF flag is now cleared and
the task no longer stops when it comes out of the signal handler and
returns back to the original execution stream.

Thanks.

Re: [PATCH] ptrace(2) single-stepping into signal handlers

[Daniel Jacobowitz]

On Fri, Jun 03, 2005 at 02:07:40PM -0400, John Blackwood wrote:
> > Subject: Re: [PATCH] ptrace(2) single-stepping into signal handlers
> > From: Daniel Jacobowitz <dan@debian.org>
> > Date: Fri, 3 Jun 2005 13:34:33 -0400
> > To: John Blackwood <john.blackwood@ccur.com>
> > CC: linux-kernel@vger.kernel.org, roland@redhat.com, ak@suse.de, 
> akpm@osdl.org, bugsy@ccur.com
> >
> > On Fri, Jun 03, 2005 at 01:21:20PM -0400, John Blackwood wrote:
> 
> > >> The reason for this behavior is due to the fact that:
> > >>
> > >> - We saved off the eflags (with the TF bit set) in setup_sigcontext()
> > >>   before we single stepped into the user's signal handler.
> >
> >
> > You didn't say what kernel you were using.  I believe this was fixed
> > some time ago.
> 
> Hi Dan,
> 
> I observed this behavior in a 2.6.11.10 kernel.  The code in 2.6.11.11
> looks the same in this area... this is the i386 code that I am speaking of.
> 
> I guess that 'some time ago' is more recent than that?
> 
> 
> If so, then please excuse me... and it's great that this is fixed.

I'm not sure of the timeline, but could you check that in a current
2.6.12 GIT snapshot?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] ptrace(2) single-stepping into signal handlers

[John Blackwood]

 > Subject: Re: [PATCH] ptrace(2) single-stepping into signal handlers
 > From: Daniel Jacobowitz <dan@debian.org>
 > Date: Fri, 3 Jun 2005 13:34:33 -0400
 > To: John Blackwood <john.blackwood@ccur.com>
 > CC: linux-kernel@vger.kernel.org, roland@redhat.com, ak@suse.de, 
akpm@osdl.org, bugsy@ccur.com
 >
 > On Fri, Jun 03, 2005 at 01:21:20PM -0400, John Blackwood wrote:

 > >> The reason for this behavior is due to the fact that:
 > >>
 > >> - We saved off the eflags (with the TF bit set) in setup_sigcontext()
 > >>   before we single stepped into the user's signal handler.
 >
 >
 > You didn't say what kernel you were using.  I believe this was fixed
 > some time ago.

Hi Dan,

I observed this behavior in a 2.6.11.10 kernel.  The code in 2.6.11.11
looks the same in this area... this is the i386 code that I am speaking of.

I guess that 'some time ago' is more recent than that?


If so, then please excuse me... and it's great that this is fixed.