[RFC,v6,4/8] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode.

  On 03/04/2014 06:32 PM, Tom Tromey wrote:
> From: Pedro Alves <palves@redhat.com>
> 
> The other part of PR gdb/13860 is about console execution commands
> in MI getting their output half lost.  E.g., take the finish command,
> executed on a frontend's GDB console:
> 

...

> +
>  static void
>  mi_on_normal_stop (struct bpstats *bs, int print_frame)
>  {
> @@ -445,6 +470,47 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
>  
>  	  current_uiout = saved_uiout;
>  	}
> +      /* Otherwise, frame information has already been printed by
> +	 normal_stop.  */
> +      else if (target_can_async_p ())
> +	{
> +	  /* However, CLI execution commands (-interpreter-exec
> +	     console "next", for example) in async mode have the
> +	     opposite issue.  normal_stop has already printed frame
> +	     information to MI uiout, but nothing has printed the same
> +	     information to the CLI channel.  We should print the
> +	     source line to the console when stepping or other similar
> +	     commands, iff the step was started by a console command
> +	     (but not if it was started with -exec-step or similar).
> +	     Breakpoint hits should always be mirrored to the
> +	     console.  */

I thought about this mirroring of breakpoints to the console
only done in async, and given we're trying to make the CLI vs MI
output match, it seemed a little silly to not do the same in
sync mode.  So I removed the target_can_async_p() check above,
and extended the comments to explain why we do this.  I then
extended the tests to make sure the CLI output appears at
the right times, and that it doesn't appear when it shouldn't.
See below.  Let me know what you think.

> +	  struct thread_info *tp = inferior_thread ();
> +
> +	  if ((tp->control.command_interp != NULL
> +	       && tp->control.command_interp != top_level_interpreter ())
> +	      || (!tp->control.stop_step
> +		  && !tp->control.proceed_to_finish))
> +	    {
> +	      struct mi_interp *mi = top_level_interpreter_data ();
> +	      struct target_waitstatus last;
> +	      ptid_t last_ptid;
> +	      struct ui_out *cli_uiout;
> +	      struct cleanup *old_chain;
> +
> +	      /* Sets the current uiout to a new temporary CLI uiout
> +		 assigned to STREAM.  */
> +	      cli_uiout = cli_out_new (mi->out);
> +	      old_chain = make_cleanup (ui_out_free_cleanup, cli_uiout);
> +
> +	      make_cleanup (restore_current_uiout_cleanup, current_uiout);
> +	      current_uiout = cli_uiout;
> +
> +	      get_last_target_status (&last_ptid, &last);
> +	      print_stop_event (&last);
> +
> +	      do_cleanups (old_chain);
> +	    }
> +	}

------------------
[PATCH] PR gdb/13860: don't lose '-interpreter-exec console
 EXECUTION_COMMAND''s output in async mode.

The other part of PR gdb/13860 is about console execution commands in
MI getting their output half lost.  E.g., take the finish command,
executed on a frontend's GDB console:

sync:

  finish
  &"finish\n"
  ~"Run till exit from #0  usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
  ^running
  *running,thread-id="1"
  (gdb)
  ~"0x00000000004004d7 in foo () at stepinf.c:6\n"
  ~"6\t    usleep (10);\n"
  ~"Value returned is $1 = 0\n"
  *stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},thread-id="1",stopped-threads="all",core="1"

async:

  finish
  &"finish\n"
  ~"Run till exit from #0  usleep (useconds=10) at ../sysdeps/unix/sysv/linux/usleep.c:27\n"
  ^running
  *running,thread-id="1"
  (gdb)
  *stopped,reason="function-finished",frame={addr="0x00000000004004d7",func="foo",args=[],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="6"},gdb-result-var="$1",return-value="0",thread-id="1",stopped-threads="all",core="0"

Note how all the "Value returned" etc. output is missing in async mode.

The same happens with e.g., catchpoints:

  =breakpoint-modified,bkpt={number="1",type="catchpoint",disp="keep",enabled="y",what="22016",times="1"}
  ~"\nCatchpoint "
  ~"1 (forked process 22016), 0x0000003791cbd8a6 in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:131\n"
  ~"131\t  pid = ARCH_FORK ();\n"
  *stopped,reason="fork",disp="keep",bkptno="1",newpid="22016",frame={addr="0x0000003791cbd8a6",func="__libc_fork",args=[],file="../nptl/sysdeps/unix/sysv/linux/fork.c",fullname="/usr/src/debug/glibc-2.14-394-g8f3b1ff/nptl/sysdeps/unix/sysv/linux/fork.c",line="131"},thread-id="1",stopped-threads="all",core="0"

where all those ~ lines are missing in async mode, or just the "step"
current line indication:

  s
  &"s\n"
  ^running
  *running,thread-id="all"
  (gdb)
  ~"13\t  foo ();\n"
  *stopped,frame={addr="0x00000000004004ef",func="main",args=[{name="argc",value="1"},{name="argv",value="0x7fffffffdd78"}],file="stepinf.c",fullname="/home/pedro/gdb/tests/stepinf.c",line="13"},thread-id="1",stopped-threads="all",core="3"
  (gdb)

Or in the case of the PRs example, the "Stopped due to shared library
event" note:

  start
  &"start\n"
  ~"Temporary breakpoint 1 at 0x400608: file ../../../src/gdb/testsuite/gdb.mi/solib-main.c, line 21.\n"
  =breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x0000000000400608",func="main",file="../../../src/gdb/testsuite/gdb.mi/solib-main.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/solib-main.c",line="21",times="0",original-location="main"}
  ~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main \n"
  =thread-group-started,id="i1",pid="21990"
  =thread-created,id="1",group-id="i1"
  ^running
  *running,thread-id="all"
  (gdb)
  =library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1"
  ~"Stopped due to shared library event (no libraries added or removed)\n"
  *stopped,reason="solib-event",thread-id="1",stopped-threads="all",core="3"
  (gdb)

IMO, if you're typing execution commands in a frontend's console, you
expect to see their output.  Indeed it's what you get in sync mode.  I
think async mode should do the same.  Deciding what to mirror to the
console wrt to breakpoints and random stops gets messy real fast.
E.g., say "s" trips on a breakpoint.  We'd clearly want to mirror the
event to the console in this case.  But what about more complicated
cases like "s&; thread n; s&", and one of those steps spawning a new
thread, and that thread hitting a breakpoint?  It's impossible in
general to track whether the thread had any relation to the commands
that had been executed.  So I think we should just simplify and always
mirror breakpoints and random events to the console.

Notes:

  - mi->out is the same as gdb_stdout when MI is the current
    interpreter.  I think that referring to that directly is cleaner.
    An earlier revision of this patch made the changes that are now
    done in mi_on_normal_stop directly in infrun.c:normal_stop, and so
    not having an obvious place to put the new uiout by then, and not
    wanting to abuse CLI's uiout, I made a temporary uiout when
    necessary.

  - Hopefuly the rest of the patch is more or less obvious given the
    comments added.

Tested on x86_64 Fedora 17, no regressions.

2014-03-18  Pedro Alves  <palves@redhat.com>

	PR gdb/13860
	* gdbthread.h (struct thread_control_state): New field
	`command_interp'.
	* infrun.c (follow_fork): Copy the new thread control field to the
	child fork thread.
	(clear_proceed_status_thread): Clear the new thread control field.
	(proceed): Set the new thread control field.
	* interps.h (command_interp): Declare.
	* interps.c (command_interpreter): New global.
	(command_interp): New function.
	(interp_exec): Set `command_interpreter' while here.
	* cli-out.c (cli_uiout_dtor): New function.
	(cli_ui_out_impl): Install it.
	* mi/mi-interp.c: Include cli-out.h.
	(mi_cmd_interpreter_exec): Add comment.
	(restore_current_uiout_cleanup): New function.
	(ui_out_free_cleanup): New function.
	(mi_on_normal_stop): If finishing an execution command started by
	a CLI command, or any kind of breakpoint-like event triggered,
	print the stop event to the output (CLI) stream.
	* mi/mi-out.c (mi_ui_out_impl): Install NULL `dtor' handler.

2014-03-18  Pedro Alves  <palves@redhat.com>

	PR gdb/13860
	* gdb.mi/mi-cli.exp (line_callee4_next_step): New global.
	(top level): Test that output related to execution commands is
	sent to the console with CLI commands, but not with MI commands.
	Test that breakpoint events are always mirrored to the console.
	Also expect the new source line to be output after a "next" in
	async mode too.  Make it a pass/fail test.
	* gdb.mi/mi-solib.exp: Test that the CLI solib event note is
	output.
	* lib/mi-support.exp (mi_gdb_expect_cli_output): New procedure.
---
 gdb/cli-out.c                     | 13 ++++++-
 gdb/gdbthread.h                   |  5 +++
 gdb/infrun.c                      | 14 +++++++
 gdb/interps.c                     | 36 +++++++++++++++++-
 gdb/interps.h                     |  2 +
 gdb/mi/mi-interp.c                | 77 +++++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-cli.exp   | 52 ++++++++++++++++++++++----
 gdb/testsuite/gdb.mi/mi-solib.exp | 11 ++++++
 gdb/testsuite/lib/mi-support.exp  | 24 ++++++++++++
 9 files changed, 225 insertions(+), 9 deletions(-)

[RFC,v6,4/8] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode.

Commit Message

Comments

Patch