[Openmp-commits] [openmp] [OpenMP] Fix work-stealing stack clobber with taskwait (PR #126049)

[Julian Brown via Openmp-commits]

jtb20 wrote:

> TBH at the first glance I think the fix makes sense but after looking it more, I'm not sure I understand the underlying issue. I don't follow why and when `node` could be "overridden". Isn't it a stack corruption?

Yes, it's a stack corruption bug. Two different tasks on two different threads access the same, supposedly thread-local stack for **one** of those threads. E.g:

In kmp_tasking.cpp:__kmp_execute_tasks_template, the first thread steals a task (__kmp_steal_task), then immediately executes it (__kmp_invoke_task). __kmp_invoke_task calls __kmp_release_deps via __kmp_task_finish.

The second thread executes by the non-task-stealing path: __kmp_remove_my_task called from __kmp_execute_tasks_template, then __kmp_invoke_task, etc. as above.

But, on the task-stealing path, the task's depnode pointer is still pointing to the stack from its **original** thread, not the one it actually executes on (the stack frame that actually lives in __kmpc_omp_taskwait_deps_51, that the fix for PR85963 took pains to keep alive). So when the second thread comes along and allocates a "new" depnode, it's actually using a chunk of memory that is still in use by the first thread.

An alternative fix might be to add more locking in an appropriate place, but that seems like it'd be more error-prone and probably slower.

https://github.com/llvm/llvm-project/pull/126049