Re: [PATCH 2/2] [gdb] Handle nullptr stream in gdb_flush

[Simon Marchi <simark@simark.ca>]

On 4/25/25 1:18 PM, Tom de Vries wrote:
> Using the trigger patch described in the previous commit, I get:
> ...
> $ gdb
> (gdb) <q>error detected on stdin
> 
> Fatal signal: Segmentation fault
> ----- Backtrace -----
> 0x64c7b3 gdb_internal_backtrace_1
> 	/data/vries/gdb/src/gdb/bt-utils.c:127
> 0x64c937 _Z22gdb_internal_backtracev
> 	/data/vries/gdb/src/gdb/bt-utils.c:196
> 0x94db83 handle_fatal_signal
> 	/data/vries/gdb/src/gdb/event-top.c:1021
> 0x94dd48 handle_sigsegv
> 	/data/vries/gdb/src/gdb/event-top.c:1098
> 0x7f372be578ff ???
> 0x10b7c0a _Z9gdb_flushP7ui_file
> 	/data/vries/gdb/src/gdb/utils.c:1527
> 0xd4b938 gdbpy_flush
> 	/data/vries/gdb/src/gdb/python/python.c:1624
> 0x7f372d73b276 _PyCFunction_FastCallDict
> 	Objects/methodobject.c:231
> 0x7f372d73b276 _PyCFunction_FastCallKeywords
> 	Objects/methodobject.c:294
> 0x7f372d794a09 call_function
> 	Python/ceval.c:4851
> 0x7f372d78e838 _PyEval_EvalFrameDefault
> 	Python/ceval.c:3351
> 0x7f372d796e6e PyEval_EvalFrameEx
> 	Python/ceval.c:754
> 0x7f372d796e6e _PyFunction_FastCall
> 	Python/ceval.c:4933
> 0x7f372d796e6e _PyFunction_FastCallDict
> 	Python/ceval.c:5035
> 0x7f372d6fefc8 _PyObject_FastCallDict
> 	Objects/abstract.c:2310
> 0x7f372d6fefc8 _PyObject_Call_Prepend
> 	Objects/abstract.c:2373
> 0x7f372d6fe162 _PyObject_FastCallDict
> 	Objects/abstract.c:2331
> 0x7f372d700705 callmethod
> 	Objects/abstract.c:2583
> 0x7f372d700705 _PyObject_CallMethodId
> 	Objects/abstract.c:2640
> 0x7f372d812a41 flush_std_files
> 	Python/pylifecycle.c:699
> 0x7f372d81281d Py_FinalizeEx
> 	Python/pylifecycle.c:768
> 0xd4d49b finalize_python
> 	/data/vries/gdb/src/gdb/python/python.c:2308
> 0x9587eb _Z17ext_lang_shutdownv
> 	/data/vries/gdb/src/gdb/extension.c:330
> 0xfd98df _Z10quit_forcePii
> 	/data/vries/gdb/src/gdb/top.c:1817
> 0x6b3080 _Z12quit_commandPKci
> 	/data/vries/gdb/src/gdb/cli/cli-cmds.c:483
> 0x1056577 stdin_event_handler
> 	/data/vries/gdb/src/gdb/ui.c:131
> 0x1986970 handle_file_event
> 	/data/vries/gdb/src/gdbsupport/event-loop.cc:551
> 0x1986f4b gdb_wait_for_event
> 	/data/vries/gdb/src/gdbsupport/event-loop.cc:672
> 0x1985e0c _Z16gdb_do_one_eventi
> 	/data/vries/gdb/src/gdbsupport/event-loop.cc:263
> 0xb66f2e start_event_loop
> 	/data/vries/gdb/src/gdb/main.c:402
> 0xb670ba captured_command_loop
> 	/data/vries/gdb/src/gdb/main.c:466
> 0xb68b9b captured_main
> 	/data/vries/gdb/src/gdb/main.c:1344
> 0xb68c44 _Z8gdb_mainP18captured_main_args
> 	/data/vries/gdb/src/gdb/main.c:1363
> 0x41a3b1 main
> 	/data/vries/gdb/src/gdb/gdb.c:38
> ---------------------
> 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
> ...
> 
> Fix this in gdb_flush by checking for nullptr stream parameter, such that we
> get instead:
> ...
> $ gdb
> (gdb) <q>error detected on stdin
> $ q
> ...
> 
> Tested on x86_64-linux.
> ---
>  gdb/utils.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/utils.c b/gdb/utils.c
> index ce3c26ef140..90063e4a96d 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1524,7 +1524,8 @@ pager_file::flush ()
>  void
>  gdb_flush (struct ui_file *stream)
>  {
> -  stream->flush ();
> +  if (stream != nullptr)
> +    stream->flush ();

Can we add the nullptr check only where it's needed?

I'm thinking that we should move towards removing gdb_flush (replacing
the call sites to call ui_file::flush directly).  And when we do so, we
wouldn't want to add nullptr checks everywhere that gdb_flush is
currently called.

Simon