Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#2659 syscall restart: switch to using SA_RESTART #2660

Merged
merged 2 commits into from
Oct 6, 2017

Conversation

derekbruening
Copy link
Contributor

Changes the strategy for handling interrupted syscalls to use SA_RESTART,
which adds easy handling of interruptions of auto-restart syscalls in
native code, such as during attach or in client C code.

This also simplifies auto-restart of app syscalls, as we now have foolproof
identification of auto-restart situations (before our syscall-number-based
check was inaccurate) and the kernel has restored the clobbered register
value for us. That eliminates the TLS store we were doing on ARM and the
decode on x86.

For fragment-inlined syscalls, we don't need to do anything anymore: we
deliver the signal immediately and the kernel has already set up the proper
resumption state.

For gencode syscalls, for sane post-syscall handling, we leverage i#1145's
auto-restart emulation and undo what the kernel did. This lets us go back
to dispatch for a clean sequence of events.

Updates synch and translation checks for post-syscall to also account for
syscall-restart locations.

For sysenter, this change means we now see threads at the int 0x80 restart
point whose fall-through is our vsyscall hook. To avoid interpreting our
own hook we map that PC to our displaced code, and mapping back in
dr_fragment_app_pc().

Fixes a race between going native and unhooking the vsyscall (to avoid
takeover problems) by redirecting threads at vsyscall or the int 0x80
restart to our own gencode.

Adds a test of a client making a blocking auto-restart syscall to ensure it
is not terminated with EINTR. This was not simple to arrange in a non-racy
manner and required adding a new feature of immediately delivering a signal
that arrives in DR or client code where the receiving thread is at a safe
point (because if we delay, the test cannot ensure the signal was received).

Fixes #2659

Changes the strategy for handling interrupted syscalls to use SA_RESTART,
which adds easy handling of interruptions of auto-restart syscalls in
native code, such as during attach or in client C code.

This also simplifies auto-restart of app syscalls, as we now have foolproof
identification of auto-restart situations (before our syscall-number-based
check was inaccurate) and the kernel has restored the clobbered register
value for us.  That eliminates the TLS store we were doing on ARM and the
decode on x86.

For fragment-inlined syscalls, we don't need to do anything anymore: we
deliver the signal immediately and the kernel has already set up the proper
resumption state.

For gencode syscalls, for sane post-syscall handling, we leverage i#1145's
auto-restart emulation and undo what the kernel did.  This lets us go back
to dispatch for a clean sequence of events.

Updates synch and translation checks for post-syscall to also account for
syscall-restart locations.

For sysenter, this change means we now see threads at the int 0x80 restart
point whose fall-through is our vsyscall hook.  To avoid interpreting our
own hook we map that PC to our displaced code, and mapping back in
dr_fragment_app_pc().

Fixes a race between going native and unhooking the vsyscall (to avoid
takeover problems) by redirecting threads at vsyscall or the int 0x80
restart to our own gencode.

Adds a test of a client making a blocking auto-restart syscall to ensure it
is not terminated with EINTR.  This was not simple to arrange in a non-racy
manner and required adding a new feature of immediately delivering a signal
that arrives in DR or client code where the receiving thread is at a safe
point (because if we delay, the test cannot ensure the signal was received).

Fixes #2659
@derekbruening
Copy link
Contributor Author

appveyor is api.detach #2246

@derekbruening derekbruening merged commit 35a6c54 into master Oct 6, 2017
@derekbruening derekbruening deleted the i1216-syscall-restart branch October 6, 2017 06:34
fhahn pushed a commit that referenced this pull request Dec 4, 2017
Changes the strategy for handling interrupted syscalls to use SA_RESTART,
which adds easy handling of interruptions of auto-restart syscalls in
native code, such as during attach or in client C code.

This also simplifies auto-restart of app syscalls, as we now have foolproof
identification of auto-restart situations (before our syscall-number-based
check was inaccurate) and the kernel has restored the clobbered register
value for us.  That eliminates the TLS store we were doing on ARM and the
decode on x86.

For fragment-inlined syscalls, we don't need to do anything anymore: we
deliver the signal immediately and the kernel has already set up the proper
resumption state.

For gencode syscalls, for sane post-syscall handling, we leverage i#1145's
auto-restart emulation and undo what the kernel did.  This lets us go back
to dispatch for a clean sequence of events.

Updates synch and translation checks for post-syscall to also account for
syscall-restart locations.

For sysenter, this change means we now see threads at the int 0x80 restart
point whose fall-through is our vsyscall hook.  To avoid interpreting our
own hook we map that PC to our displaced code, and mapping back in
dr_fragment_app_pc().

Fixes a race between going native and unhooking the vsyscall (to avoid
takeover problems) by redirecting threads at vsyscall or the int 0x80
restart to our own gencode.

Adds a test of a client making a blocking auto-restart syscall to ensure it
is not terminated with EINTR.  This was not simple to arrange in a non-racy
manner and required adding a new feature of immediately delivering a signal
that arrives in DR or client code where the receiving thread is at a safe
point (because if we delay, the test cannot ensure the signal was received).

Fixes #2659
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant