I think there are some problems with the current s390 implementation of single-step. In particular, this code: case PTRACE_SINGLESTEP: /* set the trap flag. */ if (!valid_signal(data)) return -EIO; clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE); child->exit_code = data; if (data) set_tsk_thread_flag(child, TIF_SINGLE_STEP); else set_single_step(child); /* give it a chance to run. */ wake_up_process(child); return 0; The canonical version of that logic does set_single_step unconditionally. This difference prevents cleaning up s390 ptrace to share more common code for the non-arch magic parts of ptrace. But I think that in fact this difference also creates a bug in the current implementation, which would be fixed by reorganizing things so that the PTRACE_SINGLESTEP code could indeed match other machines (and so disappear in favor of common code). The s390 TIF_SINGLE_STEP flag means "single-step trap has occurred". Once set, it causes traps.c:do_single_step to be called before the next return to user mode--that is what calls notifiers (kprobes) and/or sends SIGTRAP. The PER exception handler (entry64.S:pgm_per_std, pgm_svcper) sets the flag. It is not clear to me why pgm_per_std doesn't call some code that leads to do_single_step directly, as the single-step hardware exception handler does on other machines. But I'm happy to assume there is some s390-specific reason it can't or shouldn't be done that way. This is the background as I understand it, please correct me as needed. The case where the current s390 PTRACE_SINGLESTEP code differs from the norm is data!=0, i.e. single-step with a signal delivery. My guess is that this code is intended to ensure that single-step into a signal handler generates a SIGTRAP with the PC still at the very first instruction of the handler, not the second. That is indeed the desired behavior for that case. However, I suspect this implementation is broken for other cases. That is, when the signal is ignored, or default-ignored, or blocked, i.e. data!=0 but the target thread is going to resume at its existing PC as it would for data==0, not at a new PC after signal handler setup. In that case, it looks to me like the thread would get an immediate SIGTRAP without executing another instruction--a single-step that actually goes zero steps. (I have not actually tested this scenario to verify my analysis. A simple test would be a breakpoint or other trap at the beginning of a basic block several insns long, and then: wait; A=PEEKUSR,PSWADDR; SINGLESTEP,SIGWINCH; wait; B=PEEKUSR,PSWADDR; assert(B != A).) If I'm right, this needs to be fixed in the current code. Personally, my actual motivation is to normalize the s390 code path with all others so we can clean up ptrace to have much less work going on in arch code. On x86, ptrace uses a TIF_SINGLESTEP flag, which has a different meaning from s390's TIF_SINGLE_STEP. x86 TIF_SINGLESTEP means "should single-step", and is set in addition to the hardware state that makes the next user instruction take a hardware single-step trap. The step-into-handler problem is solved using this: after handler setup it does: if (test_thread_flag(TIF_SINGLESTEP)) ptrace_notify(SIGTRAP); The flag is also used for making single-step into the system call instruction cause a trap immediately after the syscall completes, rather than one user instruction later as would otherwise happen, because the x86 single-step hardware does not treat the syscall like a normal user insn from the user-mode perspective. If the s390 system call instruction works such that the pgm_per* stuff gets called as for a normal user instruction, then you don't need anything like this for the step-into-system call case. So it might be enough to add after signal handler setup: if (current->thread.per_info.single_step) ptrace_notify(SIGTRAP); Then PTRACE_SINGLESTEP need not touch TIF_SINGLE_STEP, and can always call set_single_step, matching the style of other machines. This will make it possible to just rename some functions and define a macro and then use ptrace_request for CONT, SINGLESTEP, et al following my -mm patches for that. Thanks, Roland - To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
