DISPATCH-2039 Add option -DQD_DISABLE_MEMORY_POOL for CMake build (#110)

Author: jiridanek

README.adoc

@@ -306,12 +306,18 @@ Existing build directory can be opened with `cmake-gui -S .. -B .`
 Other options include `Debug` (disables optimizations) and `Coverage`.
 
 |`-DQD_ENABLE_ASSERTIONS=`
-|Seting this to `ON` enables asserts irrespective of `CMAKE_BUILD_TYPE`.
+|Setting this to `ON` enables asserts irrespective of `CMAKE_BUILD_TYPE`.
 
 |`-DRUNTIME_CHECK=`
-|Enables C/C++ runtime checkers.See "Run Time Validation" chapter above.
+|Enables C/C++ runtime checkers. See "Run Time Validation" chapter above.
 
 |`-DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON`
 |With CMake 3.9+, compiles the project with LTO (Link Time Optimization) enabled.
 Older CMake will only honor this option with the Intel compiler on Linux.
+
+|`-DQD_DISABLE_MEMORY_POOL=ON`
+|Dispatch will immediately free memory, instead of returning it to memory pool.
+This option will *break* safe pointers, resulting in crashes, therefore is suitable only for debugging.
+When combined with `-DRUNTIME_CHECK=asan`, the pointer breakages are much less frequent.
+
 |===

cmake/RuntimeChecks.cmake

@@ -31,6 +31,16 @@
 # This file updates the QDROUTERD_RUNNER and CMAKE_C_FLAGS
 # appropriately for use when running the ctest suite.
 
+# Disabling memory pool turns use-after-poison asan warnings into use-after-free warnings. The latter come with
+# a freeing stacktrace that makes them easier to triage.
+#
+# Safe pointers require memory pool to work (memory may never be relinquished to the OS, otherwise safe pointers may
+# break). Therefore, only disable memory pool for special debugging purposes. Sanitizers minimize memory reuse and
+# make it significantly less likely that a safe pointer breaks.
+set(QD_DISABLE_MEMORY_POOL OFF CACHE STRING "Disables memory pool. Should be only used with asan or msan RUNTIME_CHECK")
+if (QD_DISABLE_MEMORY_POOL)
+  add_definitions(-DQD_DISABLE_MEMORY_POOL)
+endif()
 
 # Valgrind configuration
 #
@@ -85,6 +95,9 @@ set(RUNTIME_CHECK ${RUNTIME_CHECK_DEFAULT} CACHE STRING "Enable runtime checks.
 if(CMAKE_BUILD_TYPE MATCHES "Coverage" AND RUNTIME_CHECK)
   message(FATAL_ERROR "Cannot set RUNTIME_CHECK with CMAKE_BUILD_TYPE=Coverage")
 endif()
+if(QD_DISABLE_MEMORY_POOL AND NOT RUNTIME_CHECK)
+  message(FATAL_ERROR "Do not set QD_DISABLE_MEMORY_POOL without enabling RUNTIME_CHECK at the same time")
+endif()
 
 if(RUNTIME_CHECK STREQUAL "memcheck")
   assert_has_valgrind()

include/qpid/dispatch/alloc_pool.h

@@ -43,7 +43,7 @@ DEQ_DECLARE(qd_alloc_pool_t, qd_alloc_pool_list_t);
 typedef struct {
     int  transfer_batch_size;
     int  local_free_list_max;
-    int  global_free_list_max;
+    int  global_free_list_max;  ///< -1 means unlimited
 } qd_alloc_config_t;
 
 /** Allocation statistics. */

src/alloc_pool.c

@@ -260,8 +260,9 @@ struct qd_alloc_pool_t {
     qd_alloc_linked_stack_t free_list;
 };
 
-qd_alloc_config_t qd_alloc_default_config_big   = {16,  32, 0};
-qd_alloc_config_t qd_alloc_default_config_small = {64, 128, 0};
+qd_alloc_config_t qd_alloc_default_config_big   = {16,  32, -1};
+qd_alloc_config_t qd_alloc_default_config_small = {64, 128, -1};
+qd_alloc_config_t qd_alloc_default_config_asan  = { 1,   0,  0};
 #define BIG_THRESHOLD 2000
 
 static sys_mutex_t          *init_lock = 0;
@@ -281,7 +282,11 @@ static void qd_alloc_init(qd_alloc_type_desc_t *desc)
             desc->config = desc->total_size > BIG_THRESHOLD ?
                 &qd_alloc_default_config_big : &qd_alloc_default_config_small;
 
+#if defined(QD_DISABLE_MEMORY_POOL)
+        desc->config = &qd_alloc_default_config_asan;
+#else
         assert (desc->config->local_free_list_max >= desc->config->transfer_batch_size);
+#endif
 
         desc->global_pool = NEW(qd_alloc_pool_t);
         DEQ_ITEM_INIT(desc->global_pool);
@@ -489,7 +494,7 @@ void qd_dealloc(qd_alloc_type_desc_t *desc, qd_alloc_pool_t **tpool, char *p)
     // If there's a global_free_list size limit, remove items until the limit is
     // not exceeded.
     //
-    if (desc->config->global_free_list_max != 0) {
+    if (desc->config->global_free_list_max != -1) {
         while (DEQ_SIZE(desc->global_pool->free_list) > desc->config->global_free_list_max) {
             item = pop_stack(&desc->global_pool->free_list);
             FREE_CACHE_ALIGNED(item);
@@ -500,7 +505,11 @@ void qd_dealloc(qd_alloc_type_desc_t *desc, qd_alloc_pool_t **tpool, char *p)
     sys_mutex_unlock(desc->lock);
 }
 
-
+#if defined(QD_DISABLE_MEMORY_POOL)
+// disabling alloc pool causes use-after-free; no way to check if memory
+// has been freed to the OS before accessing it from `qd_alloc_deref_safe_ptr`
+ATTRIBUTE_NO_SANITIZE_ADDRESS
+#endif
 uint32_t qd_alloc_sequence(void *p)
 {
     if (!p)

src/qd_asan_interface.h

@@ -81,8 +81,8 @@ void __asan_unpoison_memory_region(void const volatile *addr, size_t size);
 #endif  // QD_HAS_ADDRESS_SANITIZER
 
 // https://github.com/google/sanitizers/wiki/AddressSanitizer#turning-off-instrumentation
-
-#if QD_HAS_ADDRESS_SANITIZER
+#if QD_HAS_ADDRESS_SANITIZER || QD_HAS_THREAD_SANITIZER
+// note: tsan can also detect some invalid memory accesses and that will crash the binary
 # define ATTRIBUTE_NO_SANITIZE_ADDRESS __attribute__((no_sanitize_address)) __attribute__((no_sanitize("address")))
 #else
 # define ATTRIBUTE_NO_SANITIZE_ADDRESS