public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix -exec-run not running asynchronously with mi-async on
@ 2016-05-07 11:24 Simon Marchi
  2016-05-17 18:56 ` Pedro Alves
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2016-05-07 11:24 UTC (permalink / raw)
  To: gdb-patches; +Cc: simon.marchi, Simon Marchi

From: Simon Marchi <simon.marchi@ericsson.com>

This patch is a long overdue v2 of:

  https://sourceware.org/ml/gdb-patches/2015-03/msg00110.html

The problem is that when doing -exec-run on a freshly started GDB, the
only target pushed is the dummy one.  When mi_async_p is called to know
whether the run should be async, it queries whether the current target
(dummy) supports async, and the answer is no.  The fix is to make the
code query the target that will be used for the run, which is not
necessarily the current target.

I cleaned up the test a little bit.

No regressions in the gdb.mi directory using the unix, native-gdbserver
and native-extended-gdbserver boards.  The test doesn't pass when
forcing maint set target-async off, obviously, since it makes mi-async
have no effect.  It doesn't seem like other tests are checking for that
eventuality, so I didn't in the new test.

gdb/ChangeLog:

	* mi/mi-main.c (run_one_inferior): Use run target to determine
	whether to run async or not.
	(mi_cmd_exec_run): Likewise.

gdb/testsuite/ChangeLog:

	* gdb.mi/mi-async-run.exp: New file.
	* gdb.mi/mi-async-run.c: New file.
---
 gdb/mi/mi-main.c                      | 12 ++++++---
 gdb/testsuite/gdb.mi/mi-async-run.c   | 30 +++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-async-run.exp | 50 +++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-async-run.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-async-run.exp

diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 7cb7bf5..904cee0 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -415,6 +415,8 @@ run_one_inferior (struct inferior *inf, void *arg)
 {
   int start_p = *(int *) arg;
   const char *run_cmd = start_p ? "start" : "run";
+  struct target_ops *run_target = find_run_target ();
+  int async_p = run_target->to_can_async_p (run_target) && mi_async;
 
   if (inf->pid != 0)
     {
@@ -435,8 +437,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_cmd, mi_async_p (),
-			  mi_async_p () ? "&" : NULL);
+  mi_execute_cli_command (run_cmd, async_p,
+			  async_p ? "&" : NULL);
   return 0;
 }
 
@@ -489,9 +491,11 @@ mi_cmd_exec_run (char *command, char **argv, int argc)
   else
     {
       const char *run_cmd = start_p ? "start" : "run";
+      struct target_ops *run_target = find_run_target ();
+      int async_p = run_target->to_can_async_p (run_target) && mi_async;
 
-      mi_execute_cli_command (run_cmd, mi_async_p (),
-			      mi_async_p () ? "&" : NULL);
+      mi_execute_cli_command (run_cmd, async_p,
+			      async_p ? "&" : NULL);
     }
 }
 
diff --git a/gdb/testsuite/gdb.mi/mi-async-run.c b/gdb/testsuite/gdb.mi/mi-async-run.c
new file mode 100644
index 0000000..2910cbf
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-async-run.c
@@ -0,0 +1,30 @@
+/* Copyright 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+
+int main ()
+{
+  int i;
+
+  for (i = 0; i < 30; i++)
+    {
+      sleep (1);
+    }
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-async-run.exp b/gdb/testsuite/gdb.mi/mi-async-run.exp
new file mode 100644
index 0000000..c08a4a6
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-async-run.exp
@@ -0,0 +1,50 @@
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+
+standard_testfile
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested "mi-async-run.exp"
+     return -1
+}
+
+# Test the resolution of PR 18077
+#
+# When doing an -exec-run with a fresh copy of GDB, it would result in
+# synchronous execution, even though mi-async was on.
+
+proc test_async_run {} {
+    global GDBFLAGS
+
+    save_vars { GDBFLAGS } {
+	global binfile
+
+	set GDBFLAGS [concat $GDBFLAGS " -ex \"set mi-async on\""]
+
+	gdb_exit
+	if [mi_gdb_start] {
+	    continue
+	}
+
+	mi_gdb_load ${binfile}
+	mi_run_cmd
+	mi_gdb_test "123-exec-interrupt --all" "123\\^done" "send interrupt command"
+	mi_expect_interrupt "expect interrupt"
+    }
+}
+
+test_async_run
-- 
2.8.2

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix -exec-run not running asynchronously with mi-async on
  2016-05-07 11:24 [PATCH] Fix -exec-run not running asynchronously with mi-async on Simon Marchi
@ 2016-05-17 18:56 ` Pedro Alves
  2016-05-17 21:01   ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Pedro Alves @ 2016-05-17 18:56 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: Simon Marchi

On 05/07/2016 12:23 PM, Simon Marchi wrote:

> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 7cb7bf5..904cee0 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -415,6 +415,8 @@ run_one_inferior (struct inferior *inf, void *arg)
>  {
>    int start_p = *(int *) arg;
>    const char *run_cmd = start_p ? "start" : "run";
> +  struct target_ops *run_target = find_run_target ();
> +  int async_p = run_target->to_can_async_p (run_target) && mi_async;

It's standard practice to put the cheap check first:

  int async_p = mi_async && run_target->to_can_async_p (run_target);

I think it reads a bit more naturally, even.

> @@ -489,9 +491,11 @@ mi_cmd_exec_run (char *command, char **argv, int argc)
>    else
>      {
>        const char *run_cmd = start_p ? "start" : "run";
> +      struct target_ops *run_target = find_run_target ();
> +      int async_p = run_target->to_can_async_p (run_target) && mi_async;

Ditto.

>  
> -      mi_execute_cli_command (run_cmd, mi_async_p (),
> -			      mi_async_p () ? "&" : NULL);
> +      mi_execute_cli_command (run_cmd, async_p,
> +			      async_p ? "&" : NULL);
>      }
>  }
>  

> +int main ()

Line break after int.

> +{
> +  int i;
> +
> +  for (i = 0; i < 30; i++)
> +    {
> +      sleep (1);
> +    }
> +
> +  return 0;
> +}

Otherwise OK, master and 7.11.1.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix -exec-run not running asynchronously with mi-async on
  2016-05-17 18:56 ` Pedro Alves
@ 2016-05-17 21:01   ` Simon Marchi
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2016-05-17 21:01 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Simon Marchi

On 2016-05-17 14:56, Pedro Alves wrote:
> On 05/07/2016 12:23 PM, Simon Marchi wrote:
> 
>> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
>> index 7cb7bf5..904cee0 100644
>> --- a/gdb/mi/mi-main.c
>> +++ b/gdb/mi/mi-main.c
>> @@ -415,6 +415,8 @@ run_one_inferior (struct inferior *inf, void *arg)
>>  {
>>    int start_p = *(int *) arg;
>>    const char *run_cmd = start_p ? "start" : "run";
>> +  struct target_ops *run_target = find_run_target ();
>> +  int async_p = run_target->to_can_async_p (run_target) && mi_async;
> 
> It's standard practice to put the cheap check first:
> 
>   int async_p = mi_async && run_target->to_can_async_p (run_target);
> 
> I think it reads a bit more naturally, even.

Agreed, done.

>> @@ -489,9 +491,11 @@ mi_cmd_exec_run (char *command, char **argv, int 
>> argc)
>>    else
>>      {
>>        const char *run_cmd = start_p ? "start" : "run";
>> +      struct target_ops *run_target = find_run_target ();
>> +      int async_p = run_target->to_can_async_p (run_target) && 
>> mi_async;
> 
> Ditto.

Done.

>> 
>> -      mi_execute_cli_command (run_cmd, mi_async_p (),
>> -			      mi_async_p () ? "&" : NULL);
>> +      mi_execute_cli_command (run_cmd, async_p,
>> +			      async_p ? "&" : NULL);
>>      }
>>  }
>> 
> 
>> +int main ()
> 
> Line break after int.

Done.

>> +{
>> +  int i;
>> +
>> +  for (i = 0; i < 30; i++)
>> +    {
>> +      sleep (1);
>> +    }
>> +
>> +  return 0;
>> +}
> 
> Otherwise OK, master and 7.11.1.

Thanks, pushed to master and gdb-7.11-branch.

Simon

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-05-17 21:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-07 11:24 [PATCH] Fix -exec-run not running asynchronously with mi-async on Simon Marchi
2016-05-17 18:56 ` Pedro Alves
2016-05-17 21:01   ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).