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

Allow usage of SIGILL for signal trampolines #963

Merged
merged 1 commit into from
Apr 21, 2021
Merged

Conversation

mxz297
Copy link
Member

@mxz297 mxz297 commented Feb 1, 2021

Add a new environment variable DYNINST_SIGNAL_TRAMPOLINE_SIGILL to control whether we use SIGILL as the signal for trampolines.

If DYNINST_SIGNAL_TRAMPOLINE_SIGILL is set, we use SIGILL as signal trampolines and the mutator will generate illegal instructions in the mutatee.

In the case of binary rewriting, DYNINST_SIGNAL_TRAMPOLINE_SIGILL should be consistently set or unset when rewriting the binary and running the rewritten binaries.

However, SIGTRAP is always intercepted by GDB, causing it is
almost impossible to debug through signal trampolines.

In this commit, we add a new environment variable DYNINST_SIGNAL_TRAMPOLINE_SIGILL
to control whether we use SIGILL as the signal for trampolines.

If DYNINST_SIGNAL_TRAMPOLINE_SIGILL is set, we use SIGILL as signal trampolines
and the mutator will generate illegal instructions in the mutatee.

In the case of binary rewriting, DYNINST_SIGNAL_TRAMPOLINE_SIGILL should be
consistently set or unset when rewriting the binary and running the rewritten binaries.
@mxz297
Copy link
Member Author

mxz297 commented Feb 1, 2021

@hainest I don't have a good way to test this. I remembered that you had a use case where you have to debug through trap trampolines. Can you check whether PR fixes your problem?

@hainest
Copy link
Contributor

hainest commented Feb 2, 2021

I do. I will test it out when I'm done testing some PRs. Thanks!

@hainest
Copy link
Contributor

hainest commented Feb 5, 2021

https://bottle.cs.wisc.edu/search?dyninst_branch=PR963

Turning on DYNINST_SIGNAL_TRAMPOLINE_SIGILL, causes regressions- mostly in proccontrol tests. Note that I only used the new flag on cayenne.

@mxz297
Copy link
Member Author

mxz297 commented Feb 5, 2021

@hainest It is expected that turning on DYNINST_SIGNAL_TRAMPOLINE_SIGILL will cause proccontrol to behave differently. Proccontrol will basically see SIGILL as fatal errors and terminate the mutatee.

The purpose of DYNINST_SIGNAL_TRAMPOLINE_SIGILL is to enable debugging rewritten binaries. For statically rewritten binaries, proccontrol is out of the scope. Proccontrol is only active during dynamic instrumentation. In addition, it is even more difficult to debug dynamic instrumentation as the mutator uses ptrace to attach to the mutatee, which prevents GDB to attach to the mutatee (a process can only be attached with ptrace by one process).

So, for this PR, I think the criteria is to see what happens with the rewriter tests under GDB and whether it would help with debugging.

@mxz297
Copy link
Member Author

mxz297 commented Feb 5, 2021

Of course, this PR also needs to be tested when not turning on DYNINST_SIGNAL_TRAMPOLINE_SIGILL.

@hainest
Copy link
Contributor

hainest commented Feb 10, 2021

It is expected that turning on DYNINST_SIGNAL_TRAMPOLINE_SIGILL will cause proccontrol to behave differently. Proccontrol will basically see SIGILL as fatal errors and terminate the mutatee.

That makes sense. Can we add this to the documentation for this flag?

Of course, this PR also needs to be tested when not turning on DYNINST_SIGNAL_TRAMPOLINE_SIGILL.

It was. Only the run on cayenne was done with the flag turned on. Running without the flag shows no regressions.

@hainest hainest self-assigned this Feb 10, 2021
@hainest hainest added this to the Release 11.0 milestone Feb 10, 2021
@hainest hainest changed the title Use SIGILL for signal trampolines Allow usage of SIGILL for signal trampolines Apr 21, 2021
@hainest hainest merged commit 7ce24bf into master Apr 21, 2021
@hainest hainest deleted the sigtrampoline branch April 21, 2021 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants