RFC: add relative file name handling for linespecs

RFC: add relative file name handling for linespecs

[Tom Tromey]

I'd appreciate comments on this patch.

This patch changes gdb to allow relative file names to match a source
file name if all the trailing components match.  Previously this
behavior was only allowed if the source file name did not have any
directory separators.

That is, now "break gcc/expr.c:1000" will work as many users expect.
See the recent thread on gdb@.

This needs at least a doc review.

Built and regtested on x86-64 F15.

Tom

2012-01-10  Tom Tromey  <tromey@redhat.com>

	* NEWS: Add item.
	* symtab.h (compare_filenames_for_search): Declare.
	* symtab.c (compare_filenames_for_search): New function.
	(iterate_over_some_symtabs): Use it.
	* symfile.h (struct quick_symbol_functions)
	<map_symtabs_matching_filename>: Change spec.
	* psymtab.c (partial_map_symtabs_matching_filename): Use
	compare_filenames_for_search.  Update for new spec.
	* dwarf2read.c (dw2_map_symtabs_matching_filename): Use
	compare_filenames_for_search.  Update for new spec.

2012-01-10  Tom Tromey  <tromey@redhat.com>

	* gdb.texinfo (Specify Location): Document relative file name
	handling.

2012-01-10  Tom Tromey  <tromey@redhat.com>

	* gdb.linespec/linespec.exp: Change some tests to use $decimal.
	Add test for relative directory.

diff --git a/gdb/NEWS b/gdb/NEWS
index a9a7859..10661c1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -9,6 +9,11 @@
 * The binary "gdbtui" can no longer be built or installed.
   Use "gdb -tui" instead.
 
+* The filename part of a linespec will now match trailing components
+  of a source file name.  For example, "break gcc/expr.c:1000" will
+  now set a breakpoint in build/gcc/expr.c, but not
+  build/libcpp/expr.c.
+
 *** Changes in GDB 7.4
 
 * GDB now handles ambiguous linespecs more consistently; the existing
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2f4aa4f..1f3b2a1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6492,6 +6492,11 @@ linespec.
 
 @item @var{filename}:@var{linenum}
 Specifies the line @var{linenum} in the source file @var{filename}.
+If @var{filename} is a relative file name, then it will match any
+source file name with the same trailing components.  For example, if
+@var{filename} is @samp{gcc/expr.c}, then it will match source file
+name of @file{/build/trunk/gcc/expr.c}, but not
+@file{/build/trunk/libcpp/expr.c} or @file{/build/trunk/gcc/x-expr.c}.
 
 @item @var{function}
 Specifies the line that begins the body of the function @var{function}.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index d09b27a..861b8de 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2402,8 +2402,8 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
 {
   int i;
   const char *name_basename = lbasename (name);
-  int check_basename = name_basename == name;
-  struct dwarf2_per_cu_data *base_cu = NULL;
+  int name_len = strlen (name);
+  int is_abs = IS_ABSOLUTE_PATH (name);
 
   dw2_setup (objfile);
 
@@ -2426,7 +2426,9 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
 	{
 	  const char *this_name = file_data->file_names[j];
 
-	  if (FILENAME_CMP (name, this_name) == 0)
+	  if (FILENAME_CMP (name, this_name) == 0
+	      || (!is_abs && compare_filenames_for_search (this_name,
+							   name, name_len)))
 	    {
 	      if (dw2_map_expand_apply (objfile, per_cu,
 					name, full_path, real_path,
@@ -2434,9 +2436,12 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
 		return 1;
 	    }
 
-	  if (check_basename && ! base_cu
-	      && FILENAME_CMP (lbasename (this_name), name) == 0)
-	    base_cu = per_cu;
+	    {
+	      if (dw2_map_expand_apply (objfile, per_cu,
+					name, full_path, real_path,
+					callback, data))
+		return 1;
+	    }
 
 	  /* Before we invoke realpath, which can get expensive when many
 	     files are involved, do a quick comparison of the basenames.  */
@@ -2450,7 +2455,10 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
 							      file_data, j);
 
 	      if (this_real_name != NULL
-		  && FILENAME_CMP (full_path, this_real_name) == 0)
+		  && (FILENAME_CMP (full_path, this_real_name) == 0
+		      || (!is_abs
+			  && compare_filenames_for_search (this_real_name,
+							   name, name_len))))
 		{
 		  if (dw2_map_expand_apply (objfile, per_cu,
 					    name, full_path, real_path,
@@ -2465,7 +2473,10 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
 							      file_data, j);
 
 	      if (this_real_name != NULL
-		  && FILENAME_CMP (real_path, this_real_name) == 0)
+		  && (FILENAME_CMP (real_path, this_real_name) == 0
+		      || (!is_abs
+			  && compare_filenames_for_search (this_real_name,
+							   name, name_len))))
 		{
 		  if (dw2_map_expand_apply (objfile, per_cu,
 					    name, full_path, real_path,
@@ -2476,14 +2487,6 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
 	}
     }
 
-  if (base_cu)
-    {
-      if (dw2_map_expand_apply (objfile, base_cu,
-				name, full_path, real_path,
-				callback, data))
-	return 1;
-    }
-
   return 0;
 }
 
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 1f9457f..998bc05 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -164,10 +164,14 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
 {
   struct partial_symtab *pst;
   const char *name_basename = lbasename (name);
+  int name_len = strlen (name);
+  int is_abs = IS_ABSOLUTE_PATH (name);
 
   ALL_OBJFILE_PSYMTABS_REQUIRED (objfile, pst)
   {
-    if (FILENAME_CMP (name, pst->filename) == 0)
+    if (FILENAME_CMP (name, pst->filename) == 0
+	|| (!is_abs && compare_filenames_for_search (pst->filename,
+						     name, name_len)))
       {
 	if (partial_map_expand_apply (objfile, name, full_path, real_path,
 				      pst, callback, data))
@@ -186,7 +190,9 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
       {
 	psymtab_to_fullname (pst);
 	if (pst->fullname != NULL
-	    && FILENAME_CMP (full_path, pst->fullname) == 0)
+	    && (FILENAME_CMP (full_path, pst->fullname) == 0
+		|| (!is_abs && compare_filenames_for_search (pst->fullname,
+							     name, name_len))))
 	  {
 	    if (partial_map_expand_apply (objfile, name, full_path, real_path,
 					  pst, callback, data))
@@ -203,7 +209,10 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
             rp = gdb_realpath (pst->fullname);
             make_cleanup (xfree, rp);
           }
-	if (rp != NULL && FILENAME_CMP (real_path, rp) == 0)
+	if (rp != NULL
+	    && (FILENAME_CMP (real_path, rp) == 0
+		|| (!is_abs && compare_filenames_for_search (real_path,
+							     name, name_len))))
 	  {
 	    if (partial_map_expand_apply (objfile, name, full_path, real_path,
 					  pst, callback, data))
@@ -212,17 +221,6 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
       }
   }
 
-  /* Now, search for a matching tail (only if name doesn't have any dirs).  */
-
-  if (name_basename == name)
-    ALL_OBJFILE_PSYMTABS_REQUIRED (objfile, pst)
-    {
-      if (FILENAME_CMP (lbasename (pst->filename), name) == 0)
-	if (partial_map_expand_apply (objfile, name, full_path, real_path, pst,
-				      callback, data))
-	  return 1;
-    }
-
   return 0;
 }
 
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 6d39162..6b664cd 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -153,7 +153,7 @@ struct quick_symbol_functions
   /* Expand and iterate over each "partial" symbol table in OBJFILE
      where the source file is named NAME.
 
-     If there is no '/' in the name, a match after a '/' in the symbol
+     If NAME is not absolute, a match after a '/' in the symbol
      table's file name will also work.  FULL_PATH is the absolute file
      name, and REAL_PATH is the same, run through gdb_realpath.
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index ab6a1aa..9471b81 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -145,6 +145,32 @@ multiple_symbols_select_mode (void)
 
 const struct block *block_found;
 
+/* See whether FILENAME matches SEARCH_NAME using the rule that we
+   advertise to the user.  SEARCH_LEN is the length of SEARCH_NAME.
+   We assume that SEARCH_NAME is a relative path.  Returns true if
+   they match, false otherwise.  */
+
+int
+compare_filenames_for_search (const char *filename, const char *search_name,
+			      int search_len)
+{
+  int len = strlen (filename);
+  int offset;
+
+  if (len < search_len)
+    return 0;
+
+  /* The tail of FILENAME must match.  */
+  if (FILENAME_CMP (filename + len - search_len, search_name) != 0)
+    return 0;
+
+  /* Either the names must completely match, or the character
+     preceding the trailing SEARCH_NAME segment of FILENAME must be a
+     directory separator.  */
+  return (len == search_len
+	  || IS_DIR_SEPARATOR (filename[len - search_len - 1]));
+}
+
 /* Check for a symtab of a specific name by searching some symtabs.
    This is a helper function for callbacks of iterate_over_symtabs.
 
@@ -169,15 +195,24 @@ iterate_over_some_symtabs (const char *name,
   struct symtab *s = NULL;
   struct cleanup *cleanup;
   const char* base_name = lbasename (name);
+  int name_len = strlen (name);
+  int is_abs = IS_ABSOLUTE_PATH (name);
 
   for (s = first; s != NULL && s != after_last; s = s->next)
     {
+      /* Exact match is always ok.  */
       if (FILENAME_CMP (name, s->filename) == 0)
 	{
 	  if (callback (s, data))
 	    return 1;
 	}
 
+      if (!is_abs && compare_filenames_for_search (s->filename, name, name_len))
+	{
+	  if (callback (s, data))
+	    return 1;
+	}
+
     /* Before we invoke realpath, which can get expensive when many
        files are involved, do a quick comparison of the basenames.  */
     if (! basenames_may_differ
@@ -196,6 +231,13 @@ iterate_over_some_symtabs (const char *name,
 	    if (callback (s, data))
 	      return 1;
           }
+
+	if (fp != NULL && !is_abs && compare_filenames_for_search (fp, name,
+								   name_len))
+	  {
+	    if (callback (s, data))
+	      return 1;
+	  }
       }
 
     if (real_path != NULL)
@@ -212,24 +254,16 @@ iterate_over_some_symtabs (const char *name,
 		if (callback (s, data))
 		  return 1;
 	      }
+
+	    if (!is_abs && compare_filenames_for_search (rp, name, name_len))
+	      {
+		if (callback (s, data))
+		  return 1;
+	      }
           }
       }
     }
 
-  /* Now, search for a matching tail (only if name doesn't have any dirs).  */
-
-  if (lbasename (name) == name)
-    {
-      for (s = first; s != NULL && s != after_last; s = s->next)
-	{
-	  if (FILENAME_CMP (lbasename (s->filename), name) == 0)
-	    {
-	      if (callback (s, data))
-		return 1;
-	    }
-	}
-    }
-
   return 0;
 }
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 7b96ab3..4836dd6 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1239,6 +1239,10 @@ struct objfile *lookup_objfile_from_block (const struct block *block);
 
 extern int basenames_may_differ;
 
+int compare_filenames_for_search (const char *filename,
+				  const char *search_name,
+				  int search_len);
+
 int iterate_over_some_symtabs (const char *name,
 			       const char *full_path,
 			       const char *real_path,
diff --git a/gdb/testsuite/gdb.linespec/linespec.exp b/gdb/testsuite/gdb.linespec/linespec.exp
index 4ac2fd8..02f796a 100644
--- a/gdb/testsuite/gdb.linespec/linespec.exp
+++ b/gdb/testsuite/gdb.linespec/linespec.exp
@@ -45,18 +45,19 @@ if {$l1 != $l2} {
 }
 
 gdb_test "break thefile.cc:$l1" \
-    "Breakpoint 1 at $hex: thefile.cc:$l1. \[(\]2 locations\[)\]" \
+    "Breakpoint $decimal at $hex: thefile.cc:$l1. \[(\]2 locations\[)\]" \
     "multi-location break using file:line"
 
-# We'd like this to work, but it currently does not.
-# gdb_test "break one/thefile.cc:$l1"
+gdb_test "break one/thefile.cc:$l1" \
+    "Breakpoint $decimal at $hex: file .*thefile.cc, line $l1." \
+    "single-location break using dir/file:line"
 
 gdb_test "break dupname" \
-    "Breakpoint 2 at $hex: dupname. \[(\]2 locations\[)\]" \
+    "Breakpoint $decimal at $hex: dupname. \[(\]2 locations\[)\]" \
     "multi-location break using duplicate function name"
 
 gdb_test "break dupname:label" \
-    "Breakpoint 3 at $hex: dupname:label. \[(\]2 locations\[)\]" \
+    "Breakpoint $decimal at $hex: dupname:label. \[(\]2 locations\[)\]" \
     "multi-location break using duplicate function name and label"
 
 gdb_test_no_output "set breakpoint pending off" \

Re: RFC: add relative file name handling for linespecs

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Date: Tue, 10 Jan 2012 09:06:08 -0700
> 
> This needs at least a doc review.

Thanks, the documentation parts are okay.

> +/* See whether FILENAME matches SEARCH_NAME using the rule that we
> +   advertise to the user.

Why not say explicitly how the names are matched?  "The rule we
advertise" doesn't give any hint where to look for that rule, so the
issue remains a mystery and requires to read the source to glean what
this does.

> +  /* Either the names must completely match, or the character
> +     preceding the trailing SEARCH_NAME segment of FILENAME must be a
> +     directory separator.  */
> +  return (len == search_len
> +	  || IS_DIR_SEPARATOR (filename[len - search_len - 1]));

What about the (somewhat perverse) case of "d:foo/bar" and "foo/bar"
on Windows?

Re: RFC: add relative file name handling for linespecs

[Joel Brobecker]

> 2012-01-10  Tom Tromey  <tromey@redhat.com>
> 
> 	* NEWS: Add item.
> 	* symtab.h (compare_filenames_for_search): Declare.
> 	* symtab.c (compare_filenames_for_search): New function.
> 	(iterate_over_some_symtabs): Use it.
> 	* symfile.h (struct quick_symbol_functions)
> 	<map_symtabs_matching_filename>: Change spec.
> 	* psymtab.c (partial_map_symtabs_matching_filename): Use
> 	compare_filenames_for_search.  Update for new spec.
> 	* dwarf2read.c (dw2_map_symtabs_matching_filename): Use
> 	compare_filenames_for_search.  Update for new spec.

FWIW, the patch looks good to me.  I think that this is an awesome
enhancement.

One thing I noticed is that we might want to apply the same treatement
to the "clear" command... It uses the filename_cmp function directly,
instead of using the FILENAME_CMP macro.

-- 
Joel

Re: RFC: add relative file name handling for linespecs

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Tom> +/* See whether FILENAME matches SEARCH_NAME using the rule that we
Tom> +   advertise to the user.

Eli> Why not say explicitly how the names are matched?  "The rule we
Eli> advertise" doesn't give any hint where to look for that rule, so the
Eli> issue remains a mystery and requires to read the source to glean what
Eli> this does.

I will mention the manual.

Tom> +  /* Either the names must completely match, or the character
Tom> +     preceding the trailing SEARCH_NAME segment of FILENAME must be a
Tom> +     directory separator.  */
Tom> +  return (len == search_len
Tom> +	  || IS_DIR_SEPARATOR (filename[len - search_len - 1]));

Eli> What about the (somewhat perverse) case of "d:foo/bar" and "foo/bar"
Eli> on Windows?

Would something like this work?

  || (HAS_DRIVE_SPEC (filename) && len - search_len == 2)

Tom

Re: RFC: add relative file name handling for linespecs

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 11 Jan 2012 14:49:37 -0700
> 
> Eli> What about the (somewhat perverse) case of "d:foo/bar" and "foo/bar"
> Eli> on Windows?
> 
> Would something like this work?
> 
>   || (HAS_DRIVE_SPEC (filename) && len - search_len == 2)

Yes, I think it will.

Thanks.

Re: RFC: add relative file name handling for linespecs

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Tom> || (HAS_DRIVE_SPEC (filename) && len - search_len == 2)

Eli> Yes, I think it will.

I ended up going with:

	  || (HAS_DRIVE_SPEC (filename)
	      && STRIP_DRIVE_SPEC (filename) == &filename[len - search_len]));

... because STRIP_DRIVE_SPEC exists already and this lets us avoid
hard-coding a "2" into the code here.

Tom

Re: RFC: add relative file name handling for linespecs

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> One thing I noticed is that we might want to apply the same treatement
Joel> to the "clear" command... It uses the filename_cmp function directly,
Joel> instead of using the FILENAME_CMP macro.

Thanks for noticing this.
I fixed this in my latest; I'm re-testing it all now.

Tom

Re: RFC: add relative file name handling for linespecs

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> This patch changes gdb to allow relative file names to match a source
Tom> file name if all the trailing components match.  Previously this
Tom> behavior was only allowed if the source file name did not have any
Tom> directory separators.

Here is the version I am checking in.

Built and regtested on x86-64 F15.  Some new tests included.

Tom

2012-01-16  Tom Tromey  <tromey@redhat.com>

	* NEWS: Add item.
	* symtab.h (compare_filenames_for_search): Declare.
	* symtab.c (compare_filenames_for_search): New function.
	(iterate_over_some_symtabs): Use it.
	* symfile.h (struct quick_symbol_functions)
	<map_symtabs_matching_filename>: Change spec.
	* psymtab.c (partial_map_symtabs_matching_filename): Use
	compare_filenames_for_search.  Update for new spec.
	* dwarf2read.c (dw2_map_symtabs_matching_filename): Use
	compare_filenames_for_search.  Update for new spec.
	* breakpoint.c (clear_command): Use compare_filenames_for_search.

2012-01-16  Tom Tromey  <tromey@redhat.com>

	* gdb.texinfo (Specify Location): Document relative file name
	handling.

2012-01-16  Tom Tromey  <tromey@redhat.com>

	* gdb.linespec/linespec.exp: Change some tests to use $decimal.
	Add tests for relative directory.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.480
diff -u -r1.480 NEWS
--- NEWS	16 Jan 2012 19:44:13 -0000	1.480
+++ NEWS	16 Jan 2012 20:39:16 -0000
@@ -25,6 +25,11 @@
   ** A new class, gdb.printing.FlagEnumerationPrinter, can be used to
      apply "flag enum"-style pretty-printing to any enum.
 
+* The filename part of a linespec will now match trailing components
+  of a source file name.  For example, "break gcc/expr.c:1000" will
+  now set a breakpoint in build/gcc/expr.c, but not
+  build/libcpp/expr.c.
+
 *** Changes in GDB 7.4
 
 * GDB now handles ambiguous linespecs more consistently; the existing
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.641
diff -u -r1.641 breakpoint.c
--- breakpoint.c	16 Jan 2012 17:44:35 -0000	1.641
+++ breakpoint.c	16 Jan 2012 20:39:17 -0000
@@ -10070,6 +10070,8 @@
   make_cleanup (VEC_cleanup (breakpoint_p), &found);
   for (i = 0; i < sals.nelts; i++)
     {
+      int is_abs, sal_name_len;
+
       /* If exact pc given, clear bpts at that pc.
          If line given (pc == 0), clear all bpts on specified line.
          If defaulting, clear all bpts on default line
@@ -10083,6 +10085,8 @@
          1              0             <can't happen> */
 
       sal = sals.sals[i];
+      is_abs = sal.symtab == NULL ? 1 : IS_ABSOLUTE_PATH (sal.symtab->filename);
+      sal_name_len = is_abs ? 0 : strlen (sal.symtab->filename);
 
       /* Find all matching breakpoints and add them to 'found'.  */
       ALL_BREAKPOINTS (b)
@@ -10102,13 +10106,24 @@
 				  && (loc->address == sal.pc)
 				  && (!section_is_overlay (loc->section)
 				      || loc->section == sal.section));
-		  int line_match = ((default_match || sal.explicit_line)
-				    && loc->source_file != NULL
-				    && sal.symtab != NULL
-				    && sal.pspace == loc->pspace
-				    && filename_cmp (loc->source_file,
-						     sal.symtab->filename) == 0
-				    && loc->line_number == sal.line);
+		  int line_match = 0;
+
+		  if ((default_match || sal.explicit_line)
+		      && loc->source_file != NULL
+		      && sal.symtab != NULL
+		      && sal.pspace == loc->pspace
+		      && loc->line_number == sal.line)
+		    {
+		      if (filename_cmp (loc->source_file,
+					sal.symtab->filename) == 0)
+			line_match = 1;
+		      else if (!IS_ABSOLUTE_PATH (sal.symtab->filename)
+			       && compare_filenames_for_search (loc->source_file,
+								sal.symtab->filename,
+								sal_name_len))
+			line_match = 1;
+		    }
+
 		  if (pc_match || line_match)
 		    {
 		      match = 1;
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.607
diff -u -r1.607 dwarf2read.c
--- dwarf2read.c	16 Jan 2012 19:44:13 -0000	1.607
+++ dwarf2read.c	16 Jan 2012 20:39:18 -0000
@@ -2403,8 +2403,8 @@
 {
   int i;
   const char *name_basename = lbasename (name);
-  int check_basename = name_basename == name;
-  struct dwarf2_per_cu_data *base_cu = NULL;
+  int name_len = strlen (name);
+  int is_abs = IS_ABSOLUTE_PATH (name);
 
   dw2_setup (objfile);
 
@@ -2427,7 +2427,9 @@
 	{
 	  const char *this_name = file_data->file_names[j];
 
-	  if (FILENAME_CMP (name, this_name) == 0)
+	  if (FILENAME_CMP (name, this_name) == 0
+	      || (!is_abs && compare_filenames_for_search (this_name,
+							   name, name_len)))
 	    {
 	      if (dw2_map_expand_apply (objfile, per_cu,
 					name, full_path, real_path,
@@ -2435,9 +2437,12 @@
 		return 1;
 	    }
 
-	  if (check_basename && ! base_cu
-	      && FILENAME_CMP (lbasename (this_name), name) == 0)
-	    base_cu = per_cu;
+	    {
+	      if (dw2_map_expand_apply (objfile, per_cu,
+					name, full_path, real_path,
+					callback, data))
+		return 1;
+	    }
 
 	  /* Before we invoke realpath, which can get expensive when many
 	     files are involved, do a quick comparison of the basenames.  */
@@ -2451,7 +2456,10 @@
 							      file_data, j);
 
 	      if (this_real_name != NULL
-		  && FILENAME_CMP (full_path, this_real_name) == 0)
+		  && (FILENAME_CMP (full_path, this_real_name) == 0
+		      || (!is_abs
+			  && compare_filenames_for_search (this_real_name,
+							   name, name_len))))
 		{
 		  if (dw2_map_expand_apply (objfile, per_cu,
 					    name, full_path, real_path,
@@ -2466,7 +2474,10 @@
 							      file_data, j);
 
 	      if (this_real_name != NULL
-		  && FILENAME_CMP (real_path, this_real_name) == 0)
+		  && (FILENAME_CMP (real_path, this_real_name) == 0
+		      || (!is_abs
+			  && compare_filenames_for_search (this_real_name,
+							   name, name_len))))
 		{
 		  if (dw2_map_expand_apply (objfile, per_cu,
 					    name, full_path, real_path,
@@ -2477,14 +2488,6 @@
 	}
     }
 
-  if (base_cu)
-    {
-      if (dw2_map_expand_apply (objfile, base_cu,
-				name, full_path, real_path,
-				callback, data))
-	return 1;
-    }
-
   return 0;
 }
 
Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.38
diff -u -r1.38 psymtab.c
--- psymtab.c	4 Jan 2012 08:17:09 -0000	1.38
+++ psymtab.c	16 Jan 2012 20:39:18 -0000
@@ -164,10 +164,14 @@
 {
   struct partial_symtab *pst;
   const char *name_basename = lbasename (name);
+  int name_len = strlen (name);
+  int is_abs = IS_ABSOLUTE_PATH (name);
 
   ALL_OBJFILE_PSYMTABS_REQUIRED (objfile, pst)
   {
-    if (FILENAME_CMP (name, pst->filename) == 0)
+    if (FILENAME_CMP (name, pst->filename) == 0
+	|| (!is_abs && compare_filenames_for_search (pst->filename,
+						     name, name_len)))
       {
 	if (partial_map_expand_apply (objfile, name, full_path, real_path,
 				      pst, callback, data))
@@ -186,7 +190,9 @@
       {
 	psymtab_to_fullname (pst);
 	if (pst->fullname != NULL
-	    && FILENAME_CMP (full_path, pst->fullname) == 0)
+	    && (FILENAME_CMP (full_path, pst->fullname) == 0
+		|| (!is_abs && compare_filenames_for_search (pst->fullname,
+							     name, name_len))))
 	  {
 	    if (partial_map_expand_apply (objfile, name, full_path, real_path,
 					  pst, callback, data))
@@ -203,7 +209,10 @@
             rp = gdb_realpath (pst->fullname);
             make_cleanup (xfree, rp);
           }
-	if (rp != NULL && FILENAME_CMP (real_path, rp) == 0)
+	if (rp != NULL
+	    && (FILENAME_CMP (real_path, rp) == 0
+		|| (!is_abs && compare_filenames_for_search (real_path,
+							     name, name_len))))
 	  {
 	    if (partial_map_expand_apply (objfile, name, full_path, real_path,
 					  pst, callback, data))
@@ -212,17 +221,6 @@
       }
   }
 
-  /* Now, search for a matching tail (only if name doesn't have any dirs).  */
-
-  if (name_basename == name)
-    ALL_OBJFILE_PSYMTABS_REQUIRED (objfile, pst)
-    {
-      if (FILENAME_CMP (lbasename (pst->filename), name) == 0)
-	if (partial_map_expand_apply (objfile, name, full_path, real_path, pst,
-				      callback, data))
-	  return 1;
-    }
-
   return 0;
 }
 
Index: symfile.h
===================================================================
RCS file: /cvs/src/src/gdb/symfile.h,v
retrieving revision 1.102
diff -u -r1.102 symfile.h
--- symfile.h	4 Jan 2012 08:17:13 -0000	1.102
+++ symfile.h	16 Jan 2012 20:39:18 -0000
@@ -153,7 +153,7 @@
   /* Expand and iterate over each "partial" symbol table in OBJFILE
      where the source file is named NAME.
 
-     If there is no '/' in the name, a match after a '/' in the symbol
+     If NAME is not absolute, a match after a '/' in the symbol
      table's file name will also work.  FULL_PATH is the absolute file
      name, and REAL_PATH is the same, run through gdb_realpath.
 
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.289
diff -u -r1.289 symtab.c
--- symtab.c	4 Jan 2012 08:17:13 -0000	1.289
+++ symtab.c	16 Jan 2012 20:39:19 -0000
@@ -145,6 +145,35 @@
 
 const struct block *block_found;
 
+/* See whether FILENAME matches SEARCH_NAME using the rule that we
+   advertise to the user.  (The manual's description of linespecs
+   describes what we advertise).  SEARCH_LEN is the length of
+   SEARCH_NAME.  We assume that SEARCH_NAME is a relative path.
+   Returns true if they match, false otherwise.  */
+
+int
+compare_filenames_for_search (const char *filename, const char *search_name,
+			      int search_len)
+{
+  int len = strlen (filename);
+  int offset;
+
+  if (len < search_len)
+    return 0;
+
+  /* The tail of FILENAME must match.  */
+  if (FILENAME_CMP (filename + len - search_len, search_name) != 0)
+    return 0;
+
+  /* Either the names must completely match, or the character
+     preceding the trailing SEARCH_NAME segment of FILENAME must be a
+     directory separator.  */
+  return (len == search_len
+	  || IS_DIR_SEPARATOR (filename[len - search_len - 1])
+	  || (HAS_DRIVE_SPEC (filename)
+	      && STRIP_DRIVE_SPEC (filename) == &filename[len - search_len]));
+}
+
 /* Check for a symtab of a specific name by searching some symtabs.
    This is a helper function for callbacks of iterate_over_symtabs.
 
@@ -169,15 +198,24 @@
   struct symtab *s = NULL;
   struct cleanup *cleanup;
   const char* base_name = lbasename (name);
+  int name_len = strlen (name);
+  int is_abs = IS_ABSOLUTE_PATH (name);
 
   for (s = first; s != NULL && s != after_last; s = s->next)
     {
+      /* Exact match is always ok.  */
       if (FILENAME_CMP (name, s->filename) == 0)
 	{
 	  if (callback (s, data))
 	    return 1;
 	}
 
+      if (!is_abs && compare_filenames_for_search (s->filename, name, name_len))
+	{
+	  if (callback (s, data))
+	    return 1;
+	}
+
     /* Before we invoke realpath, which can get expensive when many
        files are involved, do a quick comparison of the basenames.  */
     if (! basenames_may_differ
@@ -196,6 +234,13 @@
 	    if (callback (s, data))
 	      return 1;
           }
+
+	if (fp != NULL && !is_abs && compare_filenames_for_search (fp, name,
+								   name_len))
+	  {
+	    if (callback (s, data))
+	      return 1;
+	  }
       }
 
     if (real_path != NULL)
@@ -212,24 +257,16 @@
 		if (callback (s, data))
 		  return 1;
 	      }
+
+	    if (!is_abs && compare_filenames_for_search (rp, name, name_len))
+	      {
+		if (callback (s, data))
+		  return 1;
+	      }
           }
       }
     }
 
-  /* Now, search for a matching tail (only if name doesn't have any dirs).  */
-
-  if (lbasename (name) == name)
-    {
-      for (s = first; s != NULL && s != after_last; s = s->next)
-	{
-	  if (FILENAME_CMP (lbasename (s->filename), name) == 0)
-	    {
-	      if (callback (s, data))
-		return 1;
-	    }
-	}
-    }
-
   return 0;
 }
 
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.197
diff -u -r1.197 symtab.h
--- symtab.h	4 Jan 2012 08:17:13 -0000	1.197
+++ symtab.h	16 Jan 2012 20:39:19 -0000
@@ -1239,6 +1239,10 @@
 
 extern int basenames_may_differ;
 
+int compare_filenames_for_search (const char *filename,
+				  const char *search_name,
+				  int search_len);
+
 int iterate_over_some_symtabs (const char *name,
 			       const char *full_path,
 			       const char *real_path,
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.912
diff -u -r1.912 gdb.texinfo
--- doc/gdb.texinfo	16 Jan 2012 19:44:14 -0000	1.912
+++ doc/gdb.texinfo	16 Jan 2012 20:39:24 -0000
@@ -6492,6 +6492,11 @@
 
 @item @var{filename}:@var{linenum}
 Specifies the line @var{linenum} in the source file @var{filename}.
+If @var{filename} is a relative file name, then it will match any
+source file name with the same trailing components.  For example, if
+@var{filename} is @samp{gcc/expr.c}, then it will match source file
+name of @file{/build/trunk/gcc/expr.c}, but not
+@file{/build/trunk/libcpp/expr.c} or @file{/build/trunk/gcc/x-expr.c}.
 
 @item @var{function}
 Specifies the line that begins the body of the function @var{function}.
Index: testsuite/gdb.linespec/linespec.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.linespec/linespec.exp,v
retrieving revision 1.3
diff -u -r1.3 linespec.exp
--- testsuite/gdb.linespec/linespec.exp	4 Jan 2012 08:17:54 -0000	1.3
+++ testsuite/gdb.linespec/linespec.exp	16 Jan 2012 20:39:27 -0000
@@ -44,19 +44,24 @@
     error "somebody incompatibly modified the source files needed by linespec.exp"
 }
 
+gdb_test "break one/thefile.cc:$l1" \
+    "Breakpoint $decimal at $hex: file .*thefile.cc, line $l1." \
+    "single-location break using dir/file:line"
+
+gdb_test "clear one/thefile.cc:$l1" \
+    "Deleted breakpoint $decimal *" \
+    "clear breakpoint using dir/file:line"
+
 gdb_test "break thefile.cc:$l1" \
-    "Breakpoint 1 at $hex: thefile.cc:$l1. \[(\]2 locations\[)\]" \
+    "Breakpoint $decimal at $hex: thefile.cc:$l1. \[(\]2 locations\[)\]" \
     "multi-location break using file:line"
 
-# We'd like this to work, but it currently does not.
-# gdb_test "break one/thefile.cc:$l1"
-
 gdb_test "break dupname" \
-    "Breakpoint 2 at $hex: dupname. \[(\]2 locations\[)\]" \
+    "Breakpoint $decimal at $hex: dupname. \[(\]2 locations\[)\]" \
     "multi-location break using duplicate function name"
 
 gdb_test "break dupname:label" \
-    "Breakpoint 3 at $hex: dupname:label. \[(\]2 locations\[)\]" \
+    "Breakpoint $decimal at $hex: dupname:label. \[(\]2 locations\[)\]" \
     "multi-location break using duplicate function name and label"
 
 gdb_test_no_output "set breakpoint pending off" \

Performance regression (12x): Re: RFC: add relative file name handling for linespecs

[Jan Kratochvil]

Hi Tom,

I have noticed nightly regression testing to start randomly timing out a lot
for gdb.ada/* testcases, narrowed it down to this check-in.

The failures are not reproducible standalone but they happen during the heavy
nigly load, still the testcase runtime is proving enough something is wrong.


Thanks,
Jan


On Mon, 16 Jan 2012 21:40:05 +0100, Tom Tromey wrote:
> Here is the version I am checking in.

commit 024e0e8b29c57131c65424b31e931e6f8f511248
Author: Tom Tromey <tromey@redhat.com>
Date:   Mon Jan 16 20:40:47 2012 +0000

    gdb
        * NEWS: Add item.
        * symtab.h (compare_filenames_for_search): Declare.
        * symtab.c (compare_filenames_for_search): New function.
        (iterate_over_some_symtabs): Use it.
        * symfile.h (struct quick_symbol_functions)
        <map_symtabs_matching_filename>: Change spec.
        * psymtab.c (partial_map_symtabs_matching_filename): Use
        compare_filenames_for_search.  Update for new spec.
        * dwarf2read.c (dw2_map_symtabs_matching_filename): Use
        compare_filenames_for_search.  Update for new spec.
        * breakpoint.c (clear_command): Use compare_filenames_for_search.
    gdb/doc
        * gdb.texinfo (Specify Location): Document relative file name
        handling.
    gdb/testsuite
        * gdb.linespec/linespec.exp: Change some tests to use $decimal.
        Add tests for relative directory.

sync;time runtest gdb.ada/tasks.exp
024e0e8b29c57131c65424b31e931e6f8f511248 0m6.412s
eb28de4a09f016d3de7caccec781eb70995a4001 0m0.538s

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

[Tom Tromey]

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

Jan> The failures are not reproducible standalone but they happen during
Jan> the heavy nigly load, still the testcase runtime is proving enough
Jan> something is wrong.

I will investigate it soon.

Tom

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

[Tom Tromey]

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

Jan> I have noticed nightly regression testing to start randomly timing
Jan> out a lot for gdb.ada/* testcases, narrowed it down to this
Jan> check-in.

I looked into this.

I still don't know why my patch exposed this problem, but I don't think
it is actually particular to my patch -- instead, I think it is a latent
bug that the patch exposed.

Profiling gdb on gdb.ada/tasks.exp showed:

 time   seconds   seconds    calls  ms/call  ms/call  name    
 60.67      3.07     3.07 80168399     0.00     0.00  advance_wild_match
  9.49      3.55     0.48  1797312     0.00     0.00  match_partial_symbol
  8.50      3.98     0.43 22218321     0.00     0.00  wild_match
[...]

I tracked this down to ada_iterate_over_symbols calling
ada_lookup_symbol_list.  The former expects to iterate upwards starting
from a given block; but the latter will search all top-level blocks of
all objfiles if no local symbol matches.  This means we are doing an N^2
search, because linespec is also doing this sort of iteration.

This fix seems to fix the issue.  The resulting gdb is still a bit
slower than the status quo ante, but not absurdly so.  The profile now
looks like:

 11.11      0.03     0.03    40318     0.00     0.00  d_demangle
  7.41      0.05     0.02   424963     0.00     0.00  advance_wild_match
  7.41      0.07     0.02   351053     0.00     0.00  hash_continue
[...]


Built and regtested on x86-64 Fedora 15.

Joel, is this ok with you?

Tom

2012-01-31  Tom Tromey  <tromey@redhat.com>

	* ada-lang.c (ada_lookup_symbol_list_full): New function, from
	ada_lookup_symbol_list.  Add 'full_search' argument.
	(ada_lookup_symbol_list): Rewrite in terms of
	ada_lookup_symbol_list_full.
	(ada_iterate_over_symbols): Use ada_lookup_symbol_list_full.

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 55e318f..089f666 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4979,16 +4979,18 @@ add_nonlocal_symbols (struct obstack *obstackp, const char *name,
    the next call of ada_lookup_symbol_list.  Any non-function/non-enumeral 
    symbol match within the nest of blocks whose innermost member is BLOCK0,
    is the one match returned (no other matches in that or
-     enclosing blocks is returned).  If there are any matches in or
-   surrounding BLOCK0, then these alone are returned.  Otherwise, the
-   search extends to global and file-scope (static) symbol tables.
+   enclosing blocks is returned).  If there are any matches in or
+   surrounding BLOCK0, then these alone are returned.  Otherwise, if
+   FULL_SEARCH is non-zero, then the search extends to global and
+   file-scope (static) symbol tables.
    Names prefixed with "standard__" are handled specially: "standard__" 
    is first stripped off, and only static and global symbols are searched.  */
 
-int
-ada_lookup_symbol_list (const char *name0, const struct block *block0,
-                        domain_enum namespace,
-                        struct ada_symbol_info **results)
+static int
+ada_lookup_symbol_list_full (const char *name0, const struct block *block0,
+			     domain_enum namespace,
+			     struct ada_symbol_info **results,
+			     int full_search)
 {
   struct symbol *sym;
   struct block *block;
@@ -5026,7 +5028,7 @@ ada_lookup_symbol_list (const char *name0, const struct block *block0,
 
   ada_add_local_symbols (&symbol_list_obstack, name, block, namespace,
                          wild_match);
-  if (num_defns_collected (&symbol_list_obstack) > 0)
+  if (num_defns_collected (&symbol_list_obstack) > 0 || !full_search)
     goto done;
 
   /* No non-global symbols found.  Check our cache to see if we have
@@ -5070,6 +5072,17 @@ done:
   return ndefns;
 }
 
+/* A wrapper for ada_lookup_symbol_list_full that sets
+   FULL_SEARCH=1.  */
+
+int
+ada_lookup_symbol_list (const char *name0, const struct block *block0,
+			domain_enum namespace,
+			struct ada_symbol_info **results)
+{
+  return ada_lookup_symbol_list_full (name0, block0, namespace, results, 1);
+}
+
 /* If NAME is the name of an entity, return a string that should
    be used to look that entity up in Ada units.  This string should
    be deallocated after use using xfree.
@@ -5106,7 +5119,7 @@ ada_iterate_over_symbols (const struct block *block,
   int ndefs, i;
   struct ada_symbol_info *results;
 
-  ndefs = ada_lookup_symbol_list (name, block, domain, &results);
+  ndefs = ada_lookup_symbol_list_full (name, block, domain, &results, 0);
   for (i = 0; i < ndefs; ++i)
     {
       if (! (*callback) (results[i].sym, data))

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

[Joel Brobecker]

Hi Tom,

> Jan> I have noticed nightly regression testing to start randomly timing
> Jan> out a lot for gdb.ada/* testcases, narrowed it down to this
> Jan> check-in.
> 
> I looked into this.

I missed this message! You should have told me and I would have looked
into it.

> 2012-01-31  Tom Tromey  <tromey@redhat.com>
> 
> 	* ada-lang.c (ada_lookup_symbol_list_full): New function, from
> 	ada_lookup_symbol_list.  Add 'full_search' argument.
> 	(ada_lookup_symbol_list): Rewrite in terms of
> 	ada_lookup_symbol_list_full.
> 	(ada_iterate_over_symbols): Use ada_lookup_symbol_list_full.

Thanks a lot for the patch. I agree that the patch looks correct,
but I'm a little confused by the name of the new function (the _full
suffix suggests that the lookup is always a full search).

Traditionally, we've used a _1 suffix instead, which is very
unimaginative and yet so convenient.  I was going to change
your patch this way, but in the end, I'm just thinking I should
just add the parameter and update all callers. There are not that
many. I suspect you might have thought about this too, only to
just try the minimum change.

Would it be OK with you if I made that change instead?

-- 
Joel

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> I missed this message! You should have told me and I would have looked
Joel> into it.

Oh, sure, now you tell me ;-)

Joel> Thanks a lot for the patch. I agree that the patch looks correct,
Joel> but I'm a little confused by the name of the new function (the _full
Joel> suffix suggests that the lookup is always a full search).

Joel> Traditionally, we've used a _1 suffix instead, which is very
Joel> unimaginative and yet so convenient.  I was going to change
Joel> your patch this way, but in the end, I'm just thinking I should
Joel> just add the parameter and update all callers. There are not that
Joel> many. I suspect you might have thought about this too, only to
Joel> just try the minimum change.

Joel> Would it be OK with you if I made that change instead?

I will do it, it is no big deal.

Tom

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> [...], but in the end, I'm just thinking I should just add the
Joel> parameter and update all callers.

Here's the patch.  Let me know what you think.  I'm running regression
tests on it now, but I don't expect any problems.

Tom

2012-02-01  Tom Tromey  <tromey@redhat.com>

	* ada-lang.c (resolve_subexp): Update.
	(ada_lookup_symbol_list): Add 'full_search' argument.
	(ada_iterate_over_symbols): Pass 0 as full_search argument to
	ada_lookup_symbol_list.
	(ada_lookup_encoded_symbol): Update.
	(get_var_value): Update.
	* ada-exp.y (block_lookup): Update.
	(write_var_or_type): Update.
	(write_name_assoc): Update.
	* ada-lang.h (ada_lookup_symbol_list): Update.

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index d8c21a4..d1c7367 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -1021,7 +1021,7 @@ block_lookup (struct block *context, char *raw_name)
   else
     name = ada_encode (raw_name);
 
-  nsyms = ada_lookup_symbol_list (name, context, VAR_DOMAIN, &syms);
+  nsyms = ada_lookup_symbol_list (name, context, VAR_DOMAIN, &syms, 1);
   if (context == NULL
       && (nsyms == 0 || SYMBOL_CLASS (syms[0].sym) != LOC_BLOCK))
     symtab = lookup_symtab (name);
@@ -1278,7 +1278,7 @@ write_var_or_type (struct block *block, struct stoken name0)
 
 	  encoded_name[tail_index] = '\0';
 	  nsyms = ada_lookup_symbol_list (encoded_name, block,
-					  VAR_DOMAIN, &syms);
+					  VAR_DOMAIN, &syms, 1);
 	  encoded_name[tail_index] = terminator;
 
 	  /* A single symbol may rename a package or object. */
@@ -1427,7 +1427,7 @@ write_name_assoc (struct stoken name)
     {
       struct ada_symbol_info *syms;
       int nsyms = ada_lookup_symbol_list (name.ptr, expression_context_block,
-					  VAR_DOMAIN, &syms);
+					  VAR_DOMAIN, &syms, 1);
       if (nsyms != 1 || SYMBOL_CLASS (syms[0].sym) == LOC_TYPEDEF)
 	write_exp_op_with_string (OP_NAME, name);
       else
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 55e318f..de165fa 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -3057,7 +3057,7 @@ resolve_subexp (struct expression **expp, int *pos, int deprocedure_p,
             ada_lookup_symbol_list (SYMBOL_LINKAGE_NAME
                                     (exp->elts[pc + 2].symbol),
                                     exp->elts[pc + 1].block, VAR_DOMAIN,
-                                    &candidates);
+                                    &candidates, 1);
 
           if (n_candidates > 1)
             {
@@ -3149,7 +3149,7 @@ resolve_subexp (struct expression **expp, int *pos, int deprocedure_p,
               ada_lookup_symbol_list (SYMBOL_LINKAGE_NAME
                                       (exp->elts[pc + 5].symbol),
                                       exp->elts[pc + 4].block, VAR_DOMAIN,
-                                      &candidates);
+                                      &candidates, 1);
             if (n_candidates == 1)
               i = 0;
             else
@@ -3201,7 +3201,7 @@ resolve_subexp (struct expression **expp, int *pos, int deprocedure_p,
           n_candidates =
             ada_lookup_symbol_list (ada_encode (ada_decoded_op_name (op)),
                                     (struct block *) NULL, VAR_DOMAIN,
-                                    &candidates);
+                                    &candidates, 1);
           i = ada_resolve_function (candidates, n_candidates, argvec, nargs,
                                     ada_decoded_op_name (op), NULL);
           if (i < 0)
@@ -4979,16 +4979,18 @@ add_nonlocal_symbols (struct obstack *obstackp, const char *name,
    the next call of ada_lookup_symbol_list.  Any non-function/non-enumeral 
    symbol match within the nest of blocks whose innermost member is BLOCK0,
    is the one match returned (no other matches in that or
-     enclosing blocks is returned).  If there are any matches in or
-   surrounding BLOCK0, then these alone are returned.  Otherwise, the
-   search extends to global and file-scope (static) symbol tables.
+   enclosing blocks is returned).  If there are any matches in or
+   surrounding BLOCK0, then these alone are returned.  Otherwise, if
+   FULL_SEARCH is non-zero, then the search extends to global and
+   file-scope (static) symbol tables.
    Names prefixed with "standard__" are handled specially: "standard__" 
    is first stripped off, and only static and global symbols are searched.  */
 
 int
 ada_lookup_symbol_list (const char *name0, const struct block *block0,
-                        domain_enum namespace,
-                        struct ada_symbol_info **results)
+			domain_enum namespace,
+			struct ada_symbol_info **results,
+			int full_search)
 {
   struct symbol *sym;
   struct block *block;
@@ -5026,7 +5028,7 @@ ada_lookup_symbol_list (const char *name0, const struct block *block0,
 
   ada_add_local_symbols (&symbol_list_obstack, name, block, namespace,
                          wild_match);
-  if (num_defns_collected (&symbol_list_obstack) > 0)
+  if (num_defns_collected (&symbol_list_obstack) > 0 || !full_search)
     goto done;
 
   /* No non-global symbols found.  Check our cache to see if we have
@@ -5106,7 +5108,7 @@ ada_iterate_over_symbols (const struct block *block,
   int ndefs, i;
   struct ada_symbol_info *results;
 
-  ndefs = ada_lookup_symbol_list (name, block, domain, &results);
+  ndefs = ada_lookup_symbol_list (name, block, domain, &results, 0);
   for (i = 0; i < ndefs; ++i)
     {
       if (! (*callback) (results[i].sym, data))
@@ -5121,7 +5123,8 @@ ada_lookup_encoded_symbol (const char *name, const struct block *block0,
   struct ada_symbol_info *candidates;
   int n_candidates;
 
-  n_candidates = ada_lookup_symbol_list (name, block0, namespace, &candidates);
+  n_candidates = ada_lookup_symbol_list (name, block0, namespace, &candidates,
+					 1);
 
   if (n_candidates == 0)
     return NULL;
@@ -10359,7 +10362,7 @@ get_var_value (char *name, char *err_msg)
   int nsyms;
 
   nsyms = ada_lookup_symbol_list (name, get_selected_block (0), VAR_DOMAIN,
-                                  &syms);
+                                  &syms, 1);
 
   if (nsyms != 1)
     {
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 9ab7b2d..375fab0 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -219,7 +219,8 @@ extern enum language ada_update_initial_language (enum language);
 extern void clear_ada_sym_cache (void);
 
 extern int ada_lookup_symbol_list (const char *, const struct block *,
-                                   domain_enum, struct ada_symbol_info**);
+                                   domain_enum, struct ada_symbol_info**,
+				   int);
 
 extern char *ada_fold_name (const char *);
 

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

[Joel Brobecker]

> Joel> [...], but in the end, I'm just thinking I should just add the
> Joel> parameter and update all callers.
> 
> Here's the patch.  Let me know what you think.  I'm running regression
> tests on it now, but I don't expect any problems.

Thank you!

> 2012-02-01  Tom Tromey  <tromey@redhat.com>
> 
> 	* ada-lang.c (resolve_subexp): Update.
> 	(ada_lookup_symbol_list): Add 'full_search' argument.
> 	(ada_iterate_over_symbols): Pass 0 as full_search argument to
> 	ada_lookup_symbol_list.
> 	(ada_lookup_encoded_symbol): Update.
> 	(get_var_value): Update.
> 	* ada-exp.y (block_lookup): Update.
> 	(write_var_or_type): Update.
> 	(write_name_assoc): Update.
> 	* ada-lang.h (ada_lookup_symbol_list): Update.

This does look good to me.

-- 
Joel

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This does look good to me.

I'll check it in shortly.

Tom

Re: RFC: add relative file name handling for linespecs

[Jan Kratochvil]

Hi Eli,

On Wed, 11 Jan 2012 22:49:37 +0100, Tom Tromey wrote:
> Tom> +  /* Either the names must completely match, or the character
> Tom> +     preceding the trailing SEARCH_NAME segment of FILENAME must be a
> Tom> +     directory separator.  */
> Tom> +  return (len == search_len
> Tom> +	  || IS_DIR_SEPARATOR (filename[len - search_len - 1]));
> 
> Eli> What about the (somewhat perverse) case of "d:foo/bar" and "foo/bar"
> Eli> on Windows?
> 
> Would something like this work?
> 
>   || (HAS_DRIVE_SPEC (filename) && len - search_len == 2)

FILENAME here is from the debug info file (not from the user), I believe on
MinGW the path from DWARF is absolute (like in UNIX) and it has the format
d:\foo\bar (not d:foo\bar), doesn't it?

Whether user can / cannot then specify d:foo/bar is unrelated to the patch Tom
did above.  It could be implemented but it would be a different code/patch.

Not that it would be a bug but the current code there
          || (HAS_DRIVE_SPEC (filename)
              && STRIP_DRIVE_SPEC (filename) == &filename[len - search_len]));
can IMO never match on real world MinGW DWARF files.


Thanks,
Jan

Re: RFC: add relative file name handling for linespecs

[Eli Zaretskii]

> Date: Sat, 8 Dec 2012 17:34:07 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org, Tom Tromey <tromey@redhat.com>
> 
> Hi Eli,
> 
> On Wed, 11 Jan 2012 22:49:37 +0100, Tom Tromey wrote:
> > Tom> +  /* Either the names must completely match, or the character
> > Tom> +     preceding the trailing SEARCH_NAME segment of FILENAME must be a
> > Tom> +     directory separator.  */
> > Tom> +  return (len == search_len
> > Tom> +	  || IS_DIR_SEPARATOR (filename[len - search_len - 1]));
> > 
> > Eli> What about the (somewhat perverse) case of "d:foo/bar" and "foo/bar"
> > Eli> on Windows?
> > 
> > Would something like this work?
> > 
> >   || (HAS_DRIVE_SPEC (filename) && len - search_len == 2)
> 
> FILENAME here is from the debug info file (not from the user), I believe on
> MinGW the path from DWARF is absolute (like in UNIX) and it has the format
> d:\foo\bar (not d:foo\bar), doesn't it?

If this comes only from debug info, I guess you are right, although it
would be good to make sure GCC never writes such names into debug info.

[testcase patch] New testcase: DOS drive letters in linespec [Re: RFC: add relative file name handling for linespecs]

[Jan Kratochvil]

On Sat, 08 Dec 2012 19:32:39 +0100, Eli Zaretskii wrote:
> > FILENAME here is from the debug info file (not from the user), I believe on
> > MinGW the path from DWARF is absolute (like in UNIX) and it has the format
> > d:\foo\bar (not d:foo\bar), doesn't it?
> 
> If this comes only from debug info, I guess you are right, although it
> would be good to make sure GCC never writes such names into debug info.

I have tested the new testcase below by patching include/filenames.h:
-#if defined(__MSDOS__) || defined(_WIN32) || defined(__OS2__) || defined (__CYGWIN__)
+#if defined(__MSDOS__) || defined(_WIN32) || defined(__OS2__) || defined (__CYGWIN__) || 1

And therefore unfortunately the testcase cannot work on UNIX hosts.

I will check it in.


Thanks,
Jan


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

	* symtab.c (compare_filenames_for_search): New comment for
	HAS_DRIVE_SPEC.

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

	* gdb.dwarf2/dw2-dos-drive.S: New file.
	* gdb.dwarf2/dw2-dos-drive.exp: New file.

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8e14344..6985b9e 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -164,7 +164,12 @@ compare_filenames_for_search (const char *filename, const char *search_name)
 
   /* Either the names must completely match, or the character
      preceding the trailing SEARCH_NAME segment of FILENAME must be a
-     directory separator.  */
+     directory separator.
+
+     The HAS_DRIVE_SPEC purpose is to make FILENAME "c:file.c"
+     compatible with SEARCH_NAME "file.c".  In such case a compiler had
+     to put the "c:file.c" name into debug info.  Such compatibility
+     works only on GDB built for DOS host.  */
   return (len == search_len
 	  || IS_DIR_SEPARATOR (filename[len - search_len - 1])
 	  || (HAS_DRIVE_SPEC (filename)
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S
new file mode 100644
index 0000000..682ba4e
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S
@@ -0,0 +1,75 @@
+/* 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/>.  */
+
+	.text
+pc_start:
+	.byte	0
+pc_end:
+
+	.section	.debug_info
+d:
+	.4byte	.Ldebug_info_end - 1f	/* Length of Compilation Unit Info */
+1:
+	.2byte	0x3	/* DWARF version number */
+	.4byte	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
+	.byte	0x4	/* Pointer Size (in bytes) */
+dieb:
+	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
+	.ascii	"GCC\0"	/* DW_AT_producer */
+	.byte	0x2	/* DW_AT_language = DW_LANG_C */
+	.ascii	"z:file.c\0"	/* DW_AT_name */
+
+	.uleb128 0x2		/* (DIE (0xd3) DW_TAG_subprogram) */
+	.byte	0x1		/* DW_AT_external */
+	.ascii	"func\0"	/* DW_AT_name */
+	.4byte	pc_start	/* DW_AT_low_pc */
+	.4byte	pc_end		/* DW_AT_high_pc */
+	.byte	0x1		/* DW_AT_prototyped */
+
+	.byte	0x0	/* end of children of DIE 0xb */
+.Ldebug_info_end:
+
+	.section	.debug_abbrev
+.Ldebug_abbrev0:
+
+	.uleb128 0x1	/* (abbrev code) */
+	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
+	.byte	0x1	/* DW_children_yes */
+	.uleb128 0x25	/* (DW_AT_producer) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x13	/* (DW_AT_language) */
+	.uleb128 0xb	/* (DW_FORM_data1) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.byte	0x0
+	.byte	0x0
+
+	.uleb128 0x2	/* (abbrev code) */
+	.uleb128 0x2e	/* (DW_TAG_subprogram) */
+	.byte	0x0	/* DW_children_no */
+	.uleb128 0x3f	/* (DW_AT_external) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.uleb128 0x3	/* (DW_AT_name) */
+	.uleb128 0x8	/* (DW_FORM_string) */
+	.uleb128 0x11	/* (DW_AT_low_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x12	/* (DW_AT_high_pc) */
+	.uleb128 0x1	/* (DW_FORM_addr) */
+	.uleb128 0x27	/* (DW_AT_prototyped) */
+	.uleb128 0xc	/* (DW_FORM_flag) */
+	.byte	0x0
+	.byte	0x0
+
+	.byte	0x0
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.exp b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.exp
new file mode 100644
index 0000000..ecfd23a
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.exp
@@ -0,0 +1,41 @@
+# 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/>.
+load_lib dwarf.exp
+
+if {![dwarf2_support]} {
+    return 0  
+}
+
+standard_testfile .S
+set executable ${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object {}] != "" } {
+    return -1
+}
+
+clean_restart $executable
+
+gdb_test_no_output "set breakpoint pending off"
+
+gdb_test "break 'z:file.c':func" {Breakpoint [0-9]+ at .*}
+
+set dos [expr [istarget "*-*-cygwin*"] || [istarget "i?86-*-mingw*"] \
+	      || [istarget "*-*-msdosdjgpp*"] || [istarget "*-*-go32*"] ]
+
+if { $dos } {
+    gdb_test "break file.c:func" {Breakpoint [0-9]+ at .*}
+} else {
+    unsupported "break file.c:func"
+}

[commit] [testcase patch] New testcase: DOS drive letters in linespec [Re: RFC: add relative file name handling for linespecs]

[Jan Kratochvil]

On Mon, 07 Jan 2013 16:16:53 +0100, Jan Kratochvil wrote:
> gdb/
> 2013-01-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* symtab.c (compare_filenames_for_search): New comment for
> 	HAS_DRIVE_SPEC.
> 
> gdb/testsuite/
> 2013-01-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.dwarf2/dw2-dos-drive.S: New file.
> 	* gdb.dwarf2/dw2-dos-drive.exp: New file.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-01/msg00104.html


Jan