[PATCH] D43949: [scudo] Secondary allocator overhaul to support Windows

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added inline comments.


================
Comment at: lib/scudo/scudo_allocator_secondary.h:108
     const uptr Ptr = UserBeg - Chunk::getHeaderSize();
-    ReservedAddressRange *StoredRange = getReservedAddressRange(Ptr);
-    *StoredRange = AddressRange;
+    LargeChunkHeader *H = LargeChunk::getHeader(Ptr);
+    H->StoredRange = AddressRange;
----------------
cryptoad wrote:
> alekseyshl wrote:
> > This is definitely confusing. One getHeader() expects user mem ptr, another expects chunk header ptr. Not sure what's the best way to improve it yet.
> Initially I was going to add a comment with some chunk layout, eg:
> ```+------------------+
> | Guard page(s)    |
> +------------------+
> | Unused space*    |
> +------------------+
> | LargeChunkHeader |
> +------------------+
> | PackedHeader     |
> +------------------+
> | Data             |
> +------------------+
> | Unused space**   |
> +------------------+
> | Guard page(s)    |
> +------------------+```
> The frontend deals with the {Unp,P}ackedHeader, and the Secondary being part of the Backend deals with the LargeChunkHeader.
> While I agree that the distinction can be tenuous to make, it makes sense to me that the 2 `getHeader` work on different pointers.
I would agree with Frontend/Backend separation argument, should this class not be aware of and not use "Chunk::getHeaderSize()".


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D43949