This is an archive of the discontinued LLVM Phabricator instance.

[dsymutil] Force mmap'ing of binaries
ClosedPublic

Authored by JDevlieghere on Jun 20 2018, 3:28 PM.

Details

Summary

After the recent refactoring that introduced parallel handling of different object, the binary holder became unique per object file. This defeats its optimization of caching archives, leading to an archive being opened for every binary it contains. This is obviously unfortunate and will need to be refactored soon.

Luckily in practice, the impact of this is limited as most files are mmap'ed instead of memcopy'd. There's a caveat however: when the memory buffer requires a null terminator and it's a multiple of the page size, we allocate instead of mmap'ing. If this happens for a static archive, we end up with N copies of it in memory, where N is the number of objects in the archive, leading to exuberant memory usage. This provided a stopgap solution to ensure that all the files it loads are mmap in memory by removing the requirement for a terminating null byte.

Diff Detail

Repository
rL LLVM

Event Timeline

JDevlieghere created this revision.Jun 20 2018, 3:28 PM
friss added a comment.Jun 20 2018, 3:33 PM

If you have a local test case with an archive that triggers the malloc behavior, can you run an Asanified dsymutil on it with your patch applied? I'd like to make sure that nothing in dsymutil is going to access one past the end of the file.

If you have a local test case with an archive that triggers the malloc behavior, can you run an Asanified dsymutil on it with your patch applied? I'd like to make sure that nothing in dsymutil is going to access one past the end of the file.

Done, no invalid memory access detected. This sounds right to me, as the MemoryBuffer sets the size of the buffer and libObject does a relatively good job at bounds checking.

friss accepted this revision.Jun 21 2018, 8:21 AM

LGTM Thanks!

This revision is now accepted and ready to land.Jun 21 2018, 8:21 AM
This revision was automatically updated to reflect the committed changes.