[PATCH 1/2] Fix detach with target remote (PR gdb/28080)
Pedro Alves
pedro@palves.net
Mon Jul 12 19:33:10 GMT 2021
Commit 408f66864a1a823591b26420410c982174c239a2 ("detach in all-stop
with threads running") regressed "detach" with "target remote":
(gdb) detach
Detaching from program: target:/any/program, process 3671843
Detaching from process 3671843
Ending remote debugging.
[Inferior 1 (process 3671843) detached]
In main
terminate called after throwing an instance of 'gdb_exception_error'
Aborted (core dumped)
Here's the exception above being thrown:
(top-gdb) bt
#0 throw_error (error=TARGET_CLOSE_ERROR, fmt=0x555556035588 "Remote connection closed") at src/gdbsupport/common-exceptions.cc:222
#1 0x0000555555bbaa46 in remote_target::readchar (this=0x555556a11040, timeout=10000) at src/gdb/remote.c:9440
#2 0x0000555555bbb9e5 in remote_target::getpkt_or_notif_sane_1 (this=0x555556a11040, buf=0x555556a11058, forever=0, expecting_notif=0, is_notif=0x0) at src/gdb/remote.c:9928
#3 0x0000555555bbbda9 in remote_target::getpkt_sane (this=0x555556a11040, buf=0x555556a11058, forever=0) at src/gdb/remote.c:10030
#4 0x0000555555bc0e75 in remote_target::remote_hostio_send_command (this=0x555556a11040, command_bytes=13, which_packet=14, remote_errno=0x7fffffffcfd0, attachment=0x0, attachment_len=0x0) at src/gdb/remote.c:12137
#5 0x0000555555bc1b6c in remote_target::remote_hostio_close (this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12455
#6 0x0000555555bc1bb4 in remote_target::fileio_close (During symbol reading: .debug_line address at offset 0x64f417 is 0 [in module build/gdb/gdb]
this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12462
#7 0x0000555555c9274c in target_fileio_close (fd=3, target_errno=0x7fffffffcfd0) at src/gdb/target.c:3365
#8 0x000055555595a19d in gdb_bfd_iovec_fileio_close (abfd=0x555556b9f8a0, stream=0x555556b11530) at src/gdb/gdb_bfd.c:439
#9 0x0000555555e09e3f in opncls_bclose (abfd=0x555556b9f8a0) at src/bfd/opncls.c:599
#10 0x0000555555e0a2c7 in bfd_close_all_done (abfd=0x555556b9f8a0) at src/bfd/opncls.c:847
#11 0x0000555555e0a27a in bfd_close (abfd=0x555556b9f8a0) at src/bfd/opncls.c:814
#12 0x000055555595a9d3 in gdb_bfd_close_or_warn (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:626
#13 0x000055555595ad29 in gdb_bfd_unref (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:715
#14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573
#15 0x0000555555ae955a in std::_Sp_counted_ptr<objfile*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:377
#16 0x000055555572b7c8 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:155
#17 0x00005555557263c3 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x555556bf0588, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:730
#18 0x0000555555ae745e in std::__shared_ptr<objfile, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:1169
#19 0x0000555555ae747e in std::shared_ptr<objfile>::~shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr.h:103
#20 0x0000555555b1c1dc in __gnu_cxx::new_allocator<std::_List_node<std::shared_ptr<objfile> > >::destroy<std::shared_ptr<objfile> > (this=0x5555564cdd60, __p=0x555556bf0580) at /usr/include/c++/9/ext/new_allocator.h:153
#21 0x0000555555b1bb1d in std::allocator_traits<std::allocator<std::_List_node<std::shared_ptr<objfile> > > >::destroy<std::shared_ptr<objfile> > (__a=..., __p=0x555556bf0580) at /usr/include/c++/9/bits/alloc_traits.h:497
#22 0x0000555555b1b73e in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/stl_list.h:1921
#23 0x0000555555b1afeb in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/list.tcc:158
#24 0x0000555555b19576 in program_space::remove_objfile (this=0x5555564cdd20, objfile=0x555556515540) at src/gdb/progspace.c:210
#25 0x0000555555ae4502 in objfile::unlink (this=0x555556515540) at src/gdb/objfiles.c:487
#26 0x0000555555ae5a12 in objfile_purge_solibs () at src/gdb/objfiles.c:875
#27 0x0000555555c09686 in no_shared_libraries (ignored=0x0, from_tty=1) at src/gdb/solib.c:1236
#28 0x00005555559e3f5f in detach_command (args=0x0, from_tty=1) at src/gdb/infcmd.c:2769
So frame #28 already detached the remote process, and then we're
purging the shared libraries. GDB had opened remote shared libraries
via the target: sysroot, so it tries closing them. GDBserver is
tearing down already, so remote communication breaks down and we close
the remote target and throw TARGET_CLOSE_ERROR.
Note frame #14:
#14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573
That's a dtor, thus noexcept. That's the reason for the
std::terminate.
Stepping back a bit, why do we still have open remote files if we've
managed to detach already, and, we're debugging with "target remote"?
The reason is that commit 408f66864a1a823591b26420410c982174c239a2
makes detach_command hold a reference to the target, so the remote
target won't be finally closed until frame #28 returns. It's closing
the target that invalidates target file I/O handles.
This commit fixes the issue by not relying on target_close to
invalidate the target file I/O handles, instead invalidate them
immediately in remote_unpush_target. So when GDB purges the solibs,
and we end up in target_fileio_close (frame #7 above), there's nothing
to do, and we don't try to talk with the remote target anymore.
The regression isn't seem when testing with
--target_board=native-gdbserver, because that does "set sysroot" to
disable the "target:" sysroot, for test run speed reasons. So this
commit adds a testcase that explicitly tests detach with "set sysroot
target:".
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/28080
* remote.c (remote_unpush_target): Invalidate file I/O target
handles.
* target.c (fileio_handles_invalidate_target): Make extern.
* target.h (fileio_handles_invalidate_target): Declare.
gdb/testsuite/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/28080
* gdb.base/detach-sysroot-target.exp: New.
* gdb.base/detach-sysroot-target.c: New.
Reported-By: Jonah Graham <jonah@kichwacoders.com>
Change-Id: I851234910172f42a1b30e731161376c344d2727d
---
gdb/remote.c | 9 ++++
gdb/target.c | 8 +--
gdb/target.h | 6 +++
.../gdb.base/detach-sysroot-target.c | 22 ++++++++
.../gdb.base/detach-sysroot-target.exp | 53 +++++++++++++++++++
5 files changed, 92 insertions(+), 6 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/detach-sysroot-target.c
create mode 100644 gdb/testsuite/gdb.base/detach-sysroot-target.exp
diff --git a/gdb/remote.c b/gdb/remote.c
index adc53e324d0..f2271ad3b50 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5624,6 +5624,15 @@ remote_unpush_target (remote_target *target)
pop_all_targets_at_and_above (process_stratum);
generic_mourn_inferior ();
}
+
+ /* Don't rely on target_close doing this when the target is popped
+ from the last remote inferior above, because something may be
+ holding a reference to the target higher up on the stack, meaning
+ target_close won't be called yet. We lost the connection to the
+ target, so clear these now, otherwise we may later throw
+ TARGET_CLOSE_ERROR while trying to tell the remote target to
+ close the file. */
+ fileio_handles_invalidate_target (target);
}
static void
diff --git a/gdb/target.c b/gdb/target.c
index 767685fbca3..78752898191 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3121,13 +3121,9 @@ static std::vector<fileio_fh_t> fileio_fhandles;
list each time a new file is opened. */
static int lowest_closed_fd;
-/* Invalidate the target associated with open handles that were open
- on target TARG, since we're about to close (and maybe destroy) the
- target. The handles remain open from the client's perspective, but
- trying to do anything with them other than closing them will fail
- with EIO. */
+/* See target.h. */
-static void
+void
fileio_handles_invalidate_target (target_ops *targ)
{
for (fileio_fh_t &fh : fileio_fhandles)
diff --git a/gdb/target.h b/gdb/target.h
index e22f9038197..ddd4f3803cb 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2230,6 +2230,12 @@ extern LONGEST target_fileio_read_alloc (struct inferior *inf,
extern gdb::unique_xmalloc_ptr<char> target_fileio_read_stralloc
(struct inferior *inf, const char *filename);
+/* Invalidate the target associated with open handles that were open
+ on target TARG, since we're about to close (and maybe destroy) the
+ target. The handles remain open from the client's perspective, but
+ trying to do anything with them other than closing them will fail
+ with EIO. */
+extern void fileio_handles_invalidate_target (target_ops *targ);
/* Tracepoint-related operations. */
diff --git a/gdb/testsuite/gdb.base/detach-sysroot-target.c b/gdb/testsuite/gdb.base/detach-sysroot-target.c
new file mode 100644
index 00000000000..9811b15f06d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/detach-sysroot-target.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2021 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 ()
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/detach-sysroot-target.exp b/gdb/testsuite/gdb.base/detach-sysroot-target.exp
new file mode 100644
index 00000000000..5d9400f5648
--- /dev/null
+++ b/gdb/testsuite/gdb.base/detach-sysroot-target.exp
@@ -0,0 +1,53 @@
+# Copyright 2021 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 running a program from the GDB prompt and then detaching it,
+# with "set sysroot target:". Regression test for PR gdb/28080.
+#
+# It is assumed that it is only safe to tweak the sysroot on the
+# current board if it is currently either empty or already "target:".
+# If set to anything else, we skip the test.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" ${binfile} ${srcfile}]} {
+ return
+}
+
+gdb_test_multiple "show sysroot" "" {
+ -wrap -re "The current system root is \"\"\\." {
+ # Explicitly set target: sysroot.
+ gdb_test_no_output "set sysroot target:"
+ }
+ -wrap -re "The current system root is \"target:\"\\." {
+ # Nothing to do.
+ }
+ -re "$gdb_prompt $" {
+ # If testing with any other sysroot, we probably should not
+ # mess with it.
+ unsupported "sysroot is set"
+ return
+ }
+}
+
+if ![runto_main] {
+ fail "couldn't run to main"
+ return
+}
+
+# With PR gdb/28080, this would crash GDB when testing with "target
+# remote".
+set escapedbinfile [string_to_regexp ${binfile}]
+gdb_test "detach" "Detaching from program: .*$escapedbinfile, .*"
--
2.26.2
More information about the Gdb-patches
mailing list