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

Fix uninitialized var in drmemtrace offline instru #4506

Merged
merged 1 commit into from
Oct 31, 2020
Merged

Fix uninitialized var in drmemtrace offline instru #4506

merged 1 commit into from
Oct 31, 2020

Conversation

derekbruening
Copy link
Contributor

While investigating #4460 I found that reg_ptr_used in
insert_save_addr is uninitialized locally and insert_obtain_addr only
writes it when true, leaving it uninitialized for the false case: thus
we may re-instate the buffer pointer in cases where we don't need to.
I believe this is only a performance issue.

Issue: #4460

@derekbruening
Fix uninitialized var in drmemtrace offline instru
9c560ff 
While investigating #4460 I found that reg_ptr_used in
insert_save_addr is uninitialized locally and insert_obtain_addr only
writes it when true, leaving it uninitialized for the false case: thus
we may re-instate the buffer pointer in cases where we don't need to.
I believe this is only a performance issue.

Issue: #4460
@derekbruening derekbruening merged commit 420adb0 into master Oct 31, 2020
@derekbruening derekbruening deleted the i4460-uninit-var branch October 31, 2020 01:45
derekbruening added a commit that referenced this pull request Nov 2, 2020
@derekbruening
Revert "Fix uninitialized var in drmemtrace offline instru (#4506)"
fd7e255 
This reverts commit 420adb0
because it causes crashes in large apps linking drcachesim statically.
We believe that PR #4506 itself is actually correct and that it is
simply revealing an underlying problem, and once we find that problem
the plan is to re-instate it.

Issue: #4460
@derekbruening derekbruening mentioned this pull request Nov 2, 2020
Merged
derekbruening added a commit that referenced this pull request Nov 2, 2020
@derekbruening
Revert "Fix uninitialized var in drmemtrace offline instru (#4506)" (#…
33db119 
…4512)

This reverts commit 420adb0
because it causes crashes in large apps linking drcachesim statically.
We believe that PR #4506 itself is actually correct and that it is
simply revealing an underlying problem, and once we find that problem
the plan is to re-instate it.

Issue: #4460
@derekbruening
Copy link
Contributor Author

I was wrong here: drutil_insert_get_mem_addr_ex() sets *scratch_used in all cases.

gregcawthorne pushed a commit that referenced this pull request Nov 28, 2020
@derekbruening @gregcawthorne
Revert "Fix uninitialized var in drmemtrace offline instru (#4506)" (#…
04cad03 
…4512)

This reverts commit 420adb0
because it causes crashes in large apps linking drcachesim statically.
We believe that PR #4506 itself is actually correct and that it is
simply revealing an underlying problem, and once we find that problem
the plan is to re-instate it.

Issue: #4460
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abhinav92003 abhinav92003 abhinav92003 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@derekbruening @abhinav92003