[Openmp-commits] [openmp] cb1bee4 - [OpenMP] libomp: fix UB when LIBOMP_NUM_HIDDEN_HELPER_THREADS=1.

[via Openmp-commits]

Author: AndreyChurbanov
Date: 2022-02-12T03:00:38+03:00
New Revision: cb1bee4725c475bd32df3f4ce9ba2ebe30dcf9de

URL: https://github.com/llvm/llvm-project/commit/cb1bee4725c475bd32df3f4ce9ba2ebe30dcf9de
DIFF: https://github.com/llvm/llvm-project/commit/cb1bee4725c475bd32df3f4ce9ba2ebe30dcf9de.diff

LOG: [OpenMP] libomp: fix UB when LIBOMP_NUM_HIDDEN_HELPER_THREADS=1.

The __kmp_hidden_helper_threads_num set to N+1 if user requested N threads.
Thus number of worker hidden helper threads corresponds to user request,
main thread of helper team excluded as it does not participate in actual work.
This also fixes divide-by-0 issue in the code.

Fixes #48656

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

Added: 
    openmp/runtime/test/tasking/hidden_helper_task/single_helper_thread.c

Modified: 
    openmp/runtime/src/kmp_settings.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/runtime/src/kmp_settings.cpp b/openmp/runtime/src/kmp_settings.cpp
index 27ab51dbf7ed5..481f605fcd5ae 100644
--- a/openmp/runtime/src/kmp_settings.cpp
+++ b/openmp/runtime/src/kmp_settings.cpp
@@ -1245,13 +1245,25 @@ static void __kmp_stg_parse_num_hidden_helper_threads(char const *name,
   // task
   if (__kmp_hidden_helper_threads_num == 0) {
     __kmp_enable_hidden_helper = FALSE;
+  } else {
+    // Since the main thread of hidden helper team dooes not participate
+    // in tasks execution let's increment the number of threads by one
+    // so that requested number of threads do actual job.
+    __kmp_hidden_helper_threads_num++;
   }
 } // __kmp_stg_parse_num_hidden_helper_threads
 
 static void __kmp_stg_print_num_hidden_helper_threads(kmp_str_buf_t *buffer,
                                                       char const *name,
                                                       void *data) {
-  __kmp_stg_print_int(buffer, name, __kmp_hidden_helper_threads_num);
+  if (__kmp_hidden_helper_threads_num == 0) {
+    __kmp_stg_print_int(buffer, name, __kmp_hidden_helper_threads_num);
+  } else {
+    KMP_DEBUG_ASSERT(__kmp_hidden_helper_threads_num > 1);
+    // Let's exclude the main thread of hidden helper team and print
+    // number of worker threads those do actual job.
+    __kmp_stg_print_int(buffer, name, __kmp_hidden_helper_threads_num - 1);
+  }
 } // __kmp_stg_print_num_hidden_helper_threads
 
 static void __kmp_stg_parse_use_hidden_helper(char const *name,

diff  --git a/openmp/runtime/test/tasking/hidden_helper_task/single_helper_thread.c b/openmp/runtime/test/tasking/hidden_helper_task/single_helper_thread.c
new file mode 100644
index 0000000000000..a1aeda76e22fa
--- /dev/null
+++ b/openmp/runtime/test/tasking/hidden_helper_task/single_helper_thread.c
@@ -0,0 +1,21 @@
+// RUN: %libomp-compile && env LIBOMP_NUM_HIDDEN_HELPER_THREADS=1 %libomp-run
+
+// The test checks that "devide-by-0" bug fixed in runtime.
+// The fix is to increment number of threads by 1 if positive,
+// so that operation
+//   (gtid) % (__kmp_hidden_helper_threads_num - 1)
+// does not cause crash.
+
+#include <stdio.h>
+#include <omp.h>
+
+int main(){
+#pragma omp target nowait
+   {
+      printf("----- in  target region\n");
+   }
+  printf("------ before taskwait\n");
+#pragma omp taskwait
+  printf("passed\n");
+  return 0;
+}