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

[Andrey Churbanov via Phabricator via Openmp-commits]

AndreyChurbanov updated this revision to Diff 205991.
AndreyChurbanov added a comment.

Covered more cases caused memory leak (when target-teams region is followed by parallel with bigger number of threads) and use-after-free problem (when nested hot teams requested via KMP_HOT_TEAMS_MAX_LEVEL=2 and num_teams is bigger than 1).  The leak is fixed by freeing CG structure when worker threads inherit master's CG but had earlier non-NULL CG.  Use-after-free fixed by making one more freeing of CG conditional (in __kmp_free_team routine).


Repository:
  rOMP OpenMP

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63599/new/

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
@@ -4196,6 +4196,17 @@
       this_thr->th.th_cg_roots != master->th.th_cg_roots) { // CG root not set
     // Make new thread's CG root same as master's
     KMP_DEBUG_ASSERT(master->th.th_cg_roots);
+    kmp_cg_root_t *tmp = this_thr->th.th_cg_roots;
+    if (tmp) {
+      // worker changes CG, need to check if old CG should be freed
+      int i = tmp->cg_nthreads--;
+      KA_TRACE(100, ("__kmp_initialize_info: Thread %p decrement cg_nthreads"
+                     " on node %p of thread %p to %d\n",
+                     this_thr, tmp, tmp->cg_root, tmp->cg_nthreads));
+      if (i == 1) {
+        __kmp_free(tmp); // last thread left CG --> free it
+      }
+    }
     this_thr->th.th_cg_roots = master->th.th_cg_roots;
     // Increment new thread's CG root's counter to add the new thread
     this_thr->th.th_cg_roots->cg_nthreads++;
@@ -5594,7 +5605,10 @@
         KA_TRACE(100, ("__kmp_free_team: Thread %p popping node %p and moving"
                        " up to node %p. cg_nthreads was %d\n",
                        thr, tmp, thr->th.th_cg_roots, tmp->cg_nthreads));
-        __kmp_free(tmp);
+        int i = tmp->cg_nthreads--;
+        if (i == 1) {
+          __kmp_free(tmp); // free CG if we are the last thread in it
+        }
         // Restore current task's thread_limit from CG root
         if (thr->th.th_cg_roots)
           thr->th.th_current_task->td_icvs.thread_limit =
@@ -5695,6 +5709,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;
     }
@@ -7223,7 +7240,7 @@
   tmp->cg_thread_limit = thr->th.th_current_task->td_icvs.thread_limit;
   tmp->cg_nthreads = 1; // Init counter to one active thread, this one
   KA_TRACE(100, ("__kmp_teams_master: Thread %p created node %p and init"
-                 " cg_threads to 1\n",
+                 " cg_nthreads to 1\n",
                  thr, tmp));
   tmp->up = thr->th.th_cg_roots;
   thr->th.th_cg_roots = tmp;
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.205991.patch
Type: text/x-patch
Size: 3048 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20190621/fb310547/attachment.bin>