syscall auto-restart handling does not handle native syscalls: DR, clients, attach
i#1145 failed to add auto-restart handling for signals that don't enter record_pending_signal: primarily our own suspend signals, but also nudge signals. This includes all native interruption points: DR and client C code, app code for attach.
Xref #1145 (closed), #1216 (closed)
Support auto-restart in client C code
So far we've had to explicitly put loops checking for EINTR into client code where we wouldn't have to natively. Proper handling here would eliminate that.
Incorrect auto-restart for signals used during attach results in broken apps
If an attach takeover signal interrupts an auto-restart syscall, we'll break the app. We have seen this on some large apps.
Proposal: pass SA_RESTART to our own handlers!
A simple proposal here is to set SA_RESTART on our own handler. This will cause the kernel to adjust the sigcontext to restore the original xax and shift the PC back by 2. This will solve the native cases (attach, client, DR code) without any further work.
I think the reason #1145 (closed) didn't use SA_RESTART was a mis-understanding of when the kernel adjusts the PC: if the kernel adjusts on sigreturn then it messes with cases where DR changes the PC. But the kernel adjusts prior to setting up the frame.
Inlined syscalls, deliver signal immediately and leave context alone
For interrupting an inlined syscall in the cache: deliver the signal immediately as we do now, just as though the signal arrived just before the syscall executed. There is no pre- or post-syscall handling to worry about. No need to change the context: use the one the kernel reset. For a non-auto-restart syscall, leave that alone too.
For from-dispatch syscalls, have to undo the kernel's reset and then emulate
For interrupting do_syscall: first check get_at_syscall() to confirm we executed the pre-syscall part. If not, handle normally. If so, we either have to go back to dispatch to deliver the signal, in which case we may as well undo the pc-2 and xax restore and use our auto-restart emulation code, or we deliver the signal immediately and have problems w/ cross-syscall state (just like "CLS" on Windows). Having to keep emulating seems backward but it does seem like the simplest solution? Or should we try to call handle_post_system_call() and then deliver the signal immediately?
For delivering immediately, xref i#1206 "must deliver signal received in do_syscall immediately". We canceled that b/c w/ the actual interruption w/o SA_RESTART we didn't need it.
w/o doing anything we get a HANG on the eintr-noinline test
It records the signal, and returns -- but that return restarts the syscall, which blocks, and we then never deliver the signal.
-ignore_syscalls doesn't hang b/c:
record_pending_signal(23) from cache pc 0x000000005535b044
delaying until exit F2033
signal interrupted pre/post syscall itself so delivering now
We can now live w/ being inaccurate at identifying auto-restart syscalls
Our current check just looks at the syscall number which is not enough. But with SA_RESTART, the kernel tells us which ones are auto-restart, and we only need our own check for i#1216's nops -- and for the nops we can be conservative and insert nops where the syscall might be auto-restart.
Transparency of post-syscall handler
Re: is it un-transparent to have EINTR returned for an auto-restart syscall: since no app code is run and it's just DR and client-visible state it seeems ok. I guess a client counting syscall returns would have it wrong.
Avoiding this seems hard: we'd have to stack dcontexts like on Windows.
We can just pass in the xax/r0 value to restore
We can avoid storing it into TLS on ARM, and decoding on x86.
is_after_syscall checks in synch, etc. have to handle pc==syscall
We have to distinguish the case of not yet having executed the syscall yet, from having executed it but it got interrupted with the pc set backward. If we only have to do this for gencode handled from dispatch, get_at_syscall() will work.
w/ SA_RESTART, can we remove all the emulation code? and the nop insertion? => no
No: nop insertion is for non-auto-restart syscalls, and the emulation is needed to get back to dispatch. We can make it more accurate.