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#5233: Fix arm-vs-thumb signal transitions #5242

Merged
merged 1 commit into from
Dec 6, 2021

Conversation

derekbruening
Copy link
Contributor

Removes a too-early-and-thus-incorrect call to set_pc_mode_in_cpsr()
in execute_handler_from_cache()
(transfer_from_sig_handler_to_fcache_return() does this for us at the
right time).

Removes an incorrect call to dr_set_isa_mode from the cpsr in
transfer_from_sig_handler_to_fcache_return(): we want to only set the
mode from the target, not the interruption point.

Works around QEMU bugs with signals 63 and 64 by using 62 instead in
the linux.signalNNNN tests. This allows adding them to the list of
tests that work under QEMU.

Augments the linux.signalNNNN tests to vary whether the main code and
the signal handler are arm or thumb, helping to catch and test signal
transition issues.

Issue: #4719, #5233
Fixes #5233

Removes a too-early-and-thus-incorrect call to set_pc_mode_in_cpsr()
in execute_handler_from_cache()
(transfer_from_sig_handler_to_fcache_return() does this for us at the
right time).

Removes an incorrect call to dr_set_isa_mode from the cpsr in
transfer_from_sig_handler_to_fcache_return(): we want to only set the
mode from the target, not the interruption point.

Works around QEMU bugs with signals 63 and 64 by using 62 instead in
the linux.signalNNNN tests.  This allows adding them to the list of
tests that work under QEMU.

Augments the linux.signalNNNN tests to vary whether the main code and
the signal handler are arm or thumb, helping to catch and test signal
transition issues.

Issue: #4719, #5145
Fixes #5145
@derekbruening
Copy link
Contributor Author

Given that the new tests hit multiple different instances of the isa mode assert and these changes fix all of those, I'm pretty confident this is good to merge even if it doesn't fix every case hit in #5233 -- so going to merge and not wait for the filer to test. Enabling the signal tests for arm is great too. (Unfortunately they still fail under QEMU for aarch64 -- did not have time to figure out why; future work for #4719.)

@derekbruening derekbruening merged commit 5424c6f into master Dec 6, 2021
@derekbruening derekbruening deleted the i5233-signal-thumb branch December 6, 2021 23:11
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.

DynamoRIO Incorrectly switch to thumb mode when encountering the MRRC instruction on armv7l
2 participants