Merge pull request #292 from insertinterestingnamehere/debug

Fix Debug Builds
sandialabs · Oct 4, 2024 · 622f269 · 622f269
2 parents c5a7831 + 23b61eb
commit 622f269
Show file tree

Hide file tree

Showing 77 changed files with 370 additions and 516 deletions.
diff --git a/configure.ac b/configure.ac
@@ -57,10 +57,6 @@ AC_ARG_ENABLE([asserts],
               [AS_HELP_STRING([--enable-asserts],
                               [adds sanity checks to most qthread functions])])
 
-AC_ARG_ENABLE([paranoia],
-              [AS_HELP_STRING([--enable-paranoia],
-                              [adds expensive sanity checks])])
-
 AC_ARG_WITH([topology],
             [AS_HELP_STRING([--with-topology=[[topologylib]]],
                             [specify which topology interface to use. Supported
@@ -225,7 +221,6 @@ AC_CACHE_SAVE
 ## -------------------- ##
 AC_SEARCH_LIBS([pthread_create], [pthread], [],
                [AC_MSG_ERROR([Qthreads requires a working pthreads implementation.])])
-AC_CHECK_FUNCS([pthread_yield])
 
 AC_SEARCH_LIBS([nanosleep],[rt],[],
                [AC_MSG_ERROR([Cannot find nanosleep])])
@@ -368,12 +363,8 @@ AS_IF([test "x$enable_omp_affinity" = xyes],
       [AC_DEFINE([QTHREAD_OMP_AFFINITY], [1], [Enable experimental OpenMP affinity extensions. Under development])],
       [enable_omp_affinity="no"])
 
-AS_IF([test "x$enable_paranoia" == "xyes"],
-      [AC_DEFINE([QTHREAD_PARANOIA], [1], [expensive sanity checks])
-       enable_asserts=yes])
-
 AS_IF([test "x$enable_asserts" != "xyes"],
-      [AC_DEFINE([QTHREAD_NO_ASSERTS], [1], [removes sanity checks from most qthread functions])
+      [AC_DEFINE([NDEBUG], [1], [removes sanity checks from most qthread functions])
        enable_asserts=no],
       [enable_asserts=yes])
 

diff --git a/include/qt_asserts.h b/include/qt_asserts.h
@@ -37,14 +37,10 @@
  * and sending a signal to itself)
  */
 
-#ifdef QTHREAD_NO_ASSERTS
+#ifdef NDEBUG
 #define ASSERT_ONLY(x)
 #define qassert(op, val) op
 #define qassertnot(op, val) op
-#ifdef assert
-#undef assert
-#endif
-#define assert(foo)
 #define qassert_ret(assertion, retval)                                         \
   do {                                                                         \
     if (!(assertion)) { return retval; }                                       \
@@ -61,7 +57,7 @@
 #define qgoto(tag)                                                             \
   tag:
 #define tassert(foo)
-#else // ifdef QTHREAD_NO_ASSERTS
+#else // ifdef NDEBUG
 #define ASSERT_ONLY(x) x
 #define qassert(op, val) assert((op) == (val))
 #define qassertnot(op, val) assert(op != val)
@@ -78,12 +74,6 @@
   do {                                                                         \
     if (!(foo)) { QTHREAD_TRAP(); }                                            \
   } while (0)
-#endif // ifdef QTHREAD_NO_ASSERTS
-
-#ifdef QTHREAD_PARANOIA
-#define PARANOIA(x) x
-#else
-#define PARANOIA(x)
-#endif
+#endif // ifdef NDEBUG
 
 /* vim:set expandtab: */
diff --git a/include/qt_atomics.h b/include/qt_atomics.h
@@ -151,6 +151,7 @@ static inline int QTHREAD_TRYLOCK_TRY(qt_spin_trylock_t *x) {
     }                                                                          \
     {                                                                          \
       pthread_condattr_t tmp_attr;                                             \
+      pthread_condattr_init(&tmp_attr);                                        \
       qassert(pthread_condattr_setpshared(&tmp_attr, PTHREAD_PROCESS_PRIVATE), \
               0);                                                              \
       qassert(pthread_cond_init(&(c), &tmp_attr), 0);                          \
@@ -182,7 +183,8 @@ static inline int QTHREAD_TRYLOCK_TRY(qt_spin_trylock_t *x) {
     t.tv_nsec = (n.tv_usec * 1000) + 500000000;                                \
     t.tv_sec = n.tv_sec + ((t.tv_nsec >= 1000000000) ? 1 : 0);                 \
     t.tv_nsec -= ((t.tv_nsec >= 1000000000) ? 1000000000 : 0);                 \
-    qassert(pthread_cond_timedwait(&(c), &(c##_lock), &t), 0);                 \
+    int val = pthread_cond_timedwait(&(c), &(c##_lock), &t);                   \
+    qassert(val == EINVAL || val == EPERM, 0);                                 \
   } while (0)
 #define QTHREAD_COND_WAIT_DUO(c, m)                                            \
   do {                                                                         \

diff --git a/include/qt_initialized.h b/include/qt_initialized.h
@@ -1,8 +1,6 @@
 #ifndef QT_INITIALIZED_H
 #define QT_INITIALIZED_H
 
-#ifndef QTHREAD_NO_ASSERTS
 extern int qthread_library_initialized;
-#endif
 
 #endif
diff --git a/src/ds/dictionary/dictionary_simple.c b/src/ds/dictionary/dictionary_simple.c
@@ -35,7 +35,7 @@ void *qt_dictionary_put_helper(qt_dictionary *dict,
 
 // Handle the case when variable does not exist;
 // Locks become nops when qthread is no longer initialized
-#ifdef QTHREAD_NO_ASSERTS
+#ifdef NDEBUG
 static int qthread_library_initialized = 1;
 #endif
 
@@ -98,7 +98,7 @@ static int qthread_library_initialized = 1;
 
 #define GET_BUCKET(hash) ((DICT_ABS(hash)) & ((1 << BKT_POW) - 1))
 
-#ifndef QTHREAD_NO_ASSERTS
+#ifndef NDEBUG
 extern int qthread_library_initialized;
 #endif
 

diff --git a/src/mpool.c b/src/mpool.c
@@ -221,7 +221,7 @@ static qt_mpool_threadlocal_cache_t *qt_mpool_internal_getcache(qt_mpool pool) {
   {
     uintptr_t count_caches = (uintptr_t)TLS_GET(pool_cache_count);
     if (count_caches < pool->offset) {
-#if !defined(QTHREAD_NO_ASSERTS)
+#if !defined(NDEBUG)
       qthread_worker_id_t wkr = qthread_readstate(CURRENT_UNIQUE_WORKER);
       /* I don't fully understand why this is necessary. I *suspect* that
        * on thread 0, the initialization routine isn't happening
@@ -249,7 +249,7 @@ static qt_mpool_threadlocal_cache_t *qt_mpool_internal_getcache(qt_mpool pool) {
       count_caches = pool->offset;
       TLS_SET(pool_cache_count, count_caches);
     } else if (tc == NULL) {
-#if !defined(QTHREAD_NO_ASSERTS)
+#if !defined(NDEBUG)
       qthread_worker_id_t wkr = qthread_readstate(CURRENT_UNIQUE_WORKER);
       /* I don't fully understand why this is necessary. I *suspect* that
        * on thread 0, the initialization routine isn't happening

diff --git a/src/qthread.c b/src/qthread.c
@@ -196,7 +196,7 @@ static qt_mpool generic_rdata_pool = NULL;
 
 #include "qt_profiling.h"
 
-#ifndef QTHREAD_NO_ASSERTS
+#ifndef NDEBUG
 int qthread_library_initialized = 0;
 void *shep0arg = NULL;
 #endif
@@ -339,7 +339,7 @@ static void *qthread_master(void *arg) {
 
   assert(me != NULL);
   assert(me->shepherd_id <= qlib->nshepherds);
-#ifndef QTHREAD_NO_ASSERTS
+#ifndef NDEBUG
   if ((shep0arg != NULL) && (my_id == 0)) {
     if (arg != shep0arg) {
       print_error("arg = %p, shep0arg = %p\n", arg, shep0arg);
@@ -620,7 +620,7 @@ int API_FUNC qthread_initialize(void) { /*{{{ */
   qt_internal_alignment_init();
   qt_hash_initialize_subsystem();
 
-#ifndef QTHREAD_NO_ASSERTS
+#ifndef NDEBUG
   qthread_library_initialized = 1;
   MACHINE_FENCE;
 #endif
@@ -830,7 +830,7 @@ int API_FUNC qthread_initialize(void) { /*{{{ */
 #endif
                       &(qlib->shepherds[0].workers[0]),
                       &(qlib->mccoy_thread->rdata->context));
-#ifndef QTHREAD_NO_ASSERTS
+#ifndef NDEBUG
   shep0arg = &(qlib->shepherds[0].workers[0]);
 #endif
   /* this launches shepherd 0 */
@@ -1202,7 +1202,7 @@ void API_FUNC qthread_finalize(void) { /*{{{ */
   qlib = NULL;
   TLS_DELETE(shepherd_structs);
 
-#ifndef QTHREAD_NO_ASSERTS
+#ifndef NDEBUG
   MACHINE_FENCE;
   qthread_library_initialized = 0;
   MACHINE_FENCE;

diff --git a/src/sincs/snzi.c b/src/sincs/snzi.c
@@ -2,8 +2,6 @@
 #include "config.h"
 #endif
 
-#define QTHREAD_NO_ASSERTS 1
-
 #include <assert.h>
 #include <stdio.h>
 #include <stdlib.h>

diff --git a/src/threadqueues/nemesis_threadqueues.c b/src/threadqueues/nemesis_threadqueues.c
@@ -197,47 +197,6 @@ void INTERNAL qthread_steal_enable(void) {}
 
 void INTERNAL qthread_steal_disable(void) {}
 
-#ifdef QTHREAD_PARANOIA
-static void sanity_check_tq(NEMESIS_queue *q) { /*{{{*/
-  qt_threadqueue_node_t *curs;
-
-  assert(q);
-  /*if (q->head != NULL) {
-   *  assert(q->tail != NULL);
-   * }*/
-  if (q->shadow_head) { assert(q->head != q->shadow_head); }
-  if (atomic_load_explicit(&q->tail, memory_order_relaxed) != NULL) {
-    if (atomic_load_explicit(&q->head, memory_order_relaxed) == NULL) {
-      assert(q->shadow_head != NULL);
-    }
-  }
-  if ((atomic_load_explicit(&q->head, memory_order_relaxed) != NULL) ||
-      (atomic_load_explicit(&q->tail, memory_order_relaxed) != NULL)) {
-    if (q->shadow_head) {
-      curs = q->shadow_head;
-      assert(curs->thread);
-      assert(curs->thread != (void *)0x7777777777777777);
-      while (atomic_load_explicit(&curs->next, memory_order_relaxed)) {
-        curs = atomic_load_explicit(&curs->next, memory_order_relaxed);
-        assert(curs->thread);
-        assert(curs->thread != (void *)0x7777777777777777);
-      }
-    }
-    if (q->head) {
-      curs = q->head;
-      assert(curs->thread);
-      assert(curs->thread != (void *)0x7777777777777777);
-      while (atomic_load_explicit(&curs->next, memory_order_relaxed)) {
-        curs = atomic_load_explicit(&curs->next, memory_order_relaxed);
-        assert(curs->thread);
-        assert(curs->thread != (void *)0x7777777777777777);
-      }
-    }
-  }
-} /*}}}*/
-
-#endif /* ifdef QTHREAD_PARANOIA */
-
 qthread_shepherd_id_t INTERNAL
 qt_threadqueue_choose_dest(qthread_shepherd_t *curr_shep) {
   qthread_shepherd_id_t dest_shep_id = 0;
@@ -260,8 +219,6 @@ void INTERNAL qt_threadqueue_enqueue(qt_threadqueue_t *restrict q,
   assert(q);
   assert(t);
 
-  PARANOIA(sanity_check_tq(&q->q));
-
   node = ALLOC_TQNODE();
   assert(node != NULL);
   node->thread = t;
@@ -274,7 +231,6 @@ void INTERNAL qt_threadqueue_enqueue(qt_threadqueue_t *restrict q,
   } else {
     atomic_store_explicit(&prev->next, node, memory_order_relaxed);
   }
-  PARANOIA(sanity_check_tq(&q->q));
   (void)qthread_incr(&(q->advisory_queuelen), 1);
 #ifdef QTHREAD_CONDWAIT_BLOCKING_QUEUE
   /* awake waiter */
@@ -310,11 +266,10 @@ qt_scheduler_get_thread(qt_threadqueue_t *q,
 #ifdef QTHREAD_CONDWAIT_BLOCKING_QUEUE
   int i;
 #endif /* QTHREAD_CONDWAIT_BLOCKING_QUEUE */
-  PARANOIA(sanity_check_tq(&q->q));
+
   qt_threadqueue_node_t *node = qt_internal_NEMESIS_dequeue(&q->q);
   qthread_t *retval;
 
-  PARANOIA(sanity_check_tq(&q->q));
   if (node == NULL) {
 #ifdef QTHREAD_CONDWAIT_BLOCKING_QUEUE
     i = num_spins_before_condwait;
@@ -345,7 +300,6 @@ qt_scheduler_get_thread(qt_threadqueue_t *q,
   (void)qthread_incr(&(q->advisory_queuelen), -1);
   retval = node->thread;
   FREE_TQNODE(node);
-  PARANOIA(sanity_check_tq(&q->q));
   return retval;
 } /*}}} */
 
@@ -361,11 +315,8 @@ void INTERNAL qt_threadqueue_filter(qt_threadqueue_t *q,
   atomic_init(&tmp.tail, NULL);
   tmp.shadow_head = NULL;
   tmp.nemesis_advisory_queuelen = 0;
-  PARANOIA(sanity_check_tq(&q->q));
   while ((curs = qt_internal_NEMESIS_dequeue_st(&q->q))) {
     qthread_t *t = curs->thread;
-    PARANOIA(sanity_check_tq(&tmp));
-    PARANOIA(sanity_check_tq(&q->q));
     switch (f(t)) {
       case IGNORE_AND_CONTINUE: // ignore, move on
         prev = qt_internal_atomic_swap_ptr((void **)&(tmp.tail), curs);
@@ -395,7 +346,6 @@ void INTERNAL qt_threadqueue_filter(qt_threadqueue_t *q,
   }
 pushback:
   /* dequeue the rest of the queue */
-  PARANOIA(sanity_check_tq(&tmp));
   if (atomic_load_explicit(&q->q.head, memory_order_relaxed)) {
     prev = qt_internal_atomic_swap_ptr((void **)&(tmp.tail), q->q.head);
     if (prev == NULL) {
@@ -423,7 +373,6 @@ void INTERNAL qt_threadqueue_filter(qt_threadqueue_t *q,
                         memory_order_relaxed);
   q->q.shadow_head = NULL;
   q->advisory_queuelen = tmp.nemesis_advisory_queuelen;
-  PARANOIA(sanity_check_tq(&q->q));
 } /*}}}*/
 
 /* some place-holder functions */