[PATCH] D39244: [sanitizer] Random shuffling of chunks for the 32-bit Primary Allocator

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_allocator_primary32.h:341
+    const uptr kShuffleArraySize = 48;
+    uptr shuffle_array[kShuffleArraySize];
+    uptr j = 0;
----------------
cryptoad wrote:
> alekseyshl wrote:
> > How about using the newly allocated region for the array? You can fill and shuffle all the pointers at once, create the batches and then zero reg out. Still two passes, but less stack and chunks will be farther apart. The code will be simpler too and I have a feeling that we can even avoid double copying for !kShuffleArraySize.
> This is tricky because if kRandomShuffleChunks is not used with kUseSeparateSizeClassForBatch , then the batches can also use the region itself.
> Meaning while we iterate through the chunks, we could create a batch that would overlap and corrupt the array we are iterating on.
> We can't really assume any part of the region is "safe" to store the array either due to the randomness, so using the end of the region or the beginning doesn't change that.
> If kUseSeparateSizeClassForBatch  is used, then we are golden and that can work.
> Currently Scudo uses both, but I feel they should be treated distinctly nonetheless.
Rectification: even with kUseSeparateSizeClassforBatch, there is the special case of kBatchClassID for which the batches are stored in the same region.


https://reviews.llvm.org/D39244