DynamoRIO / DynamoRIO/dynamorio
Verify drx state restoration for scatter/gather expansion with other instrumentation
- Dominant language
- C
- Stars
- 3.2k
- Forks
- 629
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 30
Description
`drx_restore_state_scatter_gather` performs state restoration for blocks containing the scatter/gather expansion using various state machines. As noted by this comment, https://github.com/DynamoRIO/dynamorio/blob/6d84fea04a036038db5a3af2e979e77d2cd356c0/ext/drx/drx.c#L2540 there may be issues in this due to other instrumentation added by clients. In addition to the cases described there, there could be instrumentation added by clients that use `gpr_scratch_index` and pass this check: https://github.com/DynamoRIO/dynamorio/blob/6d84fea04a036038db5a3af2e979e77d2cd356c0/ext/drx/drx.c#L3298, e.g. loads/stores that use the mcontext base are not recognised by `instr_is_reg_spill_or_restore`, and also load of the mcontext base itself is not recognised by `instr_is_reg_spill_or_restore` after PR #5000 .
This issue is to investigate those cases and make this logic more robust if needed. One of the ideas is what's mentioned in the comment there already: use emulation labels in passed down instruction list (#3801) to make it easier to recognise the scatter/gather expansion instrs.
Contributor guide
Research direction
Start in ext/drx/drx.c at drx_restore_state_scatter_gather and the referenced comment and gpr_scratch_index check. Investigate client instrumentation involving the mcontext base, including cases not recognized by instr_is_reg_spill_or_restore, and review the emulation-label idea in issue #3801 alongside the changes from PR #5000. Done means the affected cases are verified and the restoration logic is made robust if needed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- devtools
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100