[PATCH] D63146: [scudo][standalone] Unmap memory in tests

Kostya Kortchinsky via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 11 10:32:09 PDT 2019


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


================
Comment at: lib/scudo/standalone/primary32.h:93
+      if (PossibleRegions[I])
+        unmap(reinterpret_cast<void *>(I * RegionSize), RegionSize);
+    PossibleRegions.unmapTestOnly();
----------------
morehouse wrote:
> This looks incorrect.  Is `I * RegionSize` a valid address?
It is indeed a valid address. For the 32-bit primary, the address space is split in potential regions of `RegionSize` bytes, that we index via a one or two level bytemap.


================
Comment at: lib/scudo/standalone/tests/primary_test.cc:25
   const scudo::uptr NumberOfAllocations = 32U;
-  std::unique_ptr<Primary> Allocator(new Primary);
+  auto Deleter = [](Primary *P) { P->unmapTestOnly(); };
+  std::unique_ptr<Primary, decltype(Deleter)> Allocator(new Primary, Deleter);
----------------
morehouse wrote:
> Should this also `delete P`?  Same for below.
Good point thanks, I somehow thought it would still end up deleting the original pointer with a custom deleter. I will correct those.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D63146





More information about the llvm-commits mailing list