[Openmp-commits] [openmp] ada4ad9 - [OpenMP] Fix taskgraph dependency tracking, memory access, and initialization (#136837)

[via Openmp-commits]

Author: Josep Pinot
Date: 2025-04-25T10:17:07+02:00
New Revision: ada4ad9d1f5580476918d490cb51382714cb2973

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

LOG: [OpenMP] Fix taskgraph dependency tracking, memory access, and initialization (#136837)

This commit resolves multiple issues in the OpenMP taskgraph implementation:
- Fix a potential use of uninitialized is_taskgraph and tdg fields when a task is created outside of a taskgraph construct.
- Fix use of task ID field when accessing the taskgraph’s record_map.
- Fix resizing and copying of the successors array when its capacity is exceeded.

Fixes memory management flaws, invalid memory accesses, and uninitialized data risks in taskgraph operations.

Added: 
    openmp/runtime/test/tasking/omp_record_replay_deps_multi_succ.cpp

Modified: 
    openmp/runtime/src/kmp_taskdeps.cpp
    openmp/runtime/src/kmp_tasking.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/runtime/src/kmp_taskdeps.cpp b/openmp/runtime/src/kmp_taskdeps.cpp
index e002877aaead2..abbca752f0587 100644
--- a/openmp/runtime/src/kmp_taskdeps.cpp
+++ b/openmp/runtime/src/kmp_taskdeps.cpp
@@ -243,10 +243,12 @@ static inline void __kmp_track_dependence(kmp_int32 gtid, kmp_depnode_t *source,
     }
     if (!exists) {
       if (source_info->nsuccessors >= source_info->successors_size) {
+        kmp_uint old_size = source_info->successors_size;
         source_info->successors_size = 2 * source_info->successors_size;
         kmp_int32 *old_succ_ids = source_info->successors;
         kmp_int32 *new_succ_ids = (kmp_int32 *)__kmp_allocate(
             source_info->successors_size * sizeof(kmp_int32));
+        KMP_MEMCPY(new_succ_ids, old_succ_ids, old_size * sizeof(kmp_int32));
         source_info->successors = new_succ_ids;
         __kmp_free(old_succ_ids);
       }
@@ -698,9 +700,9 @@ kmp_int32 __kmpc_omp_task_with_deps(ident_t *loc_ref, kmp_int32 gtid,
       __kmp_tdg_is_recording(new_taskdata->tdg->tdg_status)) {
     kmp_tdg_info_t *tdg = new_taskdata->tdg;
     // extend record_map if needed
-    if (new_taskdata->td_task_id >= tdg->map_size) {
+    if (new_taskdata->td_tdg_task_id >= tdg->map_size) {
       __kmp_acquire_bootstrap_lock(&tdg->graph_lock);
-      if (new_taskdata->td_task_id >= tdg->map_size) {
+      if (new_taskdata->td_tdg_task_id >= tdg->map_size) {
         kmp_uint old_size = tdg->map_size;
         kmp_uint new_size = old_size * 2;
         kmp_node_info_t *old_record = tdg->record_map;

diff  --git a/openmp/runtime/src/kmp_tasking.cpp b/openmp/runtime/src/kmp_tasking.cpp
index ea123dcb05a63..3d85a29423540 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -1614,6 +1614,8 @@ kmp_task_t *__kmp_task_alloc(ident_t *loc_ref, kmp_int32 gtid,
   taskdata->td_flags.freed = 0;
 #if OMPX_TASKGRAPH
   taskdata->td_flags.onced = 0;
+  taskdata->is_taskgraph = 0;
+  taskdata->tdg = nullptr;
 #endif
   KMP_ATOMIC_ST_RLX(&taskdata->td_incomplete_child_tasks, 0);
   // start at one because counts current task and children

diff  --git a/openmp/runtime/test/tasking/omp_record_replay_deps_multi_succ.cpp b/openmp/runtime/test/tasking/omp_record_replay_deps_multi_succ.cpp
new file mode 100644
index 0000000000000..906fab335f510
--- /dev/null
+++ b/openmp/runtime/test/tasking/omp_record_replay_deps_multi_succ.cpp
@@ -0,0 +1,56 @@
+// REQUIRES: ompx_taskgraph
+// RUN: %libomp-cxx-compile-and-run
+#include <omp.h>
+#include <cassert>
+#include <vector>
+
+constexpr const int TASKS_SIZE = 12;
+
+typedef struct ident ident_t;
+
+extern "C" {
+int __kmpc_global_thread_num(ident_t *);
+int __kmpc_start_record_task(ident_t *, int, int, int);
+void __kmpc_end_record_task(ident_t *, int, int, int);
+}
+
+void init(int &A, int val) { A = val; }
+
+void update(int &A, int &B, int val) { A = B + val; }
+
+void test(int nb, std::vector<std::vector<int>> &Ah) {
+#pragma omp parallel
+#pragma omp single
+  {
+    int gtid = __kmpc_global_thread_num(nullptr);
+    int res = __kmpc_start_record_task(nullptr, gtid, 0, 0);
+    if (res) {
+      for (int k = 0; k < nb; ++k) {
+#pragma omp task depend(inout : Ah[k][0])
+        init(Ah[k][0], k);
+
+        for (int i = 1; i < nb; ++i) {
+#pragma omp task depend(in : Ah[k][0]) depend(out : Ah[k][i])
+          update(Ah[k][i], Ah[k][0], 1);
+        }
+      }
+    }
+    __kmpc_end_record_task(nullptr, gtid, 0, 0);
+  }
+}
+
+int main() {
+  std::vector<std::vector<int>> matrix(TASKS_SIZE,
+                                       std::vector<int>(TASKS_SIZE, 0));
+
+  test(TASKS_SIZE, matrix);
+  test(TASKS_SIZE, matrix);
+
+  for (int k = 0; k < TASKS_SIZE; ++k) {
+    assert(matrix[k][0] == k);
+    for (int i = 1; i < TASKS_SIZE; ++i) {
+      assert(matrix[k][i] == k + 1);
+    }
+  }
+  return 0;
+}