[PATCH 4/4] [gdb/tui] Fix hang in tui_disable in handle_fatal_signal

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

I managed to get gdb to hang with the following backtrace:
...
 (gdb) bt
 #0  futex_wait (private=0, expected=2, futex_word=0x7f9a2fdefb00 <main_arena>)
     at ../sysdeps/nptl/futex-internal.h:146
 #1  __GI___lll_lock_wait_private (futex=futex@entry=0x7f9a2fdefb00 <main_arena>) at lowlevellock.c:34
 #2  0x00007f9a2fcaafb8 in __libc_calloc (n=<optimized out>, elem_size=<optimized out>) at malloc.c:3725
 #3  0x00000000004f98ff in xcalloc (number=1, size=24) at gdb/alloc.c:92
 #4  0x0000000000982dcb in xcnew<active_ext_lang_state> ()
     at gdbsupport/poison.h:114
 #5  0x0000000000981ee3 in set_active_ext_lang (now_active=0x1c68fc0 <extension_language_python>)
     at gdb/extension.c:843
 #6  0x0000000000d9fd41 in gdbpy_enter::gdbpy_enter (this=0x7ffeb6a67720, gdbarch=0x0, language=0x0)
     at gdb/python/python.c:231
 #7  0x0000000000d762a6 in gdbpy_tui_enabled (state=false)
     at gdb/python/py-tui.c:593
 #8  0x0000000000d7af21 in std::__invoke_impl<void, void (*&)(bool), bool> (
     __f=@0x2ead1368: 0xd7626d <gdbpy_tui_enabled(bool)>) at /usr/include/c++/15/bits/invoke.h:63
 #9  0x0000000000d7a5a5 in std::__invoke_r<void, void (*&)(bool), bool> (
     __fn=@0x2ead1368: 0xd7626d <gdbpy_tui_enabled(bool)>) at /usr/include/c++/15/bits/invoke.h:113
 #10 0x0000000000d79def in std::_Function_handler<void(bool), void (*)(bool)>::_M_invoke (__functor=...,
     __args#0=@0x7ffeb6a67854: false) at /usr/include/c++/15/bits/std_function.h:292
 #11 0x00000000010b405d in std::function<void(bool)>::operator() (this=0x2ead1368, __args#0=false)
     at /usr/include/c++/15/bits/std_function.h:593
 #12 0x00000000010b3f32 in gdb::observers::observable<bool>::notify (this=0x2fdb280 <gdb::observers::tui_enabled>,
     args#0=false) at gdbsupport/observable.h:166
 #13 0x00000000010b0616 in tui_disable () at gdb/tui/tui.c:727
 #14 0x0000000000974a64 in handle_fatal_signal (sig=6)
     at gdb/event-top.c:974
 #15 <signal handler called>
 #16 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0)
     at pthread_kill.c:44
 #17 0x00007f9a2fc9ab73 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
 #18 0x00007f9a2fc47fc6 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
 #19 0x00007f9a2fc2f917 in __GI_abort () at abort.c:79
 #20 0x00007f9a2fc307e8 in __libc_message_impl (fmt=fmt@entry=0x7f9a2fdb439e "%s\n")
     at ../sysdeps/posix/libc_fatal.c:134
 #21 0x00007f9a2fca5487 in malloc_printerr (
     str=str@entry=0x7f9a2fdb7980 "malloc(): smallbin double linked list corrupted") at malloc.c:5775
 #22 0x00007f9a2fca8d7c in _int_malloc (av=av@entry=0x7f9a2fdefb00 <main_arena>, bytes=bytes@entry=16)
     at malloc.c:3986
 #23 0x00007f9a2fca9bbd in __GI___libc_malloc (bytes=bytes@entry=16) at malloc.c:3336
 #24 0x00007f9a2fcf38ce in create_cd_newstate (hash=457, context=0, nodes=0x7ffeb6a68ab0, dfa=<optimized out>)
     at /usr/src/debug/glibc-2.40/posix/regex_internal.c:1722
 #25 re_acquire_state_context (err=err@entry=0x7ffeb6a68aac, dfa=dfa@entry=0x2eae9ce0,
     nodes=nodes@entry=0x7ffeb6a68ab0, context=context@entry=0)
     at /usr/src/debug/glibc-2.40/posix/regex_internal.c:1562
 #26 0x00007f9a2fcf778e in build_trtable (dfa=<optimized out>, state=state@entry=0x2ec5a3e0)
     at /usr/src/debug/glibc-2.40/posix/regexec.c:3331
 #27 0x00007f9a2fcf9603 in transit_state (state=0x2ec5a3e0, mctx=0x7ffeb6a6d4e0, err=0x7ffeb6a6d3fc)
     at /usr/src/debug/glibc-2.40/posix/regexec.c:2257
 #28 check_matching (p_match_first=<optimized out>, fl_longest_match=false, mctx=0x7ffeb6a6d4e0)
     at /usr/src/debug/glibc-2.40/posix/regexec.c:1120
 #29 re_search_internal (preg=preg@entry=0x7ffeb6a6d770,
     string=string@entry=0x2ec60940
     "/home/vries/.cache/debuginfod_client/df406a043351d494964e1b10e5497e9931f53cb1/debuginfo",
     length=87, start=<optimized out>, last_start=<optimized out>, stop=<optimized out>, nmatch=0,
     pmatch=0x0, eflags=0) at /usr/src/debug/glibc-2.40/posix/regexec.c:792
 #30 0x00007f9a2fcfe5d4 in __GI___regexec (preg=0x7ffeb6a6d770,
     string=0x2ec60940
     "/home/vries/.cache/debuginfod_client/df406a043351d494964e1b10e5497e9931f53cb1/debuginfo",
     nmatch=0, pmatch=0x0, eflags=0) at /usr/src/debug/glibc-2.40/posix/regexec.c:219
 #31 0x00007f9a302550fe in ?? () from /lib64/libdebuginfod.so.1
 #32 0x00000000007b8855 in debuginfod_source_query (
     build_id=0x2eae79c8 "I\v\203\342AJ\371<\234\016L\222_\236e\251k\366\021a", build_id_len=20,
     srcpath=0x2eaf73f0 "gdb/testsuite/gdb.tui/debuginfod-query.c",
     destname=0x7ffeb6a6ea60) at gdb/debuginfod-support.c:343
 #33 0x0000000000f5ef98 in open_source_file (s=0x2eb5ead0)
     at gdb/source.c:1147
 #34 0x0000000000f5f2fe in symtab_to_fullname (s=0x2eb5ead0)
     at gdb/source.c:1208
 #35 0x0000000000f547a6 in source_cache::ensure (this=0x2fded80 <g_source_cache>, s=0x2eb5ead0)
     at gdb/source-cache.c:311
 #36 0x0000000000f551d3 in source_cache::get_source_lines (this=0x2fded80 <g_source_cache>, s=0x2eb5ead0,
     first_line=15, last_line=28, lines=0x7ffeb6a6ed20)
     at gdb/source-cache.c:479
 #37 0x00000000010a0eaf in tui_source_window::set_contents (this=0x7f9a0c008d70, arch=0x2eb3a1f0, sal=...)
     at gdb/tui/tui-source.c:66
 #38 0x00000000010ab548 in tui_source_window_base::update_source_window_as_is (this=0x7f9a0c008d70,
     gdbarch=0x2eb3a1f0, sal=...) at gdb/tui/tui-winsource.c:164
 #39 0x00000000010ab4eb in tui_source_window_base::update_source_window (this=0x7f9a0c008d70, gdbarch=0x2eb3a1f0,
     sal=...) at gdb/tui/tui-winsource.c:153
 #40 0x00000000010a198a in tui_source_window::maybe_update (this=0x7f9a0c008d70, gdbarch=0x2eb3a1f0, sal=...)
     at gdb/tui/tui-source.c:223
 #41 0x00000000010ab72b in tui_source_window_base::update_source_window_with_addr (this=0x7f9a0c008d70,
     gdbarch=0x2eb3a1f0, addr=4198678) at gdb/tui/tui-winsource.c:187
 #42 0x00000000010ad134 in tui_source_window_base::rerender (this=0x7f9a0c008d70)
     at gdb/tui/tui-winsource.c:480
 #43 0x0000000001077429 in tui_before_prompt (current_gdb_prompt=0x2fe0c70 <top_prompt+16> "(gdb) ")
     at gdb/tui/tui-hooks.c:185
 #44 0x0000000000983c97 in std::__invoke_impl<void, void (*&)(char const*), char const*> (
     __f=@0x2e9d2540: 0x107738a <tui_before_prompt(char const*)>) at /usr/include/c++/15/bits/invoke.h:63
 #45 0x00000000009835d7 in std::__invoke_r<void, void (*&)(char const*), char const*> (
     __fn=@0x2e9d2540: 0x107738a <tui_before_prompt(char const*)>) at /usr/include/c++/15/bits/invoke.h:113
 #46 0x000000000098336b in std::_Function_handler<void(char const*), void (*)(char const*)>::_M_invoke (
     __functor=..., __args#0=@0x7ffeb6a6f2d0: 0x2fe0c70 <top_prompt+16> "(gdb) ")
     at /usr/include/c++/15/bits/std_function.h:292
 #47 0x000000000097677c in std::function<void(char const*)>::operator() (this=0x2e9d2540,
     __args#0=0x2fe0c70 <top_prompt+16> "(gdb) ") at /usr/include/c++/15/bits/std_function.h:593
 #48 0x00000000009763be in gdb::observers::observable<char const*>::notify (
     this=0x2fdb0c0 <gdb::observers::before_prompt>, args#0=0x2fe0c70 <top_prompt+16> "(gdb) ")
     at gdbsupport/observable.h:166
 #49 0x0000000000973d54 in notify_before_prompt (prompt=0x2fe0c70 <top_prompt+16> "(gdb) ")
     at gdb/event-top.c:518
 #50 0x0000000000973da7 in top_level_prompt () at gdb/event-top.c:534
 #51 0x0000000000973c34 in display_gdb_prompt (new_prompt=0x0)
     at gdb/event-top.c:487
 #52 0x0000000000974681 in command_line_handler (rl=...)
     at gdb/event-top.c:847
 #53 0x000000000107d633 in tui_command_line_handler (rl=...)
     at gdb/tui/tui-interp.c:101
 #54 0x000000000097375b in gdb_rl_callback_handler (rl=0x2ebbdd40 "tui enable")
     at gdb/event-top.c:288
 #55 0x00000000011e1bcf in rl_callback_read_char ()
     at readline/readline/callback.c:302
 #56 0x000000000097342f in gdb_rl_callback_read_char_wrapper_sjlj ()
     at gdb/event-top.c:197
 #57 0x0000000000973540 in gdb_rl_callback_read_char_wrapper_noexcept ()
     at gdb/event-top.c:240
 #58 0x00000000009735be in gdb_rl_callback_read_char_wrapper (client_data=0x2e6d30c0)
     at gdb/event-top.c:252
 #59 0x00000000010c7783 in stdin_event_handler (error=0, client_data=0x2e6d30c0)
     at gdb/ui.c:161
 #60 0x0000000001a90203 in handle_file_event (file_ptr=0x2eb80e90, ready_mask=1)
     at gdbsupport/event-loop.cc:551
 #61 0x0000000001a90846 in gdb_wait_for_event (block=1)
     at gdbsupport/event-loop.cc:672
 #62 0x0000000001a8f723 in gdb_do_one_event (mstimeout=-1)
     at gdbsupport/event-loop.cc:263
 #63 0x00000000006e2ff2 in interp::do_one_event (this=0x2eb2c110, mstimeout=-1)
     at gdb/interps.h:90
 #64 0x0000000000baf71c in start_event_loop () at gdb/main.c:401
 #65 0x0000000000baf907 in captured_command_loop () at gdb/main.c:466
 #66 0x0000000000bb18b0 in captured_main (context=0x7ffeb6a6fab0)
     at gdb/main.c:1373
 #67 0x0000000000bb19ac in gdb_main (args=0x7ffeb6a6fab0)
     at gdb/main.c:1392
 #68 0x0000000000419545 in main (argc=6, argv=0x7ffeb6a6fbe8)
     at gdb/gdb.c:38
...

I didn't manage to reproduce it so far.  For more details on the setup I used,
see the PR.

Regardless, I think we can conclude from the backtrace what happened:
- tui_before_prompt observer is called
- debuginfod_source_query is called
- during this call, malloc is called, and aborts with message
  "malloc(): smallbin double linked list corrupted"
- the abort is caught and handled by handle_fatal_signal
- handle_fatal_signal calls tui_disable
- tui_disable calls its observers
- one of the observers calls calloc
- calloc hangs

AFAIU, the problem in gdb is that we call calloc after abort is called.

Fix this by:
- splitting tui_disable in two:
  - tui_disable_restore_cli, and
  - tui_disable_finalize, handling tui_remove_hooks, color map clearing,
    gdb_save_tty_state and gdb::observers::tui_enabled.notify.
- using tui_disable_restore_cli instead of tui_disable in handle_fatal_signal
  and tui_exit

Logging doesn't seem to be a priority when preparing to exit, so I'm using
the TUI_SCOPED_DEBUG_ENTER_EXIT only in tui_disable.

A tricky question is what to do with internal_vproblem.  It may either quit
gdb, or not.  I investigated whether we can first do tui_disable_restore_cli,
and if the answer to "Quit this debugging session? (y or n) " is n, also do
tui_disable_finalize.  But internal_vproblem also potentially changes terminal
state, and AFAIU gdb_save_tty_state is needed before that.

So instead I replaced tui_disable with tui_disable_restore_cli and
tui_disable_finalize.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34732
---
 gdb/event-top.c      |  2 +-
 gdb/tui/tui-interp.c |  2 +-
 gdb/tui/tui-io.c     | 11 +++++++--
 gdb/tui/tui-io.h     |  3 +++
 gdb/tui/tui.c        | 59 +++++++++++++++++++++++++++++++++-----------
 gdb/tui/tui.h        | 11 ++++++++-
 gdb/utils.c          |  5 ++--
 7 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index eedf75331ba..6bae629308f 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -976,7 +976,7 @@ handle_fatal_signal (int sig)
     scoped_restore restore_current_ui
       = make_scoped_restore (&current_ui, main_ui);
 
-    tui_disable ();
+    tui_disable_restore_cli ();
   }
 #endif
 
diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
index 5d1305a8fec..8b8c56c044e 100644
--- a/gdb/tui/tui-interp.c
+++ b/gdb/tui/tui-interp.c
@@ -61,7 +61,7 @@ tui_exit (void)
 
   /* Disable the tui.  Curses mode is left leaving the screen in a
      clean state (see endwin()).  */
-  tui_disable ();
+  tui_disable_restore_cli ();
 }
 
 /* These implement the TUI interpreter.  */
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 35515fc4ca4..1c34b31b882 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -925,11 +925,18 @@ tui_setup_io (int mode)
 
       /* Clean up color information.  */
       last_style = ui_file_style ();
-      color_map.clear ();
-      color_pair_map.clear ();
     }
 }
 
+/* See tui-io.h.  */
+
+void
+tui_free_color_maps ()
+{
+  color_map.clear ();
+  color_pair_map.clear ();
+}
+
 #ifdef SIGCONT
 /* Catch SIGCONT to restore the terminal and refresh the screen.  */
 static void
diff --git a/gdb/tui/tui-io.h b/gdb/tui/tui-io.h
index 63751638717..93b0735b511 100644
--- a/gdb/tui/tui-io.h
+++ b/gdb/tui/tui-io.h
@@ -60,4 +60,7 @@ extern cli_ui_out *tui_old_uiout;
    next line.  */
 extern void tui_inject_newline_into_command_window ();
 
+/* Clean up color information.  */
+extern void tui_free_color_maps ();
+
 #endif /* GDB_TUI_TUI_IO_H */
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index a9a23f29b9a..39440ea7366 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -685,25 +685,20 @@ tui_enable (void)
   gdb::observers::tui_enabled.notify (true);
 }
 
-/* Leave the tui mode.
-   Remove the tui hooks and configure the gdb output and readline
-   back to their original state.  The curses mode is left so that
-   the terminal setting is restored to the point when we entered.  */
-void
-tui_disable (void)
-{
-  TUI_SCOPED_DEBUG_ENTER_EXIT;
+/* See tui.h.  */
 
+bool
+tui_disable_restore_cli ()
+{
   if (!tui_active)
-    return;
+    return false;
 
-  require_tui_interpreter ();
+  const char *interp = top_level_interpreter ()->name ();
+  gdb_assert (streq (interp, INTERP_TUI));
 
   /* Restore initial readline keymap.  */
   rl_set_keymap (tui_readline_standard_keymap);
 
-  /* Remove TUI hooks.  */
-  tui_remove_hooks ();
   rl_startup_hook = 0;
   rl_already_prompted = 0;
 
@@ -718,15 +713,49 @@ tui_disable (void)
      so that terminal management with the inferior works.  */
   tui_setup_io (0);
 
-  /* Update gdb's knowledge of its terminal.  */
-  gdb_save_tty_state ();
-
   tui_active = false;
   tui_update_gdb_sizes ();
+  return true;
+}
+
+/* See tui.h.  */
+
+void
+tui_disable_finalize ()
+{
+  const char *interp = top_level_interpreter ()->name ();
+  gdb_assert (streq (interp, INTERP_TUI));
+  gdb_assert (!tui_active);
+
+  /* Remove TUI hooks.  */
+  tui_remove_hooks ();
+
+  tui_free_color_maps ();
+
+  /* Update gdb's knowledge of its terminal.  */
+  gdb_save_tty_state ();
 
   gdb::observers::tui_enabled.notify (false);
 }
 
+/* Leave the tui mode.
+   Remove the tui hooks and configure the gdb output and readline
+   back to their original state.  The curses mode is left so that
+   the terminal setting is restored to the point when we entered.  */
+void
+tui_disable ()
+{
+  TUI_SCOPED_DEBUG_ENTER_EXIT;
+
+  if (!tui_active)
+    return;
+
+  require_tui_interpreter ();
+
+  if (tui_disable_restore_cli ())
+    tui_disable_finalize ();
+}
+
 /* Command wrapper for enabling tui mode.  */
 
 static void
diff --git a/gdb/tui/tui.h b/gdb/tui/tui.h
index b24fed2b322..85f3990757c 100644
--- a/gdb/tui/tui.h
+++ b/gdb/tui/tui.h
@@ -73,7 +73,16 @@ extern void tui_ensure_readline_initialized ();
 extern void tui_enable (void);
 
 /* Leave the tui mode.  */
-extern void tui_disable (void);
+extern void tui_disable ();
+
+/* Disable TUI sufficiently to print error messages and exit.  Avoids heap
+   alloc and dealloc.  Return true if finalization is needed.  */
+extern bool tui_disable_restore_cli ();
+
+/* Do the work for tui_disable that hasn't been done in
+   tui_disable_restore_cli: finalize disabling TUI, such that we can continue
+   the gdb session.  May use heap alloc and dealloc.  */
+extern void tui_disable_finalize ();
 
 enum tui_key_mode
 {
diff --git a/gdb/utils.c b/gdb/utils.c
index 1ec7454a606..077a0b97dc6 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -28,7 +28,7 @@
 #endif /* HAVE_SYS_RESOURCE_H */
 
 #ifdef TUI
-/* For tui_get_command_dimension and tui_disable.  */
+/* For tui_get_command_dimension and tui_disable_*.  */
 #include "tui/tui.h"
 #endif
 
@@ -394,7 +394,8 @@ internal_vproblem (struct internal_problem *problem,
     scoped_restore restore_current_ui
       = make_scoped_restore (&current_ui, main_ui);
 
-    tui_disable ();
+    if (tui_disable_restore_cli ())
+      tui_disable_finalize ();
   }
 #endif
 
-- 
2.51.0