[binutils-gdb] Fix gdb.base/completion.exp with --target_board=dwarf4-gdb-index

[Pedro Alves <palves at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8f14146e1317b7b416ce298fad1a4f3d1ccbeb2b

commit 8f14146e1317b7b416ce298fad1a4f3d1ccbeb2b
Author: Pedro Alves <palves@redhat.com>
Date:   Fri Jul 14 16:50:35 2017 +0100

    Fix gdb.base/completion.exp with --target_board=dwarf4-gdb-index
    
    This is the same patch as posted at
    <https://sourceware.org/ml/gdb-patches/2017-02/msg00644.html>, with
    the test at
    <https://sourceware.org/ml/gdb-patches/2017-02/msg00687.html> squashed
    in.
    
    This patch fixes:
    
     -FAIL: gdb.base/completion.exp: tab complete break break.c:ma (timeout)
     -FAIL: gdb.base/completion.exp: complete break break.c:ma
     +PASS: gdb.base/completion.exp: tab complete break break.c:ma
     +PASS: gdb.base/completion.exp: delete breakpoint for tab complete break break.c:ma
     +PASS: gdb.base/completion.exp: complete break break.c:ma
    
    When run with --target_board=dwarf4-gdb-index.
    
    The issue here is that make_file_symbol_completion_list_1, used when
    completing a symbol restricted to a given source file, uses
    lookup_symtab to look up the symtab with the given name, and search
    for matching symbols inside.  This assumes that there's only one
    symtab for the given source file.  This is an incorrect assumption
    with (for example) -fdebug-types-section, where we'll have an extra
    extra symtab containing the types.  lookup_symtab finds that symtab,
    and inside that symtab there are no functions...
    
    gdb/ChangeLog:
    2017-07-14  Pedro Alves  <palves@redhat.com>
    
    	* symtab.c (make_file_symbol_completion_list_1): Iterate over
    	symtabs matching all symtabs with SRCFILE as file name instead of
    	only considering the first hit, with lookup_symtab.
    
    gdb/testsuite/ChangeLog:
    2017-07-14  Pedro Alves  <palves@redhat.com>
    
    	* gdb.linespec/base/one/thefile.cc (z1): New function.
    	* gdb.linespec/base/two/thefile.cc (z2): New function.
    	* gdb.linespec/linespec.exp: Add tests.

Diff:
---
 gdb/ChangeLog                                  |  6 ++
 gdb/symtab.c                                   | 32 +++------
 gdb/testsuite/ChangeLog                        |  6 ++
 gdb/testsuite/gdb.linespec/base/one/thefile.cc |  5 ++
 gdb/testsuite/gdb.linespec/base/two/thefile.cc |  5 ++
 gdb/testsuite/gdb.linespec/linespec.exp        | 93 +++++++++++++++++++++++++-
 6 files changed, 121 insertions(+), 26 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 550a7a8..8c6a4f4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2017-07-14  Pedro Alves  <palves@redhat.com>
+
+	* symtab.c (make_file_symbol_completion_list_1): Iterate over
+	symtabs matching all symtabs with SRCFILE as file name instead of
+	only considering the first hit, with lookup_symtab.
+
 2017-07-14  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* ax-gdb.c (gen_aggregate_elt_ref): Remove operand_name and
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 497d520..c7f1311 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5379,13 +5379,12 @@ make_symbol_completion_list_fn (struct cmd_list_element *ignore,
 }
 
 /* Like make_symbol_completion_list, but returns a list of symbols
-   defined in a source file FILE.  */
+   defined in all source files name SRCFILE.  */
 
 static VEC (char_ptr) *
 make_file_symbol_completion_list_1 (const char *text, const char *word,
 				    const char *srcfile)
 {
-  struct symtab *s;
   /* The symbol we are completing on.  Points in same buffer as text.  */
   const char *sym_text;
   /* Length of sym_text.  */
@@ -5436,28 +5435,15 @@ make_file_symbol_completion_list_1 (const char *text, const char *word,
 
   sym_text_len = strlen (sym_text);
 
-  /* Find the symtab for SRCFILE (this loads it if it was not yet read
-     in).  */
-  s = lookup_symtab (srcfile);
-  if (s == NULL)
+  /* Go through symtabs for SRCFILE and check the externs and statics
+     for symbols which match.  */
+  iterate_over_symtabs (srcfile, [&] (symtab *s)
     {
-      /* Maybe they typed the file with leading directories, while the
-	 symbol tables record only its basename.  */
-      const char *tail = lbasename (srcfile);
-
-      if (tail > srcfile)
-	s = lookup_symtab (tail);
-    }
-
-  /* If we have no symtab for that file, return an empty list.  */
-  if (s == NULL)
-    return (return_val);
-
-  /* Go through this symtab and check the externs and statics for
-     symbols which match.  */
-  add_symtab_completions (SYMTAB_COMPUNIT (s),
-			  sym_text, sym_text_len,
-			  text, word, TYPE_CODE_UNDEF);
+      add_symtab_completions (SYMTAB_COMPUNIT (s),
+			      sym_text, sym_text_len,
+			      text, word, TYPE_CODE_UNDEF);
+      return false;
+    });
 
   return (return_val);
 }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c29f6e5..07c5a52 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2017-07-14  Pedro Alves  <palves@redhat.com>
+
+	* gdb.linespec/base/one/thefile.cc (z1): New function.
+	* gdb.linespec/base/two/thefile.cc (z2): New function.
+	* gdb.linespec/linespec.exp: Add tests.
+
 2017-07-13  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.mi/mi-vla-fortran.exp: Correct even more parameter passing
diff --git a/gdb/testsuite/gdb.linespec/base/one/thefile.cc b/gdb/testsuite/gdb.linespec/base/one/thefile.cc
index 0417b7a..34bc547 100644
--- a/gdb/testsuite/gdb.linespec/base/one/thefile.cc
+++ b/gdb/testsuite/gdb.linespec/base/one/thefile.cc
@@ -23,3 +23,8 @@ int NameSpace::overload(int x)
 {
   return x + 23;
 }
+
+int z1 ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.linespec/base/two/thefile.cc b/gdb/testsuite/gdb.linespec/base/two/thefile.cc
index 88188a5..264ae97 100644
--- a/gdb/testsuite/gdb.linespec/base/two/thefile.cc
+++ b/gdb/testsuite/gdb.linespec/base/two/thefile.cc
@@ -24,3 +24,8 @@ int NameSpace::overload(double x)
 {
   return (int) x - 23;
 }
+
+int z2 ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.linespec/linespec.exp b/gdb/testsuite/gdb.linespec/linespec.exp
index ccb73c8..b30ac10 100644
--- a/gdb/testsuite/gdb.linespec/linespec.exp
+++ b/gdb/testsuite/gdb.linespec/linespec.exp
@@ -43,6 +43,9 @@ if {$l1 != $l2} {
     error "somebody incompatibly modified the source files needed by linespec.exp"
 }
 
+gdb_test_no_output "set breakpoint pending off" \
+    "disable pending breakpoints for linespec tests"
+
 # Copying files to a remote host loses the directory prefix during
 # compilation.
 if { [is_remote host] } {
@@ -55,6 +58,93 @@ if { [is_remote host] } {
     gdb_test "clear one/thefile.cc:$l1" \
         "Deleted breakpoint $decimal *" \
         "clear breakpoint using dir/file:line"
+
+    if { [readline_is_used] } {
+	# There are functions named 'twodup' in both source files.
+	# Both should be found if we restrict the linespec to the
+	# ambiguous "thefile.cc" source filename.  Check both
+	# completion and setting the breakpoint.
+	set tst "complete unique function name in two source files"
+	send_gdb "break thefile.cc:t\t"
+	gdb_test_multiple "" $tst {
+	    -re "break thefile.cc:twodup\\(\\) " {
+		pass $tst
+
+		send_gdb "\n"
+		gdb_test "" \
+		    "Breakpoint $decimal at $hex: thefile.cc:twodup\\(\\). \[(\]2 locations\[)\]" \
+		    "set break at unique function name in two source files"
+	    }
+	}
+
+	# Check both completing and setting a breakpoint on a linespec
+	# with a source component, where there's more than one source
+	# file with the same basename.  We should find the functions
+	# in all matching sources -- one/thefile.cc and
+	# two/thefile.cc.  The "one" file has "z1()", while the "two"
+	# file has "z2()".
+	set tst "complete non-unique function name in two source files"
+	send_gdb "break thefile.cc:z\t"
+	gdb_test_multiple "" $tst {
+	    -re "break thefile.cc:z\\\x07" {
+		send_gdb "\t"
+		gdb_test_multiple "" $tst {
+		    -re "\r\nz1\\(\\)\[ \t\]+z2\\(\\)\[ \t\]+\r\n$gdb_prompt " {
+			pass $tst
+
+			send_gdb "\n"
+			gdb_test "" \
+			    "Function \"z\" not defined in \"thefile.cc\"." \
+			    "set break at non-unique function name in two source files"
+		    }
+		}
+	    }
+	}
+
+	# Now check that disambiguating the source path makes GDB only
+	# match the symbols in that file.  "z" should now have a
+	# unique completion to "z1()", and setting the breakpoint
+	# should find only one location.
+	set tst "complete unique function name in disambiguated source file"
+	send_gdb "break one/thefile.cc:z\t"
+	gdb_test_multiple "" $tst {
+	    -re "break one/thefile.cc:z1\\(\\) " {
+		pass $tst
+
+		send_gdb "\n"
+		gdb_test "" \
+		    "Breakpoint $decimal at $hex: file .*thefile.cc, line \[^\r\n\]*" \
+		    "set break at unique function name in disambiguated source file"
+		}
+	}
+
+	# Check that using a non-existing source path does not confuse
+	# GDB.  It should not match any symbol.
+	set dir_file "one/thefile.cc"
+	set non_existing "/some/non-existing/absolute/path/prefix/$dir_file"
+	set non_existing_re [string_to_regexp $non_existing]
+
+	set tst "complete functions in non-existing absolute path"
+	send_gdb "break $non_existing:\t"
+	gdb_test_multiple "" $tst {
+	    -re "break $non_existing_re:\\\x07" {
+		send_gdb "\t\t"
+		gdb_test_multiple "" $tst {
+		    -re "^\\\x07\\\x07" {
+			pass $tst
+
+			# There's a function called 'twodup' in each
+			# of the thefile.cc files.  Make sure none is
+			# picked.
+			send_gdb "twodup\n"
+			gdb_test "" \
+			    "No source file named $non_existing_re." \
+			    "set break in function in non-existing absolute path"
+		    }
+		}
+	    }
+	}
+    }
 }
 
 gdb_test "break thefile.cc:$l1" \
@@ -73,9 +163,6 @@ gdb_test "break dupname:label" \
 # not the locations.
 gdb_test "complete condition " "condition $decimal\r\ncondition $decimal\r\ncondition $decimal"
 
-gdb_test_no_output "set breakpoint pending off" \
-    "disable pending breakpoints for linespec tests"
-
 # This is PR breakpoints/12856.
 gdb_test "break lspec.cc:nosuchfunction" \
     "Function \"nosuchfunction\" not defined in \"lspec.cc\"." \