[OpenMP] Fix work-stealing stack clobber with taskwait

This patch series demonstrates and fixes a bug that causes crashes with
OpenMP 'taskwait' directives in heavily multi-threaded scenarios.

The implementation of taskwait builds a graph of dependency nodes for
tasks. Some of those dependency nodes (kmp_depnode_t) are allocated
on the stack, and some on the heap. In the former case, the stack is
specific to a given thread, and the task associated with the node is
initially bound to the same thread. This works as long as there is a
1:1 mapping between tasks and the per-thread stack.

However, kmp_tasking.cpp:__kmp_execute_tasks_template implements a
work-stealing algorithm that can take some task 'T1' from some thread's
ready queue (say, thread 'A'), and execute them on another thread (say,
thread 'B').

If that happens, task T1 may have a dependency node on thread A's stack,
and that will *not* be moved to thread B's stack when the work-stealing
takes place.

Now, in a heavily multi-threaded program, *another* task, T2, can be
invoked on thread 'A', re-using the stack slot for thread A at the same
time that T1 is using the same slot from thread 'B'.  This leads to
random crashes, often (but not always) during dependency-node cleanup
(__kmp_release_deps).

This first patch adds some instrumentation to make it more obvious when
the 1:1 mapping between tasks and thread stacks is violated. Typical
output will be (heavily truncated):

on-stack depnode moved from thread 0x5631d7d5c200 to thread 0x5631d7e15c00
Assertion failure at kmp_taskdeps.h(37): !node->dn.on_stack.

Adding debug output also affects the timing such that the bug shows up
much more frequently, at least on the system I'm working on.

Author: jtb20

openmp/runtime/src/kmp.h

@@ -2556,6 +2556,9 @@ typedef struct kmp_base_depnode {
 #endif
   std::atomic<kmp_int32> npredecessors;
   std::atomic<kmp_int32> nrefs;
+#if KMP_DEBUG
+  void *on_stack;
+#endif
 } kmp_base_depnode_t;
 
 union KMP_ALIGN_CACHE kmp_depnode {

openmp/runtime/src/kmp_taskdeps.cpp

@@ -48,6 +48,9 @@ static void __kmp_init_node(kmp_depnode_t *node) {
 #if USE_ITT_BUILD && USE_ITT_NOTIFY
   __itt_sync_create(node, "OMP task dep node", NULL, 0);
 #endif
+#if KMP_DEBUG
+  node->dn.on_stack = NULL;
+#endif
 }
 
 static inline kmp_depnode_t *__kmp_node_ref(kmp_depnode_t *node) {
@@ -1008,6 +1011,9 @@ void __kmpc_omp_taskwait_deps_51(ident_t *loc_ref, kmp_int32 gtid,
 
   kmp_depnode_t node = {0};
   __kmp_init_node(&node);
+#if KMP_DEBUG
+  node.dn.on_stack = thread;
+#endif
 
   if (!__kmp_check_deps(gtid, &node, NULL, &current_task->td_dephash,
                         DEP_BARRIER, ndeps, dep_list, ndeps_noalias,
@@ -1033,8 +1039,10 @@ void __kmpc_omp_taskwait_deps_51(ident_t *loc_ref, kmp_int32 gtid,
   // Wait until the last __kmp_release_deps is finished before we free the
   // current stack frame holding the "node" variable; once its nrefs count
   // reaches 1, we're sure nobody else can try to reference it again.
-  while (node.dn.nrefs > 1)
+  kmp_int32 nrefs;
+  while ((nrefs = node.dn.nrefs) > 1)
     KMP_YIELD(TRUE);
+  KMP_DEBUG_ASSERT(nrefs == 1);
 
 #if OMPT_SUPPORT
   __ompt_taskwait_dep_finish(current_task, taskwait_task_data);

openmp/runtime/src/kmp_taskdeps.h

@@ -22,12 +22,19 @@ static inline void __kmp_node_deref(kmp_info_t *thread, kmp_depnode_t *node) {
   if (!node)
     return;
 
+#if KMP_DEBUG
+  if (node->dn.on_stack && node->dn.on_stack != thread)
+    fprintf (stderr, "on-stack depnode moved from thread %p to thread %p\n",
+             node->dn.on_stack, thread);
+#endif
+
   kmp_int32 n = KMP_ATOMIC_DEC(&node->dn.nrefs) - 1;
   KMP_DEBUG_ASSERT(n >= 0);
   if (n == 0) {
 #if USE_ITT_BUILD && USE_ITT_NOTIFY
     __itt_sync_destroy(node);
 #endif
+    KMP_DEBUG_ASSERT(!node->dn.on_stack);
     KMP_ASSERT(node->dn.nrefs == 0);
 #if USE_FAST_MEMORY
     __kmp_fast_free(thread, node);