Enforce alignment of stinger sub-structures.

STINGER makes one large contiguous allocation, then packs several sub-
structures into this chunk of memory. The alignment of each struct
within the chunk depends on the total length of all the structs and
arrays before it.

According to section 8.1.1 of the Intel® 64 and IA-32 Architectures
Developer's Manual: Vol. 3A, "Accesses to cacheable memory that are
split across cache lines and page boundaries are not guaranteed to be
atomic". Operations like readfe() and writeef() can cause deadlocks
if they operate on pointers to unaligned 64-bit data.

Most sub-structures are composed of int64_t's, so they are aligned
regardless of size. But stinger_names includes char arrays, the
size of which is specified by the configuration variable
STINGER_NAME_STR_MAX, plus one for a null terminator byte. The
default value of 255 works, but changing this value can break the
alignment of the other structures, causing deadlocks.

This patch adds several checks to ensure that the all of the
STINGER sub-structures are aligned to an 8-byte boundary.

Author: ehein6

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;