> - it sets task->thread.trap_no/error_code under CONFIG_X86, > what should it do in the #else case?
This can't be this way. It has to be a proper arch hook of some kind. > - it sets info->si_addr = KSTK_EIP() which doesn't check > user_mode_vm(). Hopefully this is OK? Ditto. The si_code/si_addr settings are altogether arch-specific. As long as you are using x86 details with #ifdef for hack drafts, just copy the exact details used in x86's send_sigtrap. > - with or without this patch utrace-ptrace assumes that > PTRACE_SINGLESTEP after PTRACE_EVENT_SYSCALL_EXIT needs > fill_sigtrap_info()'ed signal, this may break or fix !x86 > machines, and I don't know how to check. Perhaps > ptrace_resume(PTRACE_EVENT_SYSCALL_EXIT) needs ifdef's. It should be easy enough to add a ptrace-tests case that distinguishes the two behaviors. I think we can then call the non-x86 behaviors broken and harmonize on the x86 behavior to make that test pass everywhere. > - unlike send_sigtrap()->force_sig_info() we don't unblock > SIGTRAP or reset the handler This is nicer for debugger things actually, but we don't have such niceness for real traps and won't soon. IMHO it is best to start with doing exactly what the old x86 code does, which is force_sig_info--which is also making all machines uniformly do "what a real single-step trap does" and is thus consistent in this PTRACE_SINGLESTEP case matching all others, which we have been saying is a sensible principle. > these changes looks good (but see the next patch). However, any > user-visible change is dangerous. Let's not roll it in. As well as it just being bad form to roll that into implementation detail changes, it makes things less consistent rather than more (on x86, anyway, and arguably across the board). Thanks, Roland
