Re: [RFC,PATCH 0/14] utrace/ptrace

2009-11-29 Thread Pavel Machek
On Wed 2009-11-25 16:48:18, Christoph Hellwig wrote:
 On Tue, Nov 24, 2009 at 09:01:27PM +0100, Oleg Nesterov wrote:
  Hello.
  
  This is the new iteration of Roland's utrace patch, this time
  with rewrite-ptrace-via-utrace + cleanups in utrace core.
  
  1-7 are already in -mm tree, I am sending them to simplify the
  review.
  
  8-12 don not change the behaviour, simple preparations.
  
  13-14 add utrace-ptrace and utrace
 
 Skipped over it very, very briefly.  One thing I really hate about this
 is that it introduces two ptrace implementation by adding the new one
 without removing the old one.  Given that's it's pretty much too later
 for the 2.6.33 cycle anyway I'd suggest you make sure the remaining
 two major architectures (arm and mips) get converted, and if the
 remaining minor architectures don't manage to get their homework done
 they're left without ptrace.

I don't think introducing regressions to force people to rewrite code
is a good way to go...
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html



Re: utrace-ptrace gdb testsuite tesults

2009-11-29 Thread Jan Kratochvil
On Wed, 25 Nov 2009 23:30:37 +0100, Jan Kratochvil wrote:
   Please point at some built or easily buildable kernel .rpm first.
  
  http://kojipkgs.fedoraproject.org/scratch/roland/task_1825649/
 
 OK, taken for reverification.

Followed the differences found by Qian and verified none of them (did not
verify the ppc suspicious one) has any regression in GDB testsuite.


Regards,
Jan



Re: utrace-ptrace gdb testsuite tesults

2009-11-29 Thread Jan Kratochvil
On Sun, 29 Nov 2009 23:39:59 +0100, Jan Kratochvil wrote:
 Followed the differences found by Qian and verified none of them (did not
 verify the ppc suspicious one) has any regression in GDB testsuite.

Forgot the log FYI.


Regards,
Jan


-result-2.6.31.5-127.fc12.x86_64/gdb
+result-2.6.32-0.53.rc8.496.fc13.x86_64/gdb

*attach*stop* generally unchecked, it should be covered by ptrace-testsuite and
the GDB testcases are currently racy.

-FAIL: gdb.base/follow-child.exp: break
+PASS: gdb.base/follow-child.exp: break
= unstable testcase, RH-specific, dropped as redundant to other 
testcases

/root/jkratoch/redhat/gdb-7.0-3.fc12.src/gdb-7.0-m64/gdb/testsuite
-PASS: gdb.threads/attachstop-mt.exp: attach4 stop by interrupt
-PASS: gdb.threads/attachstop-mt.exp: attach4, exit leaves process sleeping
+FAIL: gdb.threads/attachstop-mt.exp: attach4 stop by interrupt (timeout)
+FAIL: gdb.threads/attachstop-mt.exp: attach4, exit leaves process sleeping
= racy, ignored

-PASS: gdb.base/foll-fork.exp: default parent follow, no catchpoints
+FAIL: gdb.base/foll-fork.exp: (timeout) default parent follow, no catchpoints
= racy, fixed the testcase upstream

/root/jkratoch/redhat/gdb-7.0-3.fc12.src/gdb-7.0-m64/gdb/testsuite.unix.-m32
-FAIL: gdb.threads/attach-stopped.exp: threaded: attach1, exit leaves process 
stopped
+PASS: gdb.threads/attach-stopped.exp: threaded: attach1, exit leaves process 
stopped
= racy, ignored

-FAIL: gdb.base/interrupt.exp: continue
+PASS: gdb.base/interrupt.exp: continue
 FAIL: gdb.base/interrupt.exp: echo data (timeout)
 ERROR: Undefined command .
 UNRESOLVED: gdb.base/interrupt.exp: Send Control-C, second time
 FAIL: gdb.base/interrupt.exp: signal SIGINT (the program is no longer running)
-FAIL: gdb.base/interrupt.exp: echo more data (timeout)
-FAIL: gdb.base/interrupt.exp: send end of file
+PASS: gdb.base/interrupt.exp: echo more data
+FAIL: gdb.base/interrupt.exp: send end of file (eof)
= both kernels behave the same - correctly, updated erestart* tests 
set, for x86_64-x86_64-i386 (kernel-debugger-inferior) GDB needs a fix: 
http://sourceware.org/ml/gdb-patches/2009-11/msg00592.html

-FAIL: gdb.server/ext-run.exp: get process list
+PASS: gdb.server/ext-run.exp: get process list
= upstream gdbserver data corruption

-FAIL: gdb.java/jnpe.exp: next over NPE
+PASS: gdb.java/jnpe.exp: next over NPE
= fixed the testcase in archer

/root/jkratoch/redhat/gdb-7.0-3.fc12.src/gdb-7.0-m32/gdb/testsuite.unix.-m32
-ERROR: Couldn't send info inferior 16 to GDB.
-UNRESOLVED: gdb.base/multi-forks.exp: Did kill 16
+PASS: gdb.base/multi-forks.exp: Run to exit 11
= always ignored by me, IMO racy

-PASS: gdb.threads/attachstop-mt.exp: attach4 stop by interrupt
-PASS: gdb.threads/attachstop-mt.exp: attach4, exit leaves process sleeping
+FAIL: gdb.threads/attachstop-mt.exp: attach4 stop by interrupt (timeout)
+FAIL: gdb.threads/attachstop-mt.exp: attach4, exit leaves process sleeping
= racy, ignored

-FAIL: gdb.cp/constructortest.exp: running to main in runto
 PASS: gdb.cp/constructortest.exp: breaking on A::A
-FAIL: gdb.cp/constructortest.exp: continue to breakpoint: First line A
-FAIL: gdb.cp/constructortest.exp: Verify in in-charge A::A
-FAIL: gdb.cp/constructortest.exp: continue to breakpoint: First line A
-FAIL: gdb.cp/constructortest.exp: Verify in not-in-charge A::A
+PASS: gdb.cp/constructortest.exp: continue to breakpoint: First line A
+PASS: gdb.cp/constructortest.exp: Verify in in-charge A::A
+PASS: gdb.cp/constructortest.exp: continue to breakpoint: First line A
+PASS: gdb.cp/constructortest.exp: Verify in not-in-charge A::A
-FAIL: gdb.pie/break.exp: run until function breakpoint
-FAIL: gdb.pie/break.exp: run until breakpoint set at a line number
-FAIL: gdb.pie/break.exp: run until file:function(6) breakpoint
-FAIL: gdb.pie/break.exp: run until file:function(5) breakpoint (the program is 
no longer running)
-FAIL: gdb.pie/break.exp: run until file:function(4) breakpoint (the program is 
no longer running)
-FAIL: gdb.pie/break.exp: run until file:function(3) breakpoint (the program is 
no longer running)
-FAIL: gdb.pie/break.exp: run until file:function(2) breakpoint (the program is 
no longer running)
-FAIL: gdb.pie/break.exp: run until file:function(1) breakpoint (the program is 
no longer running)
-FAIL: gdb.pie/break.exp: run until quoted breakpoint (the program is no longer 
running)
-FAIL: gdb.pie/break.exp: run until file:linenum breakpoint (the program is no 
longer running)
-FAIL: gdb.pie/break.exp: breakpoint offset +1
-FAIL: gdb.pie/break.exp: step onto breakpoint (the program is no longer 
running)
+PASS: gdb.pie/break.exp: run until function breakpoint
+PASS: gdb.pie/break.exp: run until breakpoint set at a line number
+PASS: gdb.pie/break.exp: run until file:function(6) breakpoint
+PASS: gdb.pie/break.exp: run until file:function(5) breakpoint
+PASS: gdb.pie/break.exp: run until file:function(4) breakpoint
+PASS: 

Re: powerpc: syscall_dotrace() retcode (Was: powerpc: fork stepping)

2009-11-29 Thread Benjamin Herrenschmidt
On Sun, 2009-11-29 at 22:07 +0100, Oleg Nesterov wrote:
 On 11/28, Ananth N Mavinakayanahalli wrote:
 
  syscall-reset is the only failure I see on
  powerpc:
 
  errno 14 (Bad address)
  syscall-reset: syscall-reset.c:95: main: Assertion `(*__errno_location
  ()) == 38' failed.
  unexpected child status 67f
  FAIL: syscall-reset
 
 (to remind, it also fails without utrace)
 
 Once again, I know nothing about powerc, perhaps I misread the code,
 but I believe this test-case is just wrong on powerpc and should be
 fixed.
 
 On powerpc, syscall_get_nr() returns regs-gpr[0], this means this
 register is used to pass the syscall number.

Correct.

 This matches do_syscall_trace_enter(), it returns regs-gpr[0] as a
 (possibly changed by tracer) syscall nr.
 
 arch/powerpc/kernel/entry_64.S does
 
   syscall_dotrace:
 
bl  .do_syscall_trace_enter
mr  r0,r3  // I guess, r3 = r0 ?

r3 is the return value from a function so this replaces the
syscall number
...
b   syscall_dotrace_cont
 
   syscall_dotrace_cont:
 
   syscall_dotrace_cont:
 
   cmpldi  0,r0,NR_syscalls
   bge-syscall_enosys
 
   syscall_enosys:
 
   li  r3,-ENOSYS
   b   syscall_exit
 
 
 Now return to the test-case, syscall-reset.c. The tracee does
 l = syscall (-23, 1, 2, 3) and stops.
 
 The tracer does
 
   #define RETREG  offsetof(struct pt_regs, gpr[0])
   #define NEWVAL  ((long) ENOTTY)
 
   l = ptrace(PTRACE_PEEKUSER, child, RETREG, 0l);
 
 l == -23, this is correct, note syscall(-23) above.
 
   l = ptrace(PTRACE_POKEUSER, child, RETREG, NEWVAL);
 
 And expects the tracee will see NEWVAL==ENOTTY after return from
 the systame call.
 
 Of course this can't happen. We changed the syscall number, the
 new value is ENOTTY == 25 == __NR_stime, sys_stime() correctly
 returns -EFAULT.
 
 -
 
 If I change the test-case to use NEWVAL == 1000 (or any other value
 greater than NR_syscalls), then the tracee sees ENOSYS and this is
 correct too.
 
 But I do not see how it is possible to change the retcode on powerpc.
 Unlike x86, powepc doesn't set -ENOSYS in advance, before doing
 do_syscall_trace_enter() logic. This means that if the tracer cancels
 syscall, r3 will be overwritten by syscall_enosys.
 
 This probably means the kernel should be fixed too, but I am not
 brave enough to change the asm which I can't understand ;)

Yes, the asm should be changed. I suppose we could check if the result
of do_syscall_trace_enter is negative, and if it is, branch to the exit
path using r3 as the error code. Would that be ok ?

Something like this:

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 1175a85..7a88c88 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -419,6 +419,9 @@ syscall_dotrace:
stw r0,_TRAP(r1)
addir3,r1,STACK_FRAME_OVERHEAD
bl  do_syscall_trace_enter
+   cmpwi   cr0,r3,0
+   blt ret_from_syscall
+
/*
 * Restore argument registers possibly just changed.
 * We use the return value of do_syscall_trace_enter
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 9763267..ec709a7 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -240,6 +240,9 @@ syscall_dotrace:
bl  .save_nvgprs
addir3,r1,STACK_FRAME_OVERHEAD
bl  .do_syscall_trace_enter
+   cmpdi   cr0,r3,0
+   blt syscall_exit
+
/*
 * Restore argument registers possibly just changed.
 * We use the return value of do_syscall_trace_enter


Cheers,
Ben.