[PATCHv2 01/16] gdb: don't try to use readline before it's initialized

Andrew Burgess via Gdb-patches
2022-04-06T12:05:45Z

While working on a different patch, I triggered an assertion from the
initialize_current_architecture code, specifically from one of
the *_gdbarch_init functions in a *-tdep.c file. This exposes a
couple of issues with GDB.

This is easy enough to reproduce by adding 'gdb_assert (false)' into a
suitable function. For example, I added a line into i386_gdbarch_init
and can see the following issue.

I start GDB and immediately hit the assert, the output is as you'd
expect, except for the very last line:

$ ./gdb/gdb --data-directory ./gdb/data-directory/
../../src.dev-1/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
... snip ...
---------------------
../../src.dev-1/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) ../../src.dev-1/gdb/ser-event.c:212:16: runtime error: member access within null pointer of type 'struct serial'

Something goes wrong when we try to query the user. Note, I
configured GDB with --enable-ubsan, I suspect that without this the
above "error" would actually just be a crash.

The backtrace from ser-event.c:212 looks like this:

(gdb) bt 10
#0 serial_event_clear (event=0x675c020) at ../../src/gdb/ser-event.c:212
#1 0x0000000000769456 in invoke_async_signal_handlers () at ../../src/gdb/async-event.c:211
#2 0x000000000295049b in gdb_do_one_event () at ../../src/gdbsupport/event-loop.cc:194
#3 0x0000000001f015f8 in gdb_readline_wrapper (
prompt=0x67135c0 "../../src/gdb/i386-tdep.c:8455: internal-error: i386_gdbarch_init: Assertion `false' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugg"...)
at ../../src/gdb/top.c:1141
#4 0x0000000002118b64 in defaulted_query(const char *, char, typedef __va_list_tag __va_list_tag *) (
ctlstr=0x2e4eb68 "%s\nQuit this debugging session? ", defchar=0 '\000', args=0x7fffffffa6e0)
at ../../src/gdb/utils.c:934
#5 0x0000000002118f72 in query (ctlstr=0x2e4eb68 "%s\nQuit this debugging session? ")
at ../../src/gdb/utils.c:1026
#6 0x00000000021170f6 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) (problem=0x6107bc0 <internal_error_problem>, file=0x2b976c8 "../../src/gdb/i386-tdep.c",
line=8455, fmt=0x2b96d7f "%s: Assertion `%s' failed.", ap=0x7fffffffa8e8) at ../../src/gdb/utils.c:417
#7 0x00000000021175a0 in internal_verror (file=0x2b976c8 "../../src/gdb/i386-tdep.c", line=8455,
fmt=0x2b96d7f "%s: Assertion `%s' failed.", ap=0x7fffffffa8e8) at ../../src/gdb/utils.c:485
#8 0x00000000029503b3 in internal_error (file=0x2b976c8 "../../src/gdb/i386-tdep.c", line=8455,
fmt=0x2b96d7f "%s: Assertion `%s' failed.") at ../../src/gdbsupport/errors.cc:55
#9 0x000000000122d5b6 in i386_gdbarch_init (info=..., arches=0x0) at ../../src/gdb/i386-tdep.c:8455
(More stack frames follow...)

It turns out that the problem is that the async event handler
mechanism has been invoked, but this has not yet been initialized.

If we look at gdb_init (in gdb/top.c) we can indeed see the call to
gdb_init_signals is after the call to initialize_current_architecture.

If I reorder the calls, moving gdb_init_signals earlier, then the
initial error is resolved, however, things are still broken. I now
see the same "Quit this debugging session? (y or n)" prompt, but when
I provide an answer and press return GDB immediately crashes.

So what's going on now? The next problem is that the call_readline
field within the current_ui structure is not initialized, and this
callback is invoked to process the reply I entered.

The problem is that call_readline is setup as a result of calling
set_top_level_interpreter, which is called from captured_main_1.
Unfortunately, set_top_level_interpreter is called after gdb_init is
called.

I wondered how to solve this problem for a while, however, I don't
know if there's an easy "just reorder some lines" solution here.
Looking through captured_main_1 there seems to be a bunch of
dependencies between printing various things, parsing config files,
and setting up the interpreter. I'm sure there is a solution hiding
in there somewhere.... I'm just not sure I want to spend any longer
looking for it.

So.

I propose a simpler solution, more of a hack/work-around. In utils.c
we already have a function filtered_printing_initialized, this is
checked in a few places within internal_vproblem. In some of these
cases the call gates whether or not GDB will query the user.

My proposal is to add a new readline_initialized function, which
checks if the current_ui has had readline initialized yet. If this is
not the case then we should not attempt to query the user.

After this change GDB prints the error message, the backtrace, and
then aborts (including dumping core). This actually seems pretty sane
as, if GDB has not yet made it through the initialization then it
doesn't make much sense to allow the user to say "no, I don't want to
quit the debug session" (I think).
---
gdb/utils.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index 68bf3a67340..62a75d9e37f 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -306,6 +306,16 @@ struct internal_problem
bool should_print_backtrace;
};

+/* Return true if the readline callbacks have been initialized for UI.
+ This is always true once GDB is fully initialized, but during the early
+ startup phase this is initially false. */
+
+static bool
+readline_initialized (struct ui *ui)
+{
+ return ui->call_readline != nullptr;
+}
+
/* Report a problem, internal to GDB, to the user. Once the problem
has been reported, and assuming GDB didn't quit, the caller can
either allow execution to resume or throw an error. */
@@ -378,6 +388,7 @@ internal_vproblem (struct internal_problem *problem,
if (problem->should_quit != internal_problem_ask
|| !confirm
|| !filtered_printing_initialized ()
+ || !readline_initialized (current_ui)
|| problem->should_print_backtrace)
gdb_printf (gdb_stderr, "%s\n", reason.c_str ());

@@ -389,7 +400,8 @@ internal_vproblem (struct internal_problem *problem,
/* Default (yes/batch case) is to quit GDB. When in batch mode
this lessens the likelihood of GDB going into an infinite
loop. */
- if (!confirm || !filtered_printing_initialized ())
+ if (!confirm || !filtered_printing_initialized ()
+ || !readline_initialized (current_ui))
quit_p = 1;
else
quit_p = query (_("%s\nQuit this debugging session? "),
@@ -413,7 +425,8 @@ internal_vproblem (struct internal_problem *problem,
{
if (!can_dump_core_warn (LIMIT_MAX, reason.c_str ()))
dump_core_p = 0;
- else if (!filtered_printing_initialized ())
+ else if (!filtered_printing_initialized ()
+ || !readline_initialized (current_ui))
dump_core_p = 1;
else
{
--
2.25.4