Various fixes for OE (#418)

* Add default for getting chunk allocator state

Makes the API same between the two configurations.

* Reduce address space usage for Open Enclave

* Fix OE Pal concept

* Add support for Pal not to provide time.

The lazy return of pages to the OS uses a simple time
based heuristic.  This enables a PAL to not support time,
and return the memory to a central pool immediately.

* Update src/backend/backend.h

Co-authored-by: Amaury Chamayou <amaury@xargs.fr>

Co-authored-by: Amaury Chamayou <amaury@xargs.fr>

src/backend/backend.h

@@ -6,6 +6,19 @@
 #include "commonconfig.h"
 #include "pagemap.h"
 
+#if defined(SNMALLOC_CHECK_CLIENT) && !defined(OPEN_ENCLAVE)
+/**
+ * Protect meta data blocks by allocating separate from chunks for
+ * user allocations. This involves leaving gaps in address space.
+ * This is less efficient, so should only be applied for the checked
+ * build.
+ *
+ * On Open Enclave the address space is limited, so we disable this
+ * feature.
+ */
+#  define SNMALLOC_META_PROTECTED
+#endif
+
 namespace snmalloc
 {
   /**
@@ -22,14 +35,23 @@ namespace snmalloc
     // Size of local address space requests.  Currently aimed at 2MiB large
     // pages but should make this configurable (i.e. for OE, so we don't need as
     // much space).
+#ifdef OPEN_ENCLAVE
+    // Don't prefetch address space on OE, as it is limited.
+    // This could cause perf issues during warm-up phases.
+    constexpr static size_t LOCAL_CACHE_BLOCK = 0;
+#else
     constexpr static size_t LOCAL_CACHE_BLOCK = bits::one_at_bit(21);
+#endif
 
-#ifdef SNMALLOC_CHECK_CLIENT
+#ifdef SNMALLOC_META_PROTECTED
     // When protecting the meta-data, we use a smaller block for the meta-data
     // that is randomised inside a larger block.  This needs to be at least a
     // page so that we can use guard pages.
     constexpr static size_t LOCAL_CACHE_META_BLOCK =
       bits::max(MIN_CHUNK_SIZE * 2, OS_PAGE_SIZE);
+    static_assert(
+      LOCAL_CACHE_META_BLOCK <= LOCAL_CACHE_BLOCK,
+      "LOCAL_CACHE_META_BLOCK must be smaller than LOCAL_CACHE_BLOCK");
 #endif
 
   public:
@@ -103,7 +125,7 @@ namespace snmalloc
     static capptr::Chunk<void> reserve(
       AddressSpaceManager<PAL>& global, LocalState* local_state, size_t size)
     {
-#ifdef SNMALLOC_CHECK_CLIENT
+#ifdef SNMALLOC_META_PROTECTED
       constexpr auto MAX_CACHED_SIZE =
         is_meta ? LOCAL_CACHE_META_BLOCK : LOCAL_CACHE_BLOCK;
 #else
@@ -113,7 +135,7 @@ namespace snmalloc
       capptr::Chunk<void> p;
       if ((local_state != nullptr) && (size <= MAX_CACHED_SIZE))
       {
-#ifdef SNMALLOC_CHECK_CLIENT
+#ifdef SNMALLOC_META_PROTECTED
         auto& local = is_meta ? local_state->local_meta_address_space :
                                 local_state->local_address_space;
 #else
@@ -133,7 +155,7 @@ namespace snmalloc
         if (refill == nullptr)
           return nullptr;
 
-#ifdef SNMALLOC_CHECK_CLIENT
+#ifdef SNMALLOC_META_PROTECTED
         if (is_meta)
         {
           refill = sub_range(refill, LOCAL_CACHE_BLOCK, LOCAL_CACHE_META_BLOCK);
@@ -149,7 +171,7 @@ namespace snmalloc
           local_state, size);
       }
 
-#ifdef SNMALLOC_CHECK_CLIENT
+#ifdef SNMALLOC_META_PROTECTED
       // During start up we need meta-data before we have a local allocator
       // This code protects that meta-data with randomisation, and guard pages.
       if (local_state == nullptr && is_meta)
@@ -178,7 +200,7 @@ namespace snmalloc
       return p;
     }
 
-#ifdef SNMALLOC_CHECK_CLIENT
+#ifdef SNMALLOC_META_PROTECTED
     /**
      * Returns a sub-range of [return, return+sub_size] that is contained in
      * the range [base, base+full_size]. The first and last slot are not used
@@ -236,7 +258,7 @@ namespace snmalloc
 
       AddressSpaceManagerCore local_address_space;
 
-#ifdef SNMALLOC_CHECK_CLIENT
+#ifdef SNMALLOC_META_PROTECTED
       /**
        * Secondary local address space, so we can apply some randomisation
        * and guard pages to protect the meta-data.

src/backend/fixedglobalconfig.h

@@ -25,7 +25,7 @@ namespace snmalloc
 
   public:
     static ChunkAllocatorState&
-    get_chunk_allocator_state(typename Backend::LocalState*)
+    get_chunk_allocator_state(typename Backend::LocalState* = nullptr)
     {
       return chunk_allocator_state;
     }

src/mem/chunkallocator.h

@@ -131,7 +131,7 @@ namespace snmalloc
       {
         // Unsafe downcast here. Don't want vtable and RTTI.
         auto self = reinterpret_cast<DecayMemoryTimerObject*>(p);
-        ChunkAllocator::handle_decay_tick(self->state);
+        ChunkAllocator::handle_decay_tick<Pal>(self->state);
       }
 
       // Specify that we notify the ChunkAllocator every 500ms.
@@ -143,6 +143,7 @@ namespace snmalloc
       {}
     };
 
+    template<SNMALLOC_CONCEPT(ConceptPAL) Pal>
     static void handle_decay_tick(ChunkAllocatorState* state)
     {
       auto new_epoch = (state->epoch + 1) % NUM_EPOCHS;
@@ -186,6 +187,7 @@ namespace snmalloc
       size_t slab_size,
       RemoteAllocator* remote)
     {
+      using PAL = typename SharedStateHandle::Pal;
       ChunkAllocatorState& state =
         SharedStateHandle::get_chunk_allocator_state(&local_state);
 
@@ -196,13 +198,16 @@ namespace snmalloc
       }
 
       ChunkRecord* chunk_record = nullptr;
-      // Try local cache of chunks first
+      if constexpr (pal_supports<Time, PAL>)
-      for (size_t e = 0; e < NUM_EPOCHS && chunk_record == nullptr; e++)
       {
-        chunk_record =
+        // Try local cache of chunks first
-          chunk_alloc_local_state
+        for (size_t e = 0; e < NUM_EPOCHS && chunk_record == nullptr; e++)
-            .chunk_stack[slab_sizeclass][(state.epoch - e) % NUM_EPOCHS]
+        {
-            .pop();
+          chunk_record =
+            chunk_alloc_local_state
+              .chunk_stack[slab_sizeclass][(state.epoch - e) % NUM_EPOCHS]
+              .pop();
+        }
       }
 
       // Try global cache.
@@ -210,8 +215,10 @@ namespace snmalloc
       {
         chunk_record = state.decommitted_chunk_stack[slab_sizeclass].pop();
         if (chunk_record != nullptr)
-          Pal::notify_using<NoZero>(
+        {
+          PAL::template notify_using<NoZero>(
             chunk_record->meta_common.chunk.unsafe_ptr(), slab_size);
+        }
       }
 
       if (chunk_record != nullptr)
@@ -258,14 +265,31 @@ namespace snmalloc
     {
       ChunkAllocatorState& state =
         SharedStateHandle::get_chunk_allocator_state(&local_state);
-#ifdef SNMALLOC_TRACING
-      std::cout << "Return slab:" << p->meta_common.chunk.unsafe_ptr()
-                << " slab_sizeclass " << slab_sizeclass << " size "
-                << slab_sizeclass_to_size(slab_sizeclass)
-                << " memory in stacks " << state.memory_in_stacks << std::endl;
-#endif
 
-      chunk_alloc_local_state.chunk_stack[slab_sizeclass][state.epoch].push(p);
+      if constexpr (pal_supports<Time, typename SharedStateHandle::Pal>)
+      {
+        // If we have a time source use decay based local cache.
+#ifdef SNMALLOC_TRACING
+        std::cout << "Return slab:" << p->meta_common.chunk.unsafe_ptr()
+                  << " slab_sizeclass " << slab_sizeclass << " size "
+                  << slab_sizeclass_to_size(slab_sizeclass)
+                  << " memory in stacks " << state.memory_in_stacks
+                  << std::endl;
+#endif
+        chunk_alloc_local_state.chunk_stack[slab_sizeclass][state.epoch].push(
+          p);
+      }
+      else
+      {
+        // No time source share immediately with global state.
+        // Disable pages for this chunk.
+        SharedStateHandle::Pal::notify_not_using(
+          p->meta_common.chunk.unsafe_ptr(),
+          slab_sizeclass_to_size(slab_sizeclass));
+
+        // Add to global state
+        state.decommitted_chunk_stack[slab_sizeclass].push(p);
+      }
 
       state.memory_in_stacks += slab_sizeclass_to_size(slab_sizeclass);
     }
@@ -300,35 +324,43 @@ namespace snmalloc
       typename SharedStateHandle::LocalState& local_state,
       ChunkAllocatorLocalState& chunk_alloc_local_state)
     {
-      ChunkAllocatorState& state =
+      if constexpr (pal_supports<Time, typename SharedStateHandle::Pal>)
-        SharedStateHandle::get_chunk_allocator_state(&local_state);
-
-      // Register with the Pal to receive notifications.
-      if (!state.register_decay.test_and_set())
       {
-        auto timer = alloc_meta_data<
+        ChunkAllocatorState& state =
-          DecayMemoryTimerObject<typename SharedStateHandle::Pal>,
+          SharedStateHandle::get_chunk_allocator_state(&local_state);
-          SharedStateHandle>(&local_state, &state);
+
-        if (timer != nullptr)
+        // Register with the Pal to receive notifications.
+        if (!state.register_decay.test_and_set())
         {
-          SharedStateHandle::Pal::register_timer(timer);
+          auto timer = alloc_meta_data<
+            DecayMemoryTimerObject<typename SharedStateHandle::Pal>,
+            SharedStateHandle>(&local_state, &state);
+          if (timer != nullptr)
+          {
+            SharedStateHandle::Pal::register_timer(timer);
+          }
+          else
+          {
+            // We failed to register the notification.
+            // This is not catarophic, but if we can't allocate this
+            // state something else will fail shortly.
+            state.register_decay.clear();
+          }
         }
-        else
+
+        // Add to the list of local states.
+        auto* head = state.all_local.load();
+        do
         {
-          // We failed to register the notification.
+          chunk_alloc_local_state.next = head;
-          // This is not catarophic, but if we can't allocate this
+        } while (!state.all_local.compare_exchange_strong(
-          // state something else will fail shortly.
+          head, &chunk_alloc_local_state));
-          state.register_decay.clear();
-        }
       }
-
+      else
-      // Add to the list of local states.
-      auto* head = state.all_local.load();
-      do
       {
-        chunk_alloc_local_state.next = head;
+        UNUSED(local_state);
-      } while (!state.all_local.compare_exchange_strong(
+        UNUSED(chunk_alloc_local_state);
-        head, &chunk_alloc_local_state));
+      }
     }
   };
 } // namespace snmalloc

src/mem/ticker.h

@@ -83,13 +83,16 @@ namespace snmalloc
     template<typename T = void*>
     SNMALLOC_FAST_PATH T check_tick(T p = nullptr)
     {
-      // Check before decrement, so that later calcations can use
+      if constexpr (pal_supports<Time, PAL>)
-      // count_down == 0 for check on the next call.
-      // This is used if the ticks are way below the frequency of
-      // heart beat.
-      if (--count_down == 0)
       {
-        return check_tick_slow(p);
+        // Check before decrement, so that later calcations can use
+        // count_down == 0 for check on the next call.
+        // This is used if the ticks are way below the frequency of
+        // heart beat.
+        if (--count_down == 0)
+        {
+          return check_tick_slow(p);
+        }
       }
       return p;
     }

src/pal/pal_apple.h

@@ -28,7 +28,7 @@ namespace snmalloc
      * The features exported by this PAL.
      */
     static constexpr uint64_t pal_features =
-      AlignedAllocation | LazyCommit | Entropy;
+      AlignedAllocation | LazyCommit | Entropy | Time;
 
     /*
      * `page_size`

src/pal/pal_consts.h

@@ -41,6 +41,7 @@ namespace snmalloc
      * whether low memory conditions are still in effect.
      */
     LowMemoryNotification = (1 << 0),
+
     /**
      * This PAL natively supports allocation with a guaranteed alignment.  If
      * this is not supported, then we will over-allocate and round the
@@ -51,22 +52,31 @@ namespace snmalloc
      * `request()` method that takes only a size.
      */
     AlignedAllocation = (1 << 1),
+
     /**
      * This PAL natively supports lazy commit of pages. This means have large
      * allocations and not touching them does not increase memory usage. This is
      * exposed in the Pal.
      */
     LazyCommit = (1 << 2),
+
     /**
      * This Pal does not support allocation.  All memory used with this Pal
      * should be pre-allocated.
      */
     NoAllocation = (1 << 3),
+
     /**
      * This Pal provides a source of Entropy
      */
     Entropy = (1 << 4),
+
+    /**
+     * This Pal provides a millisecond time source
+     */
+    Time = (1 << 5),
   };
+
   /**
    * Flag indicating whether requested memory should be zeroed.
    */
@@ -76,6 +86,7 @@ namespace snmalloc
      * Memory should not be zeroed, contents are undefined.
      */
     NoZero,
+
     /**
      * Memory must be zeroed.  This can be lazily allocated via a copy-on-write
      * mechanism as long as any load from the memory returns zero.

src/pal/pal_noalloc.h

@@ -28,7 +28,7 @@ namespace snmalloc
    * ever use.
    */
   template<SNMALLOC_CONCEPT(PALNoAllocBase) BasePAL>
-  struct PALNoAlloc : public PalTimerDefaultImpl<BasePAL>
+  struct PALNoAlloc : public BasePAL
   {
     /**
      * Bitmap of PalFeatures flags indicating the optional features that this
@@ -38,7 +38,7 @@ namespace snmalloc
 
     static constexpr size_t page_size = BasePAL::page_size;
 
-    static constexpr size_t address_bits = Aal::address_bits;
+    static constexpr size_t address_bits = BasePAL::address_bits;
 
     /**
      * Print a stack trace.

src/pal/pal_open_enclave.h

@@ -19,6 +19,8 @@ namespace snmalloc
       UNUSED(str);
       oe_abort();
     }
+    static constexpr size_t address_bits = Aal::address_bits;
+    static constexpr size_t page_size = Aal::smallest_page_size;
   };
 
   using OpenEnclaveBasePAL = PALNoAlloc<OpenEnclaveErrorHandler>;

src/pal/pal_posix.h

@@ -125,7 +125,7 @@ namespace snmalloc
      * POSIX systems are assumed to support lazy commit. The build system checks
      * getentropy is available, only then this PAL supports Entropy.
      */
-    static constexpr uint64_t pal_features = LazyCommit
+    static constexpr uint64_t pal_features = LazyCommit | Time
 #if defined(SNMALLOC_PLATFORM_HAS_GETENTROPY)
       | Entropy
 #endif

src/pal/pal_windows.h

@@ -55,7 +55,8 @@ namespace snmalloc
      * Bitmap of PalFeatures flags indicating the optional features that this
      * PAL supports.  This PAL supports low-memory notifications.
      */
-    static constexpr uint64_t pal_features = LowMemoryNotification | Entropy
+    static constexpr uint64_t pal_features = LowMemoryNotification | Entropy |
+      Time
 #  if defined(PLATFORM_HAS_VIRTUALALLOC2) && !defined(USE_SYSTEMATIC_TESTING)
       | AlignedAllocation
 #  endif