Re: [RFC v5 5/8] make dprintf.exp pass in always-async mode

[Pedro Alves <palves@redhat.com>]

On 03/04/2014 06:32 PM, Tom Tromey wrote:
> 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.
> 
> This patch also adds a new test case, so that regression testing isn't
> dependent on the implementation of dprintf.

...

> diff --git a/gdb/testsuite/gdb.base/async-cond.exp b/gdb/testsuite/gdb.base/async-cond.exp
> new file mode 100644

...

> +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
> +    return -1
> +}
> +
> +gdb_test_no_output "set target-async on"

As discussed, I don't think we should really have these knobs.  The
test should pass in sync targets just as well.  I think it's best
to not force async, and test the whole testsuite with a board
that forces async mode (or hack gdb) to exercise this in async,
just doing that caught the issue in dprintf.exp.

Given that reasoning, I've renamed the test to condbreak-$foo.exp,
for similarity with condbreak.exp.  This was after noticing
the existence of condbreak.exp, and trying a little to exercise this
problem in that test, and giving
up -- break.c/break1.c/condbreak.exp/break.exp are just too ugly and
messy.

> +gdb_test "break g if zero()" "Breakpoint .*"
> +gdb_test "run" "Inferior.*exited.*"

Note that gdb_test "run" doesn't work with "target remote".

Running ../../../src/gdb/testsuite/gdb.base/async-cond.exp ...
ERROR: tcl error sourcing ../../../src/gdb/testsuite/gdb.base/async-cond.exp.
ERROR: gdbserver does not support run without extended-remote
    while executing
"error "gdbserver does not support $command without extended-remote""
    (procedure "gdb_test_multiple" line 23)
    invoked from within
"gdb_test_multiple $command $message {
        -re "\[\r\n\]*($pattern)\[\r\n\]+$gdb_prompt $" {
            if ![string match "" $message] then {
                pass "$message"
..."

I also extended the commit log description some.

Here's what I pushed.

-------
Subject: [PATCH] make dprintf.exp pass in target async mode

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

 Running ../../../src/gdb/testsuite/gdb.base/dprintf.exp ...
 FAIL: gdb.base/dprintf.exp: 1st dprintf, call
 FAIL: gdb.base/dprintf.exp: 2nd dprintf, call
 FAIL: gdb.base/dprintf.exp: Set dprintf function
 FAIL: gdb.base/dprintf.exp: 1st dprintf, fprintf
 FAIL: gdb.base/dprintf.exp: 2nd dprintf, fprintf

 Breakpoint 2, main (argc=1, argv=0x7fffffffd3f8) at ../../../src/gdb/testsuite/gdb.base/dprintf.c:33
 33        int loc = 1234;
 (gdb) continue
 Continuing.
 kickoff 1234
 also to stderr 1234
 At foo entry
 (gdb) FAIL: gdb.base/dprintf.exp: 1st dprintf, call

The problem is that GDB gave the prompt back to the user too early.

This happens when calling functions while handling an event that
doesn't cause a user visible stop.  dprintf with "set dprintf-style
gdb" is one such case.  This patch adds a test case that has a
breakpoint with a condition that calls a function that returns false,
so that regression testing isn't dependent on the implementation of
dprintf.

The problem happens because run_inferior_call causes GDB to forget
that it is running in sync_execution mode, so any event that runs an
inferior call causes fetch_inferior_event to display the prompt, even
if the event should not result in a user visible stop (that is, gdb
resumes the inferior and waits for the next event).

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

gdb/
2014-03-20  Tom Tromey  <tromey@redhat.com>

	PR cli/15718
	* infcall.c: Include event-top.h.
	(run_inferior_call): Call async_disable_stdin if needed.

gdb/testsuite/
2014-03-20  Tom Tromey  <tromey@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	PR cli/15718
	* gdb.base/condbreak-call-false.c: New file.
	* gdb.base/condbreak-call-false.exp: New file.
---
 gdb/ChangeLog                                   |  6 ++++
 gdb/infcall.c                                   | 10 +++++++
 gdb/testsuite/ChangeLog                         |  7 +++++
 gdb/testsuite/gdb.base/condbreak-call-false.c   | 39 +++++++++++++++++++++++++
 gdb/testsuite/gdb.base/condbreak-call-false.exp | 33 +++++++++++++++++++++
 5 files changed, 95 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/condbreak-call-false.c
 create mode 100644 gdb/testsuite/gdb.base/condbreak-call-false.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 814be2c..acb25f8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2014-03-20  Tom Tromey  <tromey@redhat.com>
+
+	PR cli/15718
+	* infcall.c: Include event-top.h.
+	(run_inferior_call): Call async_disable_stdin if needed.
+
 2014-03-20  Pedro Alves  <palves@redhat.com>
 
 	* infrun.c (prepare_to_proceed): Delete.
diff --git a/gdb/infcall.c b/gdb/infcall.c
index ad18ff1..9907263 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,13 @@ 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.  In other
+	     cases, stdin will be re-enabled by
+	     inferior_event_handler, when an exception is thrown.  */
+	  if (was_sync)
+	    async_disable_stdin ();
 	}
     }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 31e0e5d..d6fb1dc 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2014-03-20  Tom Tromey  <tromey@redhat.com>
+	    Pedro Alves  <palves@redhat.com>
+
+	PR cli/15718
+	* gdb.base/condbreak-call-false.c: New file.
+	* gdb.base/condbreak-call-false.exp: New file.
+
 2014-03-20  Pedro Alves  <palves@redhat.com>
 
 	* gdb.threads/signal-while-stepping-over-bp-other-thread.c (pid):
diff --git a/gdb/testsuite/gdb.base/condbreak-call-false.c b/gdb/testsuite/gdb.base/condbreak-call-false.c
new file mode 100644
index 0000000..ed9a3f4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-call-false.c
@@ -0,0 +1,39 @@
+/* Copyright 2013-2014 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/>.  */
+
+int
+zero (void)
+{
+  return 0;
+}
+
+int
+foo (void)
+{
+  return 23;
+}
+
+void
+bar (void)
+{
+}
+
+
+int
+main (void)
+{
+  foo ();
+  bar ();
+}
diff --git a/gdb/testsuite/gdb.base/condbreak-call-false.exp b/gdb/testsuite/gdb.base/condbreak-call-false.exp
new file mode 100644
index 0000000..d1cb2bb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/condbreak-call-false.exp
@@ -0,0 +1,33 @@
+# Copyright 2013-2014 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/>.
+
+# Test that a breakpoint condition with a function call that returns
+# false works as expected.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+gdb_test "break foo if zero()" "Breakpoint .*"
+gdb_test "break bar" "Breakpoint .*"
+
+gdb_test "c" "Breakpoint .* bar .*"
-- 
1.7.11.7