[PATCH] D100524: [scudo][standalone] Fuchsia related fixes

[Kostya Kortchinsky via Phabricator via llvm-commits]

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

While attempting to roll the latest Scudo in Fuchsia, some issues
arose. While trying to debug them, it appeared that `DCHECK`s were
also never exercised in Fuchsia. This CL fixes the following
problems:

- the size of a block in the TransferBatch class must be a multiple of the compact pointer scale. In some cases, it wasn't true, which lead to obscure crashes. Now, we round up `sizeof(TransferBatch)`. This only materialized in Fuchsia due to the specific parameters of the `DefaultConfig`;
- 2 `DCHECK` statements in Fuchsia were incorrect;
- `map()` & co. require a size multiple of a page (as enforced in Fuchsia `DCHECK`s), which wasn't the case for `PackedCounters`.
- In the Secondary, a parameter was marked as `UNUSED` while it is actually used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100524

Files:
  compiler-rt/lib/scudo/standalone/fuchsia.cpp
  compiler-rt/lib/scudo/standalone/primary64.h
  compiler-rt/lib/scudo/standalone/release.h
  compiler-rt/lib/scudo/standalone/secondary.h


Index: compiler-rt/lib/scudo/standalone/secondary.h
===================================================================
--- compiler-rt/lib/scudo/standalone/secondary.h
+++ compiler-rt/lib/scudo/standalone/secondary.h
@@ -69,7 +69,7 @@
                 UNUSED LargeBlock::Header **H, UNUSED bool *Zeroed) {
     return false;
   }
-  void store(UNUSED Options Options, UNUSED LargeBlock::Header *H) { unmap(H); }
+  void store(UNUSED Options Options, LargeBlock::Header *H) { unmap(H); }
   bool canCache(UNUSED uptr Size) { return false; }
   void disable() {}
   void enable() {}
Index: compiler-rt/lib/scudo/standalone/release.h
===================================================================
--- compiler-rt/lib/scudo/standalone/release.h
+++ compiler-rt/lib/scudo/standalone/release.h
@@ -81,7 +81,8 @@
       memset(Buffer, 0, BufferSize);
     } else {
       Buffer = reinterpret_cast<uptr *>(
-          map(nullptr, BufferSize, "scudo:counters", MAP_ALLOWNOMEM));
+          map(nullptr, roundUpTo(BufferSize, getPageSizeCached()),
+              "scudo:counters", MAP_ALLOWNOMEM));
     }
   }
   ~PackedCounterArray() {
@@ -90,7 +91,8 @@
     if (Buffer == &StaticBuffer[0])
       Mutex.unlock();
     else
-      unmap(reinterpret_cast<void *>(Buffer), BufferSize);
+      unmap(reinterpret_cast<void *>(Buffer),
+            roundUpTo(BufferSize, getPageSizeCached()));
   }
 
   bool isAllocated() const { return !!Buffer; }
Index: compiler-rt/lib/scudo/standalone/primary64.h
===================================================================
--- compiler-rt/lib/scudo/standalone/primary64.h
+++ compiler-rt/lib/scudo/standalone/primary64.h
@@ -51,7 +51,7 @@
 
   static uptr getSizeByClassId(uptr ClassId) {
     return (ClassId == SizeClassMap::BatchClassId)
-               ? sizeof(TransferBatch)
+               ? roundUpTo(sizeof(TransferBatch), 1U << CompactPtrScale)
                : SizeClassMap::getSizeByClassId(ClassId);
   }
 
Index: compiler-rt/lib/scudo/standalone/fuchsia.cpp
===================================================================
--- compiler-rt/lib/scudo/standalone/fuchsia.cpp
+++ compiler-rt/lib/scudo/standalone/fuchsia.cpp
@@ -49,7 +49,7 @@
 
 void *map(void *Addr, uptr Size, const char *Name, uptr Flags,
           MapPlatformData *Data) {
-  DCHECK_EQ(Size % PAGE_SIZE, 0);
+  DCHECK_EQ(Size % getPageSizeCached(), 0);
   const bool AllowNoMem = !!(Flags & MAP_ALLOWNOMEM);
 
   // For MAP_NOACCESS, just allocate a Vmar and return.
@@ -96,8 +96,10 @@
   // No need to track the Vmo if we don't intend on resizing it. Close it.
   if (Flags & MAP_RESIZABLE) {
     DCHECK(Data);
-    DCHECK_EQ(Data->Vmo, ZX_HANDLE_INVALID);
-    Data->Vmo = Vmo;
+    if (Data->Vmo == ZX_HANDLE_INVALID)
+      Data->Vmo = Vmo;
+    else
+      DCHECK_EQ(Data->Vmo, Vmo);
   } else {
     CHECK_EQ(_zx_handle_close(Vmo), ZX_OK);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D100524.337624.patch
Type: text/x-patch
Size: 2880 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210415/1c642e90/attachment.bin>