[Bug gdb/29297] New: [gdb] ThreadSanitizer: data race gdb/event-top.c:1211 in handle_sigterm(int) [Location sync_quit_force_run]

[Bug gdb/29297] New: [gdb] ThreadSanitizer: data race gdb/event-top.c:1211 in handle_sigterm(int) [Location sync_quit_force_run]

[vries at gcc dot gnu.org]

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

            Bug ID: 29297
           Summary: [gdb] ThreadSanitizer: data race gdb/event-top.c:1211
                    in handle_sigterm(int) [Location sync_quit_force_run]
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

With gdb build with gcc-12 and -fsanitize=thread, and test-case
gdb.base/gdb-sigterm.exp, I run into:
...
WARNING: ThreadSanitizer: data race (pid=9722)^M
  Write of size 4 at 0x00000325bc68 by thread T1:^M
  [infrun    #0 handle_sigterm(int) src/gdb/event-top.c:1211 (gdb+0x8ec01f)^M
  ...
  Previous read of size 4 at 0x00000325bc68 by main thread:^M
    [failed to restore the stack]^M
^M
  Location is global 'sync_quit_force_run' of size 4 at \
  0x00000325bc68 (gdb+0x325bc68)^M
  ...
SUMMARY: ThreadSanitizer: data race gdb/event-top.c:1211 in
handle_sigterm(int)^M
...

Write in handle_sigterm:
...
void
handle_sigterm (int sig)
{
  ...

  sync_quit_force_run = 1;
...

The read should be one of:
...
utils.c:673:  if (sync_quit_force_run)
utils.c:702:  if (sync_quit_force_run)
...

The variable is defined as:
...
defs.h:176:extern volatile int sync_quit_force_run;
event-top.c:1202:volatile int sync_quit_force_run;
...
so I suppose rather than volatile this should be atomic.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29297] [gdb] ThreadSanitizer: data race gdb/event-top.c:1211 in handle_sigterm(int) [Location sync_quit_force_run]

[vries at gcc dot gnu.org]

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

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Hmm, the log actually contains more:
...
WARNING: ThreadSanitizer: data race (pid=9722)^M
  Read of size 8 at 0x00000307b898 by thread T1:^M
  #0 set_quit_flag() gdb/extension.c:782 (gdb+0x8f6adc)^M
  ...
  Previous write of size 8 at 0x00000307b898 by main thread:^M
    [failed to restore the stack]^M
^M
  Location is global 'active_ext_lang' of size 8 at 0x307b898 (gdb+0x307b898)^M
...
and
...
WARNING: ThreadSanitizer: data race (pid=9722)^M
   Write of size 4 at 0x00000325bd08 by thread T1:^M
   #0 set_quit_flag() gdb/extension.c:787 (gdb+0x8f6b81)^M
  ...
  Previous read of size 4 at 0x00000325bd08 by main thread:^M
    #0 check_quit_flag() gdb/extension.c:817 (gdb+0x8f6ca8)^M
  ...
Location is global 'quit_flag' of size 4 at 0x325bd08 (gdb+0x325bd08)
...
and:
...
WARNING: ThreadSanitizer: data race (pid=9722)^M
  Write of size 4 at 0x7b0c00003810 by thread T1:^M
  #0 mark_async_signal_handler(async_signal_handler*) \
  gdb/async-event.c:177 (gdb+0x584481)^M
  ...
  Previous read of size 4 at 0x7b0c00003810 by main thread:^M
  #0 invoke_async_signal_handlers() gdb/async-event.c:221 (gdb+0x5845f0)^M
  ...
  Location is heap block of size 40 at 0x7b0c00003810 allocated by main \
  thread:^M
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29297] [gdb] ThreadSanitizer: data race gdb/event-top.c:1211 in handle_sigterm(int) [Location sync_quit_force_run]

[vries at gcc dot gnu.org]

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

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 14181
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14181&action=edit
gdb.log

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29297] [gdb] ThreadSanitizer: data race gdb/event-top.c:1211 in handle_sigterm(int) [Location sync_quit_force_run]

[pedro at palves dot net]

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

Pedro Alves <pedro at palves dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pedro at palves dot net

--- Comment #3 from Pedro Alves <pedro at palves dot net> ---
I think the problem starts from the fact that a thread other than the main
thread is handling a SIGTERM:

 WARNING: ThreadSanitizer: data race (pid=9722)^M
   Write of size 4 at 0x00000325bc68 by thread T1:^M
   [infrun    #0 handle_sigterm(int) src/gdb/event-top.c:1211 (gdb+0x8ec01f)^M
   ...

We start threads with a few signals blocked, but not SIGTERM.  From
gdbsupport/block-signals.h:

  block_signals ()
  {
#ifdef HAVE_SIGPROCMASK
    sigset_t mask;
    sigemptyset (&mask);
    sigaddset (&mask, SIGINT);
    sigaddset (&mask, SIGCHLD);
    sigaddset (&mask, SIGALRM);
    sigaddset (&mask, SIGWINCH);
    gdb_sigmask (SIG_BLOCK, &mask, &m_old_mask);
#endif

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29297] [gdb] ThreadSanitizer: data race gdb/event-top.c:1211 in handle_sigterm(int) [Location sync_quit_force_run]

[vries at gcc dot gnu.org]

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

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Pedro Alves from comment #3)
> I think the problem starts from the fact that a thread other than the main
> thread is handling a SIGTERM:
> 
>  WARNING: ThreadSanitizer: data race (pid=9722)^M
>    Write of size 4 at 0x00000325bc68 by thread T1:^M
>    [infrun    #0 handle_sigterm(int) src/gdb/event-top.c:1211
> (gdb+0x8ec01f)^M
>    ...
> 
> We start threads with a few signals blocked, but not SIGTERM.  From
> gdbsupport/block-signals.h:
> 
>   block_signals ()
>   {
> #ifdef HAVE_SIGPROCMASK
>     sigset_t mask;
>     sigemptyset (&mask);
>     sigaddset (&mask, SIGINT);
>     sigaddset (&mask, SIGCHLD);
>     sigaddset (&mask, SIGALRM);
>     sigaddset (&mask, SIGWINCH);
>     gdb_sigmask (SIG_BLOCK, &mask, &m_old_mask);
> #endif

Thanks for the analysis.

Sent patch blocking SIGTERM in worker threads (
https://sourceware.org/pipermail/gdb-patches/2022-June/190437.html ).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/29297] [gdb] ThreadSanitizer: data race gdb/event-top.c:1211 in handle_sigterm(int) [Location sync_quit_force_run]

[vries at gcc dot gnu.org]

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

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |13.1

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=6418644b0d09af7d1334cc034a7bf8674c061d6e

-- 
You are receiving this mail because:
You are on the CC list for the bug.