[PATCH 0/8] enable target-async by default

[PATCH 0/8] enable target-async by default

[Tom Tromey]

This series enables target-async by default.

Right now, the target-async setting just exists to let users use
another feature.  This has always seemed weird, but not very
important.  However, now I am starting to have other reasons to want
target-async enabled by default when possible (in particular this is
good for multi-target).

Most of this series is straightforward.  It begins with some latent
bug fixes, the by pulling in some related patches of Pedros (see
patches 3-5), continues by fixing some CLI bugs revealed if
target-async is always on; and finishes by fixing up MI and the docs.

The last patch is the one with the grossest hacks.

I built and regtested this on x86-64 Fedora 18.

FWIW I did not clear out the dead code from linux-nat.c yet.  I plan
to do that in a follow-up.

Tom

[PATCH 3/8] PR gdb/13860: make -interpreter-exec console "list" behave more like "list".

[Tom Tromey]

From: Pedro Alves <palves@redhat.com>

Patch 3 in this series made me notice that "list" behaves differently
in CLI vs MI.  Particularly:

  >./gdb -nx -q ./testsuite/gdb.mi/mi-cli
  Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli...done.
  (gdb) start
  Temporary breakpoint 1 at 0x40054d: file ../../../src/gdb/testsuite/gdb.mi/basics.c, line 62.
  Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli

  Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62
  62        callee1 (2, "A string argument.", 3.5);
  (gdb) list
  57      {
  58      }
  59
  60      main ()
  61      {
  62        callee1 (2, "A string argument.", 3.5);
  63        callee1 (2, "A string argument.", 3.5);
  64
  65        do_nothing (); /* Hello, World! */
  66
  (gdb)

Note the list started at line 57.  IOW, the program stopped at line
62, and GDB centered the list on that.

compare with:

  >./gdb -nx -q ./testsuite/gdb.mi/mi-cli -i=mi
  =thread-group-added,id="i1"
  ~"Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli..."
  ~"done.\n"
  (gdb)
  start
  &"start\n"
  ~"Temporary breakpoint 1 at 0x40054d: file ../../../src/gdb/testsuite/gdb.mi/basics.c, line 62.\n"
  =breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x000000000040054d",func="main",file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62",times="0",original-location="main"}
  ~"Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.mi/mi-cli \n"
  =thread-group-started,id="i1",pid="14221"
  =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"
  =library-loaded,id="/lib64/libm.so.6",target-name="/lib64/libm.so.6",host-name="/lib64/libm.so.6",symbols-loaded="0",thread-group="i1"
  =library-loaded,id="/lib64/libc.so.6",target-name="/lib64/libc.so.6",host-name="/lib64/libc.so.6",symbols-loaded="0",thread-group="i1"
  =breakpoint-modified,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x000000000040054d",func="main",file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62",times="1",original-location="main"}
  ~"\nTemporary breakpoint "
  ~"1, main () at ../../../src/gdb/testsuite/gdb.mi/basics.c:62\n"
  ~"62\t  callee1 (2, \"A string argument.\", 3.5);\n"
  *stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x000000000040054d",func="main",args=[],file="../../../src/gdb/testsuite/gdb.mi/basics.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/basics.c",line="62"},thread-id="1",stopped-threads="all",core="0"
  =breakpoint-deleted,id="1"
  (gdb)
  -interpreter-exec console list
  ~"62\t  callee1 (2, \"A string argument.\", 3.5);\n"
  ~"63\t  callee1 (2, \"A string argument.\", 3.5);\n"
  ~"64\t\n"
  ~"65\t  do_nothing (); /* Hello, World! */\n"
  ~"66\t\n"
  ~"67\t  callme (1);\n"
  ~"68\t  callme (2);\n"
  ~"69\t\n"
  ~"70\t  return 0;\n"
  ~"71\t}\n"
  ^done
  (gdb)

Here the list starts at line 62, where the program was stopped.

This happens because print_stack_frame, called from both normal_stop
and mi_on_normal_stop, is the function responsible for setting the
current sal from the selected frame, overrides the PRINT_WHAT
argument, and only after that does it decide whether to center the
current sal line or not, based on the overriden value, and it will
always decide false.

(The print_stack_frame call in mi_on_normal_stop is a little different
from the call in normal_stop, in that it is an unconditional
SRC_AND_LOC call.  The next patch will make those uniform.)

Tested on x86_64 Fedora 16, no regressions.

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

	* stack.c (print_stack_frame): Compute CENTER before overriding
	PRINT_WHAT.

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

	* gdb.mi/mi-cli.exp: Adjust expected output of "list".
---
 gdb/stack.c                     | 4 ++--
 gdb/testsuite/gdb.mi/mi-cli.exp | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index d89ff89..c64cfdc 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -159,14 +159,14 @@ print_stack_frame (struct frame_info *frame, int print_level,
 {
   volatile struct gdb_exception e;
 
+  int center = (print_what == SRC_LINE || print_what == SRC_AND_LOC);
+
   /* For mi, alway print location and address.  */
   if (ui_out_is_mi_like_p (current_uiout))
     print_what = LOC_AND_ADDRESS;
 
   TRY_CATCH (e, RETURN_MASK_ERROR)
     {
-      int center = (print_what == SRC_LINE || print_what == SRC_AND_LOC);
-
       print_frame_info (frame, print_level, print_what, 1 /* print_args */);
       set_current_sal_from_frame (frame, center);
     }
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index 59af58b..5b809e2 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -91,7 +91,7 @@ mi_gdb_test "-interpreter-exec console \"set listsize 1\"" \
 
 # {.*\~"32[ \t(\\t)]*callee1.*\\n".*\^done }
 mi_gdb_test "-interpreter-exec console \"list\"" \
-  ".*\~\"$line_main_body\[\\\\t \]*callee1.*;\\\\n\".*\\^done" \
+  ".*\~\"57\\\\t\{\\\\n\".*\\^done" \
   "-interpreter-exec console \"list\""
 
 mi_execute_to "exec-continue" "breakpoint-hit" "callee4" "" ".*basics.c" $line_callee4_body \
-- 
1.8.1.4

[PATCH 2/8] add target method delegation

[Tom Tromey]

This patch replaces some code in the record targets with target method
delegation.

Right now there are two latent problems in the record target.

First, record-full.c stores pointers to many target methods when the
record target is pushed.  Then it later delegates some calls via
these.  This is wrong because it violates the target stack contract.
In particular it is ok to unpush a target at any stratum, but
record-full does not keep track of this, so it could potentially call
into an unpushed target.

Second, RECORD_IS_USED and some other spots look at
current_target.to_stratum to determine whether a record target is in
use.  This is bad because arch_stratum is greater than record_stratum.

To fix the first problem, this patch introduces a handful of
target_delegate_* functions, which forward calls further down the
target stack.

To fix the second problem, this patch adds find_target_at to determine
whether a target appears at a given stratum.  This may seem like
overkill somehow, but I have a subsequent patch series (see archer.git
tromey/multi-target) that uses it more heavily.

	* record-full.c (record_full_beneath_to_resume_ops)
	(record_full_beneath_to_resume, record_full_beneath_to_wait_ops)
	(record_full_beneath_to_wait)
	(record_full_beneath_to_store_registers_ops)
	(record_full_beneath_to_store_registers)
	(record_full_beneath_to_xfer_partial_ops)
	(record_full_beneath_to_xfer_partial)
	(record_full_beneath_to_insert_breakpoint)
	(record_full_beneath_to_remove_breakpoint)
	(record_full_beneath_to_stopped_by_watchpoint)
	(record_full_beneath_to_stopped_data_address)
	(record_full_beneath_to_async, tmp_to_resume_ops, tmp_to_resume)
	(tmp_to_wait_ops, tmp_to_wait, tmp_to_store_registers_ops)
	(tmp_to_store_registers, tmp_to_xfer_partial_ops)
	(tmp_to_xfer_partial, tmp_to_insert_breakpoint)
	(tmp_to_remove_breakpoint, tmp_to_stopped_by_watchpoint)
	(tmp_to_stopped_data_address, tmp_to_async): Remove.
	(record_full_open_1, record_full_open): Update.  Use RECORD_IS_USED.
	(record_full_resume, record_full_wait_1)
	(record_full_stopped_by_watchpoint, record_full_stopped_data_address)
	(record_full_store_registers, record_full_xfer_partial)
	(record_full_insert_breakpoint, record_full_remove_breakpoint)
	(record_full_async, record_full_can_async_p, record_full_is_async_p)
	(record_full_core_xfer_partial): Use target delegation.
	* record.c (find_record_target): Use find_target_at.
	* record.h (RECORD_IS_USED): Use find_target_at.
	* target.c (update_current_target): Use target_delegate_xfer_partial.
	(target_delegate_xfer_partial): Now public.  Renamed from...
	(current_xfer_partial): ...this.  Remove.
	(target_delegate_async, target_delegate_is_async_p)
	(target_delegate_can_async_p, target_delegate_insert_breakpoint)
	(target_delegate_remove_breakpoint, target_delegate_wait)
	(target_delegate_resume, find_target_at)
	(target_delegate_store_registers)
	(target_delegate_stopped_by_watchpoint)
	(target_delegate_stopped_data_address): New functions.
	* target.h (target_delegate_async, target_delegate_is_async_p)
	(target_delegate_can_async_p, target_delegate_insert_breakpoint)
	(target_delegate_remove_breakpoint, target_delegate_wait)
	(target_delegate_resume, find_target_at)
	(target_delegate_store_registers)
	(target_delegate_stopped_by_watchpoint)
	(target_delegate_stopped_data_address)
	(target_delegate_xfer_partial): Declare.
---
 gdb/record-full.c | 204 ++++++-----------------------------------------------
 gdb/record.c      |   8 +--
 gdb/record.h      |   2 +-
 gdb/target.c      | 206 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 gdb/target.h      |  68 ++++++++++++++++++
 5 files changed, 285 insertions(+), 203 deletions(-)

diff --git a/gdb/record-full.c b/gdb/record-full.c
index 1c4e68b..c3c9c3d 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -215,40 +215,6 @@ static struct cmd_list_element *show_record_full_cmdlist;
 /* Command list for "record full".  */
 static struct cmd_list_element *record_full_cmdlist;
 
-/* The beneath function pointers.  */
-static struct target_ops *record_full_beneath_to_resume_ops;
-static void (*record_full_beneath_to_resume) (struct target_ops *, ptid_t, int,
-					      enum gdb_signal);
-static struct target_ops *record_full_beneath_to_wait_ops;
-static ptid_t (*record_full_beneath_to_wait) (struct target_ops *, ptid_t,
-					      struct target_waitstatus *,
-					      int);
-static struct target_ops *record_full_beneath_to_store_registers_ops;
-static void (*record_full_beneath_to_store_registers) (struct target_ops *,
-						       struct regcache *,
-						       int regno);
-static struct target_ops *record_full_beneath_to_xfer_partial_ops;
-static LONGEST
-  (*record_full_beneath_to_xfer_partial) (struct target_ops *ops,
-					  enum target_object object,
-					  const char *annex,
-					  gdb_byte *readbuf,
-					  const gdb_byte *writebuf,
-					  ULONGEST offset,
-					  LONGEST len);
-static int
-  (*record_full_beneath_to_insert_breakpoint) (struct gdbarch *,
-					       struct bp_target_info *);
-static int
-  (*record_full_beneath_to_remove_breakpoint) (struct gdbarch *,
-					       struct bp_target_info *);
-static int (*record_full_beneath_to_stopped_by_watchpoint) (void);
-static int (*record_full_beneath_to_stopped_data_address) (struct target_ops *,
-							   CORE_ADDR *);
-static void
-  (*record_full_beneath_to_async) (void (*) (enum inferior_event_type, void *),
-				   void *);
-
 static void record_full_goto_insn (struct record_full_entry *entry,
 				   enum exec_direction_kind dir);
 static void record_full_save (const char *recfilename);
@@ -795,34 +761,6 @@ record_full_exec_insn (struct regcache *regcache,
     }
 }
 
-static struct target_ops *tmp_to_resume_ops;
-static void (*tmp_to_resume) (struct target_ops *, ptid_t, int,
-			      enum gdb_signal);
-static struct target_ops *tmp_to_wait_ops;
-static ptid_t (*tmp_to_wait) (struct target_ops *, ptid_t,
-			      struct target_waitstatus *,
-			      int);
-static struct target_ops *tmp_to_store_registers_ops;
-static void (*tmp_to_store_registers) (struct target_ops *,
-				       struct regcache *,
-				       int regno);
-static struct target_ops *tmp_to_xfer_partial_ops;
-static LONGEST (*tmp_to_xfer_partial) (struct target_ops *ops,
-				       enum target_object object,
-				       const char *annex,
-				       gdb_byte *readbuf,
-				       const gdb_byte *writebuf,
-				       ULONGEST offset,
-				       LONGEST len);
-static int (*tmp_to_insert_breakpoint) (struct gdbarch *,
-					struct bp_target_info *);
-static int (*tmp_to_remove_breakpoint) (struct gdbarch *,
-					struct bp_target_info *);
-static int (*tmp_to_stopped_by_watchpoint) (void);
-static int (*tmp_to_stopped_data_address) (struct target_ops *, CORE_ADDR *);
-static int (*tmp_to_stopped_data_address) (struct target_ops *, CORE_ADDR *);
-static void (*tmp_to_async) (void (*) (enum inferior_event_type, void *), void *);
-
 static void record_full_restore (void);
 
 /* Asynchronous signal handle registered as event loop source for when
@@ -885,26 +823,6 @@ record_full_open_1 (char *name, int from_tty)
     error (_("Process record: the current architecture doesn't support "
 	     "record function."));
 
-  if (!tmp_to_resume)
-    error (_("Could not find 'to_resume' method on the target stack."));
-  if (!tmp_to_wait)
-    error (_("Could not find 'to_wait' method on the target stack."));
-  if (!tmp_to_store_registers)
-    error (_("Could not find 'to_store_registers' "
-	     "method on the target stack."));
-  if (!tmp_to_insert_breakpoint)
-    error (_("Could not find 'to_insert_breakpoint' "
-	     "method on the target stack."));
-  if (!tmp_to_remove_breakpoint)
-    error (_("Could not find 'to_remove_breakpoint' "
-	     "method on the target stack."));
-  if (!tmp_to_stopped_by_watchpoint)
-    error (_("Could not find 'to_stopped_by_watchpoint' "
-	     "method on the target stack."));
-  if (!tmp_to_stopped_data_address)
-    error (_("Could not find 'to_stopped_data_address' "
-	     "method on the target stack."));
-
   push_target (&record_full_ops);
 }
 
@@ -921,83 +839,16 @@ record_full_open (char *name, int from_tty)
     fprintf_unfiltered (gdb_stdlog, "Process record: record_full_open\n");
 
   /* Check if record target is already running.  */
-  if (current_target.to_stratum == record_stratum)
+  if (RECORD_IS_USED)
     error (_("Process record target already running.  Use \"record stop\" to "
              "stop record target first."));
 
-  /* Reset the tmp beneath pointers.  */
-  tmp_to_resume_ops = NULL;
-  tmp_to_resume = NULL;
-  tmp_to_wait_ops = NULL;
-  tmp_to_wait = NULL;
-  tmp_to_store_registers_ops = NULL;
-  tmp_to_store_registers = NULL;
-  tmp_to_xfer_partial_ops = NULL;
-  tmp_to_xfer_partial = NULL;
-  tmp_to_insert_breakpoint = NULL;
-  tmp_to_remove_breakpoint = NULL;
-  tmp_to_stopped_by_watchpoint = NULL;
-  tmp_to_stopped_data_address = NULL;
-  tmp_to_async = NULL;
-
-  /* Set the beneath function pointers.  */
-  for (t = current_target.beneath; t != NULL; t = t->beneath)
-    {
-      if (!tmp_to_resume)
-        {
-	  tmp_to_resume = t->to_resume;
-	  tmp_to_resume_ops = t;
-        }
-      if (!tmp_to_wait)
-        {
-	  tmp_to_wait = t->to_wait;
-	  tmp_to_wait_ops = t;
-        }
-      if (!tmp_to_store_registers)
-        {
-	  tmp_to_store_registers = t->to_store_registers;
-	  tmp_to_store_registers_ops = t;
-        }
-      if (!tmp_to_xfer_partial)
-        {
-	  tmp_to_xfer_partial = t->to_xfer_partial;
-	  tmp_to_xfer_partial_ops = t;
-        }
-      if (!tmp_to_insert_breakpoint)
-	tmp_to_insert_breakpoint = t->to_insert_breakpoint;
-      if (!tmp_to_remove_breakpoint)
-	tmp_to_remove_breakpoint = t->to_remove_breakpoint;
-      if (!tmp_to_stopped_by_watchpoint)
-	tmp_to_stopped_by_watchpoint = t->to_stopped_by_watchpoint;
-      if (!tmp_to_stopped_data_address)
-	tmp_to_stopped_data_address = t->to_stopped_data_address;
-      if (!tmp_to_async)
-	tmp_to_async = t->to_async;
-    }
-  if (!tmp_to_xfer_partial)
-    error (_("Could not find 'to_xfer_partial' method on the target stack."));
-
   /* Reset */
   record_full_insn_num = 0;
   record_full_insn_count = 0;
   record_full_list = &record_full_first;
   record_full_list->next = NULL;
 
-  /* Set the tmp beneath pointers to beneath pointers.  */
-  record_full_beneath_to_resume_ops = tmp_to_resume_ops;
-  record_full_beneath_to_resume = tmp_to_resume;
-  record_full_beneath_to_wait_ops = tmp_to_wait_ops;
-  record_full_beneath_to_wait = tmp_to_wait;
-  record_full_beneath_to_store_registers_ops = tmp_to_store_registers_ops;
-  record_full_beneath_to_store_registers = tmp_to_store_registers;
-  record_full_beneath_to_xfer_partial_ops = tmp_to_xfer_partial_ops;
-  record_full_beneath_to_xfer_partial = tmp_to_xfer_partial;
-  record_full_beneath_to_insert_breakpoint = tmp_to_insert_breakpoint;
-  record_full_beneath_to_remove_breakpoint = tmp_to_remove_breakpoint;
-  record_full_beneath_to_stopped_by_watchpoint = tmp_to_stopped_by_watchpoint;
-  record_full_beneath_to_stopped_data_address = tmp_to_stopped_data_address;
-  record_full_beneath_to_async = tmp_to_async;
-
   if (core_bfd)
     record_full_core_open_1 (name, from_tty);
   else
@@ -1121,8 +972,7 @@ record_full_resume (struct target_ops *ops, ptid_t ptid, int step,
       /* Make sure the target beneath reports all signals.  */
       target_pass_signals (0, NULL);
 
-      record_full_beneath_to_resume (record_full_beneath_to_resume_ops,
-				     ptid, step, signal);
+      target_delegate_resume (ops, ptid, step, signal);
     }
 
   /* We are about to start executing the inferior (or simulate it),
@@ -1212,8 +1062,7 @@ record_full_wait_1 (struct target_ops *ops,
       if (record_full_resume_step)
 	{
 	  /* This is a single step.  */
-	  return record_full_beneath_to_wait (record_full_beneath_to_wait_ops,
-					      ptid, status, options);
+	  return target_delegate_wait (ops, ptid, status, options);
 	}
       else
 	{
@@ -1224,8 +1073,7 @@ record_full_wait_1 (struct target_ops *ops,
 
 	  while (1)
 	    {
-	      ret = record_full_beneath_to_wait
-		(record_full_beneath_to_wait_ops, ptid, status, options);
+	      ret = target_delegate_wait (ops, ptid, status, options);
 	      if (status->kind == TARGET_WAITKIND_IGNORE)
 		{
 		  if (record_debug)
@@ -1309,9 +1157,7 @@ record_full_wait_1 (struct target_ops *ops,
 					    "Process record: record_full_wait "
 					    "issuing one more step in the "
 					    "target beneath\n");
-		      record_full_beneath_to_resume
-			(record_full_beneath_to_resume_ops, ptid, step,
-			 GDB_SIGNAL_0);
+		      target_delegate_resume (ops, ptid, step, GDB_SIGNAL_0);
 		      continue;
 		    }
 		}
@@ -1515,7 +1361,7 @@ record_full_stopped_by_watchpoint (void)
   if (RECORD_FULL_IS_REPLAY)
     return record_full_hw_watchpoint;
   else
-    return record_full_beneath_to_stopped_by_watchpoint ();
+    return target_delegate_stopped_by_watchpoint (find_target_at (record_stratum));
 }
 
 static int
@@ -1524,7 +1370,7 @@ record_full_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
   if (RECORD_FULL_IS_REPLAY)
     return 0;
   else
-    return record_full_beneath_to_stopped_data_address (ops, addr_p);
+    return target_delegate_stopped_data_address (ops, addr_p);
 }
 
 /* Record registers change (by user or by GDB) to list as an instruction.  */
@@ -1627,8 +1473,7 @@ record_full_store_registers (struct target_ops *ops,
 
       record_full_registers_change (regcache, regno);
     }
-  record_full_beneath_to_store_registers
-    (record_full_beneath_to_store_registers_ops, regcache, regno);
+  target_delegate_store_registers (ops, regcache, regno);
 }
 
 /* "to_xfer_partial" method.  Behavior is conditional on
@@ -1693,9 +1538,8 @@ record_full_xfer_partial (struct target_ops *ops, enum target_object object,
 	record_full_insn_num++;
     }
 
-  return record_full_beneath_to_xfer_partial
-    (record_full_beneath_to_xfer_partial_ops, object, annex,
-     readbuf, writebuf, offset, len);
+  return target_delegate_xfer_partial (ops, object, annex,
+				       readbuf, writebuf, offset, len);
 }
 
 /* This structure represents a breakpoint inserted while the record
@@ -1775,7 +1619,8 @@ record_full_insert_breakpoint (struct gdbarch *gdbarch,
       int ret;
 
       old_cleanups = record_full_gdb_operation_disable_set ();
-      ret = record_full_beneath_to_insert_breakpoint (gdbarch, bp_tgt);
+      ret = target_delegate_insert_breakpoint (find_target_at (record_stratum),
+					       gdbarch, bp_tgt);
       do_cleanups (old_cleanups);
 
       if (ret != 0)
@@ -1815,7 +1660,8 @@ record_full_remove_breakpoint (struct gdbarch *gdbarch,
 	      int ret;
 
 	      old_cleanups = record_full_gdb_operation_disable_set ();
-	      ret = record_full_beneath_to_remove_breakpoint (gdbarch, bp_tgt);
+	      ret = target_delegate_remove_breakpoint (find_target_at (record_stratum),
+						       gdbarch, bp_tgt);
 	      do_cleanups (old_cleanups);
 
 	      if (ret != 0)
@@ -1896,22 +1742,19 @@ record_full_async (void (*callback) (enum inferior_event_type event_type,
   /* If we're on top of a line target (e.g., linux-nat, remote), then
      set it to async mode as well.  Will be NULL if we're sitting on
      top of the core target, for "record restore".  */
-  if (record_full_beneath_to_async != NULL)
-    record_full_beneath_to_async (callback, context);
+  target_delegate_async (find_target_at (record_stratum), callback, context);
 }
 
 static int
 record_full_can_async_p (void)
 {
-  /* We only enable async when the user specifically asks for it.  */
-  return target_async_permitted;
+  return target_delegate_can_async_p (find_target_at (record_stratum));
 }
 
 static int
 record_full_is_async_p (void)
 {
-  /* We only enable async when the user specifically asks for it.  */
-  return target_async_permitted;
+  return target_delegate_is_async_p (find_target_at (record_stratum));
 }
 
 static enum exec_direction_kind
@@ -2237,10 +2080,10 @@ record_full_core_xfer_partial (struct target_ops *ops,
 		  else
 		    {
 		      if (!entry)
-			return record_full_beneath_to_xfer_partial
-			  (record_full_beneath_to_xfer_partial_ops,
-			   object, annex, readbuf, writebuf,
-			   offset, len);
+			return target_delegate_xfer_partial (ops,
+							     object, annex,
+							     readbuf, writebuf,
+							     offset, len);
 
 		      memcpy (readbuf, entry->buf + sec_offset,
 			      (size_t) len);
@@ -2256,9 +2099,8 @@ record_full_core_xfer_partial (struct target_ops *ops,
 	error (_("You can't do that without a process to debug."));
     }
 
-  return record_full_beneath_to_xfer_partial
-    (record_full_beneath_to_xfer_partial_ops, object, annex,
-     readbuf, writebuf, offset, len);
+  return target_delegate_xfer_partial (ops, object, annex,
+				       readbuf, writebuf, offset, len);
 }
 
 /* "to_insert_breakpoint" method for prec over corefile.  */
diff --git a/gdb/record.c b/gdb/record.c
index cbbe365..8a199f9 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -62,13 +62,7 @@ struct cmd_list_element *info_record_cmdlist = NULL;
 static struct target_ops *
 find_record_target (void)
 {
-  struct target_ops *t;
-
-  for (t = current_target.beneath; t != NULL; t = t->beneath)
-    if (t->to_stratum == record_stratum)
-      return t;
-
-  return NULL;
+  return find_target_at (record_stratum);
 }
 
 /* Check that recording is active.  Throw an error, if it isn't.  */
diff --git a/gdb/record.h b/gdb/record.h
index 86e6bc6..00fbd3f 100644
--- a/gdb/record.h
+++ b/gdb/record.h
@@ -22,7 +22,7 @@
 
 struct cmd_list_element;
 
-#define RECORD_IS_USED	(current_target.to_stratum == record_stratum)
+#define RECORD_IS_USED	(find_target_at (record_stratum) != NULL)
 
 extern unsigned int record_debug;
 
diff --git a/gdb/target.c b/gdb/target.c
index e3dcb47..2c55187 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -75,12 +75,6 @@ static LONGEST default_xfer_partial (struct target_ops *ops,
 				     const gdb_byte *writebuf,
 				     ULONGEST offset, LONGEST len);
 
-static LONGEST current_xfer_partial (struct target_ops *ops,
-				     enum target_object object,
-				     const char *annex, gdb_byte *readbuf,
-				     const gdb_byte *writebuf,
-				     ULONGEST offset, LONGEST len);
-
 static LONGEST target_xfer_partial (struct target_ops *ops,
 				    enum target_object object,
 				    const char *annex,
@@ -866,7 +860,7 @@ update_current_target (void)
   de_fault (to_stop,
 	    (void (*) (ptid_t))
 	    target_ignore);
-  current_target.to_xfer_partial = current_xfer_partial;
+  current_target.to_xfer_partial = target_delegate_xfer_partial;
   de_fault (to_rcmd,
 	    (void (*) (char *, struct ui_file *))
 	    tcomplain);
@@ -1992,14 +1986,14 @@ default_xfer_partial (struct target_ops *ops, enum target_object object,
     return -1;
 }
 
-/* The xfer_partial handler for the topmost target.  Unlike the default,
-   it does not need to handle memory specially; it just passes all
-   requests down the stack.  */
+/* See target.h.  */
 
-static LONGEST
-current_xfer_partial (struct target_ops *ops, enum target_object object,
-		      const char *annex, gdb_byte *readbuf,
-		      const gdb_byte *writebuf, ULONGEST offset, LONGEST len)
+LONGEST
+target_delegate_xfer_partial (struct target_ops *ops,
+			      enum target_object object,
+			      const char *annex, gdb_byte *readbuf,
+			      const gdb_byte *writebuf,
+			      ULONGEST offset, LONGEST len)
 {
   if (ops->beneath != NULL)
     return ops->beneath->to_xfer_partial (ops->beneath, object, annex,
@@ -2008,6 +2002,53 @@ current_xfer_partial (struct target_ops *ops, enum target_object object,
     return -1;
 }
 
+/* See target.h.  */
+
+void
+target_delegate_async (struct target_ops *self,
+		       void (*callback) (enum inferior_event_type, void *),
+		       void *datum)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_async)
+	{
+	  t->to_async (callback, datum);
+	  break;
+	}
+    }
+}
+
+/* See target.h.  */
+
+int
+target_delegate_is_async_p (struct target_ops *self)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    if (t->to_is_async_p != NULL)
+      return t->to_is_async_p ();
+
+  gdb_assert_not_reached (_("reached end of target stack during delegation"));
+}
+
+/* See target.h.  */
+
+int
+target_delegate_can_async_p (struct target_ops *self)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    if (t->to_can_async_p != NULL)
+      return t->to_can_async_p ();
+
+  gdb_assert_not_reached (_("reached end of target stack during delegation"));
+}
+
 /* Target vector read/write partial wrapper functions.  */
 
 static LONGEST
@@ -2458,6 +2499,24 @@ target_insert_breakpoint (struct gdbarch *gdbarch,
   return (*current_target.to_insert_breakpoint) (gdbarch, bp_tgt);
 }
 
+/* See target.h.  */
+
+int
+target_delegate_insert_breakpoint (struct target_ops *self,
+				   struct gdbarch *gdbarch,
+				   struct bp_target_info *bp_tgt)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_insert_breakpoint)
+	return t->to_insert_breakpoint (gdbarch, bp_tgt);
+    }
+
+  gdb_assert_not_reached (_("reached end of target stack during delegation"));
+}
+
 int
 target_remove_breakpoint (struct gdbarch *gdbarch,
 			  struct bp_target_info *bp_tgt)
@@ -2475,6 +2534,24 @@ target_remove_breakpoint (struct gdbarch *gdbarch,
   return (*current_target.to_remove_breakpoint) (gdbarch, bp_tgt);
 }
 
+/* See target.h.  */
+
+int
+target_delegate_remove_breakpoint (struct target_ops *self,
+				   struct gdbarch *gdbarch,
+				   struct bp_target_info *bp_tgt)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_remove_breakpoint)
+	return t->to_remove_breakpoint (gdbarch, bp_tgt);
+    }
+
+  gdb_assert_not_reached (_("reached end of target stack during delegation"));
+}
+
 static void
 target_info (char *args, int from_tty)
 {
@@ -2681,6 +2758,24 @@ target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
   noprocess ();
 }
 
+/* See target.h.  */
+
+ptid_t
+target_delegate_wait (struct target_ops *self,
+		      ptid_t ptid, struct target_waitstatus *status,
+		      int options)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_wait)
+	return t->to_wait (t, ptid, status, options);
+    }
+
+  gdb_assert_not_reached (_("reached end of target stack during delegation"));
+}
+
 char *
 target_pid_to_str (ptid_t ptid)
 {
@@ -2738,6 +2833,24 @@ target_resume (ptid_t ptid, int step, enum gdb_signal signal)
   noprocess ();
 }
 
+/* See target.h.  */
+
+void
+target_delegate_resume (struct target_ops *self,
+			ptid_t ptid, int step, enum gdb_signal signal)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_resume)
+	{
+	  t->to_resume (t, ptid, step, signal);
+	  break;
+	}
+    }
+}
+
 void
 target_pass_signals (int numsigs, unsigned char *pass_signals)
 {
@@ -3633,6 +3746,20 @@ find_target_beneath (struct target_ops *t)
   return t->beneath;
 }
 
+/* See target.h.  */
+
+struct target_ops *
+find_target_at (enum strata stratum)
+{
+  struct target_ops *t;
+
+  for (t = current_target.beneath; t != NULL; t = t->beneath)
+    if (t->to_stratum == stratum)
+      return t;
+
+  return NULL;
+}
+
 \f
 /* The inferior process has died.  Long live the inferior!  */
 
@@ -3985,6 +4112,24 @@ target_store_registers (struct regcache *regcache, int regno)
   noprocess ();
 }
 
+/* See target.h.  */
+
+void
+target_delegate_store_registers (struct target_ops *self,
+				 struct regcache *regcache, int regno)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_store_registers)
+	{
+	  t->to_store_registers (t, regcache, regno);
+	  break;
+	}
+    }
+}
+
 int
 target_core_of_thread (ptid_t ptid)
 {
@@ -4118,6 +4263,39 @@ target_ranged_break_num_registers (void)
 /* See target.h.  */
 
 int
+target_delegate_stopped_by_watchpoint (struct target_ops *self)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_stopped_by_watchpoint)
+	return t->to_stopped_by_watchpoint ();
+    }
+
+  gdb_assert_not_reached (_("reached end of target stack during delegation"));
+}
+
+/* See target.h.  */
+
+int
+target_delegate_stopped_data_address (struct target_ops *self,
+				      CORE_ADDR *addr_p)
+{
+  struct target_ops *t;
+
+  for (t = self->beneath; t != NULL; t = t->beneath)
+    {
+      if (t->to_stopped_data_address)
+	return t->to_stopped_data_address (t, addr_p);
+    }
+
+  gdb_assert_not_reached (_("reached end of target stack during delegation"));
+}
+
+/* See target.h.  */
+
+int
 target_supports_btrace (void)
 {
   struct target_ops *t;
diff --git a/gdb/target.h b/gdb/target.h
index 1d73bcd..2e00911 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -937,6 +937,12 @@ extern void target_disconnect (char *, int);
 
 extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
 
+/* Delegate "target_resume" to a target beneath SELF.  */
+
+extern void target_delegate_resume (struct target_ops *self,
+				    ptid_t ptid, int step,
+				    enum gdb_signal signal);
+
 /* Wait for process pid to do something.  PTID = -1 to wait for any
    pid to do something.  Return pid of child, or -1 in case of error;
    store status through argument pointer STATUS.  Note that it is
@@ -949,6 +955,13 @@ extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
 extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
 			   int options);
 
+/* Delegate "target_wait" to a target beneath SELF.  */
+
+extern ptid_t target_delegate_wait (struct target_ops *self,
+				    ptid_t ptid,
+				    struct target_waitstatus *status,
+				    int options);
+
 /* Fetch at least register REGNO, or all regs if regno == -1.  No result.  */
 
 extern void target_fetch_registers (struct regcache *regcache, int regno);
@@ -959,6 +972,12 @@ extern void target_fetch_registers (struct regcache *regcache, int regno);
 
 extern void target_store_registers (struct regcache *regcache, int regs);
 
+/* Delegate "target_store_registers" to a target beneath SELF.  */
+
+extern void target_delegate_store_registers (struct target_ops *self,
+					     struct regcache *regcache,
+					     int regno);
+
 /* Get ready to modify the registers array.  On machines which store
    individual registers, this doesn't need to do anything.  On machines
    which store all the registers in one fell swoop, this makes sure
@@ -1091,12 +1110,24 @@ int target_write_memory_blocks (VEC(memory_write_request_s) *requests,
 extern int target_insert_breakpoint (struct gdbarch *gdbarch,
 				     struct bp_target_info *bp_tgt);
 
+/* Delegate "target_insert_breakpoint" to a target beneath SELF.  */
+
+extern int target_delegate_insert_breakpoint (struct target_ops *self,
+					      struct gdbarch *gdbarch,
+					      struct bp_target_info *bp_tgt);
+
 /* Remove a breakpoint at address BP_TGT->placed_address in the target
    machine.  Result is 0 for success, or an errno value.  */
 
 extern int target_remove_breakpoint (struct gdbarch *gdbarch,
 				     struct bp_target_info *bp_tgt);
 
+/* Delegate "target_remove_breakpoint" to a target beneath SELF.  */
+
+extern int target_delegate_remove_breakpoint (struct target_ops *self,
+					      struct gdbarch *gdbarch,
+					      struct bp_target_info *bp_tgt);
+
 /* Initialize the terminal settings we record for the inferior,
    before we actually run the inferior.  */
 
@@ -1375,15 +1406,29 @@ extern int target_async_permitted;
 /* Can the target support asynchronous execution?  */
 #define target_can_async_p() (current_target.to_can_async_p ())
 
+/* Delegate "target_can_async_p" to a target beneath SELF.  */
+
+extern int target_delegate_can_async_p (struct target_ops *self);
+
 /* Is the target in asynchronous execution mode?  */
 #define target_is_async_p() (current_target.to_is_async_p ())
 
+/* Delegate "target_is_async_p" to a target beneath SELF.  */
+
+extern int target_delegate_is_async_p (struct target_ops *self);
+
 int target_supports_non_stop (void);
 
 /* Put the target in async mode with the specified callback function.  */
 #define target_async(CALLBACK,CONTEXT) \
      (current_target.to_async ((CALLBACK), (CONTEXT)))
 
+/* Delegate "target_async" to a target beneath SELF.  */
+
+extern void target_delegate_async (struct target_ops *,
+				   void (*) (enum inferior_event_type, void *),
+				   void *);
+
 #define target_execution_direction() \
   (current_target.to_execution_direction ())
 
@@ -1459,6 +1504,10 @@ extern char *target_thread_name (struct thread_info *);
 #define target_stopped_by_watchpoint \
    (*current_target.to_stopped_by_watchpoint)
 
+/* Delegate "target_stopped_by_watchpoint" to a target beneath SELF.  */
+
+extern int target_delegate_stopped_by_watchpoint (struct target_ops *self);
+
 /* Non-zero if we have steppable watchpoints  */
 
 #define target_have_steppable_watchpoint \
@@ -1532,6 +1581,11 @@ extern int target_ranged_break_num_registers (void);
 #define target_stopped_data_address(target, addr_p) \
     (*target.to_stopped_data_address) (target, addr_p)
 
+/* Delegate "target_stopped_data_address" to a target beneath SELF.  */
+
+extern int target_delegate_stopped_data_address (struct target_ops *self,
+						 CORE_ADDR *addr_p);
+
 /* Return non-zero if ADDR is within the range of a watchpoint spanning
    LENGTH bytes beginning at START.  */
 #define target_watchpoint_addr_within_range(target, addr, start, length) \
@@ -1860,6 +1914,11 @@ extern void find_default_create_inferior (struct target_ops *,
 
 extern struct target_ops *find_target_beneath (struct target_ops *);
 
+/* Find the target at STRATUM.  If no target is at that stratum,
+   return NULL.  */
+
+struct target_ops *find_target_at (enum strata stratum);
+
 /* Read OS data object of type TYPE from the target, and return it in
    XML format.  The result is NUL-terminated and returned as a string,
    allocated using xmalloc.  If an error occurs or the transfer is
@@ -1962,4 +2021,13 @@ extern void target_call_history_from (ULONGEST begin, int size, int flags);
 /* See to_call_history_range.  */
 extern void target_call_history_range (ULONGEST begin, ULONGEST end, int flags);
 
+/* Delegate "target_xfer_partial" to a target beneath SELF.  */
+
+extern LONGEST target_delegate_xfer_partial (struct target_ops *ops,
+					     enum target_object object,
+					     const char *annex,
+					     gdb_byte *readbuf,
+					     const gdb_byte *writebuf,
+					     ULONGEST offset, LONGEST len);
+
 #endif /* !defined (TARGET_H) */
-- 
1.8.1.4

RE: [PATCH 2/8] add target method delegation

[Metzger, Markus T]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Tom Tromey
> Sent: Monday, July 29, 2013 6:45 PM


> To fix the first problem, this patch introduces a handful of
> target_delegate_* functions, which forward calls further down the
> target stack.

I'm doing this explicitly in record-btrace.c.  Seems I should have added
the delegate functions, instead.  I'll change it to use your delegates.


Once we have all the delegates in place, shouldn't we rewrite the original
target_~ functions to call the respective target_delegate_~ with
&current_target as first argument?


> To fix the second problem, this patch adds find_target_at to determine
> whether a target appears at a given stratum.  This may seem like
> overkill somehow, but I have a subsequent patch series (see archer.git
> tromey/multi-target) that uses it more heavily.

I'd rather consistently add struct target_opc *self as first parameter to all
target functions.  I did this for target_prepare_to_store for exactly the
reason of delegating a request I don't handle in record-btrace.c (patch
is currently under review).

Otherwise, we rely on the fact that the target doing the delegation is at
the top of its stratum.  This holds for record targets and I may be ignorant
enough to not realize that this holds in general.  If we could have another
target on top of us in the same stratum, though, we would delegate to
ourselves.

Regards,
Markus.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH 2/8] add target method delegation

[Tom Tromey]

>>>>> "Markus" == Metzger, Markus T <markus.t.metzger@intel.com> writes:

Markus> Once we have all the delegates in place, shouldn't we rewrite
Markus> the original target_~ functions to call the respective
Markus> target_delegate_~ with &current_target as first argument?

Yeah, that would be good.

Tom> To fix the second problem, this patch adds find_target_at to determine
Tom> whether a target appears at a given stratum.  This may seem like
Tom> overkill somehow, but I have a subsequent patch series (see archer.git
Tom> tromey/multi-target) that uses it more heavily.

Markus> I'd rather consistently add struct target_opc *self as first
Markus> parameter to all target functions.

I considered this, but the difficulty is that I can't test, or even
compile, a reasonable subset of the changes.

If that doesn't bother people then I'm happy to change things that way.
I do think it would be cleaner.

Markus> Otherwise, we rely on the fact that the target doing the
Markus> delegation is at the top of its stratum.  This holds for record
Markus> targets and I may be ignorant enough to not realize that this
Markus> holds in general.  If we could have another target on top of us
Markus> in the same stratum, though, we would delegate to ourselves.

Only a single target can appear at a given stratum.  This is enforced by
push_target.  This is pretty obscure due to how the code is written; but
with my other target stack series this becomes more obvious.

So I don't think there is an issue here.  However, I do agree that it is
a violation of the abstraction.

Tom

RE: [PATCH 2/8] add target method delegation

[Metzger, Markus T]

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Tuesday, July 30, 2013 4:47 PM


> Markus> I'd rather consistently add struct target_opc *self as first
> Markus> parameter to all target functions.
> 
> I considered this, but the difficulty is that I can't test, or even
> compile, a reasonable subset of the changes.

Afaik, Sanimir wanted to look into using the gcc compile farm.

I admit that I also changed target specific code that I was not able
to compile and test.

Regards,
Markus.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH 2/8] add target method delegation

[Tom Tromey]

Tom> I considered this, but the difficulty is that I can't test, or even
Tom> compile, a reasonable subset of the changes.

Markus> Afaik, Sanimir wanted to look into using the gcc compile farm.

Yeah.  That has decent arch coverage, though not complete.
I will give it a try.  Maybe there are enough machines.

Markus> I admit that I also changed target specific code that I was not able
Markus> to compile and test.

It's unavoidable at times.
Being the loosie-goosie person I am, I tend to think we should just push
forward on such changes and let the breakage flush out obsolete ports.
But I assume I'm less conservative than the median gdb maintainer.

Tom

RE: [PATCH 2/8] add target method delegation

[Agovic, Sanimir]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
> Of Metzger, Markus T
> Sent: Tuesday, July 30, 2013 05:08 PM
> 
> > -----Original Message-----
> > From: Tom Tromey [mailto:tromey@redhat.com]
> > Sent: Tuesday, July 30, 2013 4:47 PM
> 
> > Markus> I'd rather consistently add struct target_opc *self as first
> > Markus> parameter to all target functions.
> >
> > I considered this, but the difficulty is that I can't test, or even
> > compile, a reasonable subset of the changes.
> 
> Afaik, Sanimir wanted to look into using the gcc compile farm.
> 
http://gcc20.fsffrance.org:9010/waterfall

I have some targets up and running, builds only. No tests are run so far. 

Schedulers for cvs commits and "try jobs" are in place.

Please use the existing wiki page http://sourceware.org/gdb/wiki/BuildBot to track feature requests and similar things.

 -Sanimir

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH 2/8] add target method delegation

[Tom Tromey]

Sanimir> I have some targets up and running, builds only. No tests are
Sanimir> run so far.

I have my old buildbot configuration around here.  It handled checking
for regressions.  I wasn't totally happy with it, but it was "good
enough" in most cases.

I'll send it to you; you can take what you like from it.

Sanimir> Schedulers for cvs commits and "try jobs" are in place.

Let me suggest using git rather than cvs.

Sanimir> Please use the existing wiki page
Sanimir> http://sourceware.org/gdb/wiki/BuildBot to track feature requests and
Sanimir> similar things.

Ok.

Tom

[PATCH 6/8] make dprintf.exp pass in always-async mode

[Tom Tromey]

When target-async is enabled, dprintf.exp fails.

This happens because run_inferior_call causes gdb to forget that it is
running in sync_execution mode, so something like a breakpoint
condition that makes an inferior call causes gdb to enter fully async
mode.

This patch fixes the problem by noticing when gdb was in
sync_execution mode in run_inferior_call, and taking care to restore
this state afterward.

Built and regtested on x86-64 Fedora 18.

	PR cli/15718:
	* infcall.c: Include event-top.h.
	(run_inferior_call): Call async_disable_stdin if needed.
---
 gdb/infcall.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 19af044..7398913 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -36,6 +36,7 @@
 #include "ada-lang.h"
 #include "gdbthread.h"
 #include "exceptions.h"
+#include "event-top.h"
 
 /* If we can't find a function's name from its address,
    we print this instead.  */
@@ -398,6 +399,8 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
 
   TRY_CATCH (e, RETURN_MASK_ALL)
     {
+      int was_sync = sync_execution;
+
       proceed (real_pc, GDB_SIGNAL_0, 0);
 
       /* Inferior function calls are always synchronous, even if the
@@ -407,6 +410,11 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
 	{
 	  wait_for_inferior ();
 	  normal_stop ();
+	  /* If gdb was previously in sync execution mode, then ensure
+	     that it remains so.  normal_stop calls
+	     async_enable_stdin, so reset it again here.  */
+	  if (was_sync)
+	    async_disable_stdin ();
 	}
     }
 
-- 
1.8.1.4

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

[Tom Tromey]

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:

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.

That's what this patch does.

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 I added.

Tested on x86_64 Fedora 16, no regressions.

2012-05-09  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): In async mode, 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.

	gdb/testsuite/
	* gdb.mi/mi-cli.exp: Also expect the new source line to be output
	after a "next", in async mode.  Make it a pass/fail test.
	* gdb.mi/mi-solib.exp: Test that the CLI solib event note is
	output.
---
 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                | 66 +++++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-cli.exp   | 13 +++++---
 gdb/testsuite/gdb.mi/mi-solib.exp | 11 +++++++
 8 files changed, 154 insertions(+), 6 deletions(-)

diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 380352b..cedd3af 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -40,6 +40,17 @@ static void out_field_fmt (struct ui_out *uiout, int fldno,
 			   const char *fldname,
 			   const char *format,...) ATTRIBUTE_PRINTF (4, 5);
 
+/* The destructor.  */
+
+static void
+cli_uiout_dtor (struct ui_out *ui_out)
+{
+  cli_out_data *data = ui_out_data (ui_out);
+
+  VEC_free (ui_filep, data->streams);
+  xfree (data);
+}
+
 /* These are the CLI output functions */
 
 /* Mark beginning of a table */
@@ -370,7 +381,7 @@ struct ui_out_impl cli_ui_out_impl =
   cli_wrap_hint,
   cli_flush,
   cli_redirect,
-  0,
+  cli_uiout_dtor,
   0, /* Does not need MI hacks (i.e. needs CLI hacks).  */
 };
 
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index c3b85dc..f3e8db1 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -122,6 +122,11 @@ struct thread_control_state
   /* Chain containing status of breakpoint(s) the thread stopped
      at.  */
   bpstat stop_bpstat;
+
+  /* The interpreter that issued the execution command.  NULL if the
+     thread was resumed as a result of a command applied to some other
+     thread (e.g., "next" with scheduler-locking off).  */
+  struct interp *command_interp;
 };
 
 /* Inferior thread specific part of `struct infcall_suspend_state'.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 53ecc1d..0052ef2 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -428,6 +428,7 @@ follow_fork (void)
   CORE_ADDR step_range_start = 0;
   CORE_ADDR step_range_end = 0;
   struct frame_id step_frame_id = { 0 };
+  struct interp *command_interp = NULL;
 
   if (!non_stop)
     {
@@ -479,6 +480,7 @@ follow_fork (void)
 	    step_frame_id = tp->control.step_frame_id;
 	    exception_resume_breakpoint
 	      = clone_momentary_breakpoint (tp->control.exception_resume_breakpoint);
+	    command_interp = tp->control.command_interp;
 
 	    /* For now, delete the parent's sr breakpoint, otherwise,
 	       parent/child sr breakpoints are considered duplicates,
@@ -490,6 +492,7 @@ follow_fork (void)
 	    tp->control.step_range_end = 0;
 	    tp->control.step_frame_id = null_frame_id;
 	    delete_exception_resume_breakpoint (tp);
+	    tp->control.command_interp = NULL;
 	  }
 
 	parent = inferior_ptid;
@@ -534,6 +537,7 @@ follow_fork (void)
 		    tp->control.step_frame_id = step_frame_id;
 		    tp->control.exception_resume_breakpoint
 		      = exception_resume_breakpoint;
+		    tp->control.command_interp = command_interp;
 		  }
 		else
 		  {
@@ -1993,6 +1997,8 @@ clear_proceed_status_thread (struct thread_info *tp)
 
   tp->control.proceed_to_finish = 0;
 
+  tp->control.command_interp = NULL;
+
   /* Discard any remaining commands or status from previous stop.  */
   bpstat_clear (&tp->control.stop_bpstat);
 }
@@ -2194,6 +2200,14 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
       regcache_write_pc (regcache, addr);
     }
 
+  /* Record the interpreter that issued the execution command that
+     caused this thread to resume.  If the top level interpreter is
+     MI/async, and the execution command was a CLI command
+     (next/step/etc.), we'll want to print stop event output to the MI
+     console channel (the stepped-to line, etc.), as if the user
+     entered the execution command on a real GDB console.  */
+  inferior_thread ()->control.command_interp = command_interp ();
+
   if (debug_infrun)
     fprintf_unfiltered (gdb_stdlog,
 			"infrun: proceed (addr=%s, signal=%d, step=%d)\n",
diff --git a/gdb/interps.c b/gdb/interps.c
index 25500d6..ca3b9b5 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -315,6 +315,29 @@ current_interp_display_prompt_p (void)
 						      data);
 }
 
+/* The interpreter that is active while `interp_exec' is active, NULL
+   at all other times.  */
+static struct interp *command_interpreter;
+
+/* The interpreter that was active when a command was executed.
+   Normally that'd always be CURRENT_INTERPRETER, except that MI's
+   -interpreter-exec command doesn't actually flip the current
+   interpreter when running its sub-command.  The
+   `command_interpreter' global tracks when interp_exec is called
+   (IOW, when -interpreter-exec is called).  If that is set, it is
+   INTERP in '-interpreter-exec INTERP "CMD"' or in 'interpreter-exec
+   INTERP "CMD".  Otherwise, interp_exec isn't active, and so the
+   interpreter running the command is the current interpreter.  */
+
+struct interp *
+command_interp (void)
+{
+  if (command_interpreter != NULL)
+    return command_interpreter;
+  else
+    return current_interpreter;
+}
+
 /* Run the current command interpreter's main loop.  */
 void
 current_interp_command_loop (void)
@@ -362,7 +385,18 @@ interp_exec (struct interp *interp, const char *command_str)
 {
   if (interp->procs->exec_proc != NULL)
     {
-      return interp->procs->exec_proc (interp->data, command_str);
+      struct gdb_exception ex;
+      struct interp *save_command_interp;
+
+      /* See `command_interp' for why we do this.  */
+      save_command_interp = command_interpreter;
+      command_interpreter = interp;
+
+      ex = interp->procs->exec_proc (interp->data, command_str);
+
+      command_interpreter = save_command_interp;
+
+      return ex;
     }
   return exception_none;
 }
diff --git a/gdb/interps.h b/gdb/interps.h
index 58ac6b2..6a45a7b 100644
--- a/gdb/interps.h
+++ b/gdb/interps.h
@@ -97,6 +97,8 @@ extern int current_interp_set_logging (int start_log, struct ui_file *out,
 extern void *top_level_interpreter_data (void);
 extern struct interp *top_level_interpreter (void);
 
+extern struct interp *command_interp (void);
+
 /* True if the current interpreter is in async mode, false if in sync
    mode.  If in sync mode, running a synchronous execution command
    (with execute_command, e.g, "next") will not return until the
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 0cd43f5..b4515d9 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -37,6 +37,7 @@
 #include "gdb.h"
 #include "objfiles.h"
 #include "tracepoint.h"
+#include "cli-out.h"
 
 /* These are the interpreter setup, etc. functions for the MI
    interpreter.  */
@@ -251,6 +252,10 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc)
 	     "does not support command execution"),
 	      argv[0]);
 
+  /* Note that unlike the CLI version of this command, we don't
+     actually set INTERP_TO_USE as the current interpreter, as we
+     still want gdb_stdout, etc. to point at MI streams.  */
+
   /* Insert the MI out hooks, making sure to also call the
      interpreter's hooks if it has any.  */
   /* KRS: We shouldn't need this... Events should be installed and
@@ -453,6 +458,26 @@ mi_inferior_removed (struct inferior *inf)
   gdb_flush (mi->event_channel);
 }
 
+/* Cleanup that restores a previous current uiout.  */
+
+static void
+restore_current_uiout_cleanup (void *arg)
+{
+  struct ui_out *saved_uiout = arg;
+
+  current_uiout = saved_uiout;
+}
+
+/* Cleanup that destroys the a ui_out object.  */
+
+static void
+ui_out_free_cleanup (void *arg)
+{
+  struct ui_out *uiout = arg;
+
+  ui_out_destroy (uiout);
+}
+
 static void
 mi_on_normal_stop (struct bpstats *bs, int print_frame)
 {
@@ -483,6 +508,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.  */
+	  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);
+	    }
+	}
 
       ui_out_field_int (mi_uiout, "thread-id",
 			pid_to_thread_id (inferior_ptid));
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index 5b809e2..bee296d 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -166,10 +166,15 @@ mi_gdb_test "34 next" \
     ".*34\\\^running.*\\*running,thread-id=\"all\"" \
     "34 next: run"
 
-if {!$async} {
-    gdb_expect {
-        -re "~\[^\r\n\]+\r\n" {
-        }
+# Test that the new current source line is output, given we executed
+# the console 'next' command, not -exec-next.
+set test "34 next: CLI output"
+gdb_expect {
+    -re "~\"67\[^\r\n\]+\r\n" {
+	pass $test
+    }
+    timeout {
+	fail "$test (timeout)"
     }
 }
 
diff --git a/gdb/testsuite/gdb.mi/mi-solib.exp b/gdb/testsuite/gdb.mi/mi-solib.exp
index cd400ea..0a8b43b 100644
--- a/gdb/testsuite/gdb.mi/mi-solib.exp
+++ b/gdb/testsuite/gdb.mi/mi-solib.exp
@@ -60,4 +60,15 @@ 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
 
+# Also test that the CLI solib event note is output.
+set test "CLI prints solib event"
+gdb_expect {
+    -re "~\"Stopped due to shared library event \\(no libraries added or removed\\)\\\\n" {
+	pass "$test"
+    }
+    timeout {
+	fail "$test (timeout)"
+    }
+}
+
 mi_expect_stop solib-event .* .* .* .* .* "check for solib event"
-- 
1.8.1.4

[PATCH 7/8] fix py-finish-breakpoint.exp with always-async

[Tom Tromey]

With target async enabled, py-finish-breakpoint.exp will trigger an
assertion failure.

The failure occurs because execute_command re-enters the event loop in
some circumstances, and in this case resets the sync_execution flag.
Then later gdb reaches this assertion in normal_stop:

      gdb_assert (sync_execution || !target_can_async_p ());

execute_command has a comment explaining why it dispatches events:

      /* If the interpreter is in sync mode (we're running a user
	 command's list, running command hooks or similars), and we
	 just ran a synchronous command that started the target, wait
	 for that command to end.  */

However, the code did not follow this comment -- it didn't check to
see if the command started the target.

This patch fixes the problem by noting whether the target was
executing in sync_execution mode before running the command, and then
augmenting the condition to test this as well.

Built and regtested on x86-64 Fedora 18.

	* top.c (execute_command): Only dispatch events if command
	started target.
---
 gdb/top.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/top.c b/gdb/top.c
index 467e6a2..6af0fad 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -426,6 +426,8 @@ execute_command (char *p, int from_tty)
     {
       const char *cmd = p;
       char *arg;
+      int was_executing = sync_execution && target_has_execution;
+
       line = p;
 
       /* If trace-commands is set then this will print this command.  */
@@ -481,7 +483,8 @@ execute_command (char *p, int from_tty)
 	 command's list, running command hooks or similars), and we
 	 just ran a synchronous command that started the target, wait
 	 for that command to end.  */
-      if (!interpreter_async && sync_execution)
+      if (!interpreter_async && !was_executing
+	  && sync_execution && target_has_execution)
 	{
 	  while (gdb_do_one_event () >= 0)
 	    if (!sync_execution)
-- 
1.8.1.4

Re: [PATCH 7/8] fix py-finish-breakpoint.exp with always-async

[Doug Evans]

Tom Tromey writes:
 > With target async enabled, py-finish-breakpoint.exp will trigger an
 > assertion failure.
 > 
 > The failure occurs because execute_command re-enters the event loop in
 > some circumstances, and in this case resets the sync_execution flag.
 > Then later gdb reaches this assertion in normal_stop:
 > 
 >       gdb_assert (sync_execution || !target_can_async_p ());
 > 
 > execute_command has a comment explaining why it dispatches events:
 > 
 >       /* If the interpreter is in sync mode (we're running a user
 > 	 command's list, running command hooks or similars), and we
 > 	 just ran a synchronous command that started the target, wait
 > 	 for that command to end.  */
 > 
 > However, the code did not follow this comment -- it didn't check to
 > see if the command started the target.
 > 
 > This patch fixes the problem by noting whether the target was
 > executing in sync_execution mode before running the command, and then
 > augmenting the condition to test this as well.
 > 
 > Built and regtested on x86-64 Fedora 18.
 > 
 > 	* top.c (execute_command): Only dispatch events if command
 > 	started target.
 > ---
 >  gdb/top.c | 5 ++++-
 >  1 file changed, 4 insertions(+), 1 deletion(-)
 > 
 > diff --git a/gdb/top.c b/gdb/top.c
 > index 467e6a2..6af0fad 100644
 > --- a/gdb/top.c
 > +++ b/gdb/top.c
 > @@ -426,6 +426,8 @@ execute_command (char *p, int from_tty)
 >      {
 >        const char *cmd = p;
 >        char *arg;
 > +      int was_executing = sync_execution && target_has_execution;
 > +
 >        line = p;
 >  
 >        /* If trace-commands is set then this will print this command.  */
 > @@ -481,7 +483,8 @@ execute_command (char *p, int from_tty)
 >  	 command's list, running command hooks or similars), and we
 >  	 just ran a synchronous command that started the target, wait
 >  	 for that command to end.  */
 > -      if (!interpreter_async && sync_execution)
 > +      if (!interpreter_async && !was_executing
 > +	  && sync_execution && target_has_execution)
 >  	{
 >  	  while (gdb_do_one_event () >= 0)
 >  	    if (!sync_execution)

Hi.
The patch is a bit confusing.
Assignment of was_execution tests for sync_execution,
and later we test sync_execution && !was_executing.

It may be correct to do it that way (I'm not sure ATM),
but the phrase "was_executing" doesn't really convey
any notion of what "sync_execution" means.

IOW, it'd be clearer if the patch did:

  int was_executing = target_has_execution;

and then augment the patch with whatever else is necessary
to make it correct.
[Assuming I understand correctly what the patch is trying to do.]

----
Also, is target_has_execution what you want here?
If I'm stopped at a breakpoint target_has_execution still returns non-zero.
E.g. do we want that while loop for a "next" command?
[where the target "has execution" before and after the command]
If that's *not* the case then that code needs a lot more commenting. :-)
Maybe I'm missing something of course.

Re: [PATCH 7/8] fix py-finish-breakpoint.exp with always-async

[Tom Tromey]

Doug> IOW, it'd be clearer if the patch did:
Doug>   int was_executing = target_has_execution;
Doug> and then augment the patch with whatever else is necessary
Doug> to make it correct.
Doug> [Assuming I understand correctly what the patch is trying to do.]

I think what is happening is that we need to detect the transition from
"not executing" to "executing a sync command".  But perhaps we don't
need to test sync_execution beforehand if we correctly detect execution.

Doug> Also, is target_has_execution what you want here?

Yeah, that is wrong.

Tom

Re: [PATCH 7/8] fix py-finish-breakpoint.exp with always-async

[Tom Tromey]

Doug> Also, is target_has_execution what you want here?

Tom> Yeah, that is wrong.

FWIW, dropping target_has_execution from both conditions still leaves
the test suite regression-free.  I think what happens is that
sync_execution can only be set if executing a command, and so it
suffices as a test.

I'll refresh the series soon.

Tom

Re: [PATCH 7/8] fix py-finish-breakpoint.exp with always-async

[Yao Qi]

On 07/30/2013 12:45 AM, Tom Tromey wrote:
> However, the code did not follow this comment -- it didn't check to
> see if the command started the target.
>
> This patch fixes the problem by noting whether the target was
> executing in sync_execution mode before running the command, and then
> augmenting the condition to test this as well.

The rationale looks right to me.

>
> Built and regtested on x86-64 Fedora 18.
>
> 	* top.c (execute_command): Only dispatch events if command
> 	started target.

I opened PR 14135 for an internal error we saw in 
gdb.python/py-finish-breakpoint.exp when async is on.  If it is the 
error this patch is to fix, please mention this patch fixes PR 14135 in 
ChangeLog.

http://sourceware.org/bugzilla/show_bug.cgi?id=14135

I also posted a patch for this PR, but it doesn't apply to current trunk.

-- 
Yao (齐尧)

Re: [PATCH 7/8] fix py-finish-breakpoint.exp with always-async

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> I opened PR 14135 for an internal error we saw in
Yao> gdb.python/py-finish-breakpoint.exp when async is on.  If it is the
Yao> error this patch is to fix, please mention this patch fixes PR 14135
Yao> in ChangeLog.

Yao> http://sourceware.org/bugzilla/show_bug.cgi?id=14135

Yao> I also posted a patch for this PR, but it doesn't apply to current trunk.

Oh, I'm sorry about that.  I didn't remember this bug or the patch.
I'll go look at your patch now.

Tom

[PATCH 8/8] enable target-async

[Tom Tromey]

This enables target-async by default.

Unlike the CLI, MI chose to treat target-async specially -- setting it
changes the default behavior of commands.  So, we can't get rid of the
option.  Instead we have to make it MI-only.

The hardest part of this patch, to my surprise, was getting the MI
prompt to work properly.  It was reasonably easy, and clean, to get
close to what the test suite expects; but to fix the last remaining
failure (mi-async.exp), I had to resort to a hack.

It seems to me that the MI grammar was never updated to account for
changes implied by async.

Perhaps some future MI can dispense with the prompt entirely.

Built and regtested on x86-64 Fedora 18.

	* infrun.c (set_observer_mode): Don't set target_async_permitted.
	* linux-nat.c (linux_nat_is_async_p): Always return 1.
	(linux_nat_can_async_p): Likewise.
	* mi/mi-interp.c (mi_interpreter_prompt_p): Maybe print the MI
	prompt.
	(mi_cmd_interpreter_exec): Set mi_last_was_cli.
	(mi_execute_command_input_handler): Conditionally print prompt.
	(mi_on_resume): Check sync_execution before printing prompt.
	* mi/mi-main.h (mi_last_was_cli): Declare.
	* mi/mi-main.c (mi_last_was_cli): New global.
	(mi_target_can_async_p): New function.
	(exec_continue): Maybe call async_disable_stdin.
	(run_one_inferior, mi_cmd_exec_run, mi_cmd_list_target_features):
	Use mi_target_can_async_p.
	(captured_mi_execute_command): Clear mi_last_was_cli.
	(mi_execute_async_cli_command): Use mi_target_can_async_p.
	* remote.c (remote_open_1, remote_terminal_inferior)
	(remote_terminal_ours, remote_can_async_p, remote_is_async_p):
	Don't check target_async_permitted.

	* gdb.texinfo (Non-Stop Mode): Remove "set target-async 1"
	from example.
	(Background Execution): Move target-async docs...
	(Asynchronous and non-stop modes): ... here.  Rewrite to
	MI form.

	* gdb.mi/mi-cli.exp: Don't check "$async".
---
 gdb/cli/cli-interp.c            |  1 +
 gdb/doc/gdb.texinfo             | 29 ++++++++++---------------
 gdb/infrun.c                    |  1 -
 gdb/linux-nat.c                 | 10 ++-------
 gdb/mi/mi-interp.c              | 28 +++++++++++++++++++-----
 gdb/mi/mi-main.c                | 29 +++++++++++++++++++------
 gdb/mi/mi-main.h                |  1 +
 gdb/remote.c                    | 48 +++++++++++------------------------------
 gdb/testsuite/gdb.mi/mi-cli.exp | 15 +------------
 gdb/tui/tui-interp.c            |  1 +
 10 files changed, 76 insertions(+), 87 deletions(-)

diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index 1003cc7..ef1e65b 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -25,6 +25,7 @@
 #include "top.h"		/* for "execute_command" */
 #include "gdb_string.h"
 #include "exceptions.h"
+#include "target.h"
 
 struct ui_out *cli_uiout;
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index cb393e8..4ec4880 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5672,9 +5672,6 @@ To enter non-stop mode, use this sequence of commands before you run
 or attach to your program:
 
 @smallexample
-# Enable the async interface.  
-set target-async 1
-
 # If using the CLI, pagination breaks non-stop.
 set pagination off
 
@@ -5744,21 +5741,6 @@ the program to report that some thread has stopped before prompting for
 another command.  In background execution, @value{GDBN} immediately gives
 a command prompt so that you can issue other commands while your program runs.
 
-You need to explicitly enable asynchronous mode before you can use
-background execution commands.  You can use these commands to
-manipulate the asynchronous mode setting:
-
-@table @code
-@kindex set target-async
-@item set target-async on
-Enable asynchronous mode.
-@item set target-async off
-Disable asynchronous mode.
-@kindex show target-async
-@item show target-async
-Show the current target-async setting.
-@end table
-
 If the target doesn't support async mode, @value{GDBN} issues an error
 message if you attempt to use the background execution commands.
 
@@ -28505,6 +28487,17 @@ frontend has started the executable or attached to the target, it can
 find if asynchronous execution is enabled using the
 @code{-list-target-features} command.
 
+@table @code
+@kindex -gdb-set target-async
+@item -gdb-set target-async on
+Enable asynchronous mode.  This mode only affects MI commands.
+@item -gdb-set target-async off
+Disable asynchronous mode.
+@kindex -gdb-show target-async
+@item -gdb-show target-async
+Show the current target-async setting.
+@end table
+
 Even if @value{GDBN} can accept a command while target is running,
 many commands that access the target do not work when the target is
 running.  Therefore, asynchronous command execution is most useful
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 0052ef2..be12a02 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -242,7 +242,6 @@ set_observer_mode (char *args, int from_tty,
      going out we leave it that way.  */
   if (observer_mode)
     {
-      target_async_permitted = 1;
       pagination_enabled = 0;
       non_stop = non_stop_1 = 1;
     }
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 45a6e5f..65180fb 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4768,10 +4768,7 @@ linux_trad_target (CORE_ADDR (*register_u_offset)(struct gdbarch *, int, int))
 static int
 linux_nat_is_async_p (void)
 {
-  /* NOTE: palves 2008-03-21: We're only async when the user requests
-     it explicitly with the "set target-async" command.
-     Someday, linux will always be async.  */
-  return target_async_permitted;
+  return 1;
 }
 
 /* target_can_async_p implementation.  */
@@ -4779,10 +4776,7 @@ linux_nat_is_async_p (void)
 static int
 linux_nat_can_async_p (void)
 {
-  /* NOTE: palves 2008-03-21: We're only async when the user requests
-     it explicitly with the "set target-async" command.
-     Someday, linux will always be async.  */
-  return target_async_permitted;
+  return 1;
 }
 
 static int
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index b4515d9..886fde1 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -222,11 +222,25 @@ mi_interpreter_exec (void *data, const char *command)
   return exception_none;
 }
 
+int mi_last_was_cli;
+
 /* Never display the default GDB prompt in MI case.  */
 
 static int
 mi_interpreter_prompt_p (void *data)
 {
+  if (!interp_quiet_p (NULL))
+    {
+      if (!target_is_async_p ()
+	  || (!sync_execution && (!target_async_permitted || !mi_last_was_cli)))
+	      /* && (!running_result_record_printed */
+	      /* 	  || !mi_proceeded )))) */
+	{
+	  fputs_unfiltered ("(gdb) \n", raw_stdout);
+	  gdb_flush (raw_stdout);
+	}
+    }
+
   return 0;
 }
 
@@ -252,6 +266,8 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc)
 	     "does not support command execution"),
 	      argv[0]);
 
+  mi_last_was_cli = strcmp (argv[0], "console") == 0;
+
   /* Note that unlike the CLI version of this command, we don't
      actually set INTERP_TO_USE as the current interpreter, as we
      still want gdb_stdout, etc. to point at MI streams.  */
@@ -323,8 +339,11 @@ mi_execute_command_input_handler (char *cmd)
 {
   mi_execute_command_wrapper (cmd);
 
-  fputs_unfiltered ("(gdb) \n", raw_stdout);
-  gdb_flush (raw_stdout);
+  if (!target_is_async_p () || !sync_execution)
+    {
+      fputs_unfiltered ("(gdb) \n", raw_stdout);
+      gdb_flush (raw_stdout);
+    }
 }
 
 static void
@@ -855,9 +874,8 @@ mi_on_resume (ptid_t ptid)
       /* This is what gdb used to do historically -- printing prompt even if
 	 it cannot actually accept any input.  This will be surely removed
 	 for MI3, and may be removed even earler.  */
-      /* FIXME: review the use of target_is_async_p here -- is that
-	 what we want? */
-      if (!target_is_async_p ())
+      if (/* !target_async_permitted ||  */
+	  !target_is_async_p () || sync_execution)
 	fputs_unfiltered ("(gdb) \n", raw_stdout);
     }
   gdb_flush (raw_stdout);
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index c2d8501..115308b 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -94,6 +94,10 @@ int running_result_record_printed = 1;
    command was issued.  */
 int mi_proceeded;
 
+/* Flag indicating whether the most recent command was executed via
+   the CLI interpreter.  */
+int mi_last_was_cli;
+
 extern void _initialize_mi_main (void);
 static void mi_cmd_execute (struct mi_parse *parse);
 
@@ -106,6 +110,15 @@ static int register_changed_p (int regnum, struct regcache *,
 static void output_register (struct frame_info *, int regnum, int format,
 			     int skip_unavailable);
 
+/* A wrapper for target_can_async_p that takes the MI setting into
+   account.  */
+
+static int
+mi_target_can_async_p (void)
+{
+  return target_async_permitted && target_can_async_p ();
+}
+
 /* Command implementations.  FIXME: Is this libgdb?  No.  This is the MI
    layer that calls libgdb.  Any operation used in the below should be
    formalized.  */
@@ -262,6 +275,9 @@ exec_continue (char **argv, int argc)
     {
       struct cleanup *back_to = make_cleanup_restore_integer (&sched_multi);
 
+      if (!target_async_permitted && target_can_async_p ())
+	async_disable_stdin ();
+
       if (current_context->all)
 	{
 	  sched_multi = 1;
@@ -386,8 +402,8 @@ run_one_inferior (struct inferior *inf, void *arg)
       switch_to_thread (null_ptid);
       set_current_program_space (inf->pspace);
     }
-  mi_execute_cli_command ("run", target_can_async_p (),
-			  target_can_async_p () ? "&" : NULL);
+  mi_execute_cli_command ("run", mi_target_can_async_p (),
+			  mi_target_can_async_p () ? "&" : NULL);
   return 0;
 }
 
@@ -403,8 +419,8 @@ mi_cmd_exec_run (char *command, char **argv, int argc)
     }
   else
     {
-      mi_execute_cli_command ("run", target_can_async_p (),
-			      target_can_async_p () ? "&" : NULL);
+      mi_execute_cli_command ("run", mi_target_can_async_p (),
+			      mi_target_can_async_p () ? "&" : NULL);
     }
 }
 
@@ -1789,7 +1805,7 @@ mi_cmd_list_target_features (char *command, char **argv, int argc)
       struct ui_out *uiout = current_uiout;
 
       cleanup = make_cleanup_ui_out_list_begin_end (uiout, "features");      
-      if (target_can_async_p ())
+      if (mi_target_can_async_p ())
 	ui_out_field_string (uiout, NULL, "async");
       if (target_can_execute_reverse)
 	ui_out_field_string (uiout, NULL, "reverse");
@@ -1886,6 +1902,7 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
 
   running_result_record_printed = 0;
   mi_proceeded = 0;
+  mi_last_was_cli = 0;
   switch (context->op)
     {
     case MI_COMMAND:
@@ -2204,7 +2221,7 @@ mi_execute_async_cli_command (char *cli_command, char **argv, int argc)
   struct cleanup *old_cleanups;
   char *run;
 
-  if (target_can_async_p ())
+  if (mi_target_can_async_p ())
     run = xstrprintf ("%s %s&", cli_command, argc ? *argv : "");
   else
     run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h
index d75526a..22f8827 100644
--- a/gdb/mi/mi-main.h
+++ b/gdb/mi/mi-main.h
@@ -32,6 +32,7 @@ extern char *current_token;
 
 extern int running_result_record_printed;
 extern int mi_proceeded;
+extern int mi_last_was_cli;
 
 struct mi_suppress_notification
 {
diff --git a/gdb/remote.c b/gdb/remote.c
index 1d6ac90..e160133 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4250,8 +4250,7 @@ remote_open_1 (char *name, int from_tty,
 	   "(e.g. /dev/ttyS0, /dev/ttya, COM1, etc.)."));
 
   /* See FIXME above.  */
-  if (!target_async_permitted)
-    wait_forever_enabled_p = 1;
+  wait_forever_enabled_p = 1;
 
   /* If we're connected to a running target, target_preopen will kill it.
      Ask this question first, before target_preopen has a chance to kill
@@ -4337,20 +4336,17 @@ remote_open_1 (char *name, int from_tty,
   use_threadinfo_query = 1;
   use_threadextra_query = 1;
 
-  if (target_async_permitted)
-    {
-      /* With this target we start out by owning the terminal.  */
-      remote_async_terminal_ours_p = 1;
+  /* With this target we start out by owning the terminal.  */
+  remote_async_terminal_ours_p = 1;
 
-      /* FIXME: cagney/1999-09-23: During the initial connection it is
-	 assumed that the target is already ready and able to respond to
-	 requests.  Unfortunately remote_start_remote() eventually calls
-	 wait_for_inferior() with no timeout.  wait_forever_enabled_p gets
-	 around this.  Eventually a mechanism that allows
-	 wait_for_inferior() to expect/get timeouts will be
-	 implemented.  */
-      wait_forever_enabled_p = 0;
-    }
+  /* FIXME: cagney/1999-09-23: During the initial connection it is
+     assumed that the target is already ready and able to respond to
+     requests.  Unfortunately remote_start_remote() eventually calls
+     wait_for_inferior() with no timeout.  wait_forever_enabled_p gets
+     around this.  Eventually a mechanism that allows
+     wait_for_inferior() to expect/get timeouts will be
+     implemented.  */
+  wait_forever_enabled_p = 0;
 
   /* First delete any symbols previously loaded from shared libraries.  */
   no_shared_libraries (NULL, 0);
@@ -4386,14 +4382,12 @@ remote_open_1 (char *name, int from_tty,
 	   already before throwing the exception.  */
 	if (remote_desc != NULL)
 	  remote_unpush_target ();
-	if (target_async_permitted)
-	  wait_forever_enabled_p = 1;
+	wait_forever_enabled_p = 1;
 	throw_exception (ex);
       }
   }
 
-  if (target_async_permitted)
-    wait_forever_enabled_p = 1;
+  wait_forever_enabled_p = 1;
 }
 
 /* This takes a program previously attached to and detaches it.  After
@@ -5188,10 +5182,6 @@ Give up (and stop debugging it)? ")))
 static void
 remote_terminal_inferior (void)
 {
-  if (!target_async_permitted)
-    /* Nothing to do.  */
-    return;
-
   /* FIXME: cagney/1999-09-27: Make calls to target_terminal_*()
      idempotent.  The event-loop GDB talking to an asynchronous target
      with a synchronous command calls this function from both
@@ -5211,10 +5201,6 @@ remote_terminal_inferior (void)
 static void
 remote_terminal_ours (void)
 {
-  if (!target_async_permitted)
-    /* Nothing to do.  */
-    return;
-
   /* See FIXME in remote_terminal_inferior.  */
   if (remote_async_terminal_ours_p)
     return;
@@ -11621,10 +11607,6 @@ Specify the serial device it is connected to (e.g. /dev/ttya).";
 static int
 remote_can_async_p (void)
 {
-  if (!target_async_permitted)
-    /* We only enable async when the user specifically asks for it.  */
-    return 0;
-
   /* We're async whenever the serial device is.  */
   return serial_can_async_p (remote_desc);
 }
@@ -11632,10 +11614,6 @@ remote_can_async_p (void)
 static int
 remote_is_async_p (void)
 {
-  if (!target_async_permitted)
-    /* We only enable async when the user specifically asks for it.  */
-    return 0;
-
   /* We're async whenever the serial device is.  */
   return serial_is_async_p (remote_desc);
 }
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index bee296d..08e443e 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -134,20 +134,7 @@ mi_gdb_test "500-stack-select-frame 0" \
   {500\^done} \
   "-stack-select-frame 0"
 
-# When a CLI command is entered in MI session, the respose is different in
-# sync and async modes. In sync mode normal_stop is called when current
-# interpreter is CLI. So:
-#   - print_stop_reason prints stop reason in CLI uiout, and we don't show it
-#     in MI
-#   - The stop position is printed, and appears in MI 'console' channel.
-#
-# In async mode the stop event is processed when we're back to MI interpreter,
-# so the stop reason is printed into MI uiout an.
-if {$async} {
-    set reason "end-stepping-range"
-} else {
-    set reason ""
-}
+set reason "end-stepping-range"
 
 mi_execute_to "interpreter-exec console step" $reason "callee4" "" ".*basics.c" $line_callee4_next \
     "" "check *stopped from CLI command"
diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
index 42526e6..296519b 100644
--- a/gdb/tui/tui-interp.c
+++ b/gdb/tui/tui-interp.c
@@ -30,6 +30,7 @@
 #include "tui/tui.h"
 #include "tui/tui-io.h"
 #include "exceptions.h"
+#include "target.h"
 
 /* Set to 1 when the TUI mode must be activated when we first start
    gdb.  */
-- 
1.8.1.4

Re: [PATCH 8/8] enable target-async

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: Tom Tromey <tromey@redhat.com>
> Date: Mon, 29 Jul 2013 10:45:24 -0600
> 
> This enables target-async by default.

The documentation parts of this are OK.

However, I have a question: what does this change mean for targets
that don't support target-async?  Do they always get an error message
when GDB starts?

Thanks.

Re: [PATCH 8/8] enable target-async

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> However, I have a question: what does this change mean for targets
Eli> that don't support target-async?  Do they always get an error message
Eli> when GDB starts?

I don't think they will.

All it will mean is that if the user tries to use the "&" suffix on a
command, like "run &", then he will get an error.

An MI client will still see an error if it tries to set target-async.
This is also no different from today.

Tom

[PATCH 4/8] PR gdb/13860: make "-exec-foo"'s MI output equal to "foo"'s MI output.

[Tom Tromey]

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 2a5770d..a748d8b 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -210,6 +210,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 dc1036d..53ecc1d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5922,6 +5922,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);
+
+  /* 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.
 
@@ -6044,65 +6106,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);
-
-	  /* 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 2702f4f..0cd43f5 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -479,9 +479,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);
 	  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 86a0fd6..6ad8f99 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

[PATCH 1/8] fix latent bugs in ui-out.c

[Tom Tromey]

The destructor code in ui-out.c has a latent bug, which is hidden by
the fact that nothing uses this right now.  This patch fixes the
problem.  The bug is that we don't always clear a pointer in the
ui-out object, leading to bad a free.

	* ui-out.c (clear_table, ui_out_new): Clear uiout->table.id.
---
 gdb/ui-out.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 03b1240..3c6c529 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -807,8 +807,8 @@ uo_table_header (struct ui_out *uiout, int width, enum ui_align align,
 static void
 clear_table (struct ui_out *uiout)
 {
-  if (uiout->table.id)
-    xfree (uiout->table.id);
+  xfree (uiout->table.id);
+  uiout->table.id = NULL;
   clear_header_list (uiout);
 }
 
@@ -1114,6 +1114,7 @@ ui_out_new (struct ui_out_impl *impl, void *data,
   current->field_count = 0;
   VEC_safe_push (ui_out_level_p, uiout->levels, current);
 
+  uiout->table.id = NULL;
   uiout->table.header_first = NULL;
   uiout->table.header_last = NULL;
   uiout->table.header_next = NULL;
-- 
1.8.1.4