]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdb: don't pass TARGET_WNOHANG to targets that can't async (PR 26642)
authorSimon Marchi <simon.marchi@polymtl.ca>
Tue, 13 Oct 2020 16:01:19 +0000 (12:01 -0400)
committerSimon Marchi <simon.marchi@polymtl.ca>
Tue, 13 Oct 2020 18:28:13 +0000 (14:28 -0400)
Debugging with "maintenance set target-async off" on Linux has been
broken since 5b6d1e4fa4f ("Multi-target support").

The issue is easy to reproduce:

    $ ./gdb -q --data-directory=data-directory -nx ./test
    Reading symbols from ./test...
    (gdb) maintenance set target-async off
    (gdb) start
    Temporary breakpoint 1 at 0x1151: file test.c, line 5.
    Starting program: /home/simark/build/binutils-gdb/gdb/test

... and it hangs there.

The difference between pre-5b6d1e4fa4f and 5b6d1e4fa4f is that
fetch_inferior_event now calls target_wait with TARGET_WNOHANG for
non-async-capable targets, whereas it didn't before.

For non-async-capable targets, this is how it's expected to work when
resuming execution:

1. we call resume
2. the infrun async handler is marked in prepare_to_wait, to immediately
   wake up the event loop when we get back to it
3. fetch_inferior_event calls the target's wait method without
   TARGET_WNOHANG, effectively blocking until the target has something
   to report

However, since we call the target's wait method with TARGET_WNOHANG,
this happens:

1. we call resume
2. the infrun async handler is marked in prepare_to_wait, to immediately
   wake up the event loop when we get back to it
3. fetch_inferior_event calls the target's wait method with
   TARGET_WNOHANG, the target has nothing to report yet
4. we go back to blocking on the event loop
5. SIGCHLD finally arrives, but the event loop is not woken up, because
   we are not in async mode.  Normally, we should have been stuck in
   waitpid the SIGCHLD would have unblocked us.

We end up in this situation because these two necessary conditions are
met:

1. GDB uses the TARGET_WNOHANG option with a target that can't do async.
   I don't think this makes sense.  I mean, it's technically possible,
   the doc for TARGET_WNOHANG is:

  /* Return immediately if there's no event already queued.  If this
     options is not requested, target_wait blocks waiting for an
     event.  */
  TARGET_WNOHANG = 1,

   ... which isn't in itself necessarily incompatible with synchronous
   targets.  It could be possible for a target to support non-blocking
   polls, while not having a way to asynchronously wake up the event
   loop, which is also necessary to support async.  But as of today,
   we don't expect GDB and sync targets to work this way.

2. The linux-nat target, even in the mode where it emulates a
   synchronous target (with "maintenance set target-async off") respects
   TARGET_WNOHANG.  Other non-async targets, such as windows_nat_target,
   simply don't check / support TARGET_WNOHANG, so their wait method is
   always blocking.

Fix the first issue by avoiding using TARGET_WNOHANG on non-async
targets, in do_target_wait_1.  Add an assert in target_wait to verify it
doesn't happen.

The new test gdb.base/maint-target-async-off.exp is a simple test that
just tries running to main and then to the end of the program, with
"maintenance set target-async off".

gdb/ChangeLog:

PR gdb/26642
* infrun.c (do_target_wait_1): Clear TARGET_WNOHANG if the
target can't do async.
* target.c (target_wait): Assert that we don't pass
TARGET_WNOHANG to a target that can't async.

gdb/testsuite/ChangeLog:

PR gdb/26642
* gdb.base/maint-target-async-off.c: New test.
* gdb.base/maint-target-async-off.exp: New test.

Change-Id: I69ad3a14598863d21338a8c4e78700a58ce7ad86

gdb/ChangeLog
gdb/infrun.c
gdb/target.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.base/maint-target-async-off.c [new file with mode: 0644]
gdb/testsuite/gdb.base/maint-target-async-off.exp [new file with mode: 0644]

index 5522cd7b9d3ed1f2c79808ef59a17d67a0584d43..4eed6920ae5cbb8536150e00ac8b5c75bbf84631 100644 (file)
@@ -1,3 +1,11 @@
+2020-10-13  Simon Marchi  <simon.marchi@polymtl.ca>
+
+       PR gdb/26642
+       * infrun.c (do_target_wait_1): Clear TARGET_WNOHANG if the
+       target can't do async.
+       * target.c (target_wait): Assert that we don't pass
+       TARGET_WNOHANG to a target that can't async.
+
 2020-10-08  Shahab Vahedi  <shahab@synopsys.com>
 
        * NEWS: Mention ARC support in GDBserver.
index 780d5bc791e368ad1a5c925ed51e7c2b3f1f6377..4c2e1f56d243f560b613d2017a1ad8ea965a36b5 100644 (file)
@@ -3533,6 +3533,11 @@ do_target_wait_1 (inferior *inf, ptid_t ptid,
 
   /* But if we don't find one, we'll have to wait.  */
 
+  /* We can't ask a non-async target to do a non-blocking wait, so this will be
+     a blocking wait.  */
+  if (!target_can_async_p ())
+    options &= ~TARGET_WNOHANG;
+
   if (deprecated_target_wait_hook)
     event_ptid = deprecated_target_wait_hook (ptid, status, options);
   else
index 58189e6202442d85e24d93e1b809aa50b2800b60..6b299a973a50ce6a929fa08dd41abcffb5d65239 100644 (file)
@@ -2009,7 +2009,12 @@ target_disconnect (const char *args, int from_tty)
 ptid_t
 target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
 {
-  return current_top_target ()->wait (ptid, status, options);
+  target_ops *target = current_top_target ();
+
+  if (!target->can_async_p ())
+    gdb_assert ((options & TARGET_WNOHANG) == 0);
+
+  return target->wait (ptid, status, options);
 }
 
 /* See target.h.  */
index 5ef2bf3ea138770defce4ee63aeaf66084e1ec45..99fc5df07c12d896dde71d46e8aff3223a50f4dc 100644 (file)
@@ -1,3 +1,9 @@
+2020-10-13  Simon Marchi  <simon.marchi@polymtl.ca>
+
+       PR gdb/26642
+       * gdb.base/maint-target-async-off.c: New test.
+       * gdb.base/maint-target-async-off.exp: New test.
+
 2020-09-28  Gareth Rees <grees@undo.io>  (tiny change)
 
        PR python/26586
diff --git a/gdb/testsuite/gdb.base/maint-target-async-off.c b/gdb/testsuite/gdb.base/maint-target-async-off.c
new file mode 100644 (file)
index 0000000..9d7b2f1
--- /dev/null
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 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
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/maint-target-async-off.exp b/gdb/testsuite/gdb.base/maint-target-async-off.exp
new file mode 100644 (file)
index 0000000..0b2f4a3
--- /dev/null
@@ -0,0 +1,41 @@
+# Copyright 2020 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/>.
+
+# Verify that debugging with "maintenance target-async off" works somewhat.  At
+# least running to main and to the end of the program.
+
+standard_testfile
+
+save_vars { GDBFLAGS } {
+    # Enable target-async off this way, because with board
+    # native-extended-gdbserver, the remote target is already open when
+    # returning from prepare_for_testing, and that would be too late to toggle
+    # it.
+    append GDBFLAGS { -ex "maintenance set target-async off" }
+
+    if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+       return
+    }
+}
+
+# Make sure our command-line flag worked.
+gdb_test "maintenance show target-async" "Controlling the inferior in asynchronous mode is off\\."
+
+if { ![runto_main] } {
+    fail "can't run to main"
+    return
+}
+
+gdb_continue_to_end