On 11/18, Roland McGrath wrote:
>
> > Hmm. not sure I understand how this can work. I mean, this won't
> > enable stepping (in this particular case we are discussing).
>
> I might be confused. I'm thinking of two cases: the report_syscall_entry
> pass yields UTRACE_SINGLESTEP, or that it yields UTRACE_STOP and then
> utrace_control() sets utrace->resume to UTRACE_SINGLESTEP.
>
> 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.
> The latter is e.g. PTRACE_SINGLESTEP after ptrace stop at syscall entry.
Yes, this is the case I was talking about.
> 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.
> @@ -1794,11 +1794,13 @@ static void finish_resume_report(struct
> {
> finish_report_reset(task, utrace, report);
>
> - switch (report->action) {
> - case UTRACE_STOP:
> - utrace_stop(task, utrace, report->resume_action);
> - break;
> + if (report->action == UTRACE_STOP) {
> + utrace_stop(task, utrace, report->resume_action, true);
> + report->action = start_report(utrace);
> + WARN_ON(report->action == UTRACE_STOP);
> + }
>
> + switch (report->action) {
> case UTRACE_INTERRUPT:
> if (!signal_pending(task))
> set_tsk_thread_flag(task, TIF_SIGPENDING);
Yes, this means the tracee gets TIF_SINGLESTEP right after resume,
and
> 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) ?
> > Instead of fixing utrace_report_syscall_entry(), perhaps we should
> > change utrace_stop() path to enable the stepping if needed, but this
> > means we return to "synchronous" utrace_control(UTRACE_XXXSTEP).
>
> I don't really follow what you mean by "synchronous".
Argh. I meant, from the caller pov, utrace_control(UTRACE_SINGLESTEP)
does enable_step() "immediately", like it did before utrace-cleanup
changes.
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().
> Your suggestion is
> equivalent to making utrace_finish_vfork call finish_resume_report instead
> of utrace_stop, isn't it?
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?
I'll try to read it carefully now.
Oleg.
