This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patchv2 9/11] Fix psymtab.c for real and absolute filenames


Hi,

formerly:
	[patch 7/9] Fix psymtab.c for real and absolute filenames
	http://sourceware.org/ml/gdb-patches/2013-01/msg00391.html

here was a fix that formerly it performance-regressed
default "set basenames-may-differ off", which required new bool basenames
parameter to file_matcher.


Thanks,
Jan



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

	* dwarf2read.c (dw2_expand_symtabs_matching): Add basenames parameter
	to the file_matcher parameter.  Pass 0 to it.
	(dwarf2_create_include_psymtab): Copy also DIRNAME.
	* psymtab.c (partial_map_symtabs_matching_filename): Drop handling of
	NULL psymtab_to_fullname result.
	(psymtab_to_fullname): Remove variable r.  Never return NULL, assemble
	an expected filename instead.
	(expand_symtabs_matching_via_partial): Add basenames parameter to the
	file_matcher parameter.  Call also psymtab_to_fullname, after newly
	considering BASENAMES_MAY_DIFFER.
	* source.c (rewrite_source_path): Remove static.
	* source.h (rewrite_source_path): New declaration.
	* symfile.h (struct quick_symbol_functions): Add basenames parameter to
	the expand_symtabs_matching field.  Comment it.
	* symtab.c (file_matches): New function comment.  Add parameter
	basenames, implement it.
	(search_symbols_file_matches): Add basenames parameter.  Update the
	file_matches caller.
	(search_symbols): Match FILES also against symtab_to_fullname.
	Optimize it for BASENAMES_MAY_DIFFER.

gdb/testsuite/
2013-01-25  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
@@ -3473,7 +3473,7 @@ dw2_map_matching_symbols (const char * name, domain_enum namespace,
 static void
 dw2_expand_symtabs_matching
   (struct objfile *objfile,
-   int (*file_matcher) (const char *, void *),
+   int (*file_matcher) (const char *, void *, int basenames),
    int (*name_matcher) (const char *, void *),
    enum search_domain kind,
    void *data)
@@ -3533,7 +3533,7 @@ dw2_expand_symtabs_matching
 
 	  for (j = 0; j < file_data->num_file_names; ++j)
 	    {
-	      if (file_matcher (file_data->file_names[j], data))
+	      if (file_matcher (file_data->file_names[j], data, 0))
 		{
 		  per_cu->v.quick->mark = 1;
 		  break;
@@ -4054,6 +4054,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/psymtab.c
+++ b/gdb/psymtab.c
@@ -204,9 +204,7 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
       {
 	gdb_assert (IS_ABSOLUTE_PATH (real_path));
 	gdb_assert (IS_ABSOLUTE_PATH (name));
-	psymtab_to_fullname (pst);
-	if (pst->fullname != NULL
-	    && FILENAME_CMP (real_path, pst->fullname) == 0)
+	if (filename_cmp (psymtab_to_fullname (pst), real_path) == 0)
 	  {
 	    if (partial_map_expand_apply (objfile, name, real_path,
 					  pst, callback, data))
@@ -1161,28 +1159,39 @@ 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;
 }
 
 static const char *
@@ -1354,7 +1363,7 @@ recursively_search_psymtabs (struct partial_symtab *ps,
 static void
 expand_symtabs_matching_via_partial
   (struct objfile *objfile,
-   int (*file_matcher) (const char *, void *),
+   int (*file_matcher) (const char *, void *, int basenames),
    int (*name_matcher) (const char *, void *),
    enum search_domain kind,
    void *data)
@@ -1381,7 +1390,13 @@ expand_symtabs_matching_via_partial
 	{
 	  if (ps->anonymous)
 	    continue;
-	  if (! (*file_matcher) (ps->filename, data))
+
+	  /* Before we invoke realpath, which can get expensive when many
+	     files are involved, do a quick comparison of the basenames.  */
+	  if (!(*file_matcher) (ps->filename, data, 0)
+	      && (basenames_may_differ
+		  || (*file_matcher) (lbasename (ps->filename), data, 1))
+	      && !(*file_matcher) (psymtab_to_fullname (ps), data, 0))
 	    continue;
 	}
 
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -954,7 +954,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
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -255,6 +255,8 @@ struct quick_symbol_functions
      FILE_MATCHER is called for each file in OBJFILE.  The file name
      and the DATA argument are passed to it.  If it returns zero, this
      file is skipped.  If FILE_MATCHER is NULL such file is not skipped.
+     If BASENAMES is non-zero the function should consider only base name of
+     DATA (passed file name is already only the lbasename part).
 
      Otherwise, if KIND does not match this symbol is skipped.
 
@@ -270,7 +272,7 @@ struct quick_symbol_functions
      functions.  */
   void (*expand_symtabs_matching)
     (struct objfile *objfile,
-     int (*file_matcher) (const char *, void *),
+     int (*file_matcher) (const char *, void *, int basenames),
      int (*name_matcher) (const char *, void *),
      enum search_domain kind,
      void *data);
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3274,8 +3274,11 @@ sources_info (char *ignore, int from_tty)
   do_cleanups (cleanups);
 }
 
+/* Compare FILE against all the NFILES entries of FILES.  If BASENAMES is
+   non-zero compare only lbasename of FILES.  */
+
 static int
-file_matches (const char *file, char *files[], int nfiles)
+file_matches (const char *file, char *files[], int nfiles, int basenames)
 {
   int i;
 
@@ -3283,7 +3286,9 @@ file_matches (const char *file, char *files[], int nfiles)
     {
       for (i = 0; i < nfiles; i++)
 	{
-	  if (compare_filenames_for_search (file, files[i]))
+	  if (compare_filenames_for_search (file, (basenames
+						   ? lbasename (files[i])
+						   : files[i])))
 	    return 1;
 	}
     }
@@ -3383,11 +3388,12 @@ struct search_symbols_data
 /* A callback for expand_symtabs_matching.  */
 
 static int
-search_symbols_file_matches (const char *filename, void *user_data)
+search_symbols_file_matches (const char *filename, void *user_data,
+			     int basenames)
 {
   struct search_symbols_data *data = user_data;
 
-  return file_matches (filename, data->files, data->nfiles);
+  return file_matches (filename, data->files, data->nfiles, basenames);
 }
 
 /* A callback for expand_symtabs_matching.  */
@@ -3596,7 +3602,14 @@ search_symbols (char *regexp, enum search_domain kind,
 
 	    QUIT;
 
-	    if (file_matches (real_symtab->filename, files, nfiles)
+	    /* Check first sole REAL_SYMTAB->FILENAME.  It does not need to be
+	       a substring of symtab_to_fullname as it may contain "./" etc.  */
+	    if ((file_matches (real_symtab->filename, files, nfiles, 0)
+		 || ((basenames_may_differ
+		      || file_matches (lbasename (real_symtab->filename),
+				       files, nfiles, 1))
+		     && file_matches (symtab_to_fullname (real_symtab),
+				      files, nfiles, 0)))
 		&& ((!datum.preg_p
 		     || regexec (&datum.preg, SYMBOL_NATURAL_NAME (sym), 0,
 				 NULL, 0) == 0)
--- /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,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 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\\);"
+
+clean_restart ${testfile}
+
+gdb_test_no_output "set basenames-may-differ on"
+
+gdb_test "break realname-expand-real.c:func" "^break realname-expand-real.c:func\r\nBreakpoint 1 at 0x\[0-9a-f\]+: file \[^\r\n\]*/realname-expand-link\\.c, line \[0-9\]+\\."


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]