On 11/11, Roland McGrath wrote: > > > I saw you have utrace-syscall-resumed branch but never looked at it. > > For those not concerned with its purpose, there is one thing you must > know. Every report_syscall_entry callback must do: > > if (utrace_syscall_action(action) == UTRACE_SYSCALL_RESUMED) > return UTRACE_RESUME; > > to avoid being confused by the new extra callbacks.
OK, thanks. > > Then I fix utrace-ptrace. I _think_ this will be easy too. > > ptrace_report_syscall_exit() can notice ctx->resume == UTRACE_SINGLESTEP > > and send the trap if utrace_control() doesn't set TIF_SINGLESTEP > > "synchronously". > > > > (And this means with utrace tracehook_report_syscall_exit() can do > > nothing except utrace_report_syscall_exit()). > > Today we don't call into utrace unless UTRACE_EVENT(SYSCALL_EXIT) is set. > So if nothing changes in the utrace layer, this means that ptrace will > enable UTRACE_EVENT(SYSCALL_EXIT) (i.e. TIF_SYSCALL) for PTRACE_SINGLE*. > Is that what you had in mind? Yes, exactly. > There are two possible issues with doing it that way. > > The first is purely from a ptrace perspective. Today PTRACE_SINGLE* does > not set TIF_SYSCALL. On x86 this doesn't really make a difference in the > underlying code, because TIF_SINGLESTEP takes the same path. So probably > this is fine. But on other machines I'm not entirely sure that TIF_SYSCALL > won't take a slower path than single-step alone caused already for syscall > entries. This surely pales in comparison to the overhead of the ptrace > stop and context switch and all, but still, it could be a change. On x86 > I'm not sure there's any reason not to optimize that better in the future, I see. > The other issue comes from the utrace API perspective. Here it just seems > entirely wrong to require the engine (ptrace or any other) to do any > syscall-exit magic of its own to make UTRACE_SINGLESTEP work on a syscall > insn like it does on other insns. The utrace layer should make that trap > appear transparently like the normal traps seen for UTRACE_SINGLESTEP. > > So, we could have tracehook_report_syscall_exit send the signal as it does > now upstream. But probably better is to do it all in the utrace layer, Indeed! > i.e. pass the step flag to utrace_report_syscall_exit. The problem is, we can't use step flag. step == TIF_SINGLESTEP, and it can be unset despite the fact ptrace (or other engine) did utrace_control(UTRACE_SINGLESTEP) to resume. Simple example. The tracee stopped in syscall-entry. the tracer does PTRACE_SINGLESTEP. With the recent changes in utrace-cleanup utrace_control() doesn't set TIF_SINGLESTEP, and the tracee passes syscall_trace_leave() without TIF_SINGLESTEP. But I guess utrace_report_syscall_exit() can look at utrace->resume and notice UTRACE_SINGLESTEP ? > That approximates the check that yesterday's x86 code does: > > if (test_thread_flag(TIF_SINGLESTEP) && > tracehook_consider_fatal_signal(current, SIGTRAP)) > send_sigtrap(current, regs, 0, TRAP_BRKPT); > > But still we are queuing a real signal here. That is no different from the > real hardware trap case, so perhaps that is the right thing to do for now. > (Eventually we want to have a way for all these traps to avoid becoming > real signals at all, but that is a subject for another day.) Like a real > signal, that has the problem of still being queued if the tracer detaches > immediately thereafter (so it gets delivered to the user). We could avoid > that for this fake signal by just setting some flag that tells > utrace_get_signal to synthesize it before dequeuing real signals. > It's probably not worth bothering with that complexity now. Yes, I think we have more important problems ;) Oleg.