From patchwork Thu Feb 14 20:35:23 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 31482 Received: (qmail 4877 invoked by alias); 14 Feb 2019 20:35:30 -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 4841 invoked by uid 89); 14 Feb 2019 20:35:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Keith, keith, sight, compilations 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 ESMTP; Thu, 14 Feb 2019 20:35:26 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F29E6C0799B7 for ; Thu, 14 Feb 2019 20:35:24 +0000 (UTC) Received: from theo.uglyboxes.com.com (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9C1F260D0B; Thu, 14 Feb 2019 20:35:24 +0000 (UTC) From: Keith Seitz To: gdb-patches@sourceware.org Cc: palves@redhat.com Subject: Re: [PATCH] Fix symtab/23853: symlinked default symtab Date: Thu, 14 Feb 2019 12:35:23 -0800 Message-Id: <20190214203523.11749-1-keiths@redhat.com> In-Reply-To: <2e253fcb-e844-071b-61c8-25f6992fca1f@redhat.com> References: MIME-Version: 1.0 X-IsSubscribed: yes On 11/28/18 11:17 AM, Pedro Alves wrote:> On 11/05/2018 09:19 PM, Keith Seitz wrote: >> The problem here is that when convert_sals_line_offset sets the default > > s/convert_sals_line_offset/create_sals_line_offset/ Fixed. >> Later in iterate_over_some_symtabs, symtab_to_fullname would be called >> with the result of symtab_to_fullname, but only if the user has set > > This sentence doesn't seem to make sense (symtab_to_fullname twice)? > > I think you meant something like this: > > Later in iterate_over_some_symtabs, the passed-in fullname would be compared > with the symtab's fullname but only if the user has set I've rewritten/simplified this patch, so this sentence is gone. Which is a form of fixed. :-) >> "basenames-may-differ." This flag defaults to "off" because, as >> iterate_over_some_symtabs says, realpath is very expensive to use. >> >> However, there seems little reason not to check the fullname if it is >> already known (non-NULL). That's what this patch does. >> > > My first reaction was, why are we doing that, passing down the > fullname instead of the symtab's filename, especially considering > basenames_may_differ. Pedantically looks odd at first sight. Yes, indeed it does, and after some investigation, I'm convinced enough to submit this revision using this. The symtab_to_filename/fullname thing was introduced in a series in 2007 dealing with absolute filenames. In fact, the ChangeLog entry for this particular change says that it is using "symtab_to_filename_for_display," even though the patch doesn't include such a function. [It is simply using symtab_to_filename.] Clearly, this was introduced for dealing with displaying symtab filenames, not searching for them. Since we already have the actual debug info filename in the symtab, we can look them up using that, and, as expected, it works. > Actually, if create_sals_line_offset has a default symtab handy, why > does it need to iterate over symtabs? Couldn't it just use the > default directly? That's a good question. Short answer: For developers that do elaborate #if..#endif and multiple compilations on a single source file. [See gdb.base/expand-pysmtabs.exp as an example.] Without the iteration, this test fails. This does /not/ address the previous patch's "compare fullname if it is available" approach, which I still think is correct, but I would need to find a way to trigger that in a test, and I've not done that. >> + >> +# Test GDB when source files are symlinks. > > Please mention the specific case of the default symtab somewhere. > Either here, or at the gdb_breakpoint line below. That's important > for the test, right? Done. Keith Differences from last revision: - Simplify patch -- remove fullname lookups and changes to iterate_over_some_symtabs - Fixed typos in commit log - Add more/better description of test to .exp ----- This patch attempts to fix a bug dealing with setting breakpoints in default symtabs that are symlinks. For example: (gdb) list 11 GNU General Public License for more details. 12 13 You should have received a copy of the GNU General Public License 14 along with this program. If not, see . */ 15 16 static int 17 foo (void) 18 { 19 return 0; /* break here */ 20 } (gdb) 21 22 int 23 main (void) 24 { 25 return foo (); 26 } (gdb) b 19 No line 19 in the current file. Make breakpoint pending on future shared library load? (y or [n]) The problem here is that when create_sals_line_offset sets the default symtab, it immediately calls symtab_to_fullname, passing that fullname to collect_symtabs_from_filename to find all matching symtabs. This fails because we end up looking for a symtab with the name of the actual file on disk (which is different in this case because of the symlink) instead of the one stored in the debug info. Since we already have the lookup name of the default symtab, use it instead of the fullname. [This fullname thing was originally added in 2007 in a series dealing with *displaying* absolute file names. Clearly, this instance has nothing to do with the display of file names.] gdb/ChangeLog PR symtab/23853 * linespec.c (create_sals_line_offset): Search for the default symtab's filename instead of its fullname. gdb/testsuite/ChangeLog PR symtab/23853 * gdb.base/symlink-sourcefile.c: New file. * gdb.base/symlink-sourcefile.exp: New file. --- gdb/linespec.c | 6 +-- gdb/testsuite/gdb.base/symlink-sourcefile.c | 26 +++++++++++ gdb/testsuite/gdb.base/symlink-sourcefile.exp | 45 +++++++++++++++++++ 3 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.base/symlink-sourcefile.c create mode 100644 gdb/testsuite/gdb.base/symlink-sourcefile.exp diff --git a/gdb/linespec.c b/gdb/linespec.c index cedbd39bd7..e902b11c8e 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -2102,16 +2102,14 @@ create_sals_line_offset (struct linespec_state *self, if (ls->file_symtabs->size () == 1 && ls->file_symtabs->front () == nullptr) { - const char *fullname; - set_current_program_space (self->program_space); /* Make sure we have at least a default source line. */ set_default_source_symtab_and_line (); initialize_defaults (&self->default_symtab, &self->default_line); - fullname = symtab_to_fullname (self->default_symtab); *ls->file_symtabs - = collect_symtabs_from_filename (fullname, self->search_pspace); + = collect_symtabs_from_filename (self->default_symtab->filename, + self->search_pspace); use_default = 1; } diff --git a/gdb/testsuite/gdb.base/symlink-sourcefile.c b/gdb/testsuite/gdb.base/symlink-sourcefile.c new file mode 100644 index 0000000000..12cd96892a --- /dev/null +++ b/gdb/testsuite/gdb.base/symlink-sourcefile.c @@ -0,0 +1,26 @@ +/* Copyright 2019 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 . */ + +static int +foo (void) +{ + return 0; /* break here */ +} + +int +main (void) +{ + return foo (); +} diff --git a/gdb/testsuite/gdb.base/symlink-sourcefile.exp b/gdb/testsuite/gdb.base/symlink-sourcefile.exp new file mode 100644 index 0000000000..ae43934ff4 --- /dev/null +++ b/gdb/testsuite/gdb.base/symlink-sourcefile.exp @@ -0,0 +1,45 @@ +# Copyright 2019 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 . + +# Test that GDB can find the default symtab when it is a symbolic link. +standard_testfile + +set test "file symbolic link name" +set linksrc "link-${srcfile}" +set srcfilelink [standard_output_file $linksrc] + + +# Remove any existing symlink and build executable using a +# symbolic link to the actual source file. +remote_file host delete $srcfilelink +set status [remote_exec host \ + "ln -sf $srcdir/$subdir/$srcfile $srcfilelink"] +if {[lindex $status 0] != 0} { + unsupported "$test (host does not support symbolic links)" + return 0 +} + +if {[prepare_for_testing $testfile $testfile $srcfilelink]} { + return -1 +} + +if {![runto_main]} { + untested "could not run to main" + return -1 +} + +# Using a line number ensures that the default symtab is used. +gdb_breakpoint [gdb_get_line_number "break here" $srcfile] message +gdb_continue_to_breakpoint "run to breakpoint marker"