-
Notifications
You must be signed in to change notification settings - Fork 5
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
Improve DsoHdr::erase_range #370
Conversation
a9d1ecd
to
a704bfb
Compare
`pid_fork` directly copies mappings from parent to child pid, if child pid already has mappoings this can lead to overlapping mappings. Currently this cannot happens, because child mappings are cleared before `pid_fork`. This check adds a safegard by returning early if child pid has mappings.
* Add check when re-inserting truncated last mapping * Return early in the case new mapping truncates end of a single mapping to avoid using range.first ierator that is invalidated by map::extract (the node pointed by first is extracted and then re-inserted hence the iterator should remain valid, but better be careful)
a704bfb
to
c6d5a4f
Compare
Allow new mapping to truncate existing mappings if previous mapping is a file and new mapping is anonymous because a LOAD segment can have filesize < memsize as a size optimization when the end of segment is all zeros (eg. bss). In that case elf loader / dlopen will map the whole file + some extra space to cover the difference between memsize and file size and then map an anonymous mapping over the part not in the file.
c6d5a4f
to
284e23b
Compare
Benchmark results for collatzParameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. See unchanged results
|
Benchmark results for BadBoggleSolver_runParameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. See unchanged results
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
We had to do a dedicated session to review some of the changes here.
The erase_range logics are not easy to understand and require a careful understanding of linker behaviours.
pid_fork
directly copies mappings from parent to child pid,if child pid already has mappoings this can lead to overlapping mappings.
Currently this cannot happens, because child mappings are cleared before
pid_fork
. This check adds a safegard by returning early if child pidhas mappings.
to avoid using range.first ierator that is invalidated by map::extract
(the node pointed by first is extracted and then re-inserted hence
the iterator should remain valid, but better be careful)
Allow new mapping to truncate existing mappings if previous mapping is
a file and new mapping is anonymous because a LOAD segment can have
filesize < memsize as a size optimization when the end of segment is all
zeros (eg. bss). In that case elf loader / dlopen will map the whole
file + some extra space to cover the difference between memsize and file
size and then map an anonymous mapping over the part not in the file.