i#4014 dr$sim phys: Add offline -use_physical support #5516
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
Adds support for physical addresses to offline dr$sim traces. To support simulators wanting both virtual and physical addresses, and to simplify post-processing where the virtual PC values are needed, the regular trace entries remain all virtual. A new marker type TRACE_MARKER_TYPE_PHYSICAL_ADDRESS listing the corresponding physical address is added. The mappings are assumed to not change, allowing just one marker for each newly-observed page. This is done per-thread. An explicit TRACE_MARKER_TYPE_PHYSICAL_ADDRESS_NOT_AVAILABLE marker is inserted on failure to translate, to prevent analyzers from having to infer this due to the lack of the already-sparse markers. Separately emitted pairs of virtual and physical address markers were considered, with raw2trace inserting the physical at the right place, but that presents complexities with buffer handoff and with the first buffer. Instead, the physical are inserted via memmove directly into the buffer. This does not seem to be a performance concern: the translation lookup is the bottleneck. Adds support for the new markers to the view tool. Adds a Linux x86_64 test that runs a tiny asm app and ensures a physical address marker is inserted. The test needs to run as sudo, along with its pre- and post- commands. Currently it is enabled everywhere, so a user running interactive tests will have it pause while it waits for input. This might cause issues with manually running the test suite. A number of items remain for further work: + Performance is poor: the hashtable and caching need improvement. + There is a hardcoded limit on how many markers can be added per buffer. Once this is exceeded, further markers are dropped. We should split the buffer to handle this. + We may want to add a mode that checks for mapping changes. + Missing privileges results in every physical address being 0 instead of showing the failure. We need to check the capabilities to distinguish. + Better testing that we're actually getting physical addresses for online tests. + Better offline testing with larger apps. + Basic blocks that cross a page have only the first one translated. + A file descriptor per thread is used, which will not scale well with DR's descriptor protection and might hit rlimits. Issue: #4014
|
The failure is the known flake #5514 |
abhinav92003
approved these changes
Jun 6, 2022
|
scattergather again failed: #5329 |
dolanzhao
pushed a commit
that referenced
this pull request
Jun 8, 2022
Adds support for physical addresses to offline dr$sim traces. To support simulators wanting both virtual and physical addresses, and to simplify post-processing where the virtual PC values are needed, the regular trace entries remain all virtual. A new marker type TRACE_MARKER_TYPE_PHYSICAL_ADDRESS listing the corresponding physical address is added. The mappings are assumed to not change, allowing just one marker for each newly-observed page. This is done per-thread. An explicit TRACE_MARKER_TYPE_PHYSICAL_ADDRESS_NOT_AVAILABLE marker is inserted on failure to translate, to prevent analyzers from having to infer this due to the lack of the already-sparse markers. Separately emitted pairs of virtual and physical address markers were considered, with raw2trace inserting the physical at the right place, but that presents complexities with buffer handoff and with the first buffer. Instead, the physical are inserted via memmove directly into the buffer. This does not seem to be a performance concern: the translation lookup is the bottleneck. Since the memmoves occur only on the first instance of each page, they are much rarer than all the virtual-to-physical translations. Adds support for the new markers to the view tool. Adds a Linux x86_64 test that runs a tiny asm app and ensures a physical address marker is inserted. The test needs to run as sudo, along with its pre- and post- commands. To avoid a confusing blocking password query in local runs, a new set of tests controlled by a new CMake option RUN_SUDO_TESTS is added. It is set only for automated_ci, where we assume a passwordless sudo. A number of items remain for further work: + Performance is poor: the hashtable and caching need improvement. + There is a hardcoded limit on how many markers can be added per buffer. Once this is exceeded, further markers are dropped. We should split the buffer to handle this. + We may want to add a mode that checks for mapping changes. + Missing privileges results in every physical address being 0 instead of showing the failure. We need to check the capabilities to distinguish. + Better testing that we're actually getting physical addresses for online tests. + Better offline testing with larger apps. + Basic blocks that cross a page have only the first one translated. + A file descriptor per thread is used, which will not scale well with DR's descriptor protection and might hit rlimits. + Online traces still replace all virtual addresses with physical. We should break compatibility and transition them to use these markers, with dr$sim computing the physical addresses from the markers. Issue: #4014
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.
Adds support for physical addresses to offline dr$sim traces. To
support simulators wanting both virtual and physical addresses, and to
simplify post-processing where the virtual PC values are needed, the
regular trace entries remain all virtual. A new marker type
TRACE_MARKER_TYPE_PHYSICAL_ADDRESS listing the corresponding physical
address is added. The mappings are assumed to not change, allowing
just one marker for each newly-observed page. This is done
per-thread.
An explicit TRACE_MARKER_TYPE_PHYSICAL_ADDRESS_NOT_AVAILABLE marker is
inserted on failure to translate, to prevent analyzers from having to
infer this due to the lack of the already-sparse markers.
Separately emitted pairs of virtual and physical address markers were
considered, with raw2trace inserting the physical at the right place,
but that presents complexities with buffer handoff and with the first
buffer. Instead, the physical are inserted via memmove directly into
the buffer. This does not seem to be a performance concern: the
translation lookup is the bottleneck.
Adds support for the new markers to the view tool.
Adds a Linux x86_64 test that runs a tiny asm app and ensures a
physical address marker is inserted. The test needs to run as sudo,
along with its pre- and post- commands. Currently it is enabled
everywhere, so a user running interactive tests will have it pause
while it waits for input. This might cause issues with manually
running the test suite.
A number of items remain for further work:
per buffer. Once this is exceeded, further markers are dropped.
We should split the buffer to handle this.
of showing the failure. We need to check the capabilities to distinguish.
tests.
DR's descriptor protection and might hit rlimits.
Issue: #4014