> 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.
> Roland, I guess utrace-cleanup becomes the "master" branch, yes?
> I mean, utrace-ptrace should be updated according to the recent
> changes in utrace-cleanup, and this is what you are going to send
> upstream.
Right. I was waiting to be sure you didn't object. But AFAICT we now seem
to be happy with this branch modulo more necessary fixes.
> This means you should also merge utrace-cleanup into utrace-ptrace?
Right.
> 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?
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,
too. Even in the most optimal circumstance imaginable, the whole point is
to handle a SIGTRAP signal after the syscall, so that too probably dwarfs
the overhead difference of the faster entry path.
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,
i.e. pass the step flag to utrace_report_syscall_exit. Since today
single-step generates a plain (forced) SIGTRAP, we can synthesize that.
There are at least two options here. One is to do:
if (step && (task->utrace_flags & UTRACE_SIGNAL_CORE)) {
force_sig_info(...);
}
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.
Thanks,
Roland
