[PATCH][gdbsupport] Avoid main thread in parallel for with thread sanitizer

[PATCH][gdbsupport] Avoid main thread in parallel for with thread sanitizer

[Tom de Vries]

Hi,

When running tasks in a parallel for, some elements may be handled by a worker
thread and others by the main thread.

When running with -fsanitize=thread, avoid the main thread in the parallel
for, to prevent multi-threading problems being hidden.

Tested on x86_64-linux, with -fsanitize=thread.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29359

Any comments?

Thanks,
- Tom

[gdbsupport] Avoid main thread in parallel for with thread sanitizer

---
 gdbsupport/parallel-for.h | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h
index bf40f125f0f..81a2cfa215b 100644
--- a/gdbsupport/parallel-for.h
+++ b/gdbsupport/parallel-for.h
@@ -24,6 +24,15 @@
 #include <type_traits>
 #include "gdbsupport/thread-pool.h"
 
+/* GCC does not understand __has_feature.  */
+#if !defined(__has_feature)
+# define __has_feature(x) 0
+#endif
+
+#if __has_feature(thread_sanitizer)
+#define __SANITIZE_ADDRESS__ 1
+#endif
+
 namespace gdb
 {
 
@@ -161,7 +170,22 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
       /* n_elements == n_threads * elts_per_thread + elts_left_over. */
     }
 
-  size_t count = n_threads == 0 ? 0 : n_threads - 1;
+  /* Number of worker threads we're going to use.  */
+  size_t count;
+#ifdef __SANITIZE_THREAD__
+  /* Avoid the main thread.  */
+  count = (n_threads == 0
+	   ? (/* Only use the first worker thread.  */
+	      1)
+	   : (/* Only use worker threads.  */
+	      n_threads));
+#else
+  count = (n_threads == 0
+	   ? (/* Only use the main thread.  */
+	      0)
+	   : (/* Leave at least one worker thread unused.  */
+	      n_threads - 1));
+#endif
   gdb::detail::par_for_accumulator<result_type> results (count);
 
   if (parallel_for_each_debug)

Re: [PATCH][gdbsupport] Avoid main thread in parallel for with thread sanitizer

[Tom de Vries]

On 7/18/22 12:05, Tom de Vries wrote:
> Hi,
> 
> When running tasks in a parallel for, some elements may be handled by a worker
> thread and others by the main thread.
> 
> When running with -fsanitize=thread, avoid the main thread in the parallel
> for, to prevent multi-threading problems being hidden.
> 

FWIW, in terms of parallel_for_each_debug and the example I've been 
using, with -fsanitize=thread, we have:
...
Parallel for: n_elements: 7271
Parallel for: minimum elements per thread: 10
Parallel for: elts_per_thread: 1817
Parallel for: elements on worker thread 0       : 1818
Parallel for: elements on worker thread 1       : 1818
Parallel for: elements on worker thread 2       : 1818
Parallel for: elements on worker thread 3       : 1817
Parallel for: elements on main thread           : 0
...
and without:
...
Parallel for: elements on worker thread 0       : 1818
Parallel for: elements on worker thread 1       : 1818
Parallel for: elements on worker thread 2       : 1818
Parallel for: elements on worker thread 3       : 0
Parallel for: elements on main thread           : 1817
...

Thanks,
- Tom

> Tested on x86_64-linux, with -fsanitize=thread.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29359
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdbsupport] Avoid main thread in parallel for with thread sanitizer
> 
> ---
>   gdbsupport/parallel-for.h | 26 +++++++++++++++++++++++++-
>   1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h
> index bf40f125f0f..81a2cfa215b 100644
> --- a/gdbsupport/parallel-for.h
> +++ b/gdbsupport/parallel-for.h
> @@ -24,6 +24,15 @@
>   #include <type_traits>
>   #include "gdbsupport/thread-pool.h"
>   
> +/* GCC does not understand __has_feature.  */
> +#if !defined(__has_feature)
> +# define __has_feature(x) 0
> +#endif
> +
> +#if __has_feature(thread_sanitizer)
> +#define __SANITIZE_ADDRESS__ 1
> +#endif
> +
>   namespace gdb
>   {
>   
> @@ -161,7 +170,22 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
>         /* n_elements == n_threads * elts_per_thread + elts_left_over. */
>       }
>   
> -  size_t count = n_threads == 0 ? 0 : n_threads - 1;
> +  /* Number of worker threads we're going to use.  */
> +  size_t count;
> +#ifdef __SANITIZE_THREAD__
> +  /* Avoid the main thread.  */
> +  count = (n_threads == 0
> +	   ? (/* Only use the first worker thread.  */
> +	      1)
> +	   : (/* Only use worker threads.  */
> +	      n_threads));
> +#else
> +  count = (n_threads == 0
> +	   ? (/* Only use the main thread.  */
> +	      0)
> +	   : (/* Leave at least one worker thread unused.  */
> +	      n_threads - 1));
> +#endif
>     gdb::detail::par_for_accumulator<result_type> results (count);
>   
>     if (parallel_for_each_debug)

Re: [PATCH][gdbsupport] Avoid main thread in parallel for with thread sanitizer

[Tom Tromey]

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> When running tasks in a parallel for, some elements may be handled by a worker
Tom> thread and others by the main thread.

Tom> When running with -fsanitize=thread, avoid the main thread in the parallel
Tom> for, to prevent multi-threading problems being hidden.

Tom> Tested on x86_64-linux, with -fsanitize=thread.

Tom> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29359

Tom> Any comments?

I tend to think that different behavior based on the use of a sanitizer
should be avoided.  Otherwise, the sanitizer is not checking what is
actually run.

Tom