diff mbox series

[1/2] configure: add support for thread sanitizer (--enable-tsan)

Message ID 20200312113158.24055-1-maennich@google.com
State Committed
Headers
Series [1/2] configure: add support for thread sanitizer (--enable-tsan) |

Commit Message

Matthias Männich March 12, 2020, 11:31 a.m. UTC 
  Similarly to asan and ubsan, add support for tsan conditionally at
configure time. This allows us to track down race conditions etc.

	* configure.ac: Add configure options for -fsanitize=thread

Signed-off-by: Matthias Maennich <maennich@google.com>
---
 configure.ac | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Giuliano Procida March 16, 2020, 3 p.m. UTC | #1 
On Thu, 12 Mar 2020 at 11:32, 'Matthias Maennich' via kernel-team
<kernel-team@android.com> wrote:
>
> Similarly to asan and ubsan, add support for tsan conditionally at
> configure time. This allows us to track down race conditions etc.
>
>         * configure.ac: Add configure options for -fsanitize=thread

Please verify that there's no need to add the various flags to LDFLAGS
(or no difference if you do so) thanks to CXX anf CFLAGS.
msan (for which there isn't support yet; it also requires Clang and
lld in particular) definitely requires its flag at link time.

Assuming this is the case:
Reviewed-by: Giuliano Procida <gprocida@google.com>

It's also noticeable that turning on some sanitisers affects abidiff
output. This can't be good and we should look into it when we have
time.

Regards,
Giuliano.

> Signed-off-by: Matthias Maennich <maennich@google.com>
> ---
>  configure.ac | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index ba800a72ac7a..50a50c6c8932 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -120,6 +120,12 @@ AC_ARG_ENABLE(asan,
>               ENABLE_ASAN=$enableval,
>               ENABLE_ASAN=no)
>
> +AC_ARG_ENABLE(tsan,
> +             AS_HELP_STRING([--enable-tsan=yes|no],
> +                            [enable the support of building with -fsanitize=thread)]),
> +             ENABLE_TSAN=$enableval,
> +             ENABLE_TSAN=no)
> +
>  AC_ARG_ENABLE(ubsan,
>               AS_HELP_STRING([--enable-ubsan=yes|no],
>                              [enable the support of building with -fsanitize=undefined)]),
> @@ -614,6 +620,11 @@ if test x$ENABLE_ASAN = xyes; then
>      CXXFLAGS="$CXXFLAGS -fsanitize=address"
>  fi
>
> +if test x$ENABLE_TSAN = xyes; then
> +    CFLAGS="$CFLAGS -fsanitize=thread"
> +    CXXFLAGS="$CXXFLAGS -fsanitize=thread"
> +fi
> +
>  if test x$ENABLE_UBSAN = xyes; then
>      CFLAGS="$CFLAGS -fsanitize=undefined"
>      CXXFLAGS="$CXXFLAGS -fsanitize=undefined"
> @@ -907,6 +918,7 @@ AC_MSG_NOTICE([
>      Enable python 3                               : ${ENABLE_PYTHON3}
>      Enable running tests under Valgrind            : ${enable_valgrind}
>      Enable build with -fsanitize=address          : ${ENABLE_ASAN}
> +    Enable build with -fsanitize=thread           : ${ENABLE_TSAN}
>      Enable build with -fsanitize=undefined        : ${ENABLE_UBSAN}
>      Generate html apidoc                          : ${ENABLE_APIDOC}
>      Generate html manual                          : ${ENABLE_MANUAL}
> --
> 2.25.1.481.gfbce0eb801-goog
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
Matthias Männich March 17, 2020, 11:18 p.m. UTC | #2 
Hi!

On Mon, Mar 16, 2020 at 03:00:02PM +0000, Giuliano Procida wrote:
>On Thu, 12 Mar 2020 at 11:32, 'Matthias Maennich' via kernel-team
><kernel-team@android.com> wrote:
>>
>> Similarly to asan and ubsan, add support for tsan conditionally at
>> configure time. This allows us to track down race conditions etc.
>>
>>         * configure.ac: Add configure options for -fsanitize=thread
>
>Please verify that there's no need to add the various flags to LDFLAGS
>(or no difference if you do so) thanks to CXX anf CFLAGS.
>msan (for which there isn't support yet; it also requires Clang and

Will post a patch for msan right away :-)

>lld in particular) definitely requires its flag at link time.

Autotools generates the makefiles such that CXX or CC is used for
linking and that gets passed the CXXFLAGS / CFLAGS. Having the compiler
drive the link step allows it to translate the compile flag into the
full set of linker flags. Hence, we do not need to set any LDFLAGS.

TL;DR: we are good :-)

>
>Assuming this is the case:
>Reviewed-by: Giuliano Procida <gprocida@google.com>
>
>It's also noticeable that turning on some sanitisers affects abidiff
>output. This can't be good and we should look into it when we have
>time.

When running this in our hermetic build toolchain that appears to be not
an issue. Though I do not yet see where this happens when running with
a normal autotools based build.

Cheers,
Matthias

>
>Regards,
>Giuliano.
>
>> Signed-off-by: Matthias Maennich <maennich@google.com>
>> ---
>>  configure.ac | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/configure.ac b/configure.ac
>> index ba800a72ac7a..50a50c6c8932 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -120,6 +120,12 @@ AC_ARG_ENABLE(asan,
>>               ENABLE_ASAN=$enableval,
>>               ENABLE_ASAN=no)
>>
>> +AC_ARG_ENABLE(tsan,
>> +             AS_HELP_STRING([--enable-tsan=yes|no],
>> +                            [enable the support of building with -fsanitize=thread)]),
>> +             ENABLE_TSAN=$enableval,
>> +             ENABLE_TSAN=no)
>> +
>>  AC_ARG_ENABLE(ubsan,
>>               AS_HELP_STRING([--enable-ubsan=yes|no],
>>                              [enable the support of building with -fsanitize=undefined)]),
>> @@ -614,6 +620,11 @@ if test x$ENABLE_ASAN = xyes; then
>>      CXXFLAGS="$CXXFLAGS -fsanitize=address"
>>  fi
>>
>> +if test x$ENABLE_TSAN = xyes; then
>> +    CFLAGS="$CFLAGS -fsanitize=thread"
>> +    CXXFLAGS="$CXXFLAGS -fsanitize=thread"
>> +fi
>> +
>>  if test x$ENABLE_UBSAN = xyes; then
>>      CFLAGS="$CFLAGS -fsanitize=undefined"
>>      CXXFLAGS="$CXXFLAGS -fsanitize=undefined"
>> @@ -907,6 +918,7 @@ AC_MSG_NOTICE([
>>      Enable python 3                               : ${ENABLE_PYTHON3}
>>      Enable running tests under Valgrind            : ${enable_valgrind}
>>      Enable build with -fsanitize=address          : ${ENABLE_ASAN}
>> +    Enable build with -fsanitize=thread           : ${ENABLE_TSAN}
>>      Enable build with -fsanitize=undefined        : ${ENABLE_UBSAN}
>>      Generate html apidoc                          : ${ENABLE_APIDOC}
>>      Generate html manual                          : ${ENABLE_MANUAL}
>> --
>> 2.25.1.481.gfbce0eb801-goog
>>
>> --
>> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>>
Dodji Seketeli March 18, 2020, 12:47 p.m. UTC | #3 
Hello Matthias,

Matthias Maennich <maennich@google.com> a ?crit:

> Similarly to asan and ubsan, add support for tsan conditionally at
> configure time. This allows us to track down race conditions etc.
>
> 	* configure.ac: Add configure options for -fsanitize=thread
>

Applied to master.

Thanks!
Matthias Männich March 22, 2020, 10:45 p.m. UTC | #4 
On Mon, Mar 16, 2020 at 03:00:02PM +0000, Giuliano Procida wrote:
>On Thu, 12 Mar 2020 at 11:32, 'Matthias Maennich' via kernel-team
><kernel-team@android.com> wrote:
>>
>> Similarly to asan and ubsan, add support for tsan conditionally at
>> configure time. This allows us to track down race conditions etc.
>>
>>         * configure.ac: Add configure options for -fsanitize=thread
>
>Please verify that there's no need to add the various flags to LDFLAGS
>(or no difference if you do so) thanks to CXX anf CFLAGS.
>msan (for which there isn't support yet; it also requires Clang and
>lld in particular) definitely requires its flag at link time.
>
>Assuming this is the case:
>Reviewed-by: Giuliano Procida <gprocida@google.com>
>
>It's also noticeable that turning on some sanitisers affects abidiff
>output. This can't be good and we should look into it when we have
>time.

This mystery I could now solve: elfutils before 0.178 is loading the ebl
backends with dlopen() at runtime. At least on Debian based systems,
those are not in the same directory as libelf.so and some minor magic
is used to discover them. When using the asan runtimes, the loading gets
confused and silently fails (this is due to the fact that some
architectures do not have an ebl backend and failure isn't bad then).
This leads to wrong relocations when using elfutils and eventually to
different extracted data in libabigail, which makes the tests fail. The
workaround is to use a newer elfutils version. Since 0.178, elfutils has
the backends builtin and does not load them anymore at runtime. In my
case, I just needed to rerun `make check` with a newer elfutils in
LD_LIBRARY_PATH.

Cheers,
Matthias

>
>Regards,
>Giuliano.
>
>> Signed-off-by: Matthias Maennich <maennich@google.com>
>> ---
>>  configure.ac | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/configure.ac b/configure.ac
>> index ba800a72ac7a..50a50c6c8932 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -120,6 +120,12 @@ AC_ARG_ENABLE(asan,
>>               ENABLE_ASAN=$enableval,
>>               ENABLE_ASAN=no)
>>
>> +AC_ARG_ENABLE(tsan,
>> +             AS_HELP_STRING([--enable-tsan=yes|no],
>> +                            [enable the support of building with -fsanitize=thread)]),
>> +             ENABLE_TSAN=$enableval,
>> +             ENABLE_TSAN=no)
>> +
>>  AC_ARG_ENABLE(ubsan,
>>               AS_HELP_STRING([--enable-ubsan=yes|no],
>>                              [enable the support of building with -fsanitize=undefined)]),
>> @@ -614,6 +620,11 @@ if test x$ENABLE_ASAN = xyes; then
>>      CXXFLAGS="$CXXFLAGS -fsanitize=address"
>>  fi
>>
>> +if test x$ENABLE_TSAN = xyes; then
>> +    CFLAGS="$CFLAGS -fsanitize=thread"
>> +    CXXFLAGS="$CXXFLAGS -fsanitize=thread"
>> +fi
>> +
>>  if test x$ENABLE_UBSAN = xyes; then
>>      CFLAGS="$CFLAGS -fsanitize=undefined"
>>      CXXFLAGS="$CXXFLAGS -fsanitize=undefined"
>> @@ -907,6 +918,7 @@ AC_MSG_NOTICE([
>>      Enable python 3                               : ${ENABLE_PYTHON3}
>>      Enable running tests under Valgrind            : ${enable_valgrind}
>>      Enable build with -fsanitize=address          : ${ENABLE_ASAN}
>> +    Enable build with -fsanitize=thread           : ${ENABLE_TSAN}
>>      Enable build with -fsanitize=undefined        : ${ENABLE_UBSAN}
>>      Generate html apidoc                          : ${ENABLE_APIDOC}
>>      Generate html manual                          : ${ENABLE_MANUAL}
>> --
>> 2.25.1.481.gfbce0eb801-goog
>>
>> --
>> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>>
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index ba800a72ac7a..50a50c6c8932 100644
--- a/configure.ac
+++ b/configure.ac
@@ -120,6 +120,12 @@  AC_ARG_ENABLE(asan,
 	      ENABLE_ASAN=$enableval,
 	      ENABLE_ASAN=no)
 
+AC_ARG_ENABLE(tsan,
+	      AS_HELP_STRING([--enable-tsan=yes|no],
+			     [enable the support of building with -fsanitize=thread)]),
+	      ENABLE_TSAN=$enableval,
+	      ENABLE_TSAN=no)
+
 AC_ARG_ENABLE(ubsan,
 	      AS_HELP_STRING([--enable-ubsan=yes|no],
 			     [enable the support of building with -fsanitize=undefined)]),
@@ -614,6 +620,11 @@  if test x$ENABLE_ASAN = xyes; then
     CXXFLAGS="$CXXFLAGS -fsanitize=address"
 fi
 
+if test x$ENABLE_TSAN = xyes; then
+    CFLAGS="$CFLAGS -fsanitize=thread"
+    CXXFLAGS="$CXXFLAGS -fsanitize=thread"
+fi
+
 if test x$ENABLE_UBSAN = xyes; then
     CFLAGS="$CFLAGS -fsanitize=undefined"
     CXXFLAGS="$CXXFLAGS -fsanitize=undefined"
@@ -907,6 +918,7 @@  AC_MSG_NOTICE([
     Enable python 3				   : ${ENABLE_PYTHON3}
     Enable running tests under Valgrind            : ${enable_valgrind}
     Enable build with -fsanitize=address    	   : ${ENABLE_ASAN}
+    Enable build with -fsanitize=thread    	   : ${ENABLE_TSAN}
     Enable build with -fsanitize=undefined  	   : ${ENABLE_UBSAN}
     Generate html apidoc	                   : ${ENABLE_APIDOC}
     Generate html manual	                   : ${ENABLE_MANUAL}