runtime/tracer: passthrough unexpected signals by default #488
Conversation
This file contains hidden or 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
kkysen
force-pushed
the
kkysen/post-condition-negative-control
branch
from
063b46d to
bb6f17c
Compare
kkysen
force-pushed
the
kkysen/tracer-passthrough-unexpected-signals
branch
from
5c8ecfc to
6b5aa0e
Compare
kkysen
force-pushed
the
kkysen/post-condition-negative-control
branch
from
08a547e to
c1a26d3
Compare
kkysen
force-pushed
the
kkysen/tracer-passthrough-unexpected-signals
branch
from
6b5aa0e to
e346f66
Compare
kkysen
force-pushed
the
kkysen/post-condition-negative-control
branch
from
c1a26d3 to
80f3d9c
Compare
kkysen
force-pushed
the
kkysen/tracer-passthrough-unexpected-signals
branch
from
e346f66 to
dd7993a
Compare
kkysen
force-pushed
the
kkysen/post-condition-negative-control
branch
from
80f3d9c to
8db716a
Compare
kkysen
force-pushed
the
kkysen/tracer-passthrough-unexpected-signals
branch
from
dd7993a to
f9e812b
Compare
fw-immunant
requested changes
Jan 28, 2025
Comment on lines
+132
to
+160
| if (exit_status != test_info->exit_code || | ||
| // Sometimes the signal doesn't show up with `WIFSIGNALED`, so we check the exit status as well. | ||
| (exit_status > 128 && exit_status - 128 == test_info->signal)) { |
There was a problem hiding this comment.
This seems wrong. The first half checks if the exit status is not what's expected (which is a problem). We or this against the exit status indicating we received the expected signal (which is not a problem).
| @@ -817,7 +836,7 @@ bool track_memory_map(pid_t pid, int *exit_status_out, enum trace_mode mode) { | |||
| case WAIT_SIGNALED: { | |||
| fprintf(stderr, "process received fatal signal (syscall entry)\n"); | |||
| enum control_flow cf = handle_process_exit(&maps, waited_pid); | |||
| return false; | |||
| return true; | |||
There was a problem hiding this comment.
Making this return true without setting *exit_status_out violates the doc comment for track_memory_map. This needs changes in wait_for_next_trap, which currently only writes through its copy of the exit_status_out pointer when returning WAIT_EXITED
There was a problem hiding this comment.
I changed things to always return the wait status, which can then be inspected more thoroughly with all of the W* macros.
kkysen
force-pushed
the
kkysen/post-condition-negative-control
branch
from
8db716a to
8fd3e24
Compare
kkysen
force-pushed
the
kkysen/tracer-passthrough-unexpected-signals
branch
5 times, most recently
from
7adfbaf to
44b5f63
Compare
Base automatically changed from
kkysen/post-condition-negative-control
to
main
February 19, 2025 01:48
kkysen
force-pushed
the
kkysen/tracer-passthrough-unexpected-signals
branch
from
44b5f63 to
adfaf96
Compare
kkysen
force-pushed
the
kkysen/tracer-passthrough-unexpected-signals
branch
3 times, most recently
from
ca1f7be to
f177818
Compare
This allows signals like `SIGABRT` to work in tests, so now `assert`s work correctly, like in the `post_condition` test. Since post condition functions can be arbitrary and the user can handle things any way, we don't want to impose an unnecessary restriction on what fatal signals they may raise.
When the test child process dies from a fatal signal, it seems to sometimes trigger `WIFSIGNALED`, but sometimes just shows up in the `WEXITSTATUS` as `128 + signal`. Thus, this handles both cases.
Now `STRINGIFY(EXIT_SUCCESS)` is "EXIT_SUCCESS" not "0" for example. The other expanding version is now called `EXPAND_AND_STRINGIFY`.
kkysen
force-pushed
the
kkysen/tracer-passthrough-unexpected-signals
branch
from
f177818 to
84f8c8a
Compare
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.
This allows signals like
SIGABRTto work in tests, so nowasserts work correctly, like in thepost_conditiontest. Since post condition functions can be arbitrary and the user can handle things any way, we don't want to impose an unnecessary restriction on what fatal signals they may raise.