[PATCH] D86800: [scudo][standalone] Enable secondary cache release on Fuchsia

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad created this revision.
cryptoad added reviewers: hctim, pcc, eugenis.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.
cryptoad requested review of this revision.

I had left this as a TODO, but it turns out it wasn't complicated.
By specifying `MAP_RESIZABLE`, it allows us to keep the VMO which we
can then use for release purposes.
`releasePagesToOS` also had to be called the "proper" way, as Fuchsia
requires the `Offset` field to be correct. This has no impact on
non-Fuchsia platforms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86800

Files:
  compiler-rt/lib/scudo/standalone/secondary.h
  compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp


Index: compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
===================================================================
--- compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
+++ compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
@@ -56,18 +56,12 @@
 
 TEST(ScudoSecondaryTest, SecondaryBasic) {
   testSecondaryBasic<scudo::MapAllocator<scudo::MapAllocatorNoCache>>();
-#if !SCUDO_FUCHSIA
   testSecondaryBasic<scudo::MapAllocator<scudo::MapAllocatorCache<>>>();
   testSecondaryBasic<
       scudo::MapAllocator<scudo::MapAllocatorCache<128U, 64U, 1UL << 20>>>();
-#endif
 }
 
-#if SCUDO_FUCHSIA
-using LargeAllocator = scudo::MapAllocator<scudo::MapAllocatorNoCache>;
-#else
 using LargeAllocator = scudo::MapAllocator<scudo::MapAllocatorCache<>>;
-#endif
 
 // This exercises a variety of combinations of size and alignment for the
 // MapAllocator. The size computation done here mimic the ones done by the
Index: compiler-rt/lib/scudo/standalone/secondary.h
===================================================================
--- compiler-rt/lib/scudo/standalone/secondary.h
+++ compiler-rt/lib/scudo/standalone/secondary.h
@@ -75,11 +75,6 @@
           s32 MaxReleaseToOsIntervalMs = INT32_MAX>
 class MapAllocatorCache {
 public:
-  // Fuchsia doesn't allow releasing Secondary blocks yet. Note that 0 length
-  // arrays are an extension for some compilers.
-  // FIXME(kostyak): support (partially) the cache on Fuchsia.
-  static_assert(!SCUDO_FUCHSIA || EntriesArraySize == 0U, "");
-
   // Ensure the default maximum specified fits the array.
   static_assert(DefaultMaxEntriesCount <= EntriesArraySize, "");
 
@@ -225,9 +220,10 @@
     for (uptr I = 0; I < EntriesArraySize; I++) {
       if (!Entries[I].Block || !Entries[I].Time || Entries[I].Time > Time)
         continue;
-      releasePagesToOS(Entries[I].Block, 0,
-                       Entries[I].BlockEnd - Entries[I].Block,
-                       &Entries[I].Data);
+      const uptr Base = Entries[I].MapBase;
+      const uptr Offset = Entries[I].Block - Entries[I].MapBase;
+      const uptr Size = Entries[I].BlockEnd - Entries[I].Block;
+      releasePagesToOS(Base, Offset, Size, &Entries[I].Data);
       Entries[I].Time = 0;
     }
   }
@@ -392,9 +388,9 @@
   }
 
   const uptr CommitSize = MapEnd - PageSize - CommitBase;
-  const uptr Ptr =
-      reinterpret_cast<uptr>(map(reinterpret_cast<void *>(CommitBase),
-                                 CommitSize, "scudo:secondary", 0, &Data));
+  const uptr Ptr = reinterpret_cast<uptr>(
+      map(reinterpret_cast<void *>(CommitBase), CommitSize, "scudo:secondary",
+          MAP_RESIZABLE, &Data));
   LargeBlock::Header *H = reinterpret_cast<LargeBlock::Header *>(Ptr);
   H->MapBase = MapBase;
   H->MapSize = MapEnd - MapBase;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D86800.288658.patch
Type: text/x-patch
Size: 2794 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200828/baf29405/attachment.bin>