[PATCH] D60787: [scudo][standalone] Introduce the Secondary allocator

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad marked 3 inline comments as done.
cryptoad added inline comments.


================
Comment at: lib/scudo/standalone/secondary.cc:19
+// (greater than a page) alignments.
+void *MapAllocator::allocate(uptr Size, uptr AlignmentHint, uptr *BlockEnd) {
+  DCHECK_GT(Size, AlignmentHint);
----------------
morehouse wrote:
> Would it be simpler to do the extra allocation and alignment here instead?
I went back and forth on how to deal with this and I have to say it's been annoying.
Technically the secondary doesn't (shouldn't?) know about the header size of the combined.
For N bytes aligned on A, the allocation done by the secondary includes a largeblock header, but also the block header. While this could be solved by passing the extra header bytes to the secondary, it would make it depart significantly from the primary: the primary returns a block that straddles the allocation boundary and doesn't care about anything else.
By building it that way, we have a very similar approach between Primary & Secondary: the combined rounds up the size and request the alloc. The real problem (it's still debatable if it's an actual problem) would be on 32-bit when requesting secondary allocations with large alignments. The unmapping becomes necessary due to the sparsity of the address space. For 64-bit, this is a non-issue.\
The resulting function might appear hackish in the end, but it ends up fitting decently in the combined.


================
Comment at: lib/scudo/standalone/secondary.cc:37
+    // For alignments greater than or equal to a page, the user pointer ends
+    // up on a page boundary, and our headers will live in the preceding page.
+    CommitBase = roundUpTo(MapBase + PageSize + 1, AlignmentHint) - PageSize;
----------------
morehouse wrote:
> What does "user pointer" mean?
> 
> For "users" of this function the comment seems false. `CommitBase` will always be page-aligned, but at the end of this function we return `CommitBase + HeaderSize`, which may not be page-aligned.
Here user pointer means what is returned to the user (I guess as opposition to the "combined" pointer which is the one we return here to the combined).
The combined pointer will be CommitBase+HeaderSize, which will be rounded up to the next alignment boundary.
Since the alignment is greater than or equal to a page, this means that the user pointer will be (at least) page aligned.


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60787/new/

https://reviews.llvm.org/D60787