Buffer overflow in PotentialTree with OpenMP #93
Labels
bug
Something isn't working
Comments
rtobar
added a commit
that referenced
this issue
Jun 23, 2021
There were a couple of problems with the old version of this code. Firstly, it had an unused maxthreads variable, which has now been removed. Secondly, and more importantly, it used a complex and incorrect way of calculating the number of threads (stored in the nthreads variable) that would be used in new "omp parallel" blocks: instead of directly calling omp_get_max_threads and writing its result into nthreads (the new behavior) it opened an "omp parallel" block and called omp_get_num_threads, only on the first thread of the newly created thread team. The result was then written back to nthreads. Although this seems fine, it creates a race condition, as there is no guarantee that thread #0 in the newly formed team is the same thread from which the team was created. This in turn means that the write to nthreads is not necessarily visible immediately from the parent thread. The nthreads value is later on used to allocate memory to be used by "omp parallel" regions; having a wrong value (indeed the original 1 to which nthreads is initialised to at declaration time) leads to wrongly-sized memory blocks, and to invalid writes down the line. We saw this problem when running with clang's address sanitizer, which led us to find this particular bug in the code. This problem was reported in #93. Signed-off-by: Rodrigo Tobar <[email protected]>
|
As mentioned in 511767e this was a race condition in the calculation of available OpenMP threads. With the commit above I don't get the errors anymore, so I'll merge to the |
|
This has been solved now and the fix has been merged to the |
rtobar
added a commit
that referenced
this issue
Jun 24, 2021
This is similar to the error detected in #93 and fixed in 511767e, but with the difference that in this case it doesn't seem to cause issues in real life, as most probably at this point of the execution thread #0 of the newly created team corresponds to the main thread executing the rest of the code, so the write to nthreads *is* visible. Signed-off-by: Rodrigo Tobar <[email protected]>
rtobar
added a commit
that referenced
this issue
Jun 24, 2021
This is similar to the error detected in #93 and fixed in 511767e, but with the difference that in this case it doesn't seem to cause issues in real life, as most probably at this point of the execution thread #0 of the newly created team corresponds to the main thread executing the rest of the code, so the write to nthreads *is* visible. Signed-off-by: Rodrigo Tobar <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When running with the inputs and configuration from #87/#88 with 4 OpenMP threads, 1 MPI rank and compiling with clang address sanitizer I got the following output:
The text was updated successfully, but these errors were encountered: