Do not close BFDs, breaking deleted inferior shlibs
Commit Message
On Tue, 17 Feb 2015 08:44:58 +0100, Doug Evans wrote:
> Hi. The comments in the code for the call to bfd_cache_close_all
> explain the windows situation well enough, but IIUC it is necessary
> to *not* call bfd_cache_close_all here on linux, but there are no
> comments in the code explaining this.
> Can you add something? Thanks.
Added.
> Also, IIUC there's a fragileness here that we still need to address.
> AFAICT there's nothing in the bfd cache API that prevents bfd
> from randomly closing the file in the future, and yet from reading
> the bug report it is necessary that we do not close the file.
Primarily it is always racy. At _r_debug_state (or probe) time the file may
be already mapped in inferior and already unlinked from disk.
See /proc/PID/mem discussion below.
And yes, personally I do not think there should ever be any "BFD cache" and
any closing of fds. Defaults:
ulimit -a -S
open files (-n) 1024
ulimit -a -H
open files (-n) 4096
GDB could even increase the rlimit to 4096 by default and that should be
enough for everybody incl. Google (==2000 shared libraries with separate
.debug file).
I haven't checked if -gsplit-dwarf could need even more open fds?
But for some reason BFD contains this FDs closing complexity, by default the
limit is 1024/8==128 files==64 shared libraries which is less than minimal
Linux GUI apps.
> E.g., does gdb need to keep open, for example, every shared library
> used by the inferior?
Do you mean that GDB may no longer need to access BFD for a file after it is
initially mapped? I would not depend on that...
> Perhaps a better way to go would be to teach gdb to handle
> the file going way.
Do you mean like elfutils to read for unlinked files /proc/PID/mem instead?
One cannot access unmapped areas of the file then.
Jan
gdb/ChangeLog
2015-02-16 Jan Kratochvil <jan.kratochvil@redhat.com>
* corefile.c (reopen_exec_file): Wrap bfd_cache_close_all call into
MS-Windows conditional.
* exec.c (exec_file_attach): Likewise.
* symfile.c (symbol_file_add_with_addrs): Likewise.
gdb/testsuite/ChangeLog
2015-02-16 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.base/close-deleted-bfd-lib.c: New file.
* gdb.base/close-deleted-bfd-main.c: New file.
* gdb.base/close-deleted-bfd.exp: New file.
@@ -148,10 +148,18 @@ reopen_exec_file (void)
if (exec_bfd_mtime && exec_bfd_mtime != st.st_mtime)
exec_file_attach (filename, 0);
else
- /* If we accessed the file since last opening it, close it now;
- this stops GDB from holding the executable open after it
- exits. */
- bfd_cache_close_all ();
+ {
+ /* MS-Windows cannot replace file with a new build if the file is
+ still open. Therefore close the file now, BFD will reopen it
+ when needed.
+ On other systems it is better to keep the file open, both for
+ performance reasons and also if the file gets unlinked on disk
+ it still executes fine and GDB should still be able to continue
+ debugging it. */
+#if defined _WIN32 || defined __CYGWIN__
+ bfd_cache_close_all ();
+#endif
+ }
do_cleanups (cleanups);
}
@@ -258,7 +258,11 @@ exec_file_attach (const char *filename, int from_tty)
do_cleanups (cleanups);
+ // See reopen_exec_file for the reasons.
+#if defined _WIN32 || defined __CYGWIN__
bfd_cache_close_all ();
+#endif
+
observer_notify_executable_changed ();
}
@@ -1238,7 +1238,11 @@ symbol_file_add_with_addrs (bfd *abfd, const char *name, int add_flags,
observer_notify_new_objfile (objfile);
+ // See reopen_exec_file for the reasons.
+#if defined _WIN32 || defined __CYGWIN__
bfd_cache_close_all ();
+#endif
+
return (objfile);
}
new file mode 100644
@@ -0,0 +1,22 @@
+/* Copyright 2015 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/>. */
+
+#include <signal.h>
+
+void
+foo (void)
+{
+ raise (SIGUSR1);
+}
new file mode 100644
@@ -0,0 +1,34 @@
+/* Copyright 2015 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/>. */
+
+#include <dlfcn.h>
+#include <assert.h>
+#include <stddef.h>
+
+int
+main (void)
+{
+ void *handle = dlopen (SHLIB_NAME, RTLD_LAZY);
+ void (*fp) (void);
+
+ assert (handle != NULL);
+
+ fp = (void (*) (void)) dlsym (handle, "foo");
+ assert (fp != NULL); /* break-here */
+
+ fp ();
+
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,47 @@
+# Copyright 2015 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/>.
+
+if { [skip_shlib_tests] || [is_remote target] } {
+ return 0
+}
+
+standard_testfile close-deleted-bfd-main.c close-deleted-bfd-lib.c
+
+set libname $testfile-solib
+set binfile_lib [standard_output_file $libname.so]
+
+if { [gdb_compile_shlib $srcdir/$subdir/$srcfile2 $binfile_lib debug] != "" } {
+ untested "Could not compile $binfile_lib."
+ return -1
+}
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile \
+ [list debug shlib_load additional_flags=-DSHLIB_NAME=\"$binfile_lib\"]] } {
+ return -1
+}
+
+if ![runto_main] then {
+ return 0
+}
+
+gdb_breakpoint [gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+file rename -force -- $binfile_lib $binfile_lib-moved
+
+gdb_test "continue" "\r\nProgram received signal SIGUSR1, .*"
+
+# FAIL was: BFD: reopening $$binfile_lib: No such file or directory
+gdb_test "bt" " in foo \[^\r\n\]*\r\n\[^\r\n\]* in main \[^\r\n\]*"