diff mbox series

[v4,1/2] gdb: install CLI uiout while processing early init files

Message ID 20231124161321.572511-2-richard.bunt@linaro.org
State New
Headers
Series [v4,1/2] gdb: install CLI uiout while processing early init files |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Richard Bunt Nov. 24, 2023, 4:13 p.m. UTC 
  From: Andrew Burgess <aburgess@redhat.com>

The next commit wants to use a 'show' command within an early
initialisation file, despite these commands not being in the list of
acceptable commands for use within an early initialisation file.

The problem we run into is that the early initialisation files are
processed before GDB has installed the top level interpreter.  The
interpreter is responsible to installing the default uiout (accessed
through current_uiout), and as a result code that depends on
uiout (e.g. 'show' commands) will end up dereferencing a nullptr, and
crashing GDB.

I did consider moving the interpreter installation before the early
initialisation, and this would work fine except for the new DAP
interpreter, which relies on having Python available during its
initialisation.  Which means we can't install the interpreter until
after Python has been initialised, and the early initialisation
handling has to occur before Python is setup -- that's the whole point
of this feature (to allow customisation of how Python is setup).

So, what I propose is that early within captured_main_1, we install a
temporary cli_ui_out as the current_uiout.  This will remain in place
until the top-level interpreter is installed, at which point the
temporary will be replaced.

What this means is that current_uiout will no longer be nullptr,
instead, any commands within an early initialisation file that trigger
output, will perform that output in a CLI style.

I propose that we don't update the documentation for early
initialisation files, we leave the user advice as being only 'set' and
'source' commands are acceptable.  But now, if a user does try a
'show' command, then instead of crashing, GDB will do something
predictable.

I've not added a test in this commit.  The next commit relies on this
patch and will serve as a test.

Tested-By: Richard Bunt <richard.bunt@linaro.org>

---
 gdb/main.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
diff mbox series

Patch

diff --git a/gdb/main.c b/gdb/main.c
index 486c1ffe071..eb11d6f2e46 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -56,6 +56,7 @@ 
 #include "gdbsupport/alt-stack.h"
 #include "observable.h"
 #include "serial.h"
+#include "cli-out.h"
 
 /* The selected interpreter.  */
 std::string interpreter_p;
@@ -688,6 +689,16 @@  captured_main_1 (struct captured_main_args *context)
   gdb_stdtargerr = gdb_stderr;
   gdb_stdtargin = gdb_stdin;
 
+  /* Put a CLI based uiout in place early.  If the early initialization
+     files trigger any I/O then it isn't hard to reach parts of GDB that
+     assume current_uiout is not nullptr.  Maybe we should just install the
+     CLI interpreter initially, then switch to the application requested
+     interpreter later?  But that would (potentially) result in an
+     interpreter being instantiated "just in case".  For now this feels
+     like the least effort way to protect GDB from crashing.  */
+  auto temp_uiout = std::make_unique<cli_ui_out> (gdb_stdout);
+  current_uiout = temp_uiout.get ();
+
   if (bfd_init () != BFD_INIT_MAGIC)
     error (_("fatal error: libbfd ABI mismatch"));
 
@@ -1142,6 +1153,10 @@  captured_main_1 (struct captured_main_args *context)
      look at things by now.  Initialize the default interpreter.  */
   set_top_level_interpreter (interpreter_p.c_str ());
 
+  /* The interpreter should have installed the real uiout by now.  */
+  gdb_assert (current_uiout != temp_uiout.get ());
+  temp_uiout = nullptr;
+
   if (!quiet)
     {
       /* Print all the junk at the top, with trailing "..." if we are