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

[Andrey Churbanov via Phabricator via Openmp-commits]

AndreyChurbanov marked an inline comment as done.
AndreyChurbanov added inline comments.


================
Comment at: runtime/src/kmp_csupport.cpp:444
+  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)
----------------
Hahnfeld wrote:
> Is this safe without atomics? I've only briefly looked over the code, most other usages of `cg_nthreads` are protected by `__kmp_forkjoin_lock` (at least according to comments). I don't see this here, but maybe I'm missing the relevant call.
I cannot technically prove this unfortunately, but looks like there is no need in synchronization here.  At least for now.  The reason is - for a particular contention group (CG) when master thread touches the counter its workers either already been freed of wait in a hot team. In both cases no actions can be performed in parallel with the counter.

Trying to look in more details, I found couple more cases when library produces memory leak and use-after-free problems. I'll update the patch shortly

Though the code still has problems those should be addressed in other patches.  Things like many consecutive target-teams regions executed on host, or target-teams on host with following parallel region are not correctly maintained currently.  To be fixed later.


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D63599