More fixing C++ and PLTs: static

5 files changed, 158 insertions, 26 deletions

diff --git a/gdb/linespec.c b/gdb/linespec.c
index e203fbe..d449c29 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3671,10 +3671,26 @@ name_is_plt (const char *name)
   return 0;
 }
 
-/* A helper function to classify a minimal_symbol_type according to
-   priority.  */
+/* Classes of minimal symbols.  The order of the values matters:
+   Static symbols go first as we always add them.  Trampoline symbols
+   are ignored if we find extern symbols, so trampolines go last.  See
+   search_minsyms_for_name.  */
 
-static int
+enum msym_class
+{
+  /* Static symbols.  */
+  MSYM_CLASS_STATIC,
+
+  /* Extern symbols.  */
+  MSYM_CLASS_EXTERN,
+
+  /* Trampolines.  */
+  MSYM_CLASS_TRAMPOLINE,
+};
+
+/* A helper function to classify a minimal_symbol_type.  */
+
+static enum msym_class
 classify_mtype (enum minimal_symbol_type t)
 {
   switch (t)
@@ -3682,20 +3698,18 @@ classify_mtype (enum minimal_symbol_type t)
     case mst_file_text:
     case mst_file_data:
     case mst_file_bss:
-      /* Intermediate priority.  */
-      return 1;
-
-    case mst_solib_trampoline:
-      /* Lowest priority.  */
-      return 2;
+      return MSYM_CLASS_STATIC;
 
     default:
-      /* Highest priority.  */
-      return 0;
+      return MSYM_CLASS_EXTERN;
+
+    case mst_solib_trampoline:
+      return MSYM_CLASS_TRAMPOLINE;
     }
 }
 
-/* Callback for qsort that sorts symbols by priority.  */
+/* Callback for qsort that sorts minimal symbols by class.  See
+   search_minsyms_for_name.  */
 
 static int
 compare_msyms (const void *a, const void *b)
@@ -3705,7 +3719,7 @@ compare_msyms (const void *a, const void *b)
   enum minimal_symbol_type ta = MSYMBOL_TYPE (moa->minsym);
   enum minimal_symbol_type tb = MSYMBOL_TYPE (mob->minsym);
 
-  return classify_mtype (ta) - classify_mtype (tb);
+  return (int) classify_mtype (ta) - (int) classify_mtype (tb);
 }
 
 /* Callback for iterate_over_minimal_symbols that adds the symbol to
@@ -3827,26 +3841,28 @@ search_minsyms_for_name (struct collect_info *info, const char *name,
 
     if (!VEC_empty (bound_minimal_symbol_d, local.msyms))
       {
-	int classification;
 	int ix;
 	bound_minimal_symbol_d *item;
+	int any_extern = 0;
 
 	qsort (VEC_address (bound_minimal_symbol_d, local.msyms),
 	       VEC_length (bound_minimal_symbol_d, local.msyms),
 	       sizeof (bound_minimal_symbol_d),
 	       compare_msyms);
 
-	/* Now the minsyms are in classification order.  So, we walk
-	   over them and process just the minsyms with the same
-	   classification as the very first minsym in the list.  */
-	item = VEC_index (bound_minimal_symbol_d, local.msyms, 0);
-	classification = classify_mtype (MSYMBOL_TYPE (item->minsym));
-
+	/* Now the minsyms are in classification order.  We walk over
+	   them and if we see an extern symbol, we ignore trampoline
+	   symbols.  Static symbols are always added which is why they
+	   are sorted first.  */
 	for (ix = 0;
 	     VEC_iterate (bound_minimal_symbol_d, local.msyms, ix, item);
 	     ++ix)
 	  {
-	    if (classify_mtype (MSYMBOL_TYPE (item->minsym)) != classification)
+	    enum msym_class cls = classify_mtype (MSYMBOL_TYPE (item->minsym));
+
+	    if (cls == MSYM_CLASS_EXTERN)
+	      any_extern = 1;
+	    else if (cls == MSYM_CLASS_TRAMPOLINE && any_extern)
 	      break;
 
 	    VEC_safe_push (bound_minimal_symbol_d,
diff --git a/gdb/testsuite/gdb.base/break-trampoline-2.c b/gdb/testsuite/gdb.base/break-trampoline-2.c
new file mode 100644
index 0000000..4f75290
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-trampoline-2.c
@@ -0,0 +1,26 @@
+/* Copyright 2016 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/>.  */
+
+static int
+bar (void)
+{
+  return 1; /* in static */
+}
+
+int
+call_static_bar (void)
+{
+  return bar ();
+}
diff --git a/gdb/testsuite/gdb.base/break-trampoline-solib.c b/gdb/testsuite/gdb.base/break-trampoline-solib.c
index 9f52d0b..4fcac04 100644
--- a/gdb/testsuite/gdb.base/break-trampoline-solib.c
+++ b/gdb/testsuite/gdb.base/break-trampoline-solib.c
@@ -18,3 +18,9 @@ foo (void)
 {
   return 0; /* in dso */
 }
+
+int
+bar (void)
+{
+  return 0; /* in dso */
+}
diff --git a/gdb/testsuite/gdb.base/break-trampoline.c b/gdb/testsuite/gdb.base/break-trampoline.c
index c81db33..1ff918f 100644
--- a/gdb/testsuite/gdb.base/break-trampoline.c
+++ b/gdb/testsuite/gdb.base/break-trampoline.c
@@ -13,12 +13,18 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-/* Shared library entry point.  */
+/* Shared library entry points.  */
 extern int foo (void);
+extern int bar (void);
+
+/* Defined in break-trampoline-2.c.  */
+extern int call_static_bar (void);
 
 int
 main (void)
 {
   foo ();
+  bar ();
+  call_static_bar ();
   return 0;
 }
diff --git a/gdb/testsuite/gdb.base/break-trampoline.exp b/gdb/testsuite/gdb.base/break-trampoline.exp
index e2907dc..f7b43ec 100644
--- a/gdb/testsuite/gdb.base/break-trampoline.exp
+++ b/gdb/testsuite/gdb.base/break-trampoline.exp
@@ -20,18 +20,26 @@
 #   library is loaded.  Instead the foo@plt location is replaced by
 #   the "foo" function in the shared library.
 #
+# - If there's both a static "foo" function in the program, and a
+#   "foo" extern function in a shared library, "break foo" before the
+#   shared library is loaded should set two locations: one in the
+#   static function, and another in foo@plt.  Once the shared library
+#   is loaded, we should replace the plt location with the real foo
+#   function location, and keep the "foo" static function location.
+
 if { [skip_shlib_tests] } {
     return 0
 }
 
-standard_testfile break-trampoline.c break-trampoline-solib.c
+standard_testfile break-trampoline.c break-trampoline-2.c break-trampoline-solib.c
 set exec_src ${srcdir}/${subdir}/${srcfile}
-set lib_src ${srcdir}/${subdir}/${srcfile2}
+set exec_src2 ${srcdir}/${subdir}/${srcfile2}
+set lib_src ${srcdir}/${subdir}/${srcfile3}
 
 # Test both C and C++.
 
 proc do_test {lang} {
-    global exec_src lib_src binfile gdb_prompt
+    global exec_src exec_src2 lib_src binfile gdb_prompt
     global hex decimal
 
     set lib [standard_output_file break-trampoline-solib-$lang.so]
@@ -45,7 +53,7 @@ proc do_test {lang} {
     }
 
     if { [gdb_compile_shlib ${lib_src} ${lib} $lib_opts] != ""
-	 || [gdb_compile ${exec_src} ${binfile} executable $exec_opts] != ""} {
+	 || [gdb_compile "${exec_src} ${exec_src2}" ${binfile} executable $exec_opts] != ""} {
 	untested "Could not compile $lib or $binfile."
 	return -1
     }
@@ -55,13 +63,19 @@ proc do_test {lang} {
 
     if {$lang == "c++"} {
 	set foo_plt "foo()@plt"
+	set bar_plt "bar()@plt"
 	set foo_sym "foo()"
+	set bar_sym "bar()"
     } else {
 	set foo_plt "foo@plt"
+	set bar_plt "bar@plt"
 	set foo_sym "foo"
+	set bar_sym "bar"
     }
     set foo_plt_re [string_to_regexp $foo_plt]
+    set bar_plt_re [string_to_regexp $bar_plt]
     set foo_sym_re [string_to_regexp $foo_sym]
+    set bar_sym_re [string_to_regexp $bar_sym]
 
     with_test_prefix "set break before run" {
 	clean_restart ${binfile}
@@ -95,6 +109,70 @@ proc do_test {lang} {
 	}
     }
 
+    # Similar, but run to bar instead of foo.  There's a "bar" static
+    # function in the program, which should also get a location.
+    with_test_prefix "set break before run, with static function" {
+	clean_restart ${binfile}
+	gdb_load_shlib ${lib}
+
+	gdb_test "break bar" "Breakpoint 1 .*2 locations.*"
+
+	set test "info breakpoints before run"
+	set plt_count 0
+	set static_count 0
+	gdb_test_multiple "info breakpoints" $test {
+	    -re "${ws}y${ws}${any}<${bar_plt_re}>\r\n" {
+		incr plt_count
+		exp_continue
+	    }
+	    -re "${ws}y${ws}${any} in ${bar_sym_re} at ${any}${exec_src2}:${decimal}\r\n" {
+		incr static_count
+		exp_continue
+	    }
+	    -re "$gdb_prompt $" {
+		gdb_assert \
+		    {$plt_count == 1 && $static_count == 1} \
+		    $test
+	    }
+	}
+
+	gdb_run_cmd
+	set test "run to bar"
+	gdb_test_multiple "" $test {
+	    -re "bar \\(\\) at ${any}${lib_src}:${decimal}\r\n.*$gdb_prompt $" {
+		pass $test
+	    }
+	}
+
+	set test "info breakpoints, bar resolved"
+	set plt_count 0
+	set static_count 0
+	set extern_count 0
+	gdb_test_multiple "info breakpoints" $test {
+	    -re "${ws}y${ws}${any}<${bar_plt_re}>\r\n" {
+		incr plt_count
+		exp_continue
+	    }
+	    -re "${ws}y${ws}${any} in ${bar_sym_re} at ${any}${exec_src2}:${decimal}\r\n" {
+		incr static_count
+		exp_continue
+	    }
+	    -re "${ws}y${ws}${any} in ${bar_sym_re} at ${any}${lib_src}:${decimal}\r\n" {
+		incr extern_count
+		exp_continue
+	    }
+	    -re "$gdb_prompt $" {
+		if {$plt_count > 0} {
+		    fail "$test (PR 17201)"
+		} else {
+		    gdb_assert \
+			{$static_count == 1 && $extern_count == 1} \
+			$test
+		}
+	    }
+	}
+    }
+
     # Test that setting a breakpoint on the plt symbol directly does
     # not end up resolved to the text symbol in the dso.
     with_test_prefix "set breat at plt before run" {