* [PATCH 0/2] Fix detach with target remote (PR gdb/28080)
@ 2021-07-12 19:33 Pedro Alves
2021-07-12 19:33 ` [PATCH 1/2] " Pedro Alves
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Pedro Alves @ 2021-07-12 19:33 UTC (permalink / raw)
To: gdb-patches
This series fixes PR gdb/28080 - "detach causes core dump", a
regression compared to GDB 10.
The first patch fixes the regression, and then the second patch makes
sure we don't abort in other similar situations.
This is aimed at both master and the gdb 11 branch.
Pedro Alves (2):
Fix detach with target remote (PR gdb/28080)
Avoid letting exceptions escape gdb_bfd_iovec_fileio_close (PR
gdb/28080)
gdb/gdb_bfd.c | 10 +++-
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 +++++++++++++++++++
6 files changed, 101 insertions(+), 7 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/detach-sysroot-target.c
create mode 100644 gdb/testsuite/gdb.base/detach-sysroot-target.exp
base-commit: 6bbe1a929c69e141e63265c7013fcf7f80bc792e
--
2.26.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] Fix detach with target remote (PR gdb/28080)
2021-07-12 19:33 [PATCH 0/2] Fix detach with target remote (PR gdb/28080) Pedro Alves
@ 2021-07-12 19:33 ` Pedro Alves
2021-07-13 1:07 ` Simon Marchi
2021-07-12 19:33 ` [PATCH 2/2] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close " Pedro Alves
2021-07-12 22:24 ` [PATCH 0/2] Fix detach with target remote " Jonah Graham
2 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2021-07-12 19:33 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close (PR gdb/28080)
2021-07-12 19:33 [PATCH 0/2] Fix detach with target remote (PR gdb/28080) Pedro Alves
2021-07-12 19:33 ` [PATCH 1/2] " Pedro Alves
@ 2021-07-12 19:33 ` Pedro Alves
2021-07-13 1:19 ` Simon Marchi
2021-07-12 22:24 ` [PATCH 0/2] Fix detach with target remote " Jonah Graham
2 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2021-07-12 19:33 UTC (permalink / raw)
To: gdb-patches
Before PR gdb/28080 was fixed by the previous patch, GDB was crashing
like this:
(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
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.
The previous patch fixed things such that the exception above isn't
thrown anymore. However, it's possible that e.g., the remote
connection drops just while a user types "nosharedlibrary", or some
other reason that leads to objfile::~objfile, and then we end up the
same std::terminate problem.
Also notice that frames #9-#11 are BFD frames:
#9 0x0000555555e09e3f in opncls_bclose (abfd=0x555556bc27e0) at src/bfd/opncls.c:599
#10 0x0000555555e0a2c7 in bfd_close_all_done (abfd=0x555556bc27e0) at src/bfd/opncls.c:847
#11 0x0000555555e0a27a in bfd_close (abfd=0x555556bc27e0) at src/bfd/opncls.c:814
BFD is written in C and thus throwing exceptions over such frames may
either not clean up properly, or, may abort if bfd is not compiled
with -fasynchronous-unwind-tables (x86-64 defaults that on, but not
all GCC ports do).
Thus frame #8 sees like a good place to swallow exceptions. More so
since in this spot we already close returning error. That's what this
commit does. Without the previous fix, we'd see:
(gdb) detach
Detaching from program: target:/any/program, process 2197701
Ending remote debugging.
[Inferior 1 (process 2197701) detached]
warning: while closing file: Remote connection closed
Note it prints a warning, which would still be a regression compared
to GDB 10, if it weren't for the previous fix.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/28080
* gdb_bfd.c (gdb_bfd_iovec_fileio_close): Wrap target_fileio_close
in try/catch and print warning on exception.
Change-Id: Ic7a26ddba0a4444e3377b0e7c1c89934a84545d7
---
gdb/gdb_bfd.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 3312197acd5..267a53a0edc 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -436,7 +436,15 @@ gdb_bfd_iovec_fileio_close (struct bfd *abfd, void *stream)
/* Ignore errors on close. These may happen with remote
targets if the connection has already been torn down. */
- target_fileio_close (fd, &target_errno);
+ try
+ {
+ target_fileio_close (fd, &target_errno);
+ }
+ catch (const gdb_exception &ex)
+ {
+ /* Also avoid crossing exceptions over bfd. */
+ warning (_("while closing file: %s"), ex.message->c_str ());
+ }
/* Zero means success. */
return 0;
--
2.26.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Fix detach with target remote (PR gdb/28080)
2021-07-12 19:33 [PATCH 0/2] Fix detach with target remote (PR gdb/28080) Pedro Alves
2021-07-12 19:33 ` [PATCH 1/2] " Pedro Alves
2021-07-12 19:33 ` [PATCH 2/2] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close " Pedro Alves
@ 2021-07-12 22:24 ` Jonah Graham
2 siblings, 0 replies; 9+ messages in thread
From: Jonah Graham @ 2021-07-12 22:24 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Mon, 12 Jul 2021 at 15:33, Pedro Alves <pedro@palves.net> wrote:
> This series fixes PR gdb/28080 - "detach causes core dump", a
> regression compared to GDB 10.
>
>
>
Thanks Pedro, this series fixes the issue I encountered and the Eclipse CDT
test suite now passes.
Jonah
~~~
Jonah Graham
Kichwa Coders
www.kichwacoders.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Fix detach with target remote (PR gdb/28080)
2021-07-12 19:33 ` [PATCH 1/2] " Pedro Alves
@ 2021-07-13 1:07 ` Simon Marchi
0 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2021-07-13 1:07 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-12 3:33 p.m., Pedro Alves wrote:
> 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
seem -> seen
Otherwise, LGTM.
Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close (PR gdb/28080)
2021-07-12 19:33 ` [PATCH 2/2] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close " Pedro Alves
@ 2021-07-13 1:19 ` Simon Marchi
2021-07-13 12:16 ` Pedro Alves
0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2021-07-13 1:19 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-12 3:33 p.m., Pedro Alves wrote:
> Before PR gdb/28080 was fixed by the previous patch, GDB was crashing
> like this:
>
> (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
>
> 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.
>
> The previous patch fixed things such that the exception above isn't
> thrown anymore. However, it's possible that e.g., the remote
> connection drops just while a user types "nosharedlibrary", or some
> other reason that leads to objfile::~objfile, and then we end up the
> same std::terminate problem.
>
> Also notice that frames #9-#11 are BFD frames:
>
> #9 0x0000555555e09e3f in opncls_bclose (abfd=0x555556bc27e0) at src/bfd/opncls.c:599
> #10 0x0000555555e0a2c7 in bfd_close_all_done (abfd=0x555556bc27e0) at src/bfd/opncls.c:847
> #11 0x0000555555e0a27a in bfd_close (abfd=0x555556bc27e0) at src/bfd/opncls.c:814
>
> BFD is written in C and thus throwing exceptions over such frames may
> either not clean up properly, or, may abort if bfd is not compiled
> with -fasynchronous-unwind-tables (x86-64 defaults that on, but not
Weird characters on the line above, just before fasynchronous.
> all GCC ports do).
>
> Thus frame #8 sees like a good place to swallow exceptions. More so
sees -> seems
> since in this spot we already close returning error. That's what this
I don't understand this sentence, "in this spot we already close
returning error".
> commit does. Without the previous fix, we'd see:
>
> (gdb) detach
> Detaching from program: target:/any/program, process 2197701
> Ending remote debugging.
> [Inferior 1 (process 2197701) detached]
> warning: while closing file: Remote connection closed
>
> Note it prints a warning, which would still be a regression compared
> to GDB 10, if it weren't for the previous fix.
Maybe the more correct thing to do would be to make
gdb_bfd_iovec_fileio_close return an error when it can't properly close
the file. That error would be communicated as a bfd return status to
gdb_bfd_close_or_warn, which would warn.
If we need to catch exceptions for the close operations, I suppose we
need to do that as well for the other BFD iovec operations: open, pread
and stat? A remote communication error could happen in any of those.
Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close (PR gdb/28080)
2021-07-13 1:19 ` Simon Marchi
@ 2021-07-13 12:16 ` Pedro Alves
2021-07-13 13:13 ` Simon Marchi
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2021-07-13 12:16 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 2021-07-13 2:19 a.m., Simon Marchi wrote:
>
> On 2021-07-12 3:33 p.m., Pedro Alves wrote:
>> BFD is written in C and thus throwing exceptions over such frames may
>> either not clean up properly, or, may abort if bfd is not compiled
>> with -fasynchronous-unwind-tables (x86-64 defaults that on, but not
>
> Weird characters on the line above, just before fasynchronous.
Curious, must be the result of copy/paste from the browser, for I googled the
correct spelling. They're not visible in emacs nor my Thunderbird, nor in the
archives:
https://sourceware.org/pipermail/gdb-patches/2021-July/180849.html
but I see them in the raw email source. Fixed.
>
>> all GCC ports do).
>>
>> Thus frame #8 sees like a good place to swallow exceptions. More so
>
> sees -> seems
Fixed.
>
>> since in this spot we already close returning error. That's what this
>
> I don't understand this sentence, "in this spot we already close
> returning error".
That's because the "ignore" word is missing... :-P
I've tweaked it to:
Thus frame #8 seems like a good place to swallow exceptions. More so
since in this spot we already ignore target_fileio_close return
errors. That's what this commit does. Without the previous fix, we'd
see:
>
>> commit does. Without the previous fix, we'd see:
>>
>> (gdb) detach
>> Detaching from program: target:/any/program, process 2197701
>> Ending remote debugging.
>> [Inferior 1 (process 2197701) detached]
>> warning: while closing file: Remote connection closed
>>
>> Note it prints a warning, which would still be a regression compared
>> to GDB 10, if it weren't for the previous fix.
>
> Maybe the more correct thing to do would be to make
> gdb_bfd_iovec_fileio_close return an error when it can't properly close
> the file. That error would be communicated as a bfd return status to
> gdb_bfd_close_or_warn, which would warn.
The problem with that is that it would require losing the exception
message, because the warning gdb_bfd_close_or_warn prints is just the
text version of a bfd error:
if (!ret)
warning (_("cannot close \"%s\": %s"),
name, bfd_errmsg (bfd_get_error ()));
Maybe we could add a new type of bfd error, a custom error, where the error
reason is given by callback (which gdb would install) or a string or some such, so
we could pass the gdb error through bfd. Not that different from how
bfd_error_on_input is a special error code too. But I'd rather not do this now.
I've tweaked the patch to ensure that the new warning looks exactly like the
one in gdb_bfd_close_or_warn, though. See below.
>
> If we need to catch exceptions for the close operations, I suppose we
> need to do that as well for the other BFD iovec operations: open, pread
> and stat? A remote communication error could happen in any of those.
Yes, though I don't think they're as likely to be in dtor/noexcept paths,
and it isn't as obvious to me what the return code should be. With close,
there's not much else we can do other than ignore the error. I'd rather
not touch them for now, for time constraint reasons. :-/
If you'd prefer, I can drop this patch for now. The issue addressed by this
patch isn't a regression.
From 56f2e825c7800eb226a916bc33e3948e2c02fa27 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Mon, 12 Jul 2021 18:03:22 +0100
Subject: [PATCH] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close
(PR gdb/28080)
Before PR gdb/28080 was fixed by the previous patch, GDB was crashing
like this:
(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
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.
The previous patch fixed things such that the exception above isn't
thrown anymore. However, it's possible that e.g., the remote
connection drops just while a user types "nosharedlibrary", or some
other reason that leads to objfile::~objfile, and then we end up the
same std::terminate problem.
Also notice that frames #9-#11 are BFD frames:
#9 0x0000555555e09e3f in opncls_bclose (abfd=0x555556bc27e0) at src/bfd/opncls.c:599
#10 0x0000555555e0a2c7 in bfd_close_all_done (abfd=0x555556bc27e0) at src/bfd/opncls.c:847
#11 0x0000555555e0a27a in bfd_close (abfd=0x555556bc27e0) at src/bfd/opncls.c:814
BFD is written in C and thus throwing exceptions over such frames may
either not clean up properly, or, may abort if bfd is not compiled
with -fasynchronous-unwind-tables (x86-64 defaults that on, but not
all GCC ports do).
Thus frame #8 seems like a good place to swallow exceptions. More so
since in this spot we already ignore target_fileio_close return
errors. That's what this commit does. Without the previous fix, we'd
see:
(gdb) detach
Detaching from program: target:/any/program, process 2197701
Ending remote debugging.
[Inferior 1 (process 2197701) detached]
warning: cannot close "target:/lib64/ld-linux-x86-64.so.2": Remote connection closed
Note it prints a warning, which would still be a regression compared
to GDB 10, if it weren't for the previous fix.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/28080
* gdb_bfd.c (gdb_bfd_close_warning): New.
(gdb_bfd_iovec_fileio_close): Wrap target_fileio_close in
try/catch and print warning on exception.
(gdb_bfd_close_or_warn): Use gdb_bfd_close_warning.
Change-Id: Ic7a26ddba0a4444e3377b0e7c1c89934a84545d7
---
gdb/gdb_bfd.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 3312197acd5..312442a466e 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -423,6 +423,15 @@ gdb_bfd_iovec_fileio_pread (struct bfd *abfd, void *stream, void *buf,
return pos;
}
+/* Warn that it wasn't possible to close a bfd for file NAME, because
+ of REASON. */
+
+static void
+gdb_bfd_close_warning (const char *name, const char *reason)
+{
+ warning (_("cannot close \"%s\": %s"), name, reason);
+}
+
/* Wrapper for target_fileio_close suitable for passing as the
CLOSE_FUNC argument to gdb_bfd_openr_iovec. */
@@ -436,7 +445,16 @@ gdb_bfd_iovec_fileio_close (struct bfd *abfd, void *stream)
/* Ignore errors on close. These may happen with remote
targets if the connection has already been torn down. */
- target_fileio_close (fd, &target_errno);
+ try
+ {
+ target_fileio_close (fd, &target_errno);
+ }
+ catch (const gdb_exception &ex)
+ {
+ /* Also avoid crossing exceptions over bfd. */
+ gdb_bfd_close_warning (bfd_get_filename (abfd),
+ ex.message->c_str ());
+ }
/* Zero means success. */
return 0;
@@ -626,8 +644,8 @@ gdb_bfd_close_or_warn (struct bfd *abfd)
ret = bfd_close (abfd);
if (!ret)
- warning (_("cannot close \"%s\": %s"),
- name, bfd_errmsg (bfd_get_error ()));
+ gdb_bfd_close_warning (name,
+ bfd_errmsg (bfd_get_error ()));
return ret;
}
base-commit: 6bbe1a929c69e141e63265c7013fcf7f80bc792e
--
2.26.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close (PR gdb/28080)
2021-07-13 12:16 ` Pedro Alves
@ 2021-07-13 13:13 ` Simon Marchi
2021-07-13 13:25 ` Pedro Alves
0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2021-07-13 13:13 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
>>> since in this spot we already close returning error. That's what this
>>
>> I don't understand this sentence, "in this spot we already close
>> returning error".
>
> That's because the "ignore" word is missing... :-P
>
> I've tweaked it to:
>
> Thus frame #8 seems like a good place to swallow exceptions. More so
> since in this spot we already ignore target_fileio_close return
> errors. That's what this commit does. Without the previous fix, we'd
> see:
Ah, I see. Well then it sounds like if target_fileio_close returns an
error, that error should be returned by gdb_bfd_iovec_fileio_close as
well (eventually, not in this patch).
>> Maybe the more correct thing to do would be to make
>> gdb_bfd_iovec_fileio_close return an error when it can't properly close
>> the file. That error would be communicated as a bfd return status to
>> gdb_bfd_close_or_warn, which would warn.
>
> The problem with that is that it would require losing the exception
> message, because the warning gdb_bfd_close_or_warn prints is just the
> text version of a bfd error:
>
> if (!ret)
> warning (_("cannot close \"%s\": %s"),
> name, bfd_errmsg (bfd_get_error ()));
>
> Maybe we could add a new type of bfd error, a custom error, where the error
> reason is given by callback (which gdb would install) or a string or some such, so
> we could pass the gdb error through bfd. Not that different from how
> bfd_error_on_input is a special error code too. But I'd rather not do this now.
Ah, I see. A proper solution involving BFD would be good, but we could
also bypass BFD and store the error string somewhere where we can pick
it up again in gdb_bfd_close_or_warn.
But yeah I'm fine with what you have, it's certainly not worse than the
current state.
>
> I've tweaked the patch to ensure that the new warning looks exactly like the
> one in gdb_bfd_close_or_warn, though. See below.
>
>>
>> If we need to catch exceptions for the close operations, I suppose we
>> need to do that as well for the other BFD iovec operations: open, pread
>> and stat? A remote communication error could happen in any of those.
>
> Yes, though I don't think they're as likely to be in dtor/noexcept paths,
> and it isn't as obvious to me what the return code should be. With close,
> there's not much else we can do other than ignore the error. I'd rather
> not touch them for now, for time constraint reasons. :-/
I thought an issue (which you talk about in your commit message) was for
the exception to go over BFD frames, which may not have been compiled
with -fasynchronous-unwind-tables. That could happens with any of the
callbacks.
> If you'd prefer, I can drop this patch for now. The issue addressed by this
> patch isn't a regression.
I'm fine with what you have, it makes the situation a bit better. I
just wanted to know whether the other callbacks should eventually be
converted to do the same thing, to document the situation for future us.
Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close (PR gdb/28080)
2021-07-13 13:13 ` Simon Marchi
@ 2021-07-13 13:25 ` Pedro Alves
0 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2021-07-13 13:25 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 2021-07-13 2:13 p.m., Simon Marchi wrote:
>>>> since in this spot we already close returning error. That's what this
>>>
>>> I don't understand this sentence, "in this spot we already close
>>> returning error".
>>
>> That's because the "ignore" word is missing... :-P
>>
>> I've tweaked it to:
>>
>> Thus frame #8 seems like a good place to swallow exceptions. More so
>> since in this spot we already ignore target_fileio_close return
>> errors. That's what this commit does. Without the previous fix, we'd
>> see:
>
> Ah, I see. Well then it sounds like if target_fileio_close returns an
> error, that error should be returned by gdb_bfd_iovec_fileio_close as
> well (eventually, not in this patch).
>
>>> Maybe the more correct thing to do would be to make
>>> gdb_bfd_iovec_fileio_close return an error when it can't properly close
>>> the file. That error would be communicated as a bfd return status to
>>> gdb_bfd_close_or_warn, which would warn.
>>
>> The problem with that is that it would require losing the exception
>> message, because the warning gdb_bfd_close_or_warn prints is just the
>> text version of a bfd error:
>>
>> if (!ret)
>> warning (_("cannot close \"%s\": %s"),
>> name, bfd_errmsg (bfd_get_error ()));
>>
>> Maybe we could add a new type of bfd error, a custom error, where the error
>> reason is given by callback (which gdb would install) or a string or some such, so
>> we could pass the gdb error through bfd. Not that different from how
>> bfd_error_on_input is a special error code too. But I'd rather not do this now.
>
> Ah, I see. A proper solution involving BFD would be good, but we could
> also bypass BFD and store the error string somewhere where we can pick
> it up again in gdb_bfd_close_or_warn.
And just pick a bfd_error_xxx constant, like e.g., bfd_error_system_call?
Then, if the GDB global is set, we know it's really a GDB error. That would
work, I think, yeah. As long as BFD itself doesn't look at the error to
decide anything, I guess.
>
> But yeah I'm fine with what you have, it's certainly not worse than the
> current state.
>
OK, I'll go merge the patches then.
>>
>> I've tweaked the patch to ensure that the new warning looks exactly like the
>> one in gdb_bfd_close_or_warn, though. See below.
>>
>>>
>>> If we need to catch exceptions for the close operations, I suppose we
>>> need to do that as well for the other BFD iovec operations: open, pread
>>> and stat? A remote communication error could happen in any of those.
>>
>> Yes, though I don't think they're as likely to be in dtor/noexcept paths,
>> and it isn't as obvious to me what the return code should be. With close,
>> there's not much else we can do other than ignore the error. I'd rather
>> not touch them for now, for time constraint reasons. :-/
>
> I thought an issue (which you talk about in your commit message) was for
> the exception to go over BFD frames, which may not have been compiled
> with -fasynchronous-unwind-tables. That could happens with any of the
> callbacks.
Yeah, there's that issue, and the issue with reaching the ~objfile dtor.
The BFD issue doesn't trigger on x86-64, while the dtor one triggers everywhere.
>
>> If you'd prefer, I can drop this patch for now. The issue addressed by this
>> patch isn't a regression.
>
> I'm fine with what you have, it makes the situation a bit better. I
> just wanted to know whether the other callbacks should eventually be
> converted to do the same thing, to document the situation for future us.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-07-13 13:25 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-12 19:33 [PATCH 0/2] Fix detach with target remote (PR gdb/28080) Pedro Alves
2021-07-12 19:33 ` [PATCH 1/2] " Pedro Alves
2021-07-13 1:07 ` Simon Marchi
2021-07-12 19:33 ` [PATCH 2/2] Avoid letting exceptions escape gdb_bfd_iovec_fileio_close " Pedro Alves
2021-07-13 1:19 ` Simon Marchi
2021-07-13 12:16 ` Pedro Alves
2021-07-13 13:13 ` Simon Marchi
2021-07-13 13:25 ` Pedro Alves
2021-07-12 22:24 ` [PATCH 0/2] Fix detach with target remote " Jonah Graham
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).