Re: linux-next: add utrace tree
On Mon, 2010-01-25 at 08:52 -0800, Linus Torvalds wrote: That said, I also suspect that people should still look seriously at simply just improving ptrace. For example, I suspect that the biggest problem with ptrace is really just the signalling, and that creating a new extension for JUST THAT, and then having a model where you can choose - at PTRACE_ATTACH time - how to wait for events would be a good thing. like returning a fd to poll() on ? :-) Cheers, Ben.
Re: powerpc: syscall_dotrace() retcode (Was: powerpc: fork stepping)
On Tue, 2009-12-01 at 11:27 -0800, Roland McGrath wrote: If the powerpc maintainers want to change the behavior here, that is fine by me. But there is no need for that just to satisfy general ptrace cleanups (or utrace). Normal concerns require that no such change break the ptrace behavior that userland could have relied on in the past. So off hand I don't see a reason to change at all. If every arch were to change so that registers changed at syscall-entry were left unmolested by aborting the syscall, then that might be a new consistency worth having. But short of that, I don't really see a benefit. All this implies that the ptrace-tests case relating to this needs to be tailored differently for powerpc and each other arch so it expects and verifies exactly the arch-specific behavior that's been seen in the past. Ok thanks. I'm happy to not change it then, the risk of breaking some existing assumption is too high in my book. Cheers, Ben.
Re: powerpc: syscall_dotrace() retcode (Was: powerpc: fork stepping)
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.
Re: [PATCH] powerpc ptrace block-step
On Wed, 2009-04-01 at 14:59 -0700, Roland McGrath wrote:
diff --git a/arch/powerpc/include/asm/ptrace.h
b/arch/powerpc/include/asm/ptrace.h
index c9c678f..d7692b8 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -135,7 +135,9 @@ do {
\
* These are defined as per linux/ptrace.h, which see.
*/
#define arch_has_single_step() (1)
+#define arch_has_block_step()(1)
The patch only implements it for server/classic processors, not BookE,
thus it should probably only advertise it for these :-)
Though it wouldn't be too hard to implement it for BookE using DBCR0:BRT
(Branch Taken debug event) though it might need some careful fixups such
as the one we have for single step regarding hitting exception entry
code.
Cheers,
Ben.
extern void user_enable_single_step(struct task_struct *);
+extern void user_enable_block_step(struct task_struct *);
extern void user_disable_single_step(struct task_struct *);
#endif /* __ASSEMBLY__ */
@@ -288,4 +290,6 @@ extern void user_disable_single_step(struct task_struct
*);
#define PPC_PTRACE_PEEKUSR_3264 0x91
#define PPC_PTRACE_POKEUSR_3264 0x90
+#define PTRACE_SINGLEBLOCK 0x100 /* resume execution until next branch */
+
#endif /* _ASM_POWERPC_PTRACE_H */
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 3635be6..656fea2 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -707,12 +707,29 @@ void user_enable_single_step(struct task_struct *task)
task-thread.dbcr0 |= DBCR0_IDM | DBCR0_IC;
regs-msr |= MSR_DE;
#else
+ regs-msr = ~MSR_BE;
regs-msr |= MSR_SE;
#endif
}
set_tsk_thread_flag(task, TIF_SINGLESTEP);
}
+void user_enable_block_step(struct task_struct *task)
+{
+ struct pt_regs *regs = task-thread.regs;
+
+ if (regs != NULL) {
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+ task-thread.dbcr0 = DBCR0_IDM | DBCR0_BT;
+ regs-msr |= MSR_DE;
+#else
+ regs-msr = ~MSR_SE;
+ regs-msr |= MSR_BE;
+#endif
+ }
+ set_tsk_thread_flag(task, TIF_SINGLESTEP);
+}
+
void user_disable_single_step(struct task_struct *task)
{
struct pt_regs *regs = task-thread.regs;
@@ -729,7 +746,7 @@ void user_disable_single_step(struct task_struct *task)
task-thread.dbcr0 = ~(DBCR0_IC | DBCR0_IDM);
regs-msr = ~MSR_DE;
#else
- regs-msr = ~MSR_SE;
+ regs-msr = ~(MSR_SE | MSR_BE);
#endif
}
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
