Fixed memory use-after-free problem.

Bug reported in https://bugs.llvm.org/show_bug.cgi?id=42269.
Freeing of the contention group (CG) stucture by master thread looks wrong,
because workers can leave the CG later on. Intead the freeing
is now done by the last thread leaving the CG.

Differential Revision: https://reviews.llvm.org/D63599

llvm-svn: 364456

2 files changed, 24 insertions, 3 deletions

diff --git a/openmp/runtime/src/kmp_csupport.cpp b/openmp/runtime/src/kmp_csupport.cpp
index 8e989786202..eff84e11eb3 100644
--- a/openmp/runtime/src/kmp_csupport.cpp
+++ b/openmp/runtime/src/kmp_csupport.cpp
@@ -440,7 +440,11 @@ void __kmpc_fork_teams(ident_t *loc, kmp_int32 argc, kmpc_micro microtask,
   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 =
diff --git a/openmp/runtime/src/kmp_runtime.cpp b/openmp/runtime/src/kmp_runtime.cpp
index 3f1dbfd6f69..4167d199d9a 100644
--- a/openmp/runtime/src/kmp_runtime.cpp
+++ b/openmp/runtime/src/kmp_runtime.cpp
@@ -4196,6 +4196,17 @@ static void __kmp_initialize_info(kmp_info_t *this_thr, kmp_team_t *team,
       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 @@ void __kmp_free_team(kmp_root_t *root,
         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 @@ void __kmp_free_thread(kmp_info_t *this_th) {
       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 @@ void __kmp_teams_master(int gtid) {
   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;