Add support for matching split mappings to segments. #641
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
It's not precise in some corner cases. It assumes a mapping includes at most one segment, but the loader may map memory for the entire binary at first, and under the right circumstances, this test can select the wrong segment. The filtering by sample offset works more reliably. Also, the strict test on segment memory size is less effective after adding support for matching split mappings to segments.
Update ProgramHeadersForMapping to match a mapping to any segment with overlapping file offsets, except if: - if the mapping starts before the page aligned offset associated with a segment. There is no known instance where the loader would load the content of a segment to include preceding pages of the file that don't include any segment content. The mapping must be associated with an earlier segment in such cases. - if the mappings includes the last page of the segment, but not the full segment, and it includes additional pages after the segment end. It's more likely that the mapping is associated with the following segment in such cases.
Codecov Report
@@ Coverage Diff @@
## master #641 +/- ##
==========================================
- Coverage 67.89% 67.88% -0.02%
==========================================
Files 40 40
Lines 7320 7311 -9
==========================================
- Hits 4970 4963 -7
+ Misses 1909 1908 -1
+ Partials 441 440 -1
Continue to review full report at Codecov.
|
aalexand
reviewed
Jul 20, 2021
Updated the PR description to better explain the problem we try to solve. |
aalexand
approved these changes
Jul 20, 2021
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.
Update the heuristic that matches mappings to segments to support split mappings.
The existing logic assumes a mapping is at least as large as the program segment. However, the loader, or some managing service, may split a program segment into multiple mappings for different reasons:
The list is not exhaustive. While https://github.com/google/perf_data_converter tries to combine associated split mappings into a single mapping, that heuristic also has limitations. Being able to match split mappings to segments increases symbolization coverage.