diff mbox series

[gdb/build] Use -fno-hoist-adjacent-loads for gcc <= 13

Message ID 20241016030243.31320-1-tdevries@suse.de
State Committed
Headers
Series [gdb/build] Use -fno-hoist-adjacent-loads for gcc <= 13 |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Tom de Vries Oct. 16, 2024, 3:02 a.m. UTC 
  When building gdb with gcc 12 and -fsanitize=threads while renabling
background dwarf reading by setting dwarf_synchronous to false, I run into:
...
(gdb) file amd64-watchpoint-downgrade
Reading symbols from amd64-watchpoint-downgrade...
(gdb) watch global_var
==================
WARNING: ThreadSanitizer: data race (pid=20124)
  Read of size 8 at 0x7b80000500d8 by main thread:
    #0 cooked_index_entry::full_name(obstack*, bool) const cooked-index.c:220
    #1 cooked_index::get_main_name(obstack*, language*) const cooked-index.c:735
    #2 cooked_index_worker::wait(cooked_state, bool) cooked-index.c:559
    #3 cooked_index::wait(cooked_state, bool) cooked-index.c:631
    #4 cooked_index_functions::wait(objfile*, bool) cooked-index.h:729
    #5 cooked_index_functions::compute_main_name(objfile*) cooked-index.h:806
    #6 objfile::compute_main_name() symfile-debug.c:461
    #7 find_main_name symtab.c:6503
    #8 main_language() symtab.c:6608
    #9 set_initial_language_callback symfile.c:1634
    #10 get_current_language() language.c:96
    ...

  Previous write of size 8 at 0x7b80000500d8 by thread T1:
    #0 cooked_index_shard::finalize(parent_map_map const*) \
         dwarf2/cooked-index.c:409
    #1 operator() cooked-index.c:663
    ...

  ...

SUMMARY: ThreadSanitizer: data race cooked-index.c:220 in \
  cooked_index_entry::full_name(obstack*, bool) const
==================
Hardware watchpoint 1: global_var
(gdb) PASS: gdb.arch/amd64-watchpoint-downgrade.exp: watch global_var
...

This was also reported in PR31715.

This is due do gcc PR110799 [1], generating wrong code with
-fhoist-adjacent-loads, and causing a false positive for
-fsanitize=threads.

Work around the gcc PR by forcing -fno-hoist-adjacent-loads for gcc <= 13
and -fsanitize=threads.

Tested in that same configuration on x86_64-linux.  Remaining ThreadSanitizer
problems are the ones reported in PR31626 (gdb.rust/dwindex.exp) and
PR32247 (gdb.trace/basic-libipa.exp).

PR gdb/31715
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31715

Tested-By: Bernd Edlinger <bernd.edlinger@hotmail.de>

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110799
---
 gdbsupport/common-defs.h | 7 +++++++
 1 file changed, 7 insertions(+)


base-commit: a104f0a3e62031d2a5aabfe9e82f55158647f444

Comments

Tom Tromey Oct. 17, 2024, 7:36 p.m. UTC | #1 
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Work around the gcc PR by forcing -fno-hoist-adjacent-loads for gcc <= 13
Tom> and -fsanitize=threads.

Tom> Tested in that same configuration on x86_64-linux.  Remaining ThreadSanitizer
Tom> problems are the ones reported in PR31626 (gdb.rust/dwindex.exp) and
Tom> PR32247 (gdb.trace/basic-libipa.exp).

Thank you.

I didn't mention it in the bug, but I do have mild misgivings about
this, since having compiler flags set in the source is obscure and can
cause some real head scratching.  However, it seems nice to have tsan
work automatically and it's limited to certain GCC versions as well.

Approved-By: Tom Tromey <tom@tromey.com>

Tom
Tom de Vries Oct. 17, 2024, 10:22 p.m. UTC | #2 
On 10/17/24 21:36, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Work around the gcc PR by forcing -fno-hoist-adjacent-loads for gcc <= 13
> Tom> and -fsanitize=threads.
> 
> Tom> Tested in that same configuration on x86_64-linux.  Remaining ThreadSanitizer
> Tom> problems are the ones reported in PR31626 (gdb.rust/dwindex.exp) and
> Tom> PR32247 (gdb.trace/basic-libipa.exp).
> 
> Thank you.
> 
> I didn't mention it in the bug, but I do have mild misgivings about
> this, since having compiler flags set in the source is obscure and can
> cause some real head scratching.  However, it seems nice to have tsan
> work automatically and it's limited to certain GCC versions as well.
> 

Yeah, it's not pretty, but it's meant to prevent gdb developers from 
running into the same problem again, reporting it and spending time 
analyzing it.

Thanks for the review, pushed.

- Tom

> Approved-By: Tom Tromey <tom@tromey.com>
diff mbox series

Patch

diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h
index 6120719480b..d246a84bf75 100644
--- a/gdbsupport/common-defs.h
+++ b/gdbsupport/common-defs.h
@@ -20,6 +20,13 @@ 
 #ifndef COMMON_COMMON_DEFS_H
 #define COMMON_COMMON_DEFS_H
 
+#if defined (__SANITIZE_THREAD__) && defined (__GNUC__) \
+  && !defined (__clang__) && __GNUC__ <= 13
+
+/* Work around PR gcc/110799.  */
+#pragma GCC optimize("-fno-hoist-adjacent-loads")
+#endif
+
 #include <gdbsupport/config.h>
 
 #undef PACKAGE_NAME