[PATCH] Mips: Enable asynchronous unwind tables with both ASAN and LSAN

[PATCH] Mips: Enable asynchronous unwind tables with both ASAN and LSAN

[Dimitrije Milosevic]

Enable asynchronous unwind tables with both ASAN and TSAN for correct back-trace.
LLVM currently enables asynchronous unwind tables for: ASAN, HWSAN, TSAN, MSAN, and DFSAN.
HWSAN is currently available only on AArch64, while MSAN and DFSAN are not available at all.
Also, LLVM checks is '-ffreestanding' is not passed in. '-ffreestanding' is only available in C-family frontends, hence why no such check is included.
TODO: Not sure if any tests should be added.

gcc/ChangeLog:

        * config/mips/mips.cc (mips_option_override): Enable
        asyncronous unwind tables with both ASAN and TSAN.

---
 gcc/config/mips/mips.cc | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index e64928f4113..ea2a038216c 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -20115,10 +20115,15 @@ mips_option_override (void)
  target_flags |= MASK_64BIT;
     }

-  /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables in
-     order for tracebacks to be complete but not if any
-     -fasynchronous-unwind-table were already specified.  */
-  if (flag_sanitize & SANITIZE_USER_ADDRESS
+  /* -fsanitize=address, and -fsanitize=thread need to turn
+     on -fasynchronous-unwind-tables in order for tracebacks
+     to be complete but not if any -fasynchronous-unwind-table
+     were already specified.  */
+  /* FIXME: HWSAN is currently only available on AArch64,
+      but could also utilize -fasynchronous-unwind-tables.
+     FIXME: We would also like to check if -ffreestanding is passed in.
+      However, it is only available in C-ish frontends.  */
+  if (flag_sanitize & (SANITIZE_USER_ADDRESS | SANITIZE_THREAD)
       && !global_options_set.x_flag_asynchronous_unwind_tables)
     flag_asynchronous_unwind_tables = 1;

---

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Dimitrije Milosevic]

Hi Xi, thanks for pointing this out. I'd definitely say that the https://clang.llvm.org/docs/ThreadSanitizer.html documentation is outdated. According to https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual#supported-platforms TSAN is supported on Mips64. Furthermore, there are actual code segments (in compiler-rt/lib/tsan/rtl/tsan_platforms.h, for example) related to Mips64.
I didn't add the 64-bit target check, however. Here is the updated version of the patch.

gcc/ChangeLog:

        * config/mips/mips.cc (mips_option_override): Enable
        asyncronous unwind tables with both ASAN and TSAN.

---

 gcc/config/mips/mips.cc | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index e64928f4113..2dce4007678 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -20115,10 +20115,16 @@ mips_option_override (void)
        target_flags |= MASK_64BIT;
     }

-  /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables in
-     order for tracebacks to be complete but not if any
-     -fasynchronous-unwind-table were already specified.  */
-  if (flag_sanitize & SANITIZE_USER_ADDRESS
+  /* -fsanitize=address, and -fsanitize=thread need to turn
+     on -fasynchronous-unwind-tables in order for tracebacks
+     to be complete but not if any -fasynchronous-unwind-table
+     were already specified.  */
+  /* FIXME: HWSAN is currently only available on AArch64,
+      but could also utilize -fasynchronous-unwind-tables.
+     FIXME: We would also like to check if -ffreestanding is passed in.
+      However, it is only available in C-ish frontends.  */
+  if (((flag_sanitize & SANITIZE_USER_ADDRESS)
+      || (TARGET_64BIT && (flag_sanitize & SANITIZE_THREAD)))
       && !global_options_set.x_flag_asynchronous_unwind_tables)
     flag_asynchronous_unwind_tables = 1;

---

________________________________
From: Xi Ruoyao <xry111@xry111.site>
Sent: Saturday, May 28, 2022 12:30 PM
To: Dimitrije Milosevic <Dimitrije.Milosevic@Syrmia.com>; gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>
Cc: Djordje Todorovic <Djordje.Todorovic@syrmia.com>
Subject: Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

On Thu, 2022-05-26 at 14:18 +0000, Dimitrije Milosevic wrote:
> Enable asynchronous unwind tables with both ASAN and TSAN for correct back-trace.
> LLVM currently enables asynchronous unwind tables for: ASAN, HWSAN, TSAN, MSAN, and DFSAN.
> HWSAN is currently available only on AArch64, while MSAN and DFSAN are not available at all.
> Also, LLVM checks is '-ffreestanding' is not passed in. '-ffreestanding' is only available in C-family frontends, hence why no such check is included.
> TODO: Not sure if any tests should be added.

According to https://clang.llvm.org/docs/ThreadSanitizer.html, TSAN is
not supported on MIPS.  Is this doc outdated?

--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Xi Ruoyao]

On Mon, 2022-05-30 at 07:10 +0000, Dimitrije Milosevic wrote:
> Hi Xi, thanks for pointing this out. I'd definitely say that the
> https://clang.llvm.org/docs/ThreadSanitizer.html documentation is
> outdated. According
> tohttps://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual#s
> upported-platforms TSAN is supported on Mips64. Furthermore, there are
> actual code segments (in compiler-rt/lib/tsan/rtl/tsan_platforms.h,
> for example) related to Mips64.
> I didn't add the 64-bit target check, however. Here is the updated
> version of the patch.

Well, so should we add TSAN_SUPPORTED=yes for MIPS64 in
libsanitizer/configure.tgt first?  I'll try this on my MIPS64 in a few
days.


-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Dimitrije Milosevic]

Definitely, a patch is on the way.
________________________________
From: Xi Ruoyao <xry111@xry111.site>
Sent: Tuesday, June 7, 2022 10:20 AM
To: Dimitrije Milosevic <Dimitrije.Milosevic@Syrmia.com>; gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>
Cc: Djordje Todorovic <Djordje.Todorovic@syrmia.com>
Subject: Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

On Mon, 2022-05-30 at 07:10 +0000, Dimitrije Milosevic wrote:
> Hi Xi, thanks for pointing this out. I'd definitely say that the
> https://clang.llvm.org/docs/ThreadSanitizer.html documentation is
> outdated. According
> tohttps://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual#s
> upported-platforms TSAN is supported on Mips64. Furthermore, there are
> actual code segments (in compiler-rt/lib/tsan/rtl/tsan_platforms.h,
> for example) related to Mips64.
> I didn't add the 64-bit target check, however. Here is the updated
> version of the patch.

Well, so should we add TSAN_SUPPORTED=yes for MIPS64 in
libsanitizer/configure.tgt first?  I'll try this on my MIPS64 in a few
days.


--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Xi Ruoyao]

> Well, so should we add TSAN_SUPPORTED=yes for MIPS64 in
> libsanitizer/configure.tgt first?  I'll try this on my MIPS64 in a few
> days.

Just tried TSAN_SUPPORTED=yes with asynchronous unwind tables enabled,
but I got some strange test failures for tls_race.c:

FAIL: c-c++-common/tsan/tls_race.c   -O0  output pattern test
Output was:
ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:452 "((thr_end)) <= ((tls_addr + tls_size))" (0xffec35f8c0, 0xffec35f784) (tid=748216)
    #0 __tsan::CheckUnwind() ../../../../gcc/libsanitizer/tsan/tsan_rtl.cpp:627 (libtsan.so.2+0xa30ec)
    #1 __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) ../../../../gcc/libsanitizer/sanitizer_common/sanitizer_termination.cpp:86 (libtsan.so.2+0xeb8cc)
    #2 __tsan::ImitateTlsWrite(__tsan::ThreadState*, unsigned long, unsigned long) ../../../../gcc/libsanitizer/tsan/tsan_platform_linux.cpp:452 (libtsan.so.2+0xa0cac)
    #3 __tsan::ThreadStart(__tsan::ThreadState*, unsigned int, unsigned long long, __sanitizer::ThreadType) ../../../../gcc/libsanitizer/tsan/tsan_rtl_thread.cpp:197 (libtsan.so.2+0xc0e88)
    #4 __tsan_thread_start_func ../../../../gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1009 (libtsan.so.2+0x3e5dc)
    #5 start_thread /sources/glibc-2.35/nptl/pthread_create.c:442 (libc.so.6+0xc75f4)

I've tried to diagnose the root cause but failed.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Dimitrije Milosevic]

On Saturday, June 11, 2022 2:03 PM, Xi wrote:
> Just tried TSAN_SUPPORTED=yes with asynchronous unwind tables enabled,
> but I got some strange test failures for tls_race.c:
> 
> FAIL: c-c++-common/tsan/tls_race.c   -O0  output pattern test
> Output was:
> ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:452 "((thr_end)) <= ((tls_addr + tls_size))" (0xffec35f8c0, 0xffec35f784) (tid=748216)
>     #0 __tsan::CheckUnwind() ../../../../gcc/libsanitizer/tsan/tsan_rtl.cpp:627 (libtsan.so.2+0xa30ec)
>     #1 __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) ../../../../gcc/libsanitizer/sanitizer_common/sanitizer_termination.cpp:86 (libtsan.so.2+0xeb8cc)
>     #2 __tsan::ImitateTlsWrite(__tsan::ThreadState*, unsigned long, unsigned long) ../../../../gcc/libsanitizer/tsan/tsan_platform_linux.cpp:452 (libtsan.so.2+0xa0cac)
>     #3 __tsan::ThreadStart(__tsan::ThreadState*, unsigned int, unsigned long long, __sanitizer::ThreadType) ../../../../gcc/libsanitizer/tsan/tsan_rtl_thread.cpp:197 (libtsan.so.2+0xc0e88)
>     #4 __tsan_thread_start_func ../../../../gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1009 (libtsan.so.2+0x3e5dc)
>     #5 start_thread /sources/glibc-2.35/nptl/pthread_create.c:442 (libc.so.6+0xc75f4)
>
> I've tried to diagnose the root cause but failed.

Hi Xi, thanks for looking into this. I've tried running the testsuite on a cross-toolchain (as I do not currently have access to a physical machine)
for a MIPS64R6 and the test passes successfully. Could you please verify that the test fails solely based on this change?
Kind regards,
Dimitrije

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Xi Ruoyao]

On Mon, 2022-07-04 at 14:28 +0000, Dimitrije Milosevic wrote:
> On Saturday, June 11, 2022 2:03 PM, Xi wrote:
> > Just tried TSAN_SUPPORTED=yes with asynchronous unwind tables
> > enabled,
> > but I got some strange test failures for tls_race.c:
> > 
> > FAIL: c-c++-common/tsan/tls_race.c   -O0  output pattern test
> > Output was:
> > ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:452
> > "((thr_end)) <= ((tls_addr + tls_size))" (0xffec35f8c0,
> > 0xffec35f784) (tid=748216)
> >     #0 __tsan::CheckUnwind()
> > ../../../../gcc/libsanitizer/tsan/tsan_rtl.cpp:627
> > (libtsan.so.2+0xa30ec)
> >     #1 __sanitizer::CheckFailed(char const*, int, char const*,
> > unsigned long long, unsigned long long)
> > ../../../../gcc/libsanitizer/sanitizer_common/sanitizer_termination.
> > cpp:86 (libtsan.so.2+0xeb8cc)
> >     #2 __tsan::ImitateTlsWrite(__tsan::ThreadState*, unsigned long,
> > unsigned long)
> > ../../../../gcc/libsanitizer/tsan/tsan_platform_linux.cpp:452
> > (libtsan.so.2+0xa0cac)
> >     #3 __tsan::ThreadStart(__tsan::ThreadState*, unsigned int,
> > unsigned long long, __sanitizer::ThreadType)
> > ../../../../gcc/libsanitizer/tsan/tsan_rtl_thread.cpp:197
> > (libtsan.so.2+0xc0e88)
> >     #4 __tsan_thread_start_func
> > ../../../../gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1009
> > (libtsan.so.2+0x3e5dc)
> >     #5 start_thread /sources/glibc-2.35/nptl/pthread_create.c:442
> > (libc.so.6+0xc75f4)
> > 
> > I've tried to diagnose the root cause but failed.
> 
> Hi Xi, thanks for looking into this. I've tried running the testsuite
> on a cross-toolchain (as I do not currently have access to a physical
> machine)
> for a MIPS64R6 and the test passes successfully. Could you please
> verify that the test fails solely based on this change?

I guess you mean you are running MIPS64R6 target code with qemu?  I'm
not 100% sure because maybe something is wrong in my system.  I'm now
retrying on gcc230.fsffrance.org (an EdgeRouter 4 in Cfarm) but building
GCC on it is really slow.

The changes I've tested:

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 5eb845960e1..a7f0580e9ba 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -20115,10 +20115,11 @@ mips_option_override (void)
 	target_flags |= MASK_64BIT;
     }
 
-  /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables in
-     order for tracebacks to be complete but not if any
-     -fasynchronous-unwind-table were already specified.  */
-  if (flag_sanitize & SANITIZE_USER_ADDRESS
+  /* For -fsanitize=address or -fsanitize=thread, it's needed to turn
+     on -fasynchronous-unwind-tables in order for tracebacks
+     to be complete but not if any -fasynchronous-unwind-table
+     were already specified.  */
+  if (flag_sanitize & (SANITIZE_USER_ADDRESS | SANITIZE_THREAD)
       && !global_options_set.x_flag_asynchronous_unwind_tables)
     flag_asynchronous_unwind_tables = 1;
 
diff --git a/libsanitizer/configure.tgt b/libsanitizer/configure.tgt
index fb89df4935c..52546bbe4e3 100644
--- a/libsanitizer/configure.tgt
+++ b/libsanitizer/configure.tgt
@@ -55,6 +55,9 @@ case "${target}" in
   arm*-*-linux*)
 	;;
   mips*-*-linux*)
+	if test x$ac_cv_sizeof_void_p = x8; then
+		TSAN_SUPPORTED=yes
+	fi
 	;;
   aarch64*-*-linux*)
 	if test x$ac_cv_sizeof_void_p = x8; then
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Fangrui Song]

On Mon, Jul 4, 2022 at 6:54 PM Xi Ruoyao via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Mon, 2022-07-04 at 14:28 +0000, Dimitrije Milosevic wrote:
> > On Saturday, June 11, 2022 2:03 PM, Xi wrote:
> > > Just tried TSAN_SUPPORTED=yes with asynchronous unwind tables
> > > enabled,
> > > but I got some strange test failures for tls_race.c:
> > >
> > > FAIL: c-c++-common/tsan/tls_race.c   -O0  output pattern test
> > > Output was:
> > > ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:452
> > > "((thr_end)) <= ((tls_addr + tls_size))" (0xffec35f8c0,
> > > 0xffec35f784) (tid=748216)
> > >     #0 __tsan::CheckUnwind()
> > > ../../../../gcc/libsanitizer/tsan/tsan_rtl.cpp:627
> > > (libtsan.so.2+0xa30ec)
> > >     #1 __sanitizer::CheckFailed(char const*, int, char const*,
> > > unsigned long long, unsigned long long)
> > > ../../../../gcc/libsanitizer/sanitizer_common/sanitizer_termination.
> > > cpp:86 (libtsan.so.2+0xeb8cc)
> > >     #2 __tsan::ImitateTlsWrite(__tsan::ThreadState*, unsigned long,
> > > unsigned long)
> > > ../../../../gcc/libsanitizer/tsan/tsan_platform_linux.cpp:452
> > > (libtsan.so.2+0xa0cac)
> > >     #3 __tsan::ThreadStart(__tsan::ThreadState*, unsigned int,
> > > unsigned long long, __sanitizer::ThreadType)
> > > ../../../../gcc/libsanitizer/tsan/tsan_rtl_thread.cpp:197
> > > (libtsan.so.2+0xc0e88)
> > >     #4 __tsan_thread_start_func
> > > ../../../../gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1009
> > > (libtsan.so.2+0x3e5dc)
> > >     #5 start_thread /sources/glibc-2.35/nptl/pthread_create.c:442
> > > (libc.so.6+0xc75f4)
> > >
> > > I've tried to diagnose the root cause but failed.
> >
> > Hi Xi, thanks for looking into this. I've tried running the testsuite
> > on a cross-toolchain (as I do not currently have access to a physical
> > machine)
> > for a MIPS64R6 and the test passes successfully. Could you please
> > verify that the test fails solely based on this change?
>
> I guess you mean you are running MIPS64R6 target code with qemu?  I'm
> not 100% sure because maybe something is wrong in my system.  I'm now
> retrying on gcc230.fsffrance.org (an EdgeRouter 4 in Cfarm) but building
> GCC on it is really slow.
>
> The changes I've tested:
>
> diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
> index 5eb845960e1..a7f0580e9ba 100644
> --- a/gcc/config/mips/mips.cc
> +++ b/gcc/config/mips/mips.cc
> @@ -20115,10 +20115,11 @@ mips_option_override (void)
>         target_flags |= MASK_64BIT;
>      }
>
> -  /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables in
> -     order for tracebacks to be complete but not if any
> -     -fasynchronous-unwind-table were already specified.  */
> -  if (flag_sanitize & SANITIZE_USER_ADDRESS
> +  /* For -fsanitize=address or -fsanitize=thread, it's needed to turn
> +     on -fasynchronous-unwind-tables in order for tracebacks
> +     to be complete but not if any -fasynchronous-unwind-table
> +     were already specified.  */
> +  if (flag_sanitize & (SANITIZE_USER_ADDRESS | SANITIZE_THREAD)
>        && !global_options_set.x_flag_asynchronous_unwind_tables)
>      flag_asynchronous_unwind_tables = 1;
>
> diff --git a/libsanitizer/configure.tgt b/libsanitizer/configure.tgt
> index fb89df4935c..52546bbe4e3 100644
> --- a/libsanitizer/configure.tgt
> +++ b/libsanitizer/configure.tgt
> @@ -55,6 +55,9 @@ case "${target}" in
>    arm*-*-linux*)
>         ;;
>    mips*-*-linux*)
> +       if test x$ac_cv_sizeof_void_p = x8; then
> +               TSAN_SUPPORTED=yes
> +       fi
>         ;;
>    aarch64*-*-linux*)
>         if test x$ac_cv_sizeof_void_p = x8; then
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University

Drive-by comment.

Clang considers that asan/msan/tsan/dataflow/etc enables
-fasynchronous-unwind-tables by default so I assume this GCC change is
fine.

With https://reviews.llvm.org/D102046 ("[sanitizer] Fall back to fast
unwinder"), compiler-rt may fall back to the frame pointer based
unwinder. There is not strong need to have the default
-fasynchronous-unwind-tables or -funwind-tables behavior.
However, most targets still default to omit frame pointer, so it's a
bit unfortunately that we need to enable unwind tables to get good
diagnostics.

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Xi Ruoyao]

On Mon, 2022-07-04 at 21:21 -0700, Fangrui Song wrote:
> Clang considers that asan/msan/tsan/dataflow/etc enables
> -fasynchronous-unwind-tables by default so I assume this GCC change is
> fine.

I agree it's fine, but the problem is TSAN is currently "unsupported"
within GCC (i. e. when you build gcc libtsan is not built).  So it does
not make any benefit to commit this change before making TSAN supported
on GCC side.

Dimitrije told me TSAN should be supported on 64-bit MIPS, but I can't
make it work fine with GCC.  I'll need some time to debug...

> With https://reviews.llvm.org/D102046 ("[sanitizer] Fall back to fast
> unwinder"), compiler-rt may fall back to the frame pointer based
> unwinder. There is not strong need to have the default
> -fasynchronous-unwind-tables or -funwind-tables behavior.
> However, most targets still default to omit frame pointer, so it's a
> bit unfortunately that we need to enable unwind tables to get good
> diagnostics.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Dimitrije Milosevic]

Hi Xi.
Correct, I am using a simulator.
Here are my changes:

diff --git a/libsanitizer/configure.tgt b/libsanitizer/configure.tgt
index fb89df4935c..6855a6ca9e7 100644
--- a/libsanitizer/configure.tgt
+++ b/libsanitizer/configure.tgt
@@ -55,6 +55,10 @@ case "${target}" in
   arm*-*-linux*)
        ;;
   mips*-*-linux*)
+       if test x$ac_cv_sizeof_void_p = x8; then
+               TSAN_SUPPORTED=yes
+               TSAN_TARGET_DEPENDENT_OBJECTS=tsan_rtl_mips64.lo
+       fi
        ;;
   aarch64*-*-linux*)
        if test x$ac_cv_sizeof_void_p = x8; then

Nit: Updated the code in gcc/config/mips/mips.cc a little bit, but no functional
change compared to your version.

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 8a55d2fb8f7..4ccc9e75482 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -20118,13 +20118,12 @@ mips_option_override (void)
   /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables
      in order for tracebacks to be complete but not if any
      -fasynchronous-unwind-table were already specified.  */
-  /* FIXME: TSAN could also utilize -fasynchronous-unwind-tables, but
-      should be first enabled for MIPS64.
-     FIXME: HWSAN could also utilize -fasynchronous-unwind-tables, but,
+  /* FIXME: HWSAN could also utilize -fasynchronous-unwind-tables, but,
       currently, it is only available on AArch64.
      FIXME: We would also like to check if -ffreestanding is passed in.
       However, it is only available in C-ish frontends.  */
-  if ((flag_sanitize & SANITIZE_USER_ADDRESS)
+  if (((flag_sanitize & SANITIZE_USER_ADDRESS)
+       || (TARGET_64BIT && (flag_sanitize & SANITIZE_THREAD)))
       && !global_options_set.x_flag_asynchronous_unwind_tables)
     flag_asynchronous_unwind_tables = 1;



From: Xi Ruoyao <xry111@xry111.site>
Sent: Tuesday, July 5, 2022 3:54 AM
To: Dimitrije Milosevic <Dimitrije.Milosevic@Syrmia.com>; gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>
Cc: Djordje Todorovic <Djordje.Todorovic@syrmia.com>
Subject: Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN 
 
On Mon, 2022-07-04 at 14:28 +0000, Dimitrije Milosevic wrote:
> On Saturday, June 11, 2022 2:03 PM, Xi wrote:
> > Just tried TSAN_SUPPORTED=yes with asynchronous unwind tables
> > enabled,
> > but I got some strange test failures for tls_race.c:
> > 
> > FAIL: c-c++-common/tsan/tls_race.c   -O0  output pattern test
> > Output was:
> > ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:452
> > "((thr_end)) <= ((tls_addr + tls_size))" (0xffec35f8c0,
> > 0xffec35f784) (tid=748216)
> >     #0 __tsan::CheckUnwind()
> > ../../../../gcc/libsanitizer/tsan/tsan_rtl.cpp:627
> > (libtsan.so.2+0xa30ec)
> >     #1 __sanitizer::CheckFailed(char const*, int, char const*,
> > unsigned long long, unsigned long long)
> > ../../../../gcc/libsanitizer/sanitizer_common/sanitizer_termination.
> > cpp:86 (libtsan.so.2+0xeb8cc)
> >     #2 __tsan::ImitateTlsWrite(__tsan::ThreadState*, unsigned long,
> > unsigned long)
> > ../../../../gcc/libsanitizer/tsan/tsan_platform_linux.cpp:452
> > (libtsan.so.2+0xa0cac)
> >     #3 __tsan::ThreadStart(__tsan::ThreadState*, unsigned int,
> > unsigned long long, __sanitizer::ThreadType)
> > ../../../../gcc/libsanitizer/tsan/tsan_rtl_thread.cpp:197
> > (libtsan.so.2+0xc0e88)
> >     #4 __tsan_thread_start_func
> > ../../../../gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1009
> > (libtsan.so.2+0x3e5dc)
> >     #5 start_thread /sources/glibc-2.35/nptl/pthread_create.c:442
> > (libc.so.6+0xc75f4)
> > 
> > I've tried to diagnose the root cause but failed.
> 
> Hi Xi, thanks for looking into this. I've tried running the testsuite
> on a cross-toolchain (as I do not currently have access to a physical
> machine)
> for a MIPS64R6 and the test passes successfully. Could you please
> verify that the test fails solely based on this change?

I guess you mean you are running MIPS64R6 target code with qemu?  I'm
not 100% sure because maybe something is wrong in my system.  I'm now
retrying on gcc230.fsffrance.org (an EdgeRouter 4 in Cfarm) but building
GCC on it is really slow.

The changes I've tested:

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 5eb845960e1..a7f0580e9ba 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -20115,10 +20115,11 @@ mips_option_override (void)
         target_flags |= MASK_64BIT;
     }
 
-  /* -fsanitize=address needs to turn on -fasynchronous-unwind-tables in
-     order for tracebacks to be complete but not if any
-     -fasynchronous-unwind-table were already specified.  */
-  if (flag_sanitize & SANITIZE_USER_ADDRESS
+  /* For -fsanitize=address or -fsanitize=thread, it's needed to turn
+     on -fasynchronous-unwind-tables in order for tracebacks
+     to be complete but not if any -fasynchronous-unwind-table
+     were already specified.  */
+  if (flag_sanitize & (SANITIZE_USER_ADDRESS | SANITIZE_THREAD)
       && !global_options_set.x_flag_asynchronous_unwind_tables)
     flag_asynchronous_unwind_tables = 1;
 
diff --git a/libsanitizer/configure.tgt b/libsanitizer/configure.tgt
index fb89df4935c..52546bbe4e3 100644
--- a/libsanitizer/configure.tgt
+++ b/libsanitizer/configure.tgt
@@ -55,6 +55,9 @@ case "${target}" in
   arm*-*-linux*)
         ;;
   mips*-*-linux*)
+       if test x$ac_cv_sizeof_void_p = x8; then
+               TSAN_SUPPORTED=yes
+       fi
         ;;
   aarch64*-*-linux*)
         if test x$ac_cv_sizeof_void_p = x8; then
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Xi Ruoyao]

On Tue, 2022-07-05 at 12:51 +0800, Xi Ruoyao via Gcc-patches wrote:

> I agree it's fine, but the problem is TSAN is currently "unsupported"
> within GCC (i. e. when you build gcc libtsan is not built).  So it
> does
> not make any benefit to commit this change before making TSAN
> supported
> on GCC side.
> 
> Dimitrije told me TSAN should be supported on 64-bit MIPS, but I can't
> make it work fine with GCC.  I'll need some time to debug...

Hi Fangrui,

On my system dlpi_tls_modid for libtsan.so.2 is 2, instead of 1. 
GetStaticTlsBoundary seems assuming the dlpi_tls_modid for libtsan.so to
be 1, and returning wrong values if the assumption is broken.

Is it unsafe to make such an assumption?  Or, am I being haunted by a
glibc bug?
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Mips: Enable asynchronous unwind tables with both ASAN and TSAN

[Xi Ruoyao]

On Mon, 2022-07-04 at 21:21 -0700, Fangrui Song wrote:

> > > 
> > > > FAIL: c-c++-common/tsan/tls_race.c   -O0  output pattern test
> > > > Output was:
> > > > ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:452
> > > > "((thr_end)) <= ((tls_addr + tls_size))" (0xffec35f8c0,
> > > > 0xffec35f784) (tid=748216)
> > > >     #0 __tsan::CheckUnwind()
> > > > ../../../../gcc/libsanitizer/tsan/tsan_rtl.cpp:627
> > > > (libtsan.so.2+0xa30ec)
> > > >     #1 __sanitizer::CheckFailed(char const*, int, char const*,
> > > > unsigned long long, unsigned long long)
> > > > ../../../../gcc/libsanitizer/sanitizer_common/sanitizer_termination.
> > > > cpp:86 (libtsan.so.2+0xeb8cc)
> > > >     #2 __tsan::ImitateTlsWrite(__tsan::ThreadState*, unsigned long,
> > > > unsigned long)
> > > > ../../../../gcc/libsanitizer/tsan/tsan_platform_linux.cpp:452
> > > > (libtsan.so.2+0xa0cac)
> > > >     #3 __tsan::ThreadStart(__tsan::ThreadState*, unsigned int,
> > > > unsigned long long, __sanitizer::ThreadType)
> > > > ../../../../gcc/libsanitizer/tsan/tsan_rtl_thread.cpp:197
> > > > (libtsan.so.2+0xc0e88)
> > > >     #4 __tsan_thread_start_func
> > > > ../../../../gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1009
> > > > (libtsan.so.2+0x3e5dc)
> > > >     #5 start_thread /sources/glibc-2.35/nptl/pthread_create.c:442
> > > > (libc.so.6+0xc75f4)
> > > > 
> > > > I've tried to diagnose the root cause but failed.
> > > 
> > > Hi Xi, thanks for looking into this. I've tried running the testsuite
> > > on a cross-toolchain (as I do not currently have access to a physical
> > > machine)
> > > for a MIPS64R6 and the test passes successfully. Could you please
> > > verify that the test fails solely based on this change?

I've got a clue about why this happens.  See
https://reviews.llvm.org/D129112.

It depends on how the dynamic linker arranges TLS blocks so different
version of Glibc may produce different results.
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University