i#7050: Remove preempted and faulting instruction from the drmemtraces. #7058
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ansfer. When an instruction is preempted by a kernel transfer, the instruction is not retired. The trace might not have captured all the read and write records. To avoid false positive, the invariant checker should reset the expected read and write record counters. Fixes #7050
ivankyluk
changed the title
ivankyluk
changed the title
…b.com:DynamoRIO/dynamorio into i7050-remove-preempted-instructions
…move-preempted-instructions
…er mis-placed invariant error.
ivankyluk
changed the title
derekbruening
requested changes
Nov 11, 2024
ivankyluk
changed the title
…MPLETED_INSTRUCTION marker.
…:drmemtrace::OFFLINE_FILE_VERSION_RETIRED_INSTRUCTIONS_ONLY.
…ONS_ONLY to #OFFLINE_FILE_VERSION_RETIRED_INSTRUCTIONS_ONLY.
I didn't see replies: please reply when resolving so the replies show up in the email and it's clear what the resolution was (even if just "Done" that is helpful vs a silent resolution). |
There was a problem hiding this comment.
Putting the top-level comment here to re-enable the re-request-review:
Not seeing anything major, but could you resolve the 19 open comments?
I didn't see replies: please reply when resolving so the replies show up in the email and it's clear what the resolution was (even if just "Done" that is helpful vs a silent resolution). Please reply to the 19 so it's clear what the current status is.
derekbruening
approved these changes
Nov 13, 2024
This was referenced Nov 14, 2024
ivankyluk
added a commit
that referenced
this pull request
Nov 14, 2024
… rseq_aborted. (#7086) raw2trace_t::handle_rseq_abort_marker() uses different names of the last parameter. It's interrupted in raw2trace.h but it is rseq_aborted in raw2trace.cpp. Change the name from interrupted to rseq_aborted to be consistent with raw2trace.cpp. The inconsistency was introduced by PR #7058. Fixes: #7084
ivankyluk
added a commit
that referenced
this pull request
Dec 19, 2024
…RETIRED_INSTRUCTIONS_ONLY. (#7156) Add a new trace version TRACE_ENTRY_VERSION_RETIRED_INSTRUCTIONS_ONLY for #7050 and bump the TRACE_ENTRY_VERSION. Rename OFFLINE_FILE_VERSION_RETIRED_INSTRUCTIONS_ONLY to OFFLINE_FILE_VERSION_NO_OP since there were no changes in the raw trace by #7050. #7058 incorrectly changed the OFFLINE_FILE_VERSION, it should have changed TRACE_ENTRY_VERSION instead. Issue: #7050
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change the implementation to remove preempted and faulting instructions from drmemtraces. The objective of the change is to remove non-retired instructions and the corresponding memrefs from drmemtraces.
When an instruction is removed, a new marker TRACE_MARKER_TYPE_UNCOMPLETED_INSTRUCTION will be added. Its value is the encoding of the removed instruction up to the size of a pointer.
Renamed handle_kernel_interrupt_and_markers() to handle_rseq_abort_marker() since the function has been modified to handle rseq abort.
In oder to remove preempted instructions and memrefs, a new function preempted_by_kernel_event() is added to look for KERNEL EVENT marker which may be preceded by memrefs. If a KERNEL EVENT marker is found with the same PC, the instruction and any following memrefs are removed.
Add unit tests to cover instruction and memref removed caused by a KERNEL EVENT.
Update offline-legacy-int-offs.templatex, offline-burst_aarch64_sys.templatex and signal_invariants.c to account for removed instructions.
Fixes #7050