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.

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

Support partial reading of string tables #326

Merged
merged 2 commits into from
Jun 16, 2021

Conversation

mstange
Copy link
Contributor

@mstange mstange commented Jun 15, 2021

Before this PR, StringTables are read in their entirety and do not make use of the partial read ability of ReadRef. This is fine in most cases, but it is problematic for the dyld shared cache.
The dyld shared cache has a shared string table which is about 340MB big. Only a fraction of those strings are needed by each embedded library. For example, if I dump all AppKit symbols with their names from the dyld shared cache on macOS 11.3, this patch reduces the number of bytes read from 347.4 MB to 4.8 MB.

This patch makes use of the relatively new read_bytes_at_until method. But it also extends it by one parameter, upper_bound, so that the string table's size can be honored.

We currently have a hardcoded limit of 4096 bytes for the string length in the ReadCache. It's possible that this limit is not appropriate in all the cases where read_bytes_at_until is now called. But I'm not changing it in this patch.

Copy link
Contributor

@philipc philipc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Hopefully Wasm will get mmap support eventually.

src/read/coff/symbol.rs Outdated Show resolved Hide resolved
src/read/coff/section.rs Outdated Show resolved Hide resolved
src/read/util.rs Outdated Show resolved Hide resolved
src/read/read_ref.rs Outdated Show resolved Hide resolved
src/read/read_cache.rs Outdated Show resolved Hide resolved
src/read/pe/section.rs Outdated Show resolved Hide resolved
src/read/macho/load_command.rs Outdated Show resolved Hide resolved
@philipc philipc merged commit 8273ea3 into gimli-rs:master Jun 16, 2021
@mstange
Copy link
Contributor Author

mstange commented Jul 19, 2021

@philipc Could you make a release which contains this commit?

@philipc
Copy link
Contributor

philipc commented Jul 19, 2021

Can it wait a week or so? I'm currently hoping to get an alternative to #325 implemented before doing a release.

@mstange
Copy link
Contributor Author

mstange commented Jul 19, 2021

Sure, that's fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants