Skip to content

Commit ada4ad9

Browse files
authored
[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.
1 parent 86cca00 commit ada4ad9

File tree

3 files changed

+62
-2
lines changed

3 files changed

+62
-2
lines changed

openmp/runtime/src/kmp_taskdeps.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,12 @@ static inline void __kmp_track_dependence(kmp_int32 gtid, kmp_depnode_t *source,
243243
}
244244
if (!exists) {
245245
if (source_info->nsuccessors >= source_info->successors_size) {
246+
kmp_uint old_size = source_info->successors_size;
246247
source_info->successors_size = 2 * source_info->successors_size;
247248
kmp_int32 *old_succ_ids = source_info->successors;
248249
kmp_int32 *new_succ_ids = (kmp_int32 *)__kmp_allocate(
249250
source_info->successors_size * sizeof(kmp_int32));
251+
KMP_MEMCPY(new_succ_ids, old_succ_ids, old_size * sizeof(kmp_int32));
250252
source_info->successors = new_succ_ids;
251253
__kmp_free(old_succ_ids);
252254
}
@@ -698,9 +700,9 @@ kmp_int32 __kmpc_omp_task_with_deps(ident_t *loc_ref, kmp_int32 gtid,
698700
__kmp_tdg_is_recording(new_taskdata->tdg->tdg_status)) {
699701
kmp_tdg_info_t *tdg = new_taskdata->tdg;
700702
// extend record_map if needed
701-
if (new_taskdata->td_task_id >= tdg->map_size) {
703+
if (new_taskdata->td_tdg_task_id >= tdg->map_size) {
702704
__kmp_acquire_bootstrap_lock(&tdg->graph_lock);
703-
if (new_taskdata->td_task_id >= tdg->map_size) {
705+
if (new_taskdata->td_tdg_task_id >= tdg->map_size) {
704706
kmp_uint old_size = tdg->map_size;
705707
kmp_uint new_size = old_size * 2;
706708
kmp_node_info_t *old_record = tdg->record_map;

openmp/runtime/src/kmp_tasking.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1614,6 +1614,8 @@ kmp_task_t *__kmp_task_alloc(ident_t *loc_ref, kmp_int32 gtid,
16141614
taskdata->td_flags.freed = 0;
16151615
#if OMPX_TASKGRAPH
16161616
taskdata->td_flags.onced = 0;
1617+
taskdata->is_taskgraph = 0;
1618+
taskdata->tdg = nullptr;
16171619
#endif
16181620
KMP_ATOMIC_ST_RLX(&taskdata->td_incomplete_child_tasks, 0);
16191621
// start at one because counts current task and children
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// REQUIRES: ompx_taskgraph
2+
// RUN: %libomp-cxx-compile-and-run
3+
#include <omp.h>
4+
#include <cassert>
5+
#include <vector>
6+
7+
constexpr const int TASKS_SIZE = 12;
8+
9+
typedef struct ident ident_t;
10+
11+
extern "C" {
12+
int __kmpc_global_thread_num(ident_t *);
13+
int __kmpc_start_record_task(ident_t *, int, int, int);
14+
void __kmpc_end_record_task(ident_t *, int, int, int);
15+
}
16+
17+
void init(int &A, int val) { A = val; }
18+
19+
void update(int &A, int &B, int val) { A = B + val; }
20+
21+
void test(int nb, std::vector<std::vector<int>> &Ah) {
22+
#pragma omp parallel
23+
#pragma omp single
24+
{
25+
int gtid = __kmpc_global_thread_num(nullptr);
26+
int res = __kmpc_start_record_task(nullptr, gtid, 0, 0);
27+
if (res) {
28+
for (int k = 0; k < nb; ++k) {
29+
#pragma omp task depend(inout : Ah[k][0])
30+
init(Ah[k][0], k);
31+
32+
for (int i = 1; i < nb; ++i) {
33+
#pragma omp task depend(in : Ah[k][0]) depend(out : Ah[k][i])
34+
update(Ah[k][i], Ah[k][0], 1);
35+
}
36+
}
37+
}
38+
__kmpc_end_record_task(nullptr, gtid, 0, 0);
39+
}
40+
}
41+
42+
int main() {
43+
std::vector<std::vector<int>> matrix(TASKS_SIZE,
44+
std::vector<int>(TASKS_SIZE, 0));
45+
46+
test(TASKS_SIZE, matrix);
47+
test(TASKS_SIZE, matrix);
48+
49+
for (int k = 0; k < TASKS_SIZE; ++k) {
50+
assert(matrix[k][0] == k);
51+
for (int i = 1; i < TASKS_SIZE; ++i) {
52+
assert(matrix[k][i] == k + 1);
53+
}
54+
}
55+
return 0;
56+
}

0 commit comments

Comments
 (0)