Merge pull request #229 from ehein6/alignment-check

Enforce alignment of stinger sub-structures.

Author: davidediger

CMakeLists.txt

@@ -139,6 +139,11 @@ set(STINGER_DEFAULT_NEB_FACTOR "4" CACHE STRING "Default number of edge blocks p
 set(STINGER_EDGEBLOCKSIZE "14" CACHE STRING "Number of edges per edge block")
 set(STINGER_NAME_STR_MAX "255" CACHE STRING "Max string length in physmap")
 
+MATH(EXPR STINGER_NAME_STR_MAX_ALIGN "(${STINGER_NAME_STR_MAX}+1) % 8")
+if (NOT STINGER_NAME_STR_MAX_ALIGN EQUAL 0)
+  MESSAGE(SEND_ERROR "STINGER_NAME_STR_MAX must be a multiple of 8 (minus one for null terminator).")
+endif()
+
 configure_file(${CMAKE_SOURCE_DIR}/lib/stinger_core/inc/stinger_defs.h.in ${CMAKE_BINARY_DIR}/include/stinger_core/stinger_defs.h @ONLY)
 configure_file(${CMAKE_SOURCE_DIR}/lib/stinger_core/inc/stinger_names.h.in ${CMAKE_BINARY_DIR}/include/stinger_core/stinger_names.h @ONLY)
 

lib/stinger_core/inc/stinger_names.h.in

@@ -8,6 +8,11 @@ extern "C" {
 
 #define NAME_STR_MAX @STINGER_NAME_STR_MAX@
 
+// STINGER names storage must be 8-byte aligned
+#if (NAME_STR_MAX+1) % 8 != 0
+#error STINGER_NAME_STR_MAX must be a multiple of 8 (minus one for null terminator)
+#endif
+
 #ifdef NAME_USE_SQLITE
 	#include "sqlite/sqlite3.h"
 

lib/stinger_core/src/stinger.c

@@ -467,6 +467,31 @@ stinger_graph_size (const struct stinger *S)
   return result;
 }
 
+/**
+ * Checks the size of a stinger sub-structure, to make sure we aren't
+ * violating any alignment rules. malloc() will return a pointer that
+ * is properly aligned for any struct or array of structs. But when we
+ * pack multiple structures into the same allocation, we run the risk of
+ * a struct being misaligned. As long as each sub-structure ends on an
+ * 8-byte boundary, the next one will start on an 8-byte boundary.
+ * @param sz Size of sub-structure, in bytes
+ * @param substructure_name Name of sub-structure, used when printing an error
+ */
+static void
+check_alignment(int64_t sz, const char* substructure_name)
+{
+  if (sz % sizeof(int64_t) != 0) {
+    LOG_F_A(
+      "The sub-structure %s is %lli bytes long.\n"
+      "The following struct will not be aligned to an 8-byte boundary.\n"
+      "This can cause deadlocks when x86 64-bit atomics cross a cache line. \n"
+      "See section 8.1.1 of the Intel® 64 and IA-32 Architectures Developer's Manual: Vol. 3A. \n"
+      ,substructure_name, (long long int)sz
+    );
+    abort();
+  }
+}
+
 /**
 * @brief Calculates the required memory to allocate a STINGER given a set of parameters
 *
@@ -484,21 +509,27 @@ struct stinger_size_t calculate_stinger_size(int64_t nv, int64_t nebs, int64_t n
 
   ret.vertices_start = 0;
   sz += stinger_vertices_size(nv);
+  check_alignment(sz - ret.vertices_start, "stinger_vertices");
 
   ret.physmap_start = sz;
   sz += stinger_physmap_size(nv);
+  check_alignment(sz - ret.physmap_start, "stinger_physmap");
 
   ret.ebpool_start = sz;
   sz += netypes * stinger_ebpool_size(nebs);
+  check_alignment(sz - ret.ebpool_start, "stinger_ebpool");
 
   ret.etype_names_start = sz;
   sz += stinger_names_size(netypes);
+  check_alignment(sz - ret.etype_names_start, "stinger_etype_names");
 
   ret.vtype_names_start = sz;
   sz += stinger_names_size(nvtypes);
+  check_alignment(sz - ret.vtype_names_start, "stinger_vtype_names");
 
   ret.ETA_start = sz;
   sz += netypes * stinger_etype_array_size(nebs);
+  check_alignment(sz - ret.ETA_start, "stinger_etype_array");
 
   ret.size = sz;