> > The former is e.g. PTRACE_SINGLESTEP while an unrelated engine uses > > UTRACE_EVENT(SYSCALL_ENTRY). The ptrace engine's report_quiesce returns > > UTRACE_SINGLESTEP. finish_resume_report() calls user_enable_single_step(). > > That seems fine. Right? > > In this case ptrace_report_quiesce(event) returns UTRACE_RESUME, note > that it does > > return event ? UTRACE_RESUME : ctx->resume; > > and event == SYSCALL_ENTRY. This looks correct.
With the utrace layer changes we're discussing, we need it to return UTRACE_SINGLESTEP (i.e. ctx->resume) here. AFAICT that never hurts even with today's utrace layer. > > I see. finish_resume_report() will only do utrace_stop() and then we'll > > go directly into the syscall unless someone used UTRACE_REPORT. > > Yes, > > > utrace_stop() will only set TIF_NOTIFY_RESUME and utrace->resume. In > > the real resume-to-user cases, that's fine because we'll handle that in > > utrace_resume() or utrace_get_signal() before doing anything else. > > Not sure I understand. utrace_stop() will not set TIF_NOTIFY_RESUME? > > We are talking about the case when the tracee stops in SYSCALL_ENTER, > and then the tracer does utrace_control(UTRACE_SINGLESTEP) to resume. > In this case utrace_control() sets ->resume/TIF_NOTIFY_RESUME, yes. I think you do understand fine. Yes, that's what it will do. I was just recognizing that this isn't enough in the syscall-entry case. > > this > > seems like the right idea for how to get tracehook_report_syscall_exit > > called in the cases where it is in old ptrace. > > Afaics yes. > > But, what if another engine does utrace_control(UTRACE_INTERRUPT) ? Hmm. Yes, I think this is the one case that is unlike all the others. That is, UTRACE_INTERRUPT at syscall-entry. In all other cases, nothing would care about utrace->resume until we get to get_signal_to_deliver anyway, where the UTRACE_SIGNAL_REPORT pass will trump any pending resume action anyway so you don't care that you lost track of it when UTRACE_INTERRUPT came in. Hmm. So what does UTRACE_INTERRUPT mean here anyway? It means that we should report "soon" and that signal_pending() should be true until that report. So today that means you can get into the syscall with signal_pending() and depending on the particular call that might cause a normal blocking path not to block and/or a -EINTR/-ERESTART* return, but some syscalls will just complete normally. How about if we say that UTRACE_INTERRUPT at syscall-entry means that the syscall really doesn't happen at all? That is, instead, you force an -ERESTARTNOHAND return and the normal roll-back such that you get your report_signal callback before the syscall is entered. That even seems like a useful utrace API feature, as well as conveniently smoothing over this quirk in the resume action handling. My idea here is that this makes it OK to lose UTRACE_SINGLESTEP here because from the user-mode-centric perspective no instruction has happened yet. The original guy who did UTRACE_SINGLESTEP (and perhaps never cared about syscall tracing) will get a generic report_quiesce or report_signal at which it needs to reassert UTRACE_SINGLESTEP. This merits more thought. For now, let's just leave an XXX comment about a UTRACE_INTERRUPT effectively swallowing the UTRACE_SINGLESTEP that should cause utrace_report_syscall_exit to be called. I think we can revisit this corner after we have merged up all the rest of it. > Argh. I meant, from the caller pov, utrace_control(UTRACE_SINGLESTEP) > does enable_step() "immediately", like it did before utrace-cleanup > changes. I see. From the utrace API perspective, I don't think anything changes here. In today's code, the resume action can take effect without a subsequent utrace callback. So that's the same as it ever was, and where this actually happens is not something that a utrace caller should know or can tell. > IOW, suppose the tracee is stopped, and ptrace does UTRACE_SINGLESTEP. > The tracee resumes, processes ->resume, and does enable_step(). > From the ptrace pov, it looks as if utrace_control() does enable_step() > before utrace_wakeup(). Sure. > I meant, every time the tracee stops, it should process ->resume > after wakeup. Looks like, the patch you sent in another thread > (which adds apply_resume_action()) does something like this, right? Right. Thanks, Roland
