This is an archive of the discontinued LLVM Phabricator instance.

[SectionMemoryManager] Abstract out mmap, munmap, mprotect even more ; NFC
ClosedPublic

Authored by sanjoy on Oct 25 2017, 10:47 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

sanjoy created this revision.Oct 25 2017, 10:47 AM
lhames edited edge metadata.Oct 28 2017, 1:38 AM

Out of interest, what’s the use case for this? I’d have expected sys::Memory to be good enough for block-based allocation (though there’s the known issue with PaX on BSD that I need to fix), and using a non block-based underlying allocator seems like it’d risk unnecessary fragmentation.

Out of interest, what’s the use case for this? I’d have expected sys::Memory to be good enough for block-based allocation (though there’s the known issue with PaX on BSD that I need to fix), and using a non block-based underlying allocator seems like it’d risk unnecessary fragmentation.

This is just so that the mmap can come from a file or a memfd, which will make the mapping show up with a name and not as "anon" under perf etc.

asbirlea accepted this revision.Nov 8 2017, 10:39 AM

LGTM.

include/llvm/ExecutionEngine/SectionMemoryManager.h
73 ↗(On Diff #120283)

Remove "is".

This revision is now accepted and ready to land.Nov 8 2017, 10:39 AM
jlebar accepted this revision.Nov 8 2017, 1:29 PM
jlebar added inline comments.
include/llvm/ExecutionEngine/SectionMemoryManager.h
43 ↗(On Diff #120283)

reasons

56 ↗(On Diff #120283)

, in which case

lhames added a comment.Nov 8 2017, 8:10 PM

Out of interest, what’s the use case for this? I’d have expected sys::Memory to be good enough for block-based allocation (though there’s the known issue with PaX on BSD that I need to fix), and using a non block-based underlying allocator seems like it’d risk unnecessary fragmentation.

This is just so that the mmap can come from a file or a memfd, which will make the mapping show up with a name and not as "anon" under perf etc.

Ahh. Makes sense.

Apologies for the delayed reply: I've been on the road. Thanks asbirlea and jlebar for stepping in.

This revision was automatically updated to reflect the committed changes.
sanjoy marked 3 inline comments as done.