Defensive code for alloc/dealloc during TLS teardown

CMakeLists.txt

@@ -147,6 +147,10 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY)
     set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG")
   else()
     add_compile_options(-fno-exceptions -fno-rtti -g -ftls-model=initial-exec -fomit-frame-pointer)
+    if(SNMALLOC_CI_BUILD OR (${CMAKE_BUILD_TYPE} MATCHES "Debug"))
+      # Get better stack traces in CI and Debug.
+      target_link_libraries(snmalloc_lib INTERFACE "-rdynamic")
+    endif()
 
     if((${CMAKE_SYSTEM_PROCESSOR} STREQUAL "x86_64") OR
        (${CMAKE_SYSTEM_PROCESSOR} STREQUAL "x86") OR

azure-pipelines.yml

@@ -224,7 +224,11 @@ jobs:
     matrix:
       64-bit Debug Clang:
         BuildType: Debug
-        CMakeArgs: '-GNinja -DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang.exe" -DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang++.exe"'
+        CMakeArgs: '-GNinja -DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_RC_COMPILER="C:/Program Files/LLVM/bin/llvm-rc"'
+        JFlag: -j 4
+      64-bit Release Clang:
+        BuildType: Release
+        CMakeArgs: '-GNinja -DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_RC_COMPILER="C:/Program Files/LLVM/bin/llvm-rc"'
         JFlag: -j 4
 
   steps:
@@ -237,15 +241,13 @@ jobs:
       choco install --accept-license -y Ninja llvm
     displayName: 'Dependencies'
 
-  - task: CMake@1
-    displayName: 'CMake .. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On -DSNMALLOC_RUST_SUPPORT=On'
-    inputs:
-      cmakeArgs: '.. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On -DSNMALLOC_RUST_SUPPORT=On'
-
-  - task: CMake@1
-    displayName: 'CMake --build . --config ${BuildType}'
-    inputs:
-      cmakeArgs: '--build . --config ${BuildType}'
+  - script: |
+      call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvars64.bat"
+      mkdir build
+      cd build
+      cmake .. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On -DSNMALLOC_RUST_SUPPORT=On
+      cmake --build . --config ${BuildType}
+    displayName: 'build'
 
   - script: |
       set PATH=%PATH%;"C:\Program Files\LLVM\bin\"

src/ds/aba.h

@@ -2,13 +2,25 @@
 
 #include "bits.h"
 
+/**
+ * This file contains an abstraction of ABA protection. This API should be
+ * implementable with double-word compare and exchange or with load-link
+ * store conditional.
+ *
+ * We provide a lock based implementation.
+ */
 namespace snmalloc
 {
+#ifndef NDEBUG
+  // LL/SC typically can only perform one operation at a time
+  // check this on other platforms using a thread_local.
+  inline thread_local bool operation_in_flight = false;
+#endif
+#ifdef PLATFORM_IS_X86
   template<typename T, Construction c = RequiresInit>
   class ABA
   {
   public:
-#ifdef PLATFORM_IS_X86
     struct alignas(2 * sizeof(std::size_t)) Linked
     {
       T* ptr;
@@ -28,21 +40,12 @@ namespace snmalloc
       sizeof(Linked) == (2 * sizeof(std::size_t)),
       "Expecting ABA to be the size of two pointers");
 
-    using Cmp = Linked;
-#else
-    using Cmp = T*;
-#endif
-
   private:
-#ifdef PLATFORM_IS_X86
     union
     {
       alignas(2 * sizeof(std::size_t)) std::atomic<Linked> linked;
       Independent independent;
     };
-#else
-    std::atomic<T*> raw;
-#endif
 
   public:
     ABA()
@@ -53,66 +56,116 @@ namespace snmalloc
 
     void init(T* x)
     {
-#ifdef PLATFORM_IS_X86
       independent.ptr.store(x, std::memory_order_relaxed);
       independent.aba.store(0, std::memory_order_relaxed);
-#else
-      raw.store(x, std::memory_order_relaxed);
-#endif
     }
 
-    T* peek()
-    {
-      return
-#ifdef PLATFORM_IS_X86
-        independent.ptr.load(std::memory_order_relaxed);
-#else
-        raw.load(std::memory_order_relaxed);
-#endif
-    }
+    struct Cmp;
 
     Cmp read()
     {
-      return
-#ifdef PLATFORM_IS_X86
-        Cmp{independent.ptr.load(std::memory_order_relaxed),
-            independent.aba.load(std::memory_order_relaxed)};
-#else
-        raw.load(std::memory_order_relaxed);
-#endif
+#  ifndef NDEBUG
+      if (operation_in_flight)
+        error("Only one inflight ABA operation at a time is allowed.");
+      operation_in_flight = true;
+#  endif
+      return Cmp{{independent.ptr.load(std::memory_order_relaxed),
+                  independent.aba.load(std::memory_order_relaxed)},
+                 this};
     }
 
-    static T* ptr(Cmp& from)
+    struct Cmp
     {
-#ifdef PLATFORM_IS_X86
-      return from.ptr;
-#else
-      return from;
-#endif
-    }
+      Linked old;
+      ABA* parent;
 
-    bool compare_exchange(Cmp& expect, T* value)
-    {
-#ifdef PLATFORM_IS_X86
+      T* ptr()
+      {
+        return old.ptr;
+      }
+
+      bool store_conditional(T* value)
+      {
 #  if defined(_MSC_VER) && defined(SNMALLOC_VA_BITS_64)
-      return _InterlockedCompareExchange128(
-        (volatile __int64*)&linked,
-        (__int64)(expect.aba + (uintptr_t)1),
-        (__int64)value,
-        (__int64*)&expect);
+        auto result = _InterlockedCompareExchange128(
+          (volatile __int64*)parent,
+          (__int64)(old.aba + (uintptr_t)1),
+          (__int64)value,
+          (__int64*)&old);
 #  else
 #    if defined(__GNUC__) && !defined(__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16)
 #error You must compile with -mcx16 to enable 16-byte atomic compare and swap.
 #    endif
-      Cmp xchg{value, expect.aba + 1};
+        Linked xchg{value, old.aba + 1};
+        std::atomic<Linked>& addr = parent->linked;
+
+        auto result = addr.compare_exchange_weak(
+          old, xchg, std::memory_order_relaxed, std::memory_order_relaxed);
+#  endif
+        return result;
+      }
 
-      return linked.compare_exchange_weak(
-        expect, xchg, std::memory_order_relaxed, std::memory_order_relaxed);
+      ~Cmp()
+      {
+#  ifndef NDEBUG
+        operation_in_flight = false;
 #  endif
+      }
+
+      Cmp(const Cmp&) = delete;
+    };
+  };
 #else
-      return raw.compare_exchange_weak(
-        expect, value, std::memory_order_relaxed, std::memory_order_relaxed);
-#endif
+  /**
+   * Naive implementation of ABA protection using a spin lock.
+   */
+  template<typename T, Construction c = RequiresInit>
+  class ABA
+  {
+    std::atomic<T*> ptr = nullptr;
+    std::atomic_flag lock = ATOMIC_FLAG_INIT;
+
+  public:
+    struct Cmp;
+
+    Cmp read()
+    {
+      while (lock.test_and_set(std::memory_order_acquire))
+        Aal::pause();
+
+#  ifndef NDEBUG
+      if (operation_in_flight)
+        error("Only one inflight ABA operation at a time is allowed.");
+      operation_in_flight = true;
+#  endif
+
+      return Cmp{this};
     }
+
+    struct Cmp
+    {
+      ABA* parent;
+
+    public:
+      T* ptr()
+      {
+        return parent->ptr;
+      }
+
+      bool store_conditional(T* t)
+      {
+        parent->ptr = t;
+        return true;
+      }
+
+      ~Cmp()
+      {
+        parent->lock.clear(std::memory_order_release);
+#  ifndef NDEBUG
+        operation_in_flight = false;
+#  endif
+      }
+    };
   };
+#endif
 } // namespace snmalloc

src/ds/defines.h

@@ -36,6 +36,9 @@ namespace snmalloc
   void error(const char* const str);
 } // namespace snmalloc
 
+#define TOSTRING(expr) TOSTRING2(expr)
+#define TOSTRING2(expr) #expr
+
 #ifdef NDEBUG
 #  define SNMALLOC_ASSERT(expr) \
     {}
@@ -44,7 +47,8 @@ namespace snmalloc
     { \
       if (!(expr)) \
       { \
-        snmalloc::error("assert fail"); \
+        snmalloc::error("assert fail: " #expr " in " __FILE__ \
+                        " on " TOSTRING(__LINE__)); \
       } \
     }
 #endif

src/ds/mpmcstack.h

@@ -29,9 +29,9 @@ namespace snmalloc
 
       do
       {
-        T* top = ABAT::ptr(cmp);
+        T* top = cmp.ptr();
         last->next.store(top, std::memory_order_release);
-      } while (!stack.compare_exchange(cmp, first));
+      } while (!cmp.store_conditional(first));
     }
 
     T* pop()
@@ -44,13 +44,13 @@ namespace snmalloc
 
       do
       {
-        top = ABAT::ptr(cmp);
+        top = cmp.ptr();
 
         if (top == nullptr)
           break;
 
         next = top->next.load(std::memory_order_acquire);
-      } while (!stack.compare_exchange(cmp, next));
+      } while (!cmp.store_conditional(next));
 
       return top;
     }
@@ -63,11 +63,11 @@ namespace snmalloc
 
       do
       {
-        top = ABAT::ptr(cmp);
+        top = cmp.ptr();
 
         if (top == nullptr)
           break;
-      } while (!stack.compare_exchange(cmp, nullptr));
+      } while (!cmp.store_conditional(nullptr));
 
       return top;
     }