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#5658 rseq drreg: Put native sequence by barrier #5664

Merged
merged 2 commits into from
Sep 30, 2022

Conversation

derekbruening
Copy link
Contributor

Fixes PR #5663 which fixed drreg restoration for drbbdup but broke it for non-drbbdup. bb->instr was NULL so the preinsert was an append, and the label as the last instruction moved the drreg restore point to after the native sequence!

Here, we explicitly append to avoid confusion, and we move the native sequence to the point of the barrier instead of at the final app instr, which is where the registers are actually native.

Tested on larger apps using rseq which reliably crashed without this fix. This fix also seems to fully fix drbbdup usage with drmemtrace for #2039.

Issue: #5658, #2039

Fixes PR #5663 which fixed drreg restoration for drbbdup but broke it
for non-drbbdup.  bb->instr was NULL so the preinsert was an append,
and the label as the last instruction moved the drreg restore point to
after the native sequence!

Here, we explicitly append to avoid confusion, and we move the native
sequence to the point of the barrier instead of at the final app
instr, which is where the registers are actually native.

Tested on larger apps using rseq which reliably crashed without this
fix.  This fix also seems to fully fix drbbdup usage with drmemtrace
for #2039.

Issue: #5658, #2039
core/arch/interp.c Show resolved Hide resolved
@derekbruening derekbruening merged commit 2c3dab2 into master Sep 30, 2022
@derekbruening derekbruening deleted the i5658-fix-rseq-barrier branch September 30, 2022 14:44
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.

2 participants