Re: s390 && user_enable_single_step() (Was: odd utrace testing results on s390x)

[Martin Schwidefsky <schwidefsky@de.ibm.com>]

On Tue, 5 Jan 2010 16:36:33 +0100
Oleg Nesterov <oleg@redhat.com> wrote:

> On 01/05, Martin Schwidefsky wrote:
> >
> > On Mon,  4 Jan 2010 13:11:47 -0800 (PST)
> > Roland McGrath <roland@redhat.com> wrote:
> >
> > > > This probably means that copy_process()->user_disable_single_step()
> > > > is not enough to clear the "this task wants single-stepping" copied
> > > > from parent.
> > >
> > > I would suspect s390's TIF_SINGLE_STEP flag here.  That flag means "a
> > > single-step trap occurred".  This is what causes do_single_step to be
> > > called before returning to user mode, rather than the machine trap doing it
> > > directly as is done in the other arch implementations.
> >
> > Just my thinking as well.
> 
> Oh, I am not sure. But I don't understand TIF_SINGLE_STEP on s390,
> absolutely.
> 
> For example, why do_signal() sets TIF_SINGLE_STEP? Why can't we do
> 
> 	--- a/arch/s390/kernel/signal.c
> 	+++ b/arch/s390/kernel/signal.c
> 	@@ -500,18 +500,10 @@ void do_signal(struct pt_regs *regs)
> 					clear_thread_flag(TIF_RESTORE_SIGMASK);
> 	 
> 				/*
> 	-			 * If we would have taken a single-step trap
> 	-			 * for a normal instruction, act like we took
> 	-			 * one for the handler setup.
> 	-			 */
> 	-			if (current->thread.per_info.single_step)
> 	-				set_thread_flag(TIF_SINGLE_STEP);
> 	-
> 	-			/*
> 				 * Let tracing know that we've done the handler setup.
> 				 */
> 				tracehook_signal_handler(signr, &info, &ka, regs,
> 	-					 test_thread_flag(TIF_SINGLE_STEP));
> 	+					current->thread.per_info.single_step);
> 			}
> 			return;
> 		}
> 
> ?

The reason why we set the TIF_SINGLE_STEP bit in do_signal is that we
want to be able to stop the debugged program before the first
instruction of the signal handler has been executed. The PER single
step causes a trap after an instruction has been executed. That first
instruction can do bad things to the arguments of the signal handler..

> Apart from arch/s390/signal.c, TIF_SINGLE_STEP is used by entry.S
> but I don't understand this asm at all.
> 
> Anyway. I modified the debugging patch a bit:
> 
> --- K/arch/s390/kernel/traps.c~	2009-12-22 10:41:52.909174198 -0500
> +++ K/arch/s390/kernel/traps.c	2010-01-05 09:49:19.541792379 -0500
> @@ -384,6 +384,8 @@ void __kprobes do_single_step(struct pt_
>  	}
>  	if (tracehook_consider_fatal_signal(current, SIGTRAP))
>  		force_sig(SIGTRAP, current);
> +	else
> +		printk("XXX: %d %d\n", current->pid, test_thread_flag(TIF_SINGLE_STEP));
>  }
> 
>  static void default_trap_handler(struct pt_regs * regs, long interruption_code)
> -------------------------------------------------------------------------------
> 
> Now, when I run this test-case
> 
> 	#include <stdio.h>
> 	#include <unistd.h>
> 	#include <signal.h>
> 	#include <sys/ptrace.h>
> 	#include <sys/wait.h>
> 	#include <assert.h>
> 
> 	int main(void)
> 	{
> 		int pid, status;
> 
> 		if (!(pid = fork())) {
> 			assert(ptrace(PTRACE_TRACEME) == 0);
> 			kill(getpid(), SIGSTOP);
> 
> 			if (!fork())
> 				return 43;
> 
> 			wait(&status);
> 			return WEXITSTATUS(status);
> 		}
> 
> 
> 		for (;;) {
> 			assert(pid == wait(&status));
> 			if (WIFEXITED(status))
> 				break;
> 			assert(ptrace(PTRACE_SINGLESTEP, pid, 0,0) == 0);
> 		}
> 
> 		assert(WEXITSTATUS(status) == 43);
> 		return 0;
> 	}
> 
> dmesg shows 799 lines of
> 
> 	XXX: 2389 0
> 
> 
> The kernel is 2.6.32.2 + utrace, but CONFIG_UTRACE is not set.

With or without my bug fix ?

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.