[PATCH] [gdb] Fix tsan warning: signal handler spoils errno

[Tom de Vries]

On 12/9/24 18:46, Tom de Vries wrote:
> On 12/9/24 18:11, Andrew Burgess wrote:
>> Tom de Vries <tdevries@suse.de> writes:
>>
>>> When building gdb with -fsanitize=thread and running test-case
>>> gdb.base/bg-exec-sigint-bp-cond.exp, I run into:
>>> ...
>>> ==================^M
>>> WARNING: ThreadSanitizer: signal handler spoils errno (pid=25422)^M
>>>      #0 handler_wrapper gdb/posix-hdep.c:66^M
>>>      #1 decltype ({parm#2}({parm#3}...)) gdb::handle_eintr<>() \
>>>           gdbsupport/eintr.h:67^M
>>>      #2 gdb::waitpid(int, int*, int) gdbsupport/eintr.h:78^M
>>>      #3 run_under_shell gdb/cli/cli-cmds.c:926^M
>>> ...
>>>
>>> Likewise in:
>>> - tui_sigwinch_handler with test-case gdb.python/tui-window.exp, and
>>> - handle_sighup with test-case gdb.base/quit-live.exp.
>>>
>>> Fix this by saving the original errno, and restoring it before 
>>> returning [1].
>>>
>>> Tested on x86_64-linux.
>>>
>>> [1] https://www.gnu.org/software/libc/manual/html_node/POSIX-Safety- 
>>> Concepts.html
>>> ---
>>>   gdb/event-top.c   | 4 ++++
>>>   gdb/posix-hdep.c  | 4 ++++
>>>   gdb/tui/tui-win.c | 4 ++++
>>>   3 files changed, 12 insertions(+)
>>>
>>> diff --git a/gdb/event-top.c b/gdb/event-top.c
>>> index 45ad7b990fa..b3a1e4e82c4 100644
>>> --- a/gdb/event-top.c
>>> +++ b/gdb/event-top.c
>>> @@ -1480,8 +1480,12 @@ async_do_nothing (gdb_client_data arg)
>>>   static void
>>>   handle_sighup (int sig)
>>>   {
>>> +  int save_errno = errno;
>>
>> Does:
>>
>>    scoped_restore restore_errno = make_scoped_restore (&errno);
>>
>> work here?
>>
> 
> Hi Andrew,
> 
> thanks for the review.
> 
> That seems to be the case, yes, I'll submit a v2.
> 

Submitted here ( 
https://sourceware.org/pipermail/gdb-patches/2024-December/213937.html ).

Thanks,
- Tom

> Thanks,
> - Tom
> 
>> Thanks,
>> Andrew
>>
>>> +
>>>     mark_async_signal_handler (sighup_token);
>>>     signal (sig, handle_sighup);
>>> +
>>> +  errno = save_errno;
>>>   }
>>>   /* Called by the event loop to process a SIGHUP.  */
>>> diff --git a/gdb/posix-hdep.c b/gdb/posix-hdep.c
>>> index a0d5c585ebc..bde9a9a3e7b 100644
>>> --- a/gdb/posix-hdep.c
>>> +++ b/gdb/posix-hdep.c
>>> @@ -64,9 +64,13 @@ static c_c_handler_ftype *current_handler;
>>>   static void
>>>   handler_wrapper (int num)
>>>   {
>>> +  int save_errno = errno;
>>> +
>>>     signal (num, handler_wrapper);
>>>     if (current_handler != SIG_IGN)
>>>       current_handler (num);
>>> +
>>> +  errno = save_errno;
>>>   }
>>>   /* See inferior.h.  */
>>> diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
>>> index e9a8e4651ea..9fe6288671c 100644
>>> --- a/gdb/tui/tui-win.c
>>> +++ b/gdb/tui/tui-win.c
>>> @@ -527,8 +527,12 @@ static struct async_signal_handler 
>>> *tui_sigwinch_token;
>>>   static void
>>>   tui_sigwinch_handler (int signal)
>>>   {
>>> +  int save_errno = errno;
>>> +
>>>     mark_async_signal_handler (tui_sigwinch_token);
>>>     tui_set_win_resized_to (true);
>>> +
>>> +  errno = save_errno;
>>>   }
>>>   /* Callback for asynchronously resizing TUI following a SIGWINCH 
>>> signal.  */
>>>
>>> base-commit: 01d8e0d24ad301f69b6f5ea39d073a728477507d
>>> -- 
>>> 2.43.0
>>
>