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.

Sign up for GitHub

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

Jump to bottom

[drreg] GPR value not preserved on fault when there's intervening app read #4939

Closed
abhinav92003 opened this issue Jun 7, 2021 · 0 comments · Fixed by #4944
Closed

[drreg] GPR value not preserved on fault when there's intervening app read #4939

abhinav92003 opened this issue Jun 7, 2021 · 0 comments · Fixed by #4944
Labels
Component-Extension OpSys-AArch64 OpSys-ARM OpSys-x86

Comments

@abhinav92003
Copy link
Contributor

(Copying from #3823) The following sequences causes failure to restore gpr value on fault:

reserve reg r1, spill app-val to spill slot s1

tool-write to r1

spill tool-val of r1 to spill slot s2
restore app-val of r1 from s1 // drreg_event_restore_state loses its book-keeping of r1's spill slot here
app instr that reads r1
restore tool-val of r1

fault

drreg_event_restore_state loses its book-keeping when it sees the first restore of r1:

dynamorio/ext/drreg/drreg.c

Line 1897 in e8fc651

spilled_to[GPR_IDX(reg)] = MAX_SPILLS;
.

This is demonstrated by test_asm_faultH on PR #4932.
abhinav92003 added a commit that referenced this issue Jun 9, 2021
@abhinav92003
i#4939,4933: Make drreg state restoration more robust.
0779ea4 
Revamps the drreg state restoration logic in drreg_event_restore_state to correctly
handle various corner cases like multiple restores (i#4939), aflags in reg (i#4933)
and multi-phase use (i#3823).

Makes the reconstructed ilist available to drreg_event_restore_state. This is needed
for determining whether some instr is app or tool.

The new logic looks at the complete ilist to track the movement of the app value of
gpr/aflags across spills and restores.
We do not forget a spill slot on seeing one restore (which caused i#4939), but
remember that the app value can still be found in the spill slot, in addition to
the gpr itself. Instead, a spill slot is forgetten when it is used to spill some
other reg.
We also track whether gprs/aflags contain their native app value or not. This helps
in handling cases of overlapping or nested spill regions arising out of multi-phase
use of drreg. Tool writes (except restores) to gprs/aflags clobbers the native value.
App writes to gprs/aflags installs a native value and clobbers the value in the
reg's spill slot.
We also track movement of aflags to registers more robustly (to fix i#4933).

Adds tests for the i#4939 and i#4933 cases. Test for i#3823 will be added in a
separate PR.

Issue: #4933, #4939, #3823
Fixes: #4933
Fixes: #4939
@abhinav92003 abhinav92003 mentioned this issue Jun 9, 2021
Merged
abhinav92003 added a commit that referenced this issue Jun 14, 2021
@abhinav92003
i#3801,#3823: Make drreg state restoration more robust (#4944)
f62441b 
Revamps the drreg state restoration logic in drreg_event_restore_state to
correctly handle various corner cases like multiple restores (i#4939), multiple
spills, aflags in reg (i#4933), multi-phase use (i#3823) leading to nested
or otherwise overlapping spill regions, and app instrs that look like drreg
spill/restore instrs.

Makes the reconstructed ilist available to drreg_event_restore_state (i#3801).
The additional metadata that is used by the new logic is only whether an instr
is an app or meta instr. For cases when the reconstructed ilist is not
available, we continue to use the old best-effort logic. Highlighted its caveats
in comments.

The new logic looks at the complete ilist to track the movement of the app value
of gprs/aflags across spills and restores. The native value can be in the
gpr/aflags itself or one or more spill slots. We do not forget a spill slot on
seeing one restore (which caused i#4939), but remember that the app value can
still be found in the spill slot, in addition to the gpr itself. Instead, a
spill slot is forgotten when it is used to spill some other reg or if there's an
app instr that writes to the spilled gpr/aflags.

We also track whether gprs/aflags contain their native app value or not. This
helps in handling cases of overlapping or nested spill regions arising out of
multi-phase use of drreg. Tool writes (except restores) to gprs/aflags clobber
the native value. App writes to gprs/aflags install a native value and
invalidate the value in the gpr's/aflags' spill slot. Restores from spill slots
also install a native value.

Adds tests for multi-phase gpr reservation cases (nested/overlapping spill
regions), multiple spills for gpr and multiple restores for gpr. Tests for these
scenarios for aflags will be added when multi-phase drreg support is added for
aflags.

Tracks movement of aflags to registers more robustly to fix i#4933. Also adds a
test for this case.

Avoids confusing app instrs that write using the stolen reg on AArch64 with
drreg spill or restore instrs. Also adds a test that verifies this fix.

Adds documentation about faulting fragment's reconstructed ilist being available
in restore state event callbacks.

Issue: #4933, #4939, #3823, #3801
Fixes: #4933
Fixes: #4939
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component-Extension OpSys-AArch64 OpSys-ARM OpSys-x86
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

i#3801,#3823: Make drreg state restoration more robust DynamoRIO/dynamorio
1 participant
@abhinav92003