i#1921 native sig: Add handling for signals for temp-native threads #4603
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a new execute_native_handler() feature where DR delivers a signal to a currently-native thread. For a native thread, a signal frame is set up on the appropriate stack, blocked signals are set natively rather than for emulation, and control is directly sent to the application's handler. Adds handling of asynchronous signals by not recording them for delivery later but delivering immediately. Adds a test that uses the client interface DR_EMIT_GO_NATIVE to send a thread native. The app then generates a synchronous and an asynchronous signal. Initial attempts are made to also handle a pure-native thread with no dcontex, such as late in detach, but that portion is not fully finished and will be completed separately. Handling deault-action signals is also not fully tested and likely still has some issues to be worked out. Fixes several small bugs in the go-native path hit by the test: + Moves dynamo_thread_not_under_dynamo() to dispatch_enter_native() to avoid problems unlocking bb_building_lock on the path back through dispatch. + Relaxes a TRY_EXCEPT assert to support a currently-native thread with no dcontext in TLS. + Suppresses a curiosity from a still-temporarily-native thread at exit. Issue: #1921
|
Huh, works locally. Looks like compiler complaining about a decl ordering issue or sthg, plus OSX which has a different struct. Shouldn't affect the review -- will tweak to get green later. |
abhinav92003
approved these changes
Dec 11, 2020
derekbruening
added a commit
that referenced
this pull request
Dec 23, 2020
When a signal arrives in an completely unmanaged thread with no dcontext, typically because DR is detaching, we now deliver the signal if the application has a handler for it. This requires adding support for no dcontext to several parts of the frame setup code even beyond what was added in PR #4603 for temporarily-native threads. We have to save the app's handler when we detach a thread so we know where to send a native signal. Full support is complex when we're cleaning up and have no dynamic storage, so we use a single global handler per signal. We detect whether multiple handlers are in operation in this single DR instance (quite rare: only custom non-pthread clones have this behavior) and in that case we abort like before on a native signal. Adds ATOMIC_READ_1BYTE() to complement the existing atomic operations for a cleaner read of the new multi-handler flag. Delivering the frame often overlaps with DR's frame and even DR's stack usage while delivering, if the app has no sigaltstack. We add logic to detect this overlap and avoid clobbering the stack memory. Alarm signals are still dropped, since they can arrive mid-thread-init when it is even harder to deliver. Adds a new test api.detach_signal which creates 10 threads who all sit in a loop sending 4 different alternating signals (SIGSEGV, SIGBUS, SIGURG, SIGALRM) while the main thread attaches and then detaches. When run in debug build, many many signals arrive in post-detach threads, since detach takes a while to do debug cleanup, exercising the new code. Adds a new RSTAT for native signals so we can identify when this happens in release build. Exports the stat to the API and uses it to check that at least some signals were delivered natively in the new test. Removes the fatal error on a signal arriving with no dcontext. But, non-ignore default signal actions when no handler is present are still not fully handled, along with multi-sighand-processes as mentioned, and the fatal error remains in those cases. For default actions, since the process is going to terminate anyway, the only shortcoming of this is whether a core is generated and whether the proper process exit code is raised. Issue: #1921
derekbruening
added a commit
that referenced
this pull request
Dec 23, 2020
When a signal arrives in an completely unmanaged thread with no dcontext, typically because DR is detaching, we now deliver the signal if the application has a handler for it. This requires adding support for no dcontext to several parts of the frame setup code even beyond what was added in PR #4603 for temporarily-native threads. We have to save the app's handler when we detach a thread so we know where to send a native signal. Full support is complex when we're cleaning up and have no dynamic storage, so we use a single global handler per signal. We detect whether multiple handlers are in operation in this single DR instance (quite rare: only custom non-pthread clones have this behavior) and in that case we abort like before on a native signal. Adds ATOMIC_READ_1BYTE() to complement the existing atomic operations for a cleaner read of the new multi-handler flag. Delivering the frame often overlaps with DR's frame and even DR's stack usage while delivering, if the app has no sigaltstack. We add logic to detect this overlap and avoid clobbering the stack memory. Alarm signals are still dropped, since they can arrive mid-thread-init when it is even harder to deliver. Adds a new test api.detach_signal which creates 10 threads who all sit in a loop sending 4 different alternating signals (SIGSEGV, SIGBUS, SIGURG, SIGALRM) while the main thread attaches and then detaches. When run in debug build, many many signals arrive in post-detach threads, since detach takes a while to do debug cleanup, exercising the new code. Adds a new RSTAT for native signals so we can identify when this happens in release build. Exports the stat to the API and uses it to check that at least some signals were delivered natively in the new test. Removes the fatal error on a signal arriving with no dcontext. But, non-ignore default signal actions when no handler is present are still not fully handled, along with multi-sighand-processes as mentioned, and the fatal error remains in those cases. For default actions, since the process is going to terminate anyway, the only shortcoming of this is whether a core is generated and whether the proper process exit code is raised. Issue: #1921
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a new execute_native_handler() feature where DR delivers a signal
to a currently-native thread. For a native thread, a signal frame is
set up on the appropriate stack, blocked signals are set natively
rather than for emulation, and control is directly sent to the
application's handler.
Adds handling of asynchronous signals by not recording them for
delivery later but delivering immediately.
Adds a test that uses the client interface DR_EMIT_GO_NATIVE to send a
thread native. The app then generates a synchronous and an
asynchronous signal.
Initial attempts are made to also handle a pure-native thread with no
dcontex, such as late in detach, but that portion is not fully
finished and will be completed separately. Handling deault-action
signals is also not fully tested and likely still has some issues to
be worked out.
Fixes several small bugs in the go-native path hit by the test:
avoid problems unlocking bb_building_lock on the path back through
dispatch.
with no dcontext in TLS.
Issue: #1921