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

Simon Marchi via Gdb-patches
2022-04-06T12:57:50Z

On 2022-04-06 08:04, Andrew Burgess via Gdb-patches wrote:
> 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).

I think this is reasonable. In theory, the internal error could be for
something you don't care about, and you could still want to continue.
But there's also a limit to the far-fetched theoritical scenarios we
must support. So I think it is ok to say that any internal error during
initialization is fatal. The patch LGTM.

Simon