Merge pull request #8 from Microsoft/external_pointer_fix

External pointer fix

CMakeLists.txt

@@ -64,20 +64,24 @@ enable_testing()
 
 set(TESTDIR ${CMAKE_CURRENT_SOURCE_DIR}/src/test)
 subdirlist(TEST_CATEGORIES ${TESTDIR})
-foreach(TEST_CATEGORY ${TEST_CATEGORIES})
-  subdirlist(TESTS ${TESTDIR}/${TEST_CATEGORY})
-  foreach(TEST ${TESTS})
-    unset(SRC)
-    aux_source_directory(${TESTDIR}/${TEST_CATEGORY}/${TEST} SRC)
-    set(TESTNAME "${TEST_CATEGORY}-${TEST}")
-    add_executable(${TESTNAME} ${SRC} src/override/new.cc)
-    target_include_directories(${TESTNAME} PRIVATE src)
-    linklibs(${TESTNAME})
-    add_test(${TESTNAME} ${TESTNAME})
+foreach(SUPER_SLAB_SIZE 1;16)
+  foreach(TEST_CATEGORY ${TEST_CATEGORIES})
+    subdirlist(TESTS ${TESTDIR}/${TEST_CATEGORY})
+    foreach(TEST ${TESTS})
+      unset(SRC)
+      aux_source_directory(${TESTDIR}/${TEST_CATEGORY}/${TEST} SRC)
+      set(TESTNAME "${TEST_CATEGORY}-${TEST}-${SUPER_SLAB_SIZE}")
+      add_executable(${TESTNAME} ${SRC} src/override/new.cc)
+      if (${SUPER_SLAB_SIZE} EQUAL 1)
+        target_compile_definitions(${TESTNAME} PRIVATE IS_ADDRESS_SPACE_CONSTRAINED)
+      endif()
+      target_include_directories(${TESTNAME} PRIVATE src)
+      linklibs(${TESTNAME})
+      add_test(${TESTNAME} ${TESTNAME})
+    endforeach()
   endforeach()
 endforeach()
 
-
 # The clang-format tool is installed under a variety of different names.  Try
 # to find a sensible one.  Only look for 6.0 and 7.0 versions explicitly - we
 # don't know whether our clang-format file will work with newer versions of the

src/ds/helpers.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "bits.h"
 #include "flaglock.h"
 
 namespace snmalloc
@@ -30,4 +31,22 @@ namespace snmalloc
       return obj;
     }
   };
+
+  template<size_t length, typename T>
+  class ModArray
+  {
+    static constexpr size_t rlength = bits::next_pow2_const(length);
+    T array[rlength];
+
+  public:
+    constexpr const T& operator[](const size_t i) const
+    {
+      return array[i & (rlength - 1)];
+    }
+
+    constexpr T& operator[](const size_t i)
+    {
+      return array[i & (rlength - 1)];
+    }
+  };
 }

src/mem/allocconfig.h

@@ -110,6 +110,8 @@ namespace snmalloc
   static constexpr size_t SUPERSLAB_BITS = SLAB_BITS + SLAB_COUNT_BITS;
   static constexpr size_t RESERVE_SIZE = SUPERSLAB_SIZE * RESERVE_MULTIPLE;
 
+  static_assert((1ULL << SUPERSLAB_BITS) == SUPERSLAB_SIZE, "Sanity check");
+
   // Number of slots for remote deallocation.
   static constexpr size_t REMOTE_SLOT_BITS = 6;
   static constexpr size_t REMOTE_SLOTS = 1 << REMOTE_SLOT_BITS;

src/mem/largealloc.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "../ds/flaglock.h"
+#include "../ds/helpers.h"
 #include "../ds/mpmcstack.h"
 #include "../pal/pal.h"
 #include "allocstats.h"
@@ -62,7 +63,7 @@ namespace snmalloc
     /**
      * Stack of large allocations that have been returned for reuse.
      */
-    MPMCStack<Largeslab, PreZeroed> large_stack[NUM_LARGE_CLASSES];
+    ModArray<NUM_LARGE_CLASSES, MPMCStack<Largeslab, PreZeroed>> large_stack;
 
     /**
      * Primitive allocator for structure that are required before

src/mem/metaslab.h

@@ -57,11 +57,8 @@ namespace snmalloc
     // doubly linked node into that size class's free list.
     uint16_t link;
 
-    union
-    {
-      uint8_t sizeclass;
-      uint8_t next;
-    };
+    uint8_t sizeclass;
+    uint8_t next;
 
     void add_use()
     {

src/mem/sizeclass.h

@@ -35,7 +35,7 @@ namespace snmalloc
     size_to_sizeclass_const((size_t)1 << SLAB_BITS) + 1;
 
   static constexpr size_t NUM_SIZECLASSES =
-    size_to_sizeclass_const((size_t)1 << SUPERSLAB_BITS);
+    size_to_sizeclass_const(SUPERSLAB_SIZE);
 
   // Medium classes range from (SLAB, SUPERSLAB), i.e. non-inclusive.
   static constexpr size_t NUM_MEDIUM_CLASSES =
@@ -45,14 +45,6 @@ namespace snmalloc
   static constexpr size_t NUM_LARGE_CLASSES =
     bits::ADDRESS_BITS - SUPERSLAB_BITS;
 
-  template<size_t X, size_t Y>
-  constexpr void check_same()
-  {
-    static_assert(X == Y, "Values must be the same");
-  }
-
-  static_assert(size_to_sizeclass_const(SUPERSLAB_SIZE) == NUM_SIZECLASSES);
-
   inline static size_t round_by_sizeclass(size_t rsize, size_t offset)
   {
     //    check_same<NUM_LARGE_CLASSES, Globals::num_large_classes>();

src/mem/sizeclasstable.h

@@ -1,16 +1,17 @@
 #pragma once
 
+#include "../ds/helpers.h"
 #include "superslab.h"
 
 namespace snmalloc
 {
   struct SizeClassTable
   {
-    size_t size[NUM_SIZECLASSES];
-    uint16_t bump_ptr_start[NUM_SMALL_CLASSES];
-    uint16_t short_bump_ptr_start[NUM_SMALL_CLASSES];
-    uint16_t count_per_slab[NUM_SMALL_CLASSES];
-    uint16_t medium_slab_slots[NUM_MEDIUM_CLASSES];
+    ModArray<NUM_SIZECLASSES, size_t> size;
+    ModArray<NUM_SMALL_CLASSES, uint16_t> bump_ptr_start;
+    ModArray<NUM_SMALL_CLASSES, uint16_t> short_bump_ptr_start;
+    ModArray<NUM_SMALL_CLASSES, uint16_t> count_per_slab;
+    ModArray<NUM_MEDIUM_CLASSES, uint16_t> medium_slab_slots;
 
     constexpr SizeClassTable()
     : size(),
@@ -66,6 +67,7 @@ namespace snmalloc
 
   constexpr static inline uint16_t medium_slab_free(uint8_t sizeclass)
   {
-    return sizeclass_metadata.medium_slab_slots[sizeclass - NUM_SMALL_CLASSES];
+    return sizeclass_metadata
+      .medium_slab_slots[(sizeclass - NUM_SMALL_CLASSES)];
   }
 }

src/mem/superslab.h

@@ -194,6 +194,7 @@ namespace snmalloc
       uint8_t index = (uint8_t)slab_to_index(slab);
       uint8_t n = head - index - 1;
 
+      meta[index].sizeclass = 0;
       meta[index].next = n;
       head = index;
       bool was_almost_full = is_almost_full();

src/test/func/memory/memory.cc

@@ -242,6 +242,30 @@ void test_external_pointer_large()
   }
 }
 
+void test_external_pointer_dealloc_bug()
+{
+  auto* alloc = ThreadAlloc::get();
+  constexpr size_t count = (SUPERSLAB_SIZE / SLAB_SIZE) * 2;
+  void* allocs[count];
+
+  for (size_t i = 0; i < count; i++)
+  {
+    allocs[i] = alloc->alloc(SLAB_SIZE / 2);
+  }
+
+  for (size_t i = 1; i < count; i++)
+  {
+    alloc->dealloc(allocs[i]);
+  }
+
+  for (size_t i = 0; i < count; i++)
+  {
+    Alloc::external_pointer(allocs[i]);
+  }
+
+  alloc->dealloc(allocs[0]);
+}
+
 void test_alloc_16M()
 {
   auto* alloc = ThreadAlloc::get();
@@ -264,6 +288,7 @@ int main(int argc, char** argv)
   UNUSED(argv);
 #endif
 
+  test_external_pointer_dealloc_bug();
   test_external_pointer_large();
   test_alloc_dealloc_64k();
   test_random_allocation();