Bug 29297

Summary: [gdb] ThreadSanitizer: data race gdb/event-top.c:1211 in handle_sigterm(int) [Location sync_quit_force_run]
Product: gdb Reporter: Tom de Vries <vries>
Component: gdbAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: pedro
Priority: P2    
Version: HEAD   
Target Milestone: 13.1   
Host: Target:
Build: Last reconfirmed:
Attachments: gdb.log

Description Tom de Vries 2022-06-29 11:57:36 UTC 
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.
Comment 1 Tom de Vries 2022-06-29 12:09:31 UTC 
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
...
Comment 2 Tom de Vries 2022-06-29 12:11:37 UTC 
Created attachment 14181 [details]
gdb.log
Comment 3 Pedro Alves 2022-06-29 18:39:16 UTC 
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
Comment 4 Tom de Vries 2022-06-30 10:00:16 UTC 
(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 ).