From patchwork Tue Feb 17 19:27:31 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kratochvil X-Patchwork-Id: 5141 Received: (qmail 26481 invoked by alias); 17 Feb 2015 19:27:42 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 26462 invoked by uid 89); 17 Feb 2015 19:27:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 17 Feb 2015 19:27:40 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1HJRaWL021308 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 17 Feb 2015 14:27:36 -0500 Received: from host1.jankratochvil.net ([10.40.204.17]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t1HJRV2D023280 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); Tue, 17 Feb 2015 14:27:34 -0500 Date: Tue, 17 Feb 2015 20:27:31 +0100 From: Jan Kratochvil To: Doug Evans Cc: gdb-patches Subject: Re: [patch] Do not close BFDs, breaking deleted inferior shlibs Message-ID: <20150217192731.GA24843@host1.jankratochvil.net> References: <20150216210958.GA9701@host1.jankratochvil.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes 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 * 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 * 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. diff --git a/gdb/corefile.c b/gdb/corefile.c index a042e6d..bee2b0f 100644 --- a/gdb/corefile.c +++ b/gdb/corefile.c @@ -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); } diff --git a/gdb/exec.c b/gdb/exec.c index 124074f..b1f6157 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -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 (); } diff --git a/gdb/symfile.c b/gdb/symfile.c index c2a71ec..45277b3 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -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); } diff --git a/gdb/testsuite/gdb.base/close-deleted-bfd-lib.c b/gdb/testsuite/gdb.base/close-deleted-bfd-lib.c new file mode 100644 index 0000000..782302c --- /dev/null +++ b/gdb/testsuite/gdb.base/close-deleted-bfd-lib.c @@ -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 . */ + +#include + +void +foo (void) +{ + raise (SIGUSR1); +} diff --git a/gdb/testsuite/gdb.base/close-deleted-bfd-main.c b/gdb/testsuite/gdb.base/close-deleted-bfd-main.c new file mode 100644 index 0000000..669a275 --- /dev/null +++ b/gdb/testsuite/gdb.base/close-deleted-bfd-main.c @@ -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 . */ + +#include +#include +#include + +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; +} diff --git a/gdb/testsuite/gdb.base/close-deleted-bfd.exp b/gdb/testsuite/gdb.base/close-deleted-bfd.exp new file mode 100644 index 0000000..cdbbd33 --- /dev/null +++ b/gdb/testsuite/gdb.base/close-deleted-bfd.exp @@ -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 . + +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\]*"