Re: [RFA] Fix "Segmentation fault" when "gdb -v"

[Pedro Alves <pedro@codesourcery.com>]

On Thursday 04 March 2010 07:26:20, Joel Brobecker wrote:
> > All of this code use "printf_filtered".  But this function must be
> > call after "interp_set".
> > But this part of code call before "interp_set".

Sorry I missed this could happen.  Quite obvious in hindsight.

> I think that this was an unforseen side-effect of a recent change.
> IMO, it's better to get rid of this side-effect (needing the interpreter
> to be set) in printf_filtered.

That could mean that GDB could paginate before figuring out if the
top level interpreter is MI, which isn't desirable.  Unless we
always disable pagination until there's an interpreter set, but,
now that I look, actually, we already do:

I started GDB in a really short window (3 lines height), and noticed
that "gdb --version" doesn't paginate either.  prompt_for_continue
_is_ called, but it's eneffective: it calls gdb_readline_wrapper
which first, which tries to print the pagination prompt, but that
fails because current_interp_display_prompt_p just doesn't do
anything if there's no current interpreter yet.  Then it goes into the
event loop waiting for input to the prompt, which just returns
almost immediately, because there's no waitable file registered
in the event loop yet.

> 
> We should NOT change printf_filtered into printf_unfiltered in this case
> because the same function is used in two different situations:
>   - when the user uses -v
>   - when the user types "show version"
> In the latter case, the printf_filtered is appropriate.
> 
> Now, we need to decide whether pagination should be enabled if
> the interpreter is not set. I think it makes sense to disable pagination
> in this case.  If the interpreter is not set yet, we're just printing
> stuff on stdout, we haven't started the interactive session (if any) yet...

I agree, and turns out this is what's already happening, albeit in
a "works almost by accident" way.  I think this patch below wouldn't
be that bad afterall.  It does fix the problem.

-- 
Pedro Alves

2010-03-04  Pedro Alves  <pedro@codesourcery.com>

	* utils.c (fputs_maybe_filtered): Check if there's already a top
	level interpreter before dereferencing it.  If there isn't one,
	don't paginate either.

---
 gdb/utils.c |    1 +
 1 file changed, 1 insertion(+)

Index: src/gdb/utils.c
===================================================================
--- src.orig/gdb/utils.c	2010-03-04 13:27:17.000000000 +0000
+++ src/gdb/utils.c	2010-03-04 13:29:11.000000000 +0000
@@ -2213,6 +2213,7 @@ fputs_maybe_filtered (const char *linebu
   if (stream != gdb_stdout
       || !pagination_enabled
       || (lines_per_page == UINT_MAX && chars_per_line == UINT_MAX)
+      || top_level_interpreter () == NULL
       || ui_out_is_mi_like_p (interp_ui_out (top_level_interpreter ())))
     {
       fputs_unfiltered (linebuffer, stream);