[Openmp-commits] [PATCH] D84472: [OpenMP] Fix releasing of stack memory

[Joachim Protze via Phabricator via Openmp-commits]

protze.joachim created this revision.
protze.joachim added reviewers: jlpeyton, AndreyChurbanov.
Herald added subscribers: sstefan1, guansong, yaxunl.
Herald added a reviewer: jdoerfert.
Herald added a project: OpenMP.

Starting with 787eb0c637b <https://reviews.llvm.org/rG787eb0c637b26ce88e91403584b016a42ab5d59c> I got spurious segmentation faults for some testcases. I could nail it down to `brel` trying to release the "memory" of the node allocated on the stack of __kmpc_omp_wait_deps. With this patch, you will see the assertion triggering for some of the tests in the test suite.

My proposed solution for the issue is to just patch __kmpc_omp_wait_deps:

    __kmp_init_node(&node);
  -  node.dn.on_stack = 1;
  +  // the stack owns the node
  +  __kmp_node_ref(&node);

What do you think?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84472

Files:
  openmp/runtime/src/kmp.h
  openmp/runtime/src/kmp_taskdeps.cpp
  openmp/runtime/src/kmp_taskdeps.h


Index: openmp/runtime/src/kmp_taskdeps.h
===================================================================
--- openmp/runtime/src/kmp_taskdeps.h
+++ openmp/runtime/src/kmp_taskdeps.h
@@ -21,12 +21,13 @@
 #define KMP_RELEASE_DEPNODE(gtid, n) __kmp_release_lock(&(n)->dn.lock, (gtid))
 
 static inline void __kmp_node_deref(kmp_info_t *thread, kmp_depnode_t *node) {
-  if (!node)
+  if (!node /*|| node->dn.on_stack*/)
     return;
 
   kmp_int32 n = KMP_ATOMIC_DEC(&node->dn.nrefs) - 1;
   if (n == 0) {
     KMP_ASSERT(node->dn.nrefs == 0);
+    KMP_ASSERT(node->dn.on_stack != 0);
 #if USE_FAST_MEMORY
     __kmp_fast_free(thread, node);
 #else
Index: openmp/runtime/src/kmp_taskdeps.cpp
===================================================================
--- openmp/runtime/src/kmp_taskdeps.cpp
+++ openmp/runtime/src/kmp_taskdeps.cpp
@@ -35,6 +35,7 @@
 
 static void __kmp_init_node(kmp_depnode_t *node) {
   node->dn.successors = NULL;
+  node->dn.on_stack = 0;
   node->dn.task = NULL; // will point to the right task
   // once dependences have been processed
   for (int i = 0; i < MAX_MTX_DEPS; ++i)
@@ -771,6 +772,7 @@
 
   kmp_depnode_t node = {0};
   __kmp_init_node(&node);
+  node.dn.on_stack = 1;
 
   if (!__kmp_check_deps(gtid, &node, NULL, &current_task->td_dephash,
                         DEP_BARRIER, ndeps, dep_list, ndeps_noalias,
Index: openmp/runtime/src/kmp.h
===================================================================
--- openmp/runtime/src/kmp.h
+++ openmp/runtime/src/kmp.h
@@ -2154,6 +2154,7 @@
   kmp_task_t *task; /* non-NULL if depnode is active, used under lock */
   kmp_lock_t *mtx_locks[MAX_MTX_DEPS]; /* lock mutexinoutset dependent tasks */
   kmp_int32 mtx_num_locks; /* number of locks in mtx_locks array */
+  kmp_int32 on_stack; /* the node is allocated on stack */
   kmp_lock_t lock; /* guards shared fields: task, successors */
 #if KMP_SUPPORT_GRAPH_OUTPUT
   kmp_uint32 id;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84472.280282.patch
Type: text/x-patch
Size: 1934 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20200723/fefa345d/attachment-0001.bin>