[Openmp-commits] [PATCH] D63599: Fixed memory use-after-free problem.

[Andrey Churbanov via Phabricator via Openmp-commits]

AndreyChurbanov created this revision.
AndreyChurbanov added reviewers: tlwilmar, hbae.
AndreyChurbanov added a project: OpenMP.
Herald added a subscriber: jdoerfert.

Bug reported in https://bugs.llvm.org/show_bug.cgi?id=42269.

Problem caused by recent attempt to fix memory leak on the same memory block.
Investigation showed that worker threads may leave contention group either before or after the master thread depending on the number of teams in the teams construct.
This patch makes freeing of the contention group structure conditional for master thread, and adds similar conditional freeing for worker threads. Thus both the memory leak and the use-after-free problems are fixed now.

Do not add test because the problem was only visible if library is instrumented by memory sanitizer.


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D63599

Files:
  runtime/src/kmp_csupport.cpp
  runtime/src/kmp_runtime.cpp


Index: runtime/src/kmp_runtime.cpp
===================================================================
--- runtime/src/kmp_runtime.cpp
+++ runtime/src/kmp_runtime.cpp
@@ -5695,6 +5695,9 @@
       this_th->th.th_cg_roots = tmp->up;
       __kmp_free(tmp);
     } else { // Worker thread
+      if (tmp->cg_nthreads == 0) { // last thread leaves contention group
+        __kmp_free(tmp);
+      }
       this_th->th.th_cg_roots = NULL;
       break;
     }
Index: runtime/src/kmp_csupport.cpp
===================================================================
--- runtime/src/kmp_csupport.cpp
+++ runtime/src/kmp_csupport.cpp
@@ -440,7 +440,11 @@
   KA_TRACE(100, ("__kmpc_fork_teams: Thread %p popping node %p and moving up"
                  " to node %p. cg_nthreads was %d\n",
                  this_thr, tmp, this_thr->th.th_cg_roots, tmp->cg_nthreads));
-  __kmp_free(tmp);
+  KMP_DEBUG_ASSERT(tmp->cg_nthreads);
+  int i = tmp->cg_nthreads--;
+  if (i == 1) { // check is we are the last thread in CG (not always the case)
+    __kmp_free(tmp);
+  }
   // Restore current task's thread_limit from CG root
   KMP_DEBUG_ASSERT(this_thr->th.th_cg_roots);
   this_thr->th.th_current_task->td_icvs.thread_limit =


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63599.205789.patch
Type: text/x-patch
Size: 1215 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20190620/9c09f162/attachment.bin>