[RFC] fix regression with target-async and record

[Tom Tromey <tromey at redhat dot com>]

A patch in the target cleanup series caused a regression when using
record with target-async.

The immediate problem is that record supplies to_can_async_p and
to_is_async_p methods, but does not supply a to_async method.  So,
when target-async is set, record claims to support async -- but if the
underlying target does not support async, then the to_async method
call will end up in that method's default implementation, namely
tcomplain.

This worked previously because the record target used to provide a
to_async method; one that (erroneously, only at push time) checked the
other members of the target stack, and then simply dropped to_async
calls in the "does not implement async" case.

My first thought was to simply drop tcomplain as the default for
to_async.  This works, but Pedro pointed out that the only reason
record has to supply to_can_async_p and to_is_async_p is that these
default to using the find_default_run_target machinery -- and these
defaults are only needed by "run" and "attach".

So, a nicer solution presents itself: change run and attach to
explicitly call into the default run target when needed; and change
to_is_async_p and to_can_async_p to default to "return 0".  This makes
the target stack simpler to use and lets us remove the method
implementations from record.  This is also in harmony with other plans
for the target stack; namely trying to reduce the impact of
find_default_run_target.  This approach makes it clear that
find_default_is_async_p is not needed -- it is asking whether a target
that may not even be pushed is actually async, which seems like a
nonsensical question.

While working on this I noticed that there don't seem to be any test
cases that involve both target-async and record, so this patch changes
break-precsave.exp to add some.

Built and regtested on x86-64 Fedora 18.

2014-02-25  Tom Tromey  <tromey@redhat.com>

	* target.h (struct target_ops) <to_can_async_p, to_is_async_p>:
	Use TARGET_DEFAULT_RETURN.
	(find_default_can_async_p): Declare.
	* target.c (find_default_can_async_p): No longer static.  Use
	current target if it can run.
	(find_default_is_async_p): Remove.
	* target-delegates.c: Rebuild.
	* record-full.c (record_full_can_async_p, record_full_is_async_p):
	Remove.
	(init_record_full_ops, init_record_full_core_ops): Update.
	* infcmd.c (run_command_1, attach_command): Use
	find_default_can_async_p.

2014-02-25  Tom Tromey  <tromey@redhat.com>

	* gdb.reverse/break-precsave.exp (precsave_tests): New proc.
	Add target-async tests.
---
 gdb/ChangeLog                                |  15 ++++
 gdb/infcmd.c                                 |  12 +--
 gdb/record-full.c                            |  18 -----
 gdb/target-delegates.c                       |  16 +++-
 gdb/target.c                                 |  40 ++++------
 gdb/target.h                                 |  11 ++-
 gdb/testsuite/ChangeLog                      |   5 ++
 gdb/testsuite/gdb.reverse/break-precsave.exp | 115 +++++++++++++++------------
 8 files changed, 127 insertions(+), 105 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 2d50f41..449ab2e 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -544,7 +544,7 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
 
   if (!args)
     {
-      if (target_can_async_p ())
+      if (find_default_can_async_p ())
 	async_disable_stdin ();
     }
   else
@@ -553,12 +553,12 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
 
       /* If we get a request for running in the bg but the target
          doesn't support it, error out.  */
-      if (async_exec && !target_can_async_p ())
+      if (async_exec && !find_default_can_async_p ())
 	error (_("Asynchronous execution not supported on this target."));
 
       /* If we don't get a request of running in the bg, then we need
          to simulate synchronous (fg) execution.  */
-      if (!async_exec && target_can_async_p ())
+      if (!async_exec && find_default_can_async_p ())
 	{
 	  /* Simulate synchronous execution.  */
 	  async_disable_stdin ();
@@ -2543,13 +2543,13 @@ attach_command (char *args, int from_tty)
 
       /* If we get a request for running in the bg but the target
          doesn't support it, error out.  */
-      if (async_exec && !target_can_async_p ())
+      if (async_exec && !find_default_can_async_p ())
 	error (_("Asynchronous execution not supported on this target."));
     }
 
   /* If we don't get a request of running in the bg, then we need
      to simulate synchronous (fg) execution.  */
-  if (!async_exec && target_can_async_p ())
+  if (!async_exec && find_default_can_async_p ())
     {
       /* Simulate synchronous execution.  */
       async_disable_stdin ();
@@ -2595,7 +2595,7 @@ attach_command (char *args, int from_tty)
 	 STOP_QUIETLY_NO_SIGSTOP is for.  */
       inferior->control.stop_soon = STOP_QUIETLY_NO_SIGSTOP;
 
-      if (target_can_async_p ())
+      if (find_default_can_async_p ())
 	{
 	  /* sync_execution mode.  Wait for stop.  */
 	  struct attach_command_continuation_args *a;
diff --git a/gdb/record-full.c b/gdb/record-full.c
index d35165b..aabbdf2 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1750,20 +1750,6 @@ record_full_goto_bookmark (struct target_ops *self,
   return;
 }
 
-static int
-record_full_can_async_p (struct target_ops *ops)
-{
-  /* We only enable async when the user specifically asks for it.  */
-  return target_async_permitted;
-}
-
-static int
-record_full_is_async_p (struct target_ops *ops)
-{
-  /* We only enable async when the user specifically asks for it.  */
-  return target_async_permitted;
-}
-
 static enum exec_direction_kind
 record_full_execution_direction (struct target_ops *self)
 {
@@ -1928,8 +1914,6 @@ init_record_full_ops (void)
   /* Add bookmark target methods.  */
   record_full_ops.to_get_bookmark = record_full_get_bookmark;
   record_full_ops.to_goto_bookmark = record_full_goto_bookmark;
-  record_full_ops.to_can_async_p = record_full_can_async_p;
-  record_full_ops.to_is_async_p = record_full_is_async_p;
   record_full_ops.to_execution_direction = record_full_execution_direction;
   record_full_ops.to_info_record = record_full_info;
   record_full_ops.to_save_record = record_full_save;
@@ -2174,8 +2158,6 @@ init_record_full_core_ops (void)
   /* Add bookmark target methods.  */
   record_full_core_ops.to_get_bookmark = record_full_get_bookmark;
   record_full_core_ops.to_goto_bookmark = record_full_goto_bookmark;
-  record_full_core_ops.to_can_async_p = record_full_can_async_p;
-  record_full_core_ops.to_is_async_p = record_full_is_async_p;
   record_full_core_ops.to_execution_direction
     = record_full_execution_direction;
   record_full_core_ops.to_info_record = record_full_info;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 5b27b59..d04255d 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -690,12 +690,24 @@ delegate_can_async_p (struct target_ops *self)
 }
 
 static int
+tdefault_can_async_p (struct target_ops *self)
+{
+  return 0;
+}
+
+static int
 delegate_is_async_p (struct target_ops *self)
 {
   self = self->beneath;
   return self->to_is_async_p (self);
 }
 
+static int
+tdefault_is_async_p (struct target_ops *self)
+{
+  return 0;
+}
+
 static void
 delegate_async (struct target_ops *self, async_callback_ftype *arg1, void *arg2)
 {
@@ -1939,8 +1951,8 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_pid_to_exec_file = tdefault_pid_to_exec_file;
   ops->to_log_command = tdefault_log_command;
   ops->to_get_section_table = tdefault_get_section_table;
-  ops->to_can_async_p = find_default_can_async_p;
-  ops->to_is_async_p = find_default_is_async_p;
+  ops->to_can_async_p = tdefault_can_async_p;
+  ops->to_is_async_p = tdefault_is_async_p;
   ops->to_async = tdefault_async;
   ops->to_find_memory_regions = dummy_find_memory_regions;
   ops->to_make_corefile_notes = dummy_make_corefile_notes;
diff --git a/gdb/target.c b/gdb/target.c
index 3e54d85..eb153b4 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -99,10 +99,6 @@ static char *dummy_make_corefile_notes (struct target_ops *self,
 
 static char *default_pid_to_str (struct target_ops *ops, ptid_t ptid);
 
-static int find_default_can_async_p (struct target_ops *ignore);
-
-static int find_default_is_async_p (struct target_ops *ignore);
-
 static enum exec_direction_kind default_execution_direction
     (struct target_ops *self);
 
@@ -2747,34 +2743,26 @@ find_default_create_inferior (struct target_ops *ops,
   return;
 }
 
-static int
-find_default_can_async_p (struct target_ops *ignore)
-{
-  struct target_ops *t;
-
-  /* This may be called before the target is pushed on the stack;
-     look for the default process stratum.  If there's none, gdb isn't
-     configured with a native debugger, and target remote isn't
-     connected yet.  */
-  t = find_default_run_target (NULL);
-  if (t && t->to_can_async_p != delegate_can_async_p)
-    return (t->to_can_async_p) (t);
-  return 0;
-}
+/* See target.h.  */
 
-static int
-find_default_is_async_p (struct target_ops *ignore)
+int
+find_default_can_async_p (void)
 {
-  struct target_ops *t;
-
   /* This may be called before the target is pushed on the stack;
      look for the default process stratum.  If there's none, gdb isn't
      configured with a native debugger, and target remote isn't
      connected yet.  */
-  t = find_default_run_target (NULL);
-  if (t && t->to_is_async_p != delegate_is_async_p)
-    return (t->to_is_async_p) (t);
-  return 0;
+  if (current_target.to_stratum >= process_stratum)
+    return current_target.to_can_async_p (&current_target);
+  else
+    {
+      struct target_ops *t;
+
+      t = find_default_run_target (NULL);
+      if (t && t->to_can_async_p != delegate_can_async_p)
+	return (t->to_can_async_p) (t);
+      return 0;
+    }
 }
 
 static int
diff --git a/gdb/target.h b/gdb/target.h
index 4b735dd..3d661f8 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -589,9 +589,9 @@ struct target_ops
     int to_attach_no_wait;
     /* ASYNC target controls */
     int (*to_can_async_p) (struct target_ops *)
-      TARGET_DEFAULT_FUNC (find_default_can_async_p);
+      TARGET_DEFAULT_RETURN (0);
     int (*to_is_async_p) (struct target_ops *)
-      TARGET_DEFAULT_FUNC (find_default_is_async_p);
+      TARGET_DEFAULT_RETURN (0);
     void (*to_async) (struct target_ops *, async_callback_ftype *, void *)
       TARGET_DEFAULT_NORETURN (tcomplain ());
     int (*to_supports_non_stop) (struct target_ops *);
@@ -1639,6 +1639,13 @@ extern int target_async_permitted;
 /* Can the target support asynchronous execution?  */
 #define target_can_async_p() (current_target.to_can_async_p (&current_target))
 
+/* This determines whether the target that will handle a "run" or
+   "attach" command supports asynchronous execution.  If the current
+   target has a process stratum (or above) target, then it is queried;
+   otherwise, the default run target is used.  */
+
+int find_default_can_async_p (void);
+
 /* Is the target in asynchronous execution mode?  */
 #define target_is_async_p() (current_target.to_is_async_p (&current_target))
 
diff --git a/gdb/testsuite/gdb.reverse/break-precsave.exp b/gdb/testsuite/gdb.reverse/break-precsave.exp
index cc05d43..f1e6c69 100644
--- a/gdb/testsuite/gdb.reverse/break-precsave.exp
+++ b/gdb/testsuite/gdb.reverse/break-precsave.exp
@@ -33,73 +33,86 @@ set bar_location  [gdb_get_line_number "break in bar" ]
 set main_location [gdb_get_line_number "break in main"]
 set end_location  [gdb_get_line_number "end of main"  ]
 
-runto main
+proc precsave_tests {} {
+    global foo_location bar_location main_location end_location
+    global decimal srcfile precsave gdb_prompt
 
-if [supports_process_record] {
-    # Activate process record/replay
-    gdb_test_no_output "record" "Turn on process record"
-}
+    runto main
 
-gdb_test "break $end_location" \
-    "Breakpoint $decimal at .*/$srcfile, line $end_location\." \
-    "BP at end of main"
+    if [supports_process_record] {
+	# Activate process record/replay
+	gdb_test_no_output "record" "Turn on process record"
+    }
 
-gdb_test "continue" "Breakpoint .* end of main .*" "run to end of main"
+    gdb_test "break $end_location" \
+	"Breakpoint $decimal at .*/$srcfile, line $end_location\." \
+	"BP at end of main"
 
-gdb_test "record save $precsave" \
-    "Saved core file $precsave with execution log\."  \
-    "save process recfile"
+    gdb_test "continue" "Breakpoint .* end of main .*" "run to end of main"
 
-gdb_test "kill" "" "Kill process, prepare to debug log file" \
-    "Kill the program being debugged\\? \\(y or n\\) " "y"
+    gdb_test "record save $precsave" \
+	"Saved core file $precsave with execution log\."  \
+	"save process recfile"
 
-gdb_test "record restore $precsave" \
-    "Program terminated with signal .*" \
-    "reload precord save file"
+    gdb_test "kill" "" "Kill process, prepare to debug log file" \
+	"Kill the program being debugged\\? \\(y or n\\) " "y"
 
-gdb_test "break foo" \
-    "Breakpoint $decimal at .* line $foo_location\." \
-    "set breakpoint on foo"
+    gdb_test "record restore $precsave" \
+	"Program terminated with signal .*" \
+	"reload precord save file"
 
-gdb_test "break bar" \
-    "Breakpoint $decimal at .* line $bar_location\." \
-    "set breakpoint on bar"
+    gdb_test "break foo" \
+	"Breakpoint $decimal at .* line $foo_location\." \
+	"set breakpoint on foo"
 
-gdb_continue_to_breakpoint "foo" ".*/$srcfile:$foo_location.*"
-gdb_continue_to_breakpoint "bar" ".*/$srcfile:$bar_location.*"
-gdb_test_multiple "continue" "go to end of main forward" {
-    -re ".*Breakpoint $decimal,.*/$srcfile:$end_location.*$gdb_prompt $"  {
-	pass "go to end of main forward"
-    }
-    -re "No more reverse-execution history.* end of main .*$gdb_prompt $" {
-	pass "go to end of main forward"
+    gdb_test "break bar" \
+	"Breakpoint $decimal at .* line $bar_location\." \
+	"set breakpoint on bar"
+
+    gdb_continue_to_breakpoint "foo" ".*/$srcfile:$foo_location.*"
+    gdb_continue_to_breakpoint "bar" ".*/$srcfile:$bar_location.*"
+    gdb_test_multiple "continue" "go to end of main forward" {
+	-re ".*Breakpoint $decimal,.*/$srcfile:$end_location.*$gdb_prompt $"  {
+	    pass "go to end of main forward"
+	}
+	-re "No more reverse-execution history.* end of main .*$gdb_prompt $" {
+	    pass "go to end of main forward"
+	}
     }
-}
 
-gdb_test_no_output "set exec-direction reverse" "set reverse"
+    gdb_test_no_output "set exec-direction reverse" "set reverse"
 
-gdb_continue_to_breakpoint "bar backward"  ".*/$srcfile:$bar_location.*"
-gdb_continue_to_breakpoint "foo backward"  ".*/$srcfile:$foo_location.*"
+    gdb_continue_to_breakpoint "bar backward"  ".*/$srcfile:$bar_location.*"
+    gdb_continue_to_breakpoint "foo backward"  ".*/$srcfile:$foo_location.*"
 
-gdb_test_multiple "continue" "main backward" {
-    -re ".*Breakpoint $decimal,.*/$srcfile:$main_location.*$gdb_prompt $" {
-	pass "main backward"
-    }
-    -re "No more reverse-execution history.* break in main .*$gdb_prompt $" {
-	pass "main backward"
+    gdb_test_multiple "continue" "main backward" {
+	-re ".*Breakpoint $decimal,.*/$srcfile:$main_location.*$gdb_prompt $" {
+	    pass "main backward"
+	}
+	-re "No more reverse-execution history.* break in main .*$gdb_prompt $" {
+	    pass "main backward"
+	}
     }
-}
 
-gdb_test_no_output "set exec-direction forward" "set forward"
+    gdb_test_no_output "set exec-direction forward" "set forward"
 
-gdb_continue_to_breakpoint "foo" ".*/$srcfile:$foo_location.*"
-gdb_continue_to_breakpoint "bar" ".*/$srcfile:$bar_location.*"
+    gdb_continue_to_breakpoint "foo" ".*/$srcfile:$foo_location.*"
+    gdb_continue_to_breakpoint "bar" ".*/$srcfile:$bar_location.*"
 
-gdb_test_multiple "continue" "end of record log" {
-    -re ".*Breakpoint $decimal,.*/$srcfile:$end_location.*$gdb_prompt $" {
-	pass "end of record log"
-    }
-    -re "No more reverse-execution history.* end of main .*$gdb_prompt $" {
-	pass "end of record log"
+    gdb_test_multiple "continue" "end of record log" {
+	-re ".*Breakpoint $decimal,.*/$srcfile:$end_location.*$gdb_prompt $" {
+	    pass "end of record log"
+	}
+	-re "No more reverse-execution history.* end of main .*$gdb_prompt $" {
+	    pass "end of record log"
+	}
     }
 }
+
+precsave_tests
+
+with_test_prefix "target-async" {
+    clean_restart $testfile
+    gdb_test_no_output "set target-async on"
+    precsave_tests
+}
-- 
1.8.1.4