Code review feedback.

CMakeLists.txt

@@ -9,6 +9,12 @@ option(EXPOSE_EXTERNAL_RESERVE "Expose an interface to reserve memory using the
 option(SNMALLOC_RUST_SUPPORT "Build static library for rust" OFF)
 set(CACHE_FRIENDLY_OFFSET OFF CACHE STRING "Base offset to place linked-list nodes.")
 
+if (CMAKE_BUILD_TYPE STREQUAL "Release")
+  option(USE_POSIX_COMMIT_CHECKS "Instrument Posix PAL to check for access to unused blocks of memory." Off)
+else ()
+  option(USE_POSIX_COMMIT_CHECKS "Instrument Posix PAL to check for access to unused blocks of memory." On)
+endif ()
+
 # Provide as macro so other projects can reuse
 macro(warnings_high)
   if(MSVC)
@@ -111,6 +117,11 @@ if(CACHE_FRIENDLY_OFFSET)
   target_compile_definitions(snmalloc_lib INTERFACE -DCACHE_FRIENDLY_OFFSET=${CACHE_FRIENDLY_OFFSET})
 endif()
 
+if(USE_POSIX_COMMIT_CHECKS)
+  target_compile_definitions(snmalloc_lib INTERFACE -DUSE_POSIX_COMMIT_CHECKS)
+endif()
+
+
 # To build with just the header library target define SNMALLOC_ONLY_HEADER_LIBRARY
 # in containing Cmake file.
 if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY)

src/mem/largealloc.h

@@ -230,25 +230,13 @@ namespace snmalloc
         // Reserve 4 times the amount, and put aligned leftovers into the
         // large_stack
         size_t request = bits::max(size * 4, SUPERSLAB_SIZE * 8);
-        void* p = PAL::template reserve<committed>(request);
+        void* p = PAL::template reserve<false>(request);
 
         address_t p0 = address_cast(p);
         address_t start = bits::align_up(p0, align);
         address_t p1 = p0 + request;
         address_t end = start + size;
 
-        // Turn off pages for extra allocations.
-        // Will turn pages back on for stack entries.
-        if (committed)
-        {
-          if (start - p0 > 0)
-          {
-            PAL::notify_not_using(p, start - p0);
-          }
-          assert(p1 - end > 0);
-          PAL::notify_not_using(pointer_cast<void>(end), p1 - end);
-        }
-
         for (; end < bits::align_down(p1, align); end += size)
         {
           push_space(end, large_class);
@@ -285,7 +273,11 @@ namespace snmalloc
 
         // printf("Alloc %zx (size = %zx)\n", start, size);
 
-        return pointer_cast<void>(start);
+        void* result = pointer_cast<void>(start);  
+        if (committed)
+          PAL::template notify_using<NoZero>(result, size);
+
+        return result;
       }
     }
 

src/pal/pal_freebsd_kernel.h

@@ -63,11 +63,10 @@ namespace snmalloc
     template<bool committed>
     void* reserve(size_t size, size_t align)
     {
-      size_t request = size;
       vm_offset_t addr;
       if (vmem_xalloc(
             kernel_arena,
-            request,
+            size,
             align,
             0,
             0,
@@ -81,10 +80,10 @@ namespace snmalloc
       if (committed)
       {
         if (
-          kmem_back(kernel_object, addr, request, M_ZERO | M_WAITOK) !=
+          kmem_back(kernel_object, addr, size, M_ZERO | M_WAITOK) !=
           KERN_SUCCESS)
         {
-          vmem_xfree(kernel_arena, addr, request);
+          vmem_xfree(kernel_arena, addr, size);
           return nullptr;
         }
       }

src/pal/pal_posix.h

@@ -54,7 +54,7 @@ namespace snmalloc
     void notify_not_using(void* p, size_t size) noexcept
     {
       assert(is_aligned_block<OS_PAGE_SIZE>(p, size));
-#ifndef NDEBUG
+#ifdef USE_POSIX_COMMIT_CHECKS
       mprotect(p, size, PROT_NONE);
 #else
       UNUSED(p);
@@ -77,7 +77,7 @@ namespace snmalloc
       if constexpr (zero_mem == YesZero)
         static_cast<OS*>(this)->template zero<true>(p, size);
 
-#ifndef NDEBUG
+#ifdef USE_POSIX_COMMIT_CHECKS
       mprotect(p, size, PROT_READ | PROT_WRITE);
 #else
       UNUSED(p);

src/test/func/fixed_region/fixed_region.cc

@@ -32,6 +32,9 @@ int main()
 {
   MemoryProviderStateMixin<DefaultPal> mp;
 
+  // 28 is large enough to produce a nested allocator.
+  // It is also large enough for the example to run in.
+  // For 1MiB superslabs, SUPERSLAB_BITS + 4 is not big enough for the example.
   size_t large_class = 28 - SUPERSLAB_BITS;
   size_t size = 1ULL << (SUPERSLAB_BITS + large_class);
   oe_base = mp.reserve<true>(large_class);

src/test/func/two_alloc_types/main.cc

@@ -45,6 +45,9 @@ int main()
 
   MemoryProviderStateMixin<DefaultPal> mp;
 
+  // 26 is large enough to produce a nested allocator.
+  // It is also large enough for the example to run in.
+  // For 1MiB superslabs, SUPERSLAB_BITS + 2 is not big enough for the example.
   size_t large_class = 26 - SUPERSLAB_BITS;
   size_t size = 1ULL << (SUPERSLAB_BITS + large_class);
   oe_base = mp.reserve<true>(large_class);