The parameter name in FindAvailableMemoryRange() is left_padding.
There is a variable with this name in this function that has the same
value so just use it to avoid any confusion.
Details
- Reviewers
kubamracek yln tejohnson
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I am okay with this change, but I don't understand this code or why this change is NFC. Can we get @tejohnson to take a look? (D83247)
granularity is not necessarily equal to left_padding. Also left_padding already goes into space_size, which is also passed to FindAvailableMemoryRange().
const uptr granularity = GetMmapGranularity(); ... const uptr left_padding = Max<uptr>(granularity, 1ULL << min_shadow_base_alignment); uptr space_size = shadow_size_bytes + left_padding; ... uptr shadow_start = FindAvailableMemoryRange(space_size, alignment, granularity, ...);
It's NFC because min_shadow_base_alignment is always 0 for mac. We made this change to make the interface to, and the code within, the newly refactored MapDynamicShadow versions consistent. BTW the same issue exists in the win version, can you make the same change there? The linux version doesn't call FindAvailableMemoryRange(), so no issue there.
Also left_padding already goes into space_size, which is also passed to FindAvailableMemoryRange().
const uptr granularity = GetMmapGranularity(); ... const uptr left_padding = Max<uptr>(granularity, 1ULL << min_shadow_base_alignment); uptr space_size = shadow_size_bytes + left_padding; ... uptr shadow_start = FindAvailableMemoryRange(space_size, alignment, granularity, ...);
@tejohnson Side comment about the refactor. The comments on MapDynamicShadow(...) say that is function is actually supposed to mmap the found memory as NO_ACCESS. The current implementation doesn't do this. It just finds a space but doesn't mmap it. Is this a problem?
Thanks for pointing that out. That comment came from the linux version, where it is true, but it isn't true on mac or win. I will adjust the comment to say "on linux mapped no access".