[patch 7/9] Fix psymtab.c for real and absolute filenames

[patch 7/9] Fix psymtab.c for real and absolute filenames

[Jan Kratochvil]

Hi,

partial symtabs did not handle correctly absolute filenames and they had
problems also with expansion on symlinks.

The new testcases also show FAIL->PASS for the next .gdb_index patch.


Thanks,
Jan


gdb/
2013-01-15  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (dwarf2_create_include_psymtab): Copy also DIRNAME.
	* psympriv.h (struct partial_symtab): New field realname.
	* psymtab.c (psymtab_to_realname): New declaration.
	(partial_map_symtabs_matching_filename): New variable fp.  Drop
	handling of NULL psymtab_to_fullname result.  Make variable rp const
	char *.  Use psymtab_to_realname.
	(forget_cached_source_info_partial): Clear also REALNAME.
	(psymtab_to_fullname): Remove variable r.  Never return NULL, assemble
	an expected filename instead.
	(psymtab_to_realname): New function.
	(expand_symtabs_matching_via_partial): Call also psymtab_to_fullname and
	psymtab_to_realname.
	(maintenance_info_psymtabs): Display also REALNAME.
	* source.c (rewrite_source_path): Remove static.
	* source.h (rewrite_source_path): New declaration.

gdb/testsuite/
2013-01-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/fullpath-expand-func.c: New file.
	* gdb.base/fullpath-expand.c: New file.
	* gdb.base/fullpath-expand.exp: New file.
	* gdb.base/realname-expand-real.c: New file.
	* gdb.base/realname-expand.c: New file.
	* gdb.base/realname-expand.exp: New file.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4094,6 +4094,12 @@ dwarf2_create_include_psymtab (char *name, struct partial_symtab *pst,
 {
   struct partial_symtab *subpst = allocate_psymtab (name, objfile);
 
+  if (!IS_ABSOLUTE_PATH (subpst->filename))
+    {
+      /* It shares objfile->objfile_obstack.  */
+      subpst->dirname = pst->dirname;
+    }
+
   subpst->section_offsets = pst->section_offsets;
   subpst->textlow = 0;
   subpst->texthigh = 0;
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -92,6 +92,10 @@ struct partial_symtab
 
   char *fullname;
 
+  /* FULLPATH after conversion by gdb_realpath.  */
+
+  char *realname;
+
   /* Directory in which it was compiled, or NULL if we don't know.  */
 
   const char *dirname;
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -60,6 +60,8 @@ static struct partial_symbol *lookup_partial_symbol (struct objfile *,
 
 static const char *psymtab_to_fullname (struct partial_symtab *ps);
 
+static const char *psymtab_to_realname (struct partial_symtab *ps);
+
 static struct partial_symbol *find_pc_sect_psymbol (struct objfile *,
 						    struct partial_symtab *,
 						    CORE_ADDR,
@@ -198,11 +200,12 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
        this symtab and use its absolute path.  */
     if (full_path != NULL)
       {
+	/* FP is here in xfullpath form.  */
+	const char *fp = psymtab_to_fullname (pst);
+
 	gdb_assert (IS_ABSOLUTE_PATH (full_path));
 	gdb_assert (IS_ABSOLUTE_PATH (name));
-	psymtab_to_fullname (pst);
-	if (pst->fullname != NULL
-	    && FILENAME_CMP (full_path, pst->fullname) == 0)
+	if (filename_cmp (fp, full_path) == 0)
 	  {
 	    if (partial_map_expand_apply (objfile, name, full_path, real_path,
 					  pst, callback, data))
@@ -212,17 +215,11 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
 
     if (real_path != NULL)
       {
-        char *rp = NULL;
+	const char *rp = psymtab_to_realname (pst);
 
 	gdb_assert (IS_ABSOLUTE_PATH (real_path));
 	gdb_assert (IS_ABSOLUTE_PATH (name));
-	psymtab_to_fullname (pst);
-        if (pst->fullname != NULL)
-          {
-            rp = gdb_realpath (pst->fullname);
-            make_cleanup (xfree, rp);
-          }
-	if (rp != NULL && FILENAME_CMP (real_path, rp) == 0)
+	if (filename_cmp (rp, real_path) == 0)
 	  {
 	    if (partial_map_expand_apply (objfile, name, full_path, real_path,
 					  pst, callback, data))
@@ -869,6 +866,11 @@ forget_cached_source_info_partial (struct objfile *objfile)
 	  xfree (pst->fullname);
 	  pst->fullname = NULL;
 	}
+      if (pst->realname != NULL)
+	{
+	  xfree (pst->realname);
+	  pst->realname = NULL;
+	}
     }
 }
 
@@ -1177,28 +1179,55 @@ map_symbol_filenames_psymtab (struct objfile *objfile,
 static const char *
 psymtab_to_fullname (struct partial_symtab *ps)
 {
-  int r;
-
-  if (!ps)
-    return NULL;
-  if (ps->anonymous)
-    return NULL;
+  gdb_assert (!ps->anonymous);
 
   /* Use cached copy if we have it.
      We rely on forget_cached_source_info being called appropriately
      to handle cases like the file being moved.  */
-  if (ps->fullname)
-    return ps->fullname;
+  if (ps->fullname == NULL)
+    {
+      int fd = find_and_open_source (ps->filename, ps->dirname, &ps->fullname);
 
-  r = find_and_open_source (ps->filename, ps->dirname, &ps->fullname);
+      if (fd >= 0)
+	close (fd);
+      else
+	{
+	  char *fullname;
+	  struct cleanup *back_to;
 
-  if (r >= 0)
-    {
-      close (r);
-      return ps->fullname;
-    }
+	  /* rewrite_source_path would be applied by find_and_open_source, we
+	     should report the pathname where GDB tried to find the file.  */
 
-  return NULL;
+	  if (ps->dirname == NULL || IS_ABSOLUTE_PATH (ps->filename))
+	    fullname = xstrdup (ps->filename);
+	  else
+	    fullname = concat (ps->dirname, SLASH_STRING, ps->filename, NULL);
+
+	  back_to = make_cleanup (xfree, fullname);
+	  ps->fullname = rewrite_source_path (fullname);
+	  if (ps->fullname == NULL)
+	    ps->fullname = xstrdup (fullname);
+	  do_cleanups (back_to);
+	}
+    } 
+
+  return ps->fullname;
+}
+
+/* Return absolute source filename of partial_symtab PS.
+   While psymtab_to_fullname uses xfullpath psymtab_to_realname uses
+   gdb_realpath.  */
+
+static const char *
+psymtab_to_realname (struct partial_symtab *ps)
+{
+  if (ps == NULL || ps->anonymous)
+    return NULL;
+
+  if (ps->realname == NULL)
+    ps->realname = gdb_realpath (psymtab_to_fullname (ps)); 
+
+  return ps->realname;
 }
 
 static const char *
@@ -1397,7 +1426,9 @@ expand_symtabs_matching_via_partial
 	{
 	  if (ps->anonymous)
 	    continue;
-	  if (! (*file_matcher) (ps->filename, data))
+
+	  if (!(*file_matcher) (psymtab_to_fullname (ps), data)
+	      && !(*file_matcher) (psymtab_to_realname (ps), data))
 	    continue;
 	}
 
@@ -1932,6 +1963,9 @@ maintenance_info_psymtabs (char *regexp, int from_tty)
 	      printf_filtered ("    fullname %s\n",
 			       psymtab->fullname
 			       ? psymtab->fullname : "(null)");
+	      printf_filtered ("    realname %s\n",
+			       psymtab->realname
+			       ? psymtab->realname : "(null)");
 	      printf_filtered ("    text addresses ");
 	      fputs_filtered (paddress (gdbarch, psymtab->textlow),
 			      gdb_stdout);
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -958,7 +958,7 @@ get_substitute_path_rule (const char *path)
    Return NULL if no substitution rule was specified by the user,
    or if no rule applied to the given PATH.  */
    
-static char *
+char *
 rewrite_source_path (const char *path)
 {
   const struct substitute_path_rule *rule = get_substitute_path_rule (path);
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -48,6 +48,8 @@ extern int find_and_open_source (const char *filename,
    negative number for error.  */
 extern int open_source_file (struct symtab *s);
 
+extern char *rewrite_source_path (const char *path);
+
 extern const char *symtab_to_fullname (struct symtab *s);
 
 /* Returns filename without the compile directory part, basename or absolute
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fullpath-expand-func.c
@@ -0,0 +1,21 @@
+/* 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/>.  */
+
+void
+func (void)
+{
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fullpath-expand.c
@@ -0,0 +1,25 @@
+/* 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/>.  */
+
+extern void func (void);
+
+int
+main (void)
+{
+  func ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fullpath-expand.exp
@@ -0,0 +1,44 @@
+# Copyright (C) 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 .c fullpath-expand-func.c
+
+if { [file pathtype $objdir] != "absolute" } {
+    untested "objdir $objdir is not absolute"
+    return -1
+}
+
+set saved_pwd [pwd]
+cd $srcdir
+set err [gdb_compile "${subdir}/${srcfile} ${subdir}/${srcfile2}" $binfile executable {debug}]
+cd $saved_pwd
+if { $err != "" } {
+    untested "${srcfile} or ${srcfile2} compilation failed"
+    return -1
+}
+
+set result [catch "exec realpath ${srcdir}/${subdir}/${srcfile2}" realsrcfile2]
+if { $result != 0 || $realsrcfile2 == "" } {
+    untested "invalid realpath of ${srcfile2}: result $result output $realsrcfile2"
+    return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test "rbreak $realsrcfile2:func" "^rbreak \[^\r\n\]*:func\r\nBreakpoint 1 at 0x\[0-9a-f\]+: file [string_to_regexp ${subdir}/${srcfile2}], line \[0-9\]+\\.\r\nvoid func\\(void\\);" "rbreak XXX/fullpath-expand-func.c:func"
+
+# Verify the compilation pathnames are as expected:
+gdb_test "list func" "\tfunc \\(void\\)\r\n.*"
+gdb_test "info source" "^info source\r\nCurrent source file is [string_to_regexp ${subdir}/${srcfile2}]\r\nCompilation directory is /.*"
--- /dev/null
+++ b/gdb/testsuite/gdb.base/realname-expand-real.c
@@ -0,0 +1,21 @@
+/* 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/>.  */
+
+void
+func (void)
+{
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/realname-expand.c
@@ -0,0 +1,25 @@
+/* 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/>.  */
+
+extern void func (void);
+
+int
+main (void)
+{
+  func ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/realname-expand.exp
@@ -0,0 +1,38 @@
+# Copyright (C) 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 .c realname-expand-real.c
+
+set srcdirabs [file join [pwd] $srcdir]
+set srcfilelink [standard_output_file realname-expand-link.c]
+
+remote_exec build "ln -sf ${srcdirabs}/${subdir}/${srcfile2} $srcfilelink"
+
+if { [file type $srcfilelink] != "link" } {
+    unsupported "target directory cannot have symbolic links"
+    return -1
+}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile} ${srcfilelink}" "${binfile}" \
+		  executable {debug}] != "" } {
+    untested "cannot compile ${srcdir}/${subdir}/${srcfile} and ${srcfilelink}"
+    return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test_no_output "set basenames-may-differ on"
+
+gdb_test "rbreak realname-expand-real.c:func" "^rbreak realname-expand-real.c:func\r\nBreakpoint 1 at 0x\[0-9a-f\]+: file \[^\r\n\]*/realname-expand-link\\.c, line \[0-9\]+\\.\r\nvoid func\\(void\\);"

Re: [patch 7/9] Fix psymtab.c for real and absolute filenames

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> partial symtabs did not handle correctly absolute filenames and they had
Jan> problems also with expansion on symlinks.

Jan> +	  if (!(*file_matcher) (psymtab_to_fullname (ps), data)
Jan> +	      && !(*file_matcher) (psymtab_to_realname (ps), data))
Jan>  	    continue;

Doesn't this tend to eliminate the benefit of using xfullpath?

Aside from this it looks fine.

Tom

Re: [patch 7/9] Fix psymtab.c for real and absolute filenames

[Jan Kratochvil]

On Wed, 23 Jan 2013 22:31:30 +0100, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> partial symtabs did not handle correctly absolute filenames and they had
> Jan> problems also with expansion on symlinks.
> 
       -	  if (! (*file_matcher) (ps->filename, data))
> Jan> +	  if (!(*file_matcher) (psymtab_to_fullname (ps), data)
> Jan> +	      && !(*file_matcher) (psymtab_to_realname (ps), data))
> Jan>  	    continue;
> 
> Doesn't this tend to eliminate the benefit of using xfullpath?

Primarily xfullpath is gone (in the next patch series) so this change will be
undone there.

This meant:

if ((*file_matcher) (psymtab_to_fullname (ps), data)
    || (*file_matcher) (psymtab_to_realname (ps), data))
  do-it.

Which seems correct to me.  I do not understand much your comment.

But I see there another bug, that it no longer tests ps->filename, so that it
would not match that "./gdb.base/return.c:main" because fullpath no longer has
that "./" there.


Thanks,
Jan

Re: [patch 7/9] Fix psymtab.c for real and absolute filenames

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> I do not understand much your comment.

Haha, me neither.  I see I was confused.

Tom