* [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
* 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
* [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 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
* 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
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).