[rfc 3/3] Do not use gdb_realpath for exec_bfd and objfiles

Doug Evans dje@google.com
Tue Sep 3 22:54:00 GMT 2013


Jan Kratochvil writes:
 > Hi Doug,
 > 
 > in fact all the gdb_realpath() calls are IMO a bug, it is more convenient to
 > see I am debugging /home/user/file than to see it is /.sdb5/home/user/file
 > (this example comes from common 'ln -s .sdb5/home /home' joining of partitions
 > etc.).
 > 
 > As this goes against some GDB goal I do not understand I have not changed it
 > everywhere yet.  Still this patch already makes such change for objfiles
 > and exec file, such as in "info files" (but not for source files).
 > 
 > If the displayed filenames really should remain canonical then sure this patch
 > is wrong and the original filename should be stored only internally (for
 > open_and_init_dwp_file).
 > 
 > 
 > Jan
 > 
 > 
 > gdb/
 > 2013-08-28  Jan Kratochvil  <jan.kratochvil@redhat.com>
 > 
 > 	Do not use gdb_realpath for exec_bfd and objfiles.
 > 	* dwarf2read.c (open_and_init_dwp_file): Try DWP_NAME also with
 > 	gdb_realpath.
 > 	* exec.c (exec_file_attach): Remove variable canonical_pathname, use
 > 	scratch_pathname instead.
 > 	* symfile.c (symfile_bfd_open): Remove OPF_RETURN_REALPATH from openp
 > 	call.  Twice.
 > 
 > gdb/testsuite/
 > 2013-08-28  Jan Kratochvil  <jan.kratochvil@redhat.com>
 > 
 > 	* gdb.base/dwp-symlink.c: New file.
 > 	* gdb.base/dwp-symlink.exp: New file.

Hi.
Ok by me, with a few nits.

 > --- a/gdb/dwarf2read.c
 > +++ b/gdb/dwarf2read.c
 > @@ -9637,6 +9637,14 @@ open_and_init_dwp_file (void)
 >    dbfd = open_dwp_file (dwp_name);
 >    if (dbfd == NULL)
 >      {
 > +      /* Try to find .dwp for the binary file after gdb_realpath resolving.  */
 > +      dwp_name = xstrprintf ("%s.dwp",
 > +			     gdb_realpath (dwarf2_per_objfile->objfile->name));

Needs another cleanup for the result of gdb_realpath.

 > +      make_cleanup (xfree, dwp_name);
 > +      dbfd = open_dwp_file (dwp_name);
 > +    }
 > +  if (dbfd == NULL)
 > +    {
 >        if (dwarf2_read_debug)
 >  	fprintf_unfiltered (gdb_stdlog, "DWP file not found: %s\n", dwp_name);
 >        do_cleanups (cleanups);
 > --- a/gdb/exec.c
 > +++ b/gdb/exec.c
 > @@ -182,7 +182,7 @@ exec_file_attach (char *filename, int from_tty)
 >    else
 >      {
 >        struct cleanup *cleanups;
 > -      char *scratch_pathname, *canonical_pathname;
 > +      char *scratch_pathname;
 >        int scratch_chan;
 >        struct target_section *sections = NULL, *sections_end = NULL;
 >        char **matching;
 > @@ -206,16 +206,11 @@ exec_file_attach (char *filename, int from_tty)
 >  
 >        cleanups = make_cleanup (xfree, scratch_pathname);
 >  
 > -      /* gdb_bfd_open (and its variants) prefers canonicalized pathname for
 > -	 better BFD caching.  */
 > -      canonical_pathname = gdb_realpath (scratch_pathname);
 > -      make_cleanup (xfree, canonical_pathname);
 > -
 >        if (write_files)
 > -	exec_bfd = gdb_bfd_fopen (canonical_pathname, gnutarget,
 > +	exec_bfd = gdb_bfd_fopen (scratch_pathname, gnutarget,
 >  				  FOPEN_RUB, scratch_chan);
 >        else
 > -	exec_bfd = gdb_bfd_open (canonical_pathname, gnutarget, scratch_chan);
 > +	exec_bfd = gdb_bfd_open (scratch_pathname, gnutarget, scratch_chan);
 >  
 >        if (!exec_bfd)
 >  	{
 > --- a/gdb/symfile.c
 > +++ b/gdb/symfile.c
 > @@ -1679,7 +1679,7 @@ symfile_bfd_open (char *name)
 >    name = tilde_expand (name);	/* Returns 1st new malloc'd copy.  */
 >  
 >    /* Look down path for it, allocate 2nd new malloc'd copy.  */
 > -  desc = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH, name,
 > +  desc = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, name,
 >  		O_RDONLY | O_BINARY, &absolute_name);
 >  #if defined(__GO32__) || defined(_WIN32) || defined (__CYGWIN__)
 >    if (desc < 0)
 > @@ -1687,8 +1687,8 @@ symfile_bfd_open (char *name)
 >        char *exename = alloca (strlen (name) + 5);
 >  
 >        strcat (strcpy (exename, name), ".exe");
 > -      desc = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
 > -		    exename, O_RDONLY | O_BINARY, &absolute_name);
 > +      desc = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, exename,
 > +		    O_RDONLY | O_BINARY, &absolute_name);
 >      }
 >  #endif
 >    if (desc < 0)
 > --- /dev/null
 > +++ b/gdb/testsuite/gdb.base/dwp-symlink.c
 > @@ -0,0 +1,22 @@
 > +/* This testcase is part of GDB, the GNU debugger.
 > +
 > +   Copyright 2013 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 (int argc, char **argv)
 > +{
 > +  return 0;
 > +}
 > --- /dev/null
 > +++ b/gdb/testsuite/gdb.base/dwp-symlink.exp
 > @@ -0,0 +1,77 @@
 > +# Copyright 2013 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/>.
 > +
 > +standard_testfile
 > +
 > +if [is_remote host] {
 > +    untested "remote host"
 > +    return 0
 > +}
 > +
 > +file delete [standard_output_file ${testfile}.dwp]
 > +if [file exists [standard_output_file ${testfile}.dwp]] {
 > +    unsupported "dwp file cannot be deleted"
 > +    return 0
 > +}
 > +if { [build_executable ${testfile}.exp ${testfile} ${srcfile}] == -1 } {
 > +    return -1
 > +}
 > +if ![file exists [standard_output_file ${testfile}.dwp]] {
 > +    unsupported "testsuite run run does not produce dwp files"

s/run run/run/

 > +    return 0
 > +}
 > +
 > +set thelink "${testfile}-thelink"
 > +
 > +file delete [standard_output_file ${thelink}]
 > +file delete [standard_output_file ${thelink}.dwp]
 > +# file link is only Tcl 8.4+.
 > +exec "ln" "-sf" "${testfile}" "[standard_output_file $thelink]"
 > +if ![file exists [standard_output_file $thelink]] {
 > +    unsupported "host does not support symbolic links (binary symlink is missing)"
 > +    return 0
 > +}
 > +if [file exists [standard_output_file $thelink.dwp]] {
 > +    unsupported "host does not support symbolic links (dwp symlink exists)"

error message could be better (we tried to delete a file and it's still there?).

 > +    return 0
 > +}
 > +
 > +clean_restart "$testfile"
 > +
 > +gdb_test "ptype main" {type = int \(int, char \*\*\)} "binary default, dwp default"
 > +
 > +clean_restart "$thelink"
 > +
 > +gdb_test "ptype main" {type = int \(int, char \*\*\)} "binary symlink, dwp default"
 > +
 > +gdb_exit
 > +file rename [standard_output_file ${testfile}.dwp] [standard_output_file ${thelink}.dwp]
 > +if [file exists [standard_output_file ${testfile}.dwp]] {
 > +    unsupported "host does not support symbolic links (binary symlink exists)"
 > +    return 0
 > +}
 > +if ![file exists [standard_output_file ${thelink}.dwp]] {
 > +    unsupported "host does not support symbolic links (dwp symlink is missing)"
 > +    return 0
 > +}
 > +
 > +clean_restart "$testfile"
 > +
 > +# This case cannot work.
 > +gdb_test "ptype main" {type = int \(\)} "binary default, dwp at symlink"
 > +
 > +clean_restart "$thelink"
 > +
 > +gdb_test "ptype main" {type = int \(int, char \*\*\)} "binary symlink, dwp at symlink"



More information about the Gdb-patches mailing list