public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: gdb-patches@sourceware.org
Cc: Pedro Alves <palves@redhat.com>
Subject: [PATCH v4 5/9] PR gdb/13860: make "-exec-foo"'s MI output equal to "foo"'s MI output.
Date: Tue, 22 Oct 2013 17:59:00 -0000	[thread overview]
Message-ID: <1382464769-2465-6-git-send-email-tromey@redhat.com> (raw)
In-Reply-To: <1382464769-2465-1-git-send-email-tromey@redhat.com>

From: Pedro Alves <palves@redhat.com>

Part of PR gdb/13860 is about the mi-solib.exp test's output being
different in sync vs async modes.

sync:

  >./gdb -nx -q ./testsuite/gdb.mi/solib-main -ex "set stop-on-solib-events 1" -ex "set target-async off" -i=mi
  =thread-group-added,id="i1"
  ~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main..."
  ~"done.\n"
  (gdb)
  &"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="17724"
  =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",frame={addr="0x000000379180f990",func="_dl_debug_state",args=[],from="/lib64/ld-linux-x86-64.so.2"},thread-id="1",stopped-threads="all",core="3"
  (gdb)

async:

  >./gdb -nx -q ./testsuite/gdb.mi/solib-main -ex "set stop-on-solib-events 1" -ex "set target-async on" -i=mi
  =thread-group-added,id="i1"
  ~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main..."
  ~"done.\n"
  (gdb)
  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="17729"
  =thread-created,id="1",group-id="i1"
  ^running
  *running,thread-id="all"
  =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"
  (gdb)
  *stopped,reason="solib-event",thread-id="1",stopped-threads="all",core="1"

For now, let's focus only on the *stopped event.  We see that the
async output is missing frame info.  And this causes a test failure in
async mode, as "mi_expect_stop solib-event" wants to see the frame
info.

However, if we compare the event output when a real MI execution
command is used, compared to a CLI command (e.g., run vs -exec-run,
next vs -exec-next, etc.), we see:

  >./gdb -nx -q ./testsuite/gdb.mi/solib-main -ex "set stop-on-solib-events 1" -ex "set target-async off" -i=mi
  =thread-group-added,id="i1"
  ~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main..."
  ~"done.\n"
  (gdb)
  r
  &"r\n"
  ~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/solib-main \n"
  =thread-group-started,id="i1",pid="17751"
  =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",frame={addr="0x000000379180f990",func="_dl_debug_state",args=[],from="/lib64/ld-linux-x86-64.so.2"},thread-id="1",stopped-threads="all",core="3"
  (gdb)
  -exec-run
  =thread-exited,id="1",group-id="i1"
  =thread-group-exited,id="i1"
  =library-unloaded,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",thread-group="i1"
  =thread-group-started,id="i1",pid="17754"
  =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,reason="solib-event",thread-id="1",stopped-threads="all",core="1"
  =thread-selected,id="1"
  (gdb)

As seen above, with MI commands, the *stopped event _doesn't_ have
frame info.  This is because normal_stop, as commanded by the result
of bpstat_print, skips printing frame info in this case (it's an
"event", not a "breakpoint"), and when the interpreter is MI,
mi_on_normal_stop skips calling print_stack_frame, as the normal_stop
call was already done with the MI uiout.  This explains why the async
output is different even with a CLI command.  Its because in async
mode, the mi_on_normal_stop path is always taken; it is always reached
with the MI uiout, because the stop is handled from the event loop,
instead of from within `proceed -> wait_for_inferior -> normal_stop'
with the interpreter overridden, as in sync mode.

This patch fixes the issue by making all cases output the same
*stopped event, by factoring out the print code from normal_stop, and
using it from mi_on_normal_stop as well.  I chose the *stopped output
without a frame, mainly because that is what you already get if you
use MI execution commands, the commands frontends are supposed to use
(except when implementing a console).  This patch makes it simpler to
tweak the MI output differently if desired, as we only have to change
the centralized print_stop_event (taking into account whether the
uiout is MI-like), and all different modes will change accordingly.

Tested on x86_64 Fedora 16, no regressions.  The mi-solib.exp test no
longer fails in async mode with this patch, so the patch removes the
kfail.

2012-05-09  Pedro Alves  <palves@redhat.com>

	PR gdb/13860

	gdb/
	* inferior.h (print_stop_event): Declare.
	* infrun.c (print_stop_event): New, factored out from ...
	(normal_stop): ... this.
	* mi/mi-interp.c (mi_on_normal_stop): Use print_stop_event instead
	of bpstat_print/print_stack_frame.

	gdb/testsuite/
	* gdb.mi/mi-solib.exp: Remove gdb/13860 kfail.
	* lib/mi-support.exp (mi_expect_stop): Add special handling for
	solib-event.
---
 gdb/inferior.h                    |   2 +
 gdb/infrun.c                      | 118 ++++++++++++++++++++------------------
 gdb/mi/mi-interp.c                |   3 +-
 gdb/testsuite/gdb.mi/mi-solib.exp |   4 --
 gdb/testsuite/lib/mi-support.exp  |  18 +++++-
 5 files changed, 82 insertions(+), 63 deletions(-)

diff --git a/gdb/inferior.h b/gdb/inferior.h
index fff072b..cd6d962 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -205,6 +205,8 @@ extern void start_remote (int from_tty);
 
 extern void normal_stop (void);
 
+extern void print_stop_event (struct target_waitstatus *ws);
+
 extern int signal_stop_state (int);
 
 extern int signal_print_state (int);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index b1d2dc5..49e8ab5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5964,6 +5964,68 @@ print_no_history_reason (void)
   ui_out_text (current_uiout, "\nNo more reverse-execution history.\n");
 }
 
+/* Print current location without a level number, if we have changed
+   functions or hit a breakpoint.  Print source line if we have one.
+   bpstat_print contains the logic deciding in detail what to print,
+   based on the event(s) that just occurred.  */
+
+void
+print_stop_event (struct target_waitstatus *ws)
+{
+  int bpstat_ret;
+  int source_flag;
+  int do_frame_printing = 1;
+  struct thread_info *tp = inferior_thread ();
+
+  bpstat_ret = bpstat_print (tp->control.stop_bpstat, ws->kind);
+  switch (bpstat_ret)
+    {
+    case PRINT_UNKNOWN:
+      /* FIXME: cagney/2002-12-01: Given that a frame ID does (or
+	 should) carry around the function and does (or should) use
+	 that when doing a frame comparison.  */
+      if (tp->control.stop_step
+	  && frame_id_eq (tp->control.step_frame_id,
+			  get_frame_id (get_current_frame ()))
+	  && step_start_function == find_pc_function (stop_pc))
+	{
+	  /* Finished step, just print source line.  */
+	  source_flag = SRC_LINE;
+	}
+      else
+	{
+	  /* Print location and source line.  */
+	  source_flag = SRC_AND_LOC;
+	}
+      break;
+    case PRINT_SRC_AND_LOC:
+      /* Print location and source line.  */
+      source_flag = SRC_AND_LOC;
+      break;
+    case PRINT_SRC_ONLY:
+      source_flag = SRC_LINE;
+      break;
+    case PRINT_NOTHING:
+      /* Something bogus.  */
+      source_flag = SRC_LINE;
+      do_frame_printing = 0;
+      break;
+    default:
+      internal_error (__FILE__, __LINE__, _("Unknown value."));
+    }
+
+  /* The behavior of this routine with respect to the source
+     flag is:
+     SRC_LINE: Print only source line
+     LOCATION: Print only location
+     SRC_AND_LOC: Print location and source line.  */
+  if (do_frame_printing)
+    print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1);
+
+  /* Display the auto-display expressions.  */
+  do_displays ();
+}
+
 /* Here to return control to GDB when the inferior stops for real.
    Print appropriate messages, remove breakpoints, give terminal our modes.
 
@@ -6086,65 +6148,11 @@ normal_stop (void)
     {
       select_frame (get_current_frame ());
 
-      /* Print current location without a level number, if
-         we have changed functions or hit a breakpoint.
-         Print source line if we have one.
-         bpstat_print() contains the logic deciding in detail
-         what to print, based on the event(s) that just occurred.  */
-
       /* If --batch-silent is enabled then there's no need to print the current
 	 source location, and to try risks causing an error message about
 	 missing source files.  */
       if (stop_print_frame && !batch_silent)
-	{
-	  int bpstat_ret;
-	  int source_flag;
-	  int do_frame_printing = 1;
-	  struct thread_info *tp = inferior_thread ();
-
-	  bpstat_ret = bpstat_print (tp->control.stop_bpstat, last.kind);
-	  switch (bpstat_ret)
-	    {
-	    case PRINT_UNKNOWN:
-	      /* FIXME: cagney/2002-12-01: Given that a frame ID does
-	         (or should) carry around the function and does (or
-	         should) use that when doing a frame comparison.  */
-	      if (tp->control.stop_step
-		  && frame_id_eq (tp->control.step_frame_id,
-				  get_frame_id (get_current_frame ()))
-		  && step_start_function == find_pc_function (stop_pc))
-		source_flag = SRC_LINE;		/* Finished step, just
-						   print source line.  */
-	      else
-		source_flag = SRC_AND_LOC;	/* Print location and
-						   source line.  */
-	      break;
-	    case PRINT_SRC_AND_LOC:
-	      source_flag = SRC_AND_LOC;	/* Print location and
-						   source line.  */
-	      break;
-	    case PRINT_SRC_ONLY:
-	      source_flag = SRC_LINE;
-	      break;
-	    case PRINT_NOTHING:
-	      source_flag = SRC_LINE;	/* something bogus */
-	      do_frame_printing = 0;
-	      break;
-	    default:
-	      internal_error (__FILE__, __LINE__, _("Unknown value."));
-	    }
-
-	  /* The behavior of this routine with respect to the source
-	     flag is:
-	     SRC_LINE: Print only source line
-	     LOCATION: Print only location
-	     SRC_AND_LOC: Print location and source line.  */
-	  if (do_frame_printing)
-	    print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1);
-
-	  /* Display the auto-display expressions.  */
-	  do_displays ();
-	}
+	print_stop_event (&last);
     }
 
   /* Save the function value return registers, if we care.
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 44ebd21..57d997c 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -446,9 +446,8 @@ mi_on_normal_stop (struct bpstats *bs, int print_frame)
 	  current_uiout = mi_uiout;
 
 	  get_last_target_status (&last_ptid, &last);
-	  bpstat_print (bs, last.kind);
+	  print_stop_event (&last);
 
-	  print_stack_frame (get_selected_frame (NULL), 0, SRC_AND_LOC, 1);
 	  current_uiout = saved_uiout;
 	}
 
diff --git a/gdb/testsuite/gdb.mi/mi-solib.exp b/gdb/testsuite/gdb.mi/mi-solib.exp
index a760f92..cd400ea 100644
--- a/gdb/testsuite/gdb.mi/mi-solib.exp
+++ b/gdb/testsuite/gdb.mi/mi-solib.exp
@@ -60,8 +60,4 @@ mi_gdb_test "777-gdb-set stop-on-solib-events 1" "777\\^done" \
 # commands still cause the correct MI output to be generated.
 mi_run_with_cli
 
-global async
-if { $async } {
-    setup_kfail gdb/13860 *-*-*
-}
 mi_expect_stop solib-event .* .* .* .* .* "check for solib event"
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 3d55609..6a67718 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1013,6 +1013,8 @@ proc mi_expect_stop { reason func args file line extra test } {
     global thread_selected_re
     global breakpoint_re
 
+    set any "\[^\n\]*"
+
     set after_stopped ""
     set after_reason ""
     if { [llength $extra] == 2 } {
@@ -1055,6 +1057,20 @@ proc mi_expect_stop { reason func args file line extra test } {
         return
     }
 
+    if { $reason == "solib-event" } {
+	set pattern "\\*stopped,reason=\"solib-event\",thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re|$breakpoint_re)*$prompt_re"
+	verbose -log "mi_expect_stop: expecting: $pattern"
+	gdb_expect {
+	    -re "$pattern" {
+		pass "$test"
+	    }
+	    timeout {
+		fail "$test (unknown output after running)"
+	    }
+	}
+	return
+    }
+
     set args "\\\[$args\\\]"
 
     set bn ""
@@ -1072,8 +1088,6 @@ proc mi_expect_stop { reason func args file line extra test } {
 
     set a $after_reason
 
-    set any "\[^\n\]*"
-
     verbose -log "mi_expect_stop: expecting: \\*stopped,${r}${a}${bn}frame=\{addr=\"$hex\",func=\"$func\",args=$args,(?:file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"|from=\"$file\")\}$after_stopped,thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re|$breakpoint_re)*$prompt_re"
     gdb_expect {
 	-re "\\*stopped,${r}${a}${bn}frame=\{addr=\"$hex\",func=\"$func\",args=$args,(?:file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"|from=\"$file\")\}$after_stopped,thread-id=\"$decimal\",stopped-threads=$any\r\n($thread_selected_re|$breakpoint_re)*$prompt_re" {
-- 
1.8.1.4

  reply	other threads:[~2013-10-22 17:59 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-22 17:59 [PATCH v4 0/9] enable target-async by default Tom Tromey
2013-10-22 17:59 ` Tom Tromey [this message]
2013-10-22 17:59 ` [PATCH v4 3/9] add target method delegation Tom Tromey
2013-10-28 16:05   ` Pedro Alves
2013-10-28 17:51     ` Tom Tromey
2013-10-28 17:53       ` Tom Tromey
2013-10-29 20:55         ` Tom Tromey
2013-11-08 17:44           ` Pedro Alves
2013-12-11 22:03             ` Tom Tromey
2013-12-12  2:46               ` Tom Tromey
2013-12-13 22:07                 ` Tom Tromey
2013-12-16 13:07                   ` Pedro Alves
2013-12-16 21:21                     ` Tom Tromey
2013-12-17 16:17                       ` Pedro Alves
2013-12-18 18:29                         ` Tom Tromey
2013-12-18 22:06                           ` Tom Tromey
2013-12-19 16:03                             ` Pedro Alves
2013-12-19 16:15                               ` Tom Tromey
2013-12-20 19:24                                 ` Tom Tromey
2013-11-08 16:34       ` Pedro Alves
2013-10-22 17:59 ` [PATCH v4 1/9] fix latent bugs in ui-out.c Tom Tromey
2013-10-28 15:20   ` Pedro Alves
2013-10-28 17:36     ` Tom Tromey
2013-10-22 17:59 ` [PATCH v4 2/9] add "this" pointers to more target APIs Tom Tromey
2013-10-28 16:04   ` Pedro Alves
2013-10-28 16:37     ` Tom Tromey
2013-10-28 16:44       ` Pedro Alves
2013-10-28 16:52         ` Tom Tromey
2013-11-08 18:04           ` Pedro Alves
2013-11-08 21:53             ` Tom Tromey
2013-11-09  3:35               ` Pedro Alves
2013-12-06 17:40                 ` Tom Tromey
2013-12-06 18:35                   ` Pedro Alves
2013-12-06 18:23                 ` Tom Tromey
2013-12-06 19:06                   ` Pedro Alves
2013-10-22 17:59 ` [PATCH v4 4/9] PR gdb/13860: make -interpreter-exec console "list" behave more like "list" Tom Tromey
2013-10-22 18:11 ` [PATCH v4 8/9] fix py-finish-breakpoint.exp with always-async Tom Tromey
2013-11-11 19:51   ` Pedro Alves
2013-12-09 17:53     ` Tom Tromey
2013-10-22 18:26 ` [PATCH v4 6/9] PR gdb/13860: don't lose '-interpreter-exec console EXECUTION_COMMAND''s output in async mode Tom Tromey
2013-10-22 18:26 ` [PATCH v4 9/9] enable target-async Tom Tromey
2013-10-22 20:15   ` Eli Zaretskii
2013-11-11 19:54   ` Pedro Alves
2013-11-12 20:53   ` Pedro Alves
2013-11-15  0:45     ` Tom Tromey
2013-11-18 15:42       ` Pedro Alves
2013-12-06 20:44         ` Tom Tromey
2013-12-09 12:01           ` Pedro Alves
2013-12-09 15:57             ` Tom Tromey
2014-02-21 20:23               ` Tom Tromey
2014-02-24 17:38           ` Tom Tromey
2013-10-22 19:00 ` [PATCH v4 7/9] make dprintf.exp pass in always-async mode Tom Tromey
2013-11-12  0:05   ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1382464769-2465-6-git-send-email-tromey@redhat.com \
    --to=tromey@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).