Re: [PATCH 1/2] [gdb] Fix sig_write for null gdb_stderr

[Tom de Vries <tdevries@suse.de>]

On 4/28/25 17:17, Simon Marchi wrote:
> On 4/25/25 1:18 PM, Tom de Vries wrote:
>> When running test-case gdb.tui/tui-layout-asm.exp with target board
>> dwarf5-fission-debug-types, the test-case fails and I get a core dump:
>> ...
>>   # of unexpected core files      1
>> ...
>>
>> Looking at the backtrace of the core file, what seems to be happening is that:
>> - gdbpy_flush attempts to flush gdb_stdout, which is nullptr
>> - that causes a segfault
>> - gdb intercepts this and starts to handle it using handle_fatal_signal
>> - handle_fatal_signal calls sig_write, which attempts to write to gdb_stderr,
>>    which is nullptr,
>> - that causes another segfault
>> - gdb exits
>>
>> I managed to reproduce the problem by the following trigger patch in
>> stdin_event_handler:
>> ...
>> -  if (error)
>> +  if (1 || error)
>>       {
>>         current_ui = main_ui;
>>         ui->unregister_file_handler ();
>> -      if (main_ui == ui)
>> +      if (1 || main_ui == ui)
>>   	{
>>   	  gdb_printf (gdb_stderr, _("error detected on stdin\n"));
>> +	  gdb_stderr = nullptr;
>> +	  gdb_stdout = nullptr;
>> +	  gdb_stdlog = nullptr;
>>   	  quit_command ((char *) 0, 0);
>>   	}
>> ...
>> which gives us:
>> ...
>> $ gdb
>> (gdb) <q> error detected on stdin
>> Segmentation fault (core dumped)
>> $ q
>> ...
>>
>> Fix sig_write to handle the case that gdb_stderr == nullptr, such that we get
>> instead:
>> ...
>> $ gdb
>> (gdb) <q> error detected on stdin
>>
>> Fatal signal: Segmentation fault
>> ----- Backtrace -----
>>    ...
>> ---------------------
>> A fatal error internal to GDB has been detected, further
>> debugging is not possible.  GDB will now terminate.
>>
>> This is a bug, please report it.  For instructions, see:
>> <https://www.gnu.org/software/gdb/bugs/>.
>>
>> Segmentation fault (core dumped)
>> $ q
>> ...
>>
>> Tested on x86_64-linux.
>> ---
>>   gdb/bt-utils.c  | 30 +++++++++++++++++++++---------
>>   gdb/event-top.c | 13 +++++++++++--
>>   2 files changed, 32 insertions(+), 11 deletions(-)
>>
>> diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
>> index 8e782450ae9..9e9f680f9ea 100644
>> --- a/gdb/bt-utils.c
>> +++ b/gdb/bt-utils.c
>> @@ -55,7 +55,10 @@ libbacktrace_error (void *data, const char *errmsg, int errnum)
>>   
>>     const auto sig_write = [] (const char *msg) -> void
>>     {
>> -    gdb_stderr->write_async_safe (msg, strlen (msg));
>> +    if (gdb_stderr == nullptr || gdb_stderr->fd () == -1)
>> +      std::ignore = ::write (2, msg, strlen (msg));
> 
> TIL about std::ignore.
> 
> Is it possible to factor out the lambdas:
> 
>    const auto sig_write = [] (const char *msg) -> void
>    {
>      if (gdb_stderr == nullptr || gdb_stderr->fd () == -1)
>        std::ignore = ::write (2, msg, strlen (msg));
>      else
>        gdb_stderr->write_async_safe (msg, strlen (msg));
>    };
> 
> to a function?  At first glance they all seem to be the same.
> 

I've done this in a v2 ( 
https://sourceware.org/pipermail/gdb-patches/2025-April/217564.html ) in 
a separate patch.

Thanks,
- Tom

> Otherwise, LGTM.
> 
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
> 
> Simon