[PATCH 01/11] eval.c: reverse minsym and sym

[PATCH 01/11] eval.c: reverse minsym and sym

[Pedro Alves]

I noticed that in evaluate_funcall, where we handle
OP_VAR_MSYM_VALUE/OP_VAR_VALUE to figure out the symbol's name gets
the minimal_symbol/symbol backwards.  Happens to be harmless in
practice because the symbol name is recorded in the common initial
sequence (in the general_symbol_info field).

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* eval.c (evaluate_funcall): Swap OP_VAR_MSYM_VALUE/OP_VAR_VALUE
	if then/else bodies in var_func_name extraction.
---
 gdb/eval.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 4899011a58f..a50299cbfdb 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1046,13 +1046,13 @@ evaluate_funcall (type *expect_type, expression *exp, int *pos,
 	{
 	  if (op == OP_VAR_MSYM_VALUE)
 	    {
-	      symbol *sym = exp->elts[*pos + 2].symbol;
-	      var_func_name = SYMBOL_PRINT_NAME (sym);
+	      minimal_symbol *msym = exp->elts[*pos + 2].msymbol;
+	      var_func_name = MSYMBOL_PRINT_NAME (msym);
 	    }
 	  else if (op == OP_VAR_VALUE)
 	    {
-	      minimal_symbol *msym = exp->elts[*pos + 2].msymbol;
-	      var_func_name = MSYMBOL_PRINT_NAME (msym);
+	      symbol *sym = exp->elts[*pos + 2].symbol;
+	      var_func_name = SYMBOL_PRINT_NAME (sym);
 	    }
 
 	  argvec[0] = evaluate_subexp_with_coercion (exp, pos, noside);
-- 
2.14.3

[PATCH 02/11] Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol creation)

[Pedro Alves]

Setting a breakpoint on an ifunc symbol after the ifunc has already
been resolved by the inferior should result in creating a breakpoint
location at the ifunc target.  However, that's not what happens today:

  (gdb) n
  53        i = gnu_ifunc (1);    /* break-at-call */
  (gdb)
  54        assert (i == 2);
  (gdb) b gnu_ifunc
  Breakpoint 2 at gnu-indirect-function resolver at 0x7ffff7bd36ee
  (gdb) info breakpoints
  Num     Type                   Disp Enb Address            What
  2       STT_GNU_IFUNC resolver keep y   0x00007ffff7bd36ee <gnu_ifunc+4>

The problem is that elf_gnu_ifunc_resolve_by_got never manages to
revolve an ifunc target.  The reason is that GDB never actually
creates the internal got.plt symbols:

 (gdb) p 'gnu_ifunc@got.plt'
 No symbol "gnu_ifunc@got.plt" in current context.

and this is because GDB expects that rela.plt has relocations for
.plt, while it actually has relocations for .got.plt:

 Relocation section [10] '.rela.plt' for section [22] '.got.plt' at offset 0x570 contains 2 entries:
   Offset              Type            Value               Addend Name
   0x0000000000601018  X86_64_JUMP_SLOT 000000000000000000      +0 __assert_fail
   0x0000000000601020  X86_64_JUMP_SLOT 000000000000000000      +0 gnu_ifunc

With that addressed, we now get:

 (gdb) p 'gnu_ifunc@got.plt'
 $1 = (<text from jump slot in .got.plt, no debug info>) 0x400753 <final>

And setting a breakpoint on the ifunc finds the ifunc target:

 (gdb) b gnu_ifunc
 Breakpoint 2 at 0x400753
 (gdb) info breakpoints
 Num     Type           Disp Enb Address            What
 2       breakpoint     keep y   0x0000000000400753 <final>

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* elfread.c (elf_rel_plt_read): Look for relocation for .got.plt,
	not .plt.
---
 gdb/elfread.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 103b2144c3a..454b77e9bdc 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -535,8 +535,7 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
 {
   bfd *obfd = objfile->obfd;
   const struct elf_backend_data *bed = get_elf_backend_data (obfd);
-  asection *plt, *relplt, *got_plt;
-  int plt_elf_idx;
+  asection *relplt, *got_plt;
   bfd_size_type reloc_count, reloc;
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
@@ -545,11 +544,6 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
   if (objfile->separate_debug_objfile_backlink)
     return;
 
-  plt = bfd_get_section_by_name (obfd, ".plt");
-  if (plt == NULL)
-    return;
-  plt_elf_idx = elf_section_data (plt)->this_idx;
-
   got_plt = bfd_get_section_by_name (obfd, ".got.plt");
   if (got_plt == NULL)
     {
@@ -559,9 +553,11 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
 	return;
     }
 
+  int got_plt_elf_idx = elf_section_data (got_plt)->this_idx;
+
   /* This search algorithm is from _bfd_elf_canonicalize_dynamic_reloc.  */
   for (relplt = obfd->sections; relplt != NULL; relplt = relplt->next)
-    if (elf_section_data (relplt)->this_hdr.sh_info == plt_elf_idx
+    if (elf_section_data (relplt)->this_hdr.sh_info == got_plt_elf_idx
 	&& (elf_section_data (relplt)->this_hdr.sh_type == SHT_REL
 	    || elf_section_data (relplt)->this_hdr.sh_type == SHT_RELA))
       break;
-- 
2.14.3

[PATCH 10/11] Extend GNU ifunc testcases

[Pedro Alves]

This patch extends/rewrites the gdb.base/gnu-ifunc.exp testcase to
cover the many different fixes in earlier patches.  (This was actually
what encovered most of the problems.)

The current testcase uses an ifunc symbol with the same name as the
ifunc resolver symbol and makes sure to compile the ifunc resolver
without debug info.  That does not model how ifuncs are implemented in
gcc/ifunc nowadays.  Instead, what we have is that the glibc ifunc
resolvers nowadays are written in C and end up with debug info.

Also, in some cases the ifunc target is written in assembly, but in
other cases it's written in C.  In the case of target function written
in C, if the target function has debug info, when we set a break on
the ifunc, we want to set it past the prologue of the target function.
Currently GDB gets that wrong.

To make sure we cover all the different scenarios, the testcase is
tweaked to cover all the different combinations of

 - An ifunc resolver with the same name as the user-visible symbol vs
   an ifunc resolver with a different name as the user-visible symbol.

 - ifunc resolver compiled with and without debug info.

 - ifunc target function compiled with and without debug info.

The testcase currently sets breakpoints on ifuncs, calls ifunc
functions, steps into ifunc functions, etc.  After this series, this
all works and the testcase passes cleanly.

While working on this, I noticed that "b gnu_ifunc" before and after
the inferior resolved the ifunc would end up with a breakpoint with
different locations.  That's now covered by new tests inside the new
"set-break" procedure.

It also tests other things like making sure we can't call an ifunc
without a return-type case if we don't know the type of the target.
And making sure that we pass enough arguments when we do know the
type.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/gnu-ifunc-resd.c: New file.
	* gdb.base/gnu-ifunc.c (final): Delete, moved to gnu-ifunc-resd.c.
	* gdb.base/gnu-ifunc.exp (executable): Delete.
	(staticexecutable): Adjust.
	(lib_opts, exec_opts): Delete.
	(make_binsuffix, build, set-break): New procedures.
	(misc_tests): New, with tests factored out from the top level.
	(top level): Test different combinations of ifunc resolver name,
	resolver with and with debug info, and ifunc target with and
	without debug info.  Wrap static tests with with_target_prefix.
---
 gdb/testsuite/gdb.base/gnu-ifunc-lib.c  |  12 +-
 gdb/testsuite/gdb.base/gnu-ifunc-resd.c |  22 ++
 gdb/testsuite/gdb.base/gnu-ifunc.c      |   6 -
 gdb/testsuite/gdb.base/gnu-ifunc.exp    | 384 ++++++++++++++++++++++++--------
 4 files changed, 329 insertions(+), 95 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/gnu-ifunc-resd.c

diff --git a/gdb/testsuite/gdb.base/gnu-ifunc-lib.c b/gdb/testsuite/gdb.base/gnu-ifunc-lib.c
index b9d446c92fa..7aac81faec6 100644
--- a/gdb/testsuite/gdb.base/gnu-ifunc-lib.c
+++ b/gdb/testsuite/gdb.base/gnu-ifunc-lib.c
@@ -22,10 +22,14 @@ extern int final (int arg);
 
 typedef int (*final_t) (int arg);
 
+#ifndef IFUNC_RESOLVER_ATTR
 asm (".type gnu_ifunc, %gnu_indirect_function");
-
 final_t
 gnu_ifunc (unsigned long hwcap)
+#else
+final_t
+gnu_ifunc_resolver (unsigned long hwcap)
+#endif
 {
   resolver_hwcap = hwcap;
   if (! gnu_ifunc_initialized)
@@ -33,3 +37,9 @@ gnu_ifunc (unsigned long hwcap)
   else
     return final;
 }
+
+#ifdef IFUNC_RESOLVER_ATTR
+extern int gnu_ifunc (int arg);
+
+__typeof (gnu_ifunc) gnu_ifunc __attribute__ ((ifunc ("gnu_ifunc_resolver")));
+#endif
diff --git a/gdb/testsuite/gdb.base/gnu-ifunc-resd.c b/gdb/testsuite/gdb.base/gnu-ifunc-resd.c
new file mode 100644
index 00000000000..ef19fc98fab
--- /dev/null
+++ b/gdb/testsuite/gdb.base/gnu-ifunc-resd.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2009-2018 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/>.  */
+
+int
+final (int arg)
+{
+  return arg + 1;
+}
diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.c b/gdb/testsuite/gdb.base/gnu-ifunc.c
index 78fd30d9fd2..a4abacaccee 100644
--- a/gdb/testsuite/gdb.base/gnu-ifunc.c
+++ b/gdb/testsuite/gdb.base/gnu-ifunc.c
@@ -23,12 +23,6 @@ init_stub (int arg)
   return 0;
 }
 
-int
-final (int arg)
-{
-  return arg + 1;
-}
-
 /* Make differentiation of how the gnu_ifunc call resolves before and after
    calling gnu_ifunc_pre.  This ensures the resolved function address is not
    being cached anywhere for the debugging purposes.  */
diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
index 1d0d0401c37..fa1464bec51 100644
--- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
+++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
@@ -18,139 +18,347 @@ if {[skip_shlib_tests]} {
 }
 
 standard_testfile .c
-set executable ${testfile}
-set staticexecutable ${executable}-static
+set staticexecutable ${testfile}-static
 set staticbinfile [standard_output_file ${staticexecutable}]
 
 set libfile "${testfile}-lib"
 set libsrc ${libfile}.c
-set lib_so [standard_output_file ${libfile}.so]
-# $lib_o must not have {debug}, it would override the STT_GNU_IFUNC ELF markers.
-set lib_o [standard_output_file ${libfile}.o]
 
-# We need DWARF for the "final" function as we "step" into the function and GDB
-# would step-over the "final" function if there would be no line number debug
-# information (DWARF) available.
-#
-# We must not have DWARF for the "gnu_ifunc" function as DWARF has no way to
-# express the STT_GNU_IFUNC type and it would be considered as a regular
-# function due to DWARF by GDB.  In ELF gnu-ifunc is expressed by the
-# STT_GNU_IFUNC type.
-#
-# Both functions need to be in the same shared library file but
-# gdb_compile_shlib has no way to specify source-specific compilation options.
-#
-# Therefore $libfile contains only the STT_GNU_IFUNC function with no DWARF
-# referencing all the other parts from the main executable with DWARF.
-
-set lib_opts {}
-set exec_opts [list debug shlib=$lib_so]
+set resdfile "${testfile}-resd"
+set resdsrc ${resdfile}.c
 
 if [get_compiler_info] {
     return -1
 }
 
-if { [gdb_compile_shlib ${srcdir}/${subdir}/$libsrc $lib_so $lib_opts] != ""
-     || [gdb_compile ${srcdir}/${subdir}/$srcfile $binfile executable $exec_opts] != ""} {
-    untested "failed to compile first testcase"
-    return -1
+# Return the binary suffix appended to program and library names to
+# make each testcase variant unique.
+proc make_binsuffix {resolver_attr resolver_debug resolved_debug} {
+    return "$resolver_attr-$resolver_debug-$resolved_debug"
 }
 
-# Start with a fresh gdb.
+# Compile the testcase.  RESOLVER_ATTR is true if we're testing with
+# an ifunc resolver that has a different name from the user symbol,
+# specified with GCC's __attribute__ ifunc.  RESOLVER_DEBUG is true
+# iff the resolver was compiled with debug info.  RESOLVED_DEBUG is
+# true iff the target function was compiled with debug info.
+proc build {resolver_attr resolver_debug resolved_debug} {
+    global srcdir subdir srcfile binfile
+    global libsrc lib_so libfile
+    global exec_opts executable
+    global hex gdb_prompt
+    global resdfile resdsrc
+
+    set suffix [make_binsuffix $resolver_attr $resolver_debug $resolved_debug]
+
+    set lib_so [standard_output_file ${libfile}-$suffix.so]
+    # $lib_o must not have {debug}, it would override the STT_GNU_IFUNC ELF markers.
+    set lib_o [standard_output_file ${libfile}-$suffix.o]
 
-clean_restart $executable
-gdb_load_shlib ${lib_so}
+    set exec_opts [list debug shlib=$lib_so]
+
+    set lib_opts {}
+    set resd_opts {}
+
+    if {$resolver_attr} {
+	lappend lib_opts "additional_flags=-DIFUNC_RESOLVER_ATTR"
+    }
+
+    if {$resolver_debug} {
+	lappend lib_opts "debug"
+    }
+
+    if {$resolved_debug} {
+	lappend resd_opts "debug"
+    }
+
+    set resd_o $resdfile-$suffix.o
+
+    if { [gdb_compile_shlib ${srcdir}/${subdir}/$libsrc \
+	      $lib_so $lib_opts] != ""
+	 || [gdb_compile ${srcdir}/${subdir}/$resdsrc \
+		 $resd_o object $resd_opts] != ""
+	 || [gdb_compile [list ${srcdir}/${subdir}/$srcfile $resd_o] \
+		 $binfile-$suffix executable $exec_opts] != ""} {
+	untested "failed to compile testcase"
+	return 0
+    }
 
-if ![runto_main] then {
-    fail "can't run to main"
     return 1
 }
 
-# The "if" condition is artifical to test regression of a former patch.
-gdb_breakpoint "[gdb_get_line_number "break-at-nextcall"] if i && gnu_ifunc (i) != 42"
+# Test setting a breakpoint on a ifunc function before and after the
+# ifunc is resolved.  RESOLVER_ATTR is true if we're testing with a
+# resolver specified with __attribute__ ifunc.  RESOLVER_DEBUG is true
+# iff the resolver was compiled with debug info.  RESOLVED_DEBUG is
+# true iff the target function was compiled with debug info.
+proc_with_prefix set-break {resolver_attr resolver_debug resolved_debug} {
+    global binfile libfile lib_so
+    global hex decimal
+    global gdb_prompt
+
+    set suffix [make_binsuffix $resolver_attr $resolver_debug $resolved_debug]
+
+    set lib_so [standard_output_file ${libfile}-$suffix.so]
+    clean_restart $binfile-$suffix
+    gdb_load_shlib ${lib_so}
+
+    if ![runto_main] then {
+	fail "can't run to main"
+	return 1
+    }
 
-gdb_breakpoint [gdb_get_line_number "break-at-call"]
-gdb_continue_to_breakpoint "break-at-call" ".*break-at-call.*"
+    set ws "\[ \t\]+"
 
-# Test GDB will automatically indirect the call.
+    if {$resolver_attr} {
+	set gnu_ifunc_resolver "gnu_ifunc_resolver"
+    } else {
+	set gnu_ifunc_resolver "gnu_ifunc"
+    }
 
-gdb_test "p gnu_ifunc (3)" " = 4"
+    with_test_prefix "before resolving" {
+	delete_breakpoints
+	gdb_test "break gnu_ifunc" \
+	    "Breakpoint $decimal at gnu-indirect-function resolver at $hex"
+	gdb_test "info breakpoints" \
+	    "$decimal${ws}STT_GNU_IFUNC resolver${ws}keep${ws}y${ws}$hex <${gnu_ifunc_resolver}>"
+    }
 
-# Test that the resolver received its argument.
+    global resdsrc
 
-set actual_hwcap "0x0"
-set test "info auxv"
-gdb_test_multiple $test $test {
-    -re "\r\n\\d+\\s+AT_HWCAP\[^\r\n\]+($hex)\r\n.*$gdb_prompt $" {
-	set actual_hwcap $expect_out(1,string)
+    with_test_prefix "resolve" {
+	delete_breakpoints
+	gdb_breakpoint [gdb_get_line_number "break-at-exit"]
+	gdb_continue_to_breakpoint "break-at-exit" ".*break-at-exit.*"
     }
-    -re ".*$gdb_prompt $" {
-	pass "$test (no HWCAP)"
+
+    with_test_prefix "after resolving" {
+	delete_breakpoints
+
+	if {!$resolved_debug} {
+	    # Set a breakpoint both at the ifunc, and at the ifunc's
+	    # target.  GDB should resolve both to the same address.
+	    # Start with the ifunc's target.
+	    set addr "-"
+	    set test "break final"
+	    # Extract the address without the leading "0x", because
+	    # addresses in "info break" output include leading 0s
+	    # (like "0x0000ADDR").
+	    set hex_number {[0-9a-fA-F][0-9a-fA-F]*}
+	    gdb_test_multiple $test $test {
+		-re "Breakpoint .* at 0x($hex_number)\r\n$gdb_prompt $" {
+		    set addr $expect_out(1,string)
+		    pass $test
+		}
+	    }
+
+	    # Now set a break at the ifunc.
+	    gdb_test "break gnu_ifunc" "Breakpoint .* at 0x$addr"
+	    set location "$decimal${ws}breakpoint${ws}keep${ws}y${ws}0x0*$addr${ws}<final\\+.*>"
+	} else {
+	    set lineno -1
+	    set test "break final"
+	    gdb_test_multiple $test $test {
+		-re "Breakpoint .* at $hex: file .*$resdsrc, line ($decimal)\\.\r\n$gdb_prompt $" {
+		    set lineno $expect_out(1,string)
+		    pass $test
+		}
+	    }
+	    gdb_test "break gnu_ifunc" "Breakpoint .* at $hex: file .*$resdsrc, line $lineno\\."
+	    set location "$decimal${ws}breakpoint${ws}keep${ws}y${ws}$hex in final at .*$resdsrc:$lineno"
+	}
+	gdb_test "info breakpoints" "$location\r\n$location"
     }
 }
 
-gdb_test "p/x resolver_hwcap" "= $actual_hwcap" "resolver received HWCAP"
+proc misc_tests {resolver_attr resolver_debug resolved_debug} {
+    global srcdir subdir srcfile binfile
+    global libsrc lib_so libfile
+    global exec_opts executable
+    global hex gdb_prompt
+    global resdfile resdsrc
 
-# Test GDB will skip the gnu_ifunc resolver on first call.
+    set suffix [make_binsuffix $resolver_attr $resolver_debug $resolved_debug]
 
-gdb_test "step" "\r\nfinal .*"
+    if {$resolver_attr} {
+	set gnu_ifunc_resolver "gnu_ifunc_resolver"
+    } else {
+	set gnu_ifunc_resolver "gnu_ifunc"
+    }
 
-# Test GDB will not break before the final chosen implementation.
+    # Start with a fresh gdb.
 
-# Also test a former patch regression:
-# Continuing.
-# Error in testing breakpoint condition:
-# Attempt to take address of value not located in memory.
-# 
-# Breakpoint 2, main () at ./gdb.base/gnu-ifunc.c:33
+    clean_restart $binfile-$suffix
+    gdb_load_shlib ${lib_so}
 
-gdb_test "continue" "Continuing.\r\n\r\nBreakpoint .* (at|in) .*break-at-nextcall.*" \
-	 "continue to break-at-nextcall"
+    if ![runto_main] then {
+	fail "can't run to main"
+	return 1
+    }
 
-gdb_breakpoint "gnu_ifunc"
+    # The "if" condition is artifical to test regression of a former patch.
+    gdb_breakpoint "[gdb_get_line_number "break-at-nextcall"] if i && (int) gnu_ifunc (i) != 42"
 
-gdb_continue_to_breakpoint "nextcall gnu_ifunc"
+    gdb_breakpoint [gdb_get_line_number "break-at-call"]
+    gdb_continue_to_breakpoint "break-at-call" ".*break-at-call.*"
 
-gdb_test "frame" "#0 +(0x\[0-9a-f\]+ in +)?final \\(.*" "nextcall gnu_ifunc skipped"
+    # Test GDB will automatically indirect the call.
 
+    if {!$resolver_debug && !$resolved_debug} {
+	gdb_test "p gnu_ifunc()" \
+	    "'final' has unknown return type; cast the call to its declared return type"
+	gdb_test "p gnu_ifunc (3)" \
+	    "'final' has unknown return type; cast the call to its declared return type"
+	gdb_test "p (int) gnu_ifunc (3)" " = 4"
+    } else {
+	gdb_test "p gnu_ifunc()" "Too few arguments in function call\\."
+	gdb_test "p gnu_ifunc (3)" " = 4"
+    }
 
-# Check any commands not doing an inferior call access the address of the
-# STT_GNU_IFUNC resolver, not the target function.
+    # Test that the resolver received its argument.
+
+    set actual_hwcap "0x0"
+    set test "info auxv"
+    gdb_test_multiple $test $test {
+	-re "\r\n\\d+\\s+AT_HWCAP\[^\r\n\]+($hex)\r\n.*$gdb_prompt $" {
+	    set actual_hwcap $expect_out(1,string)
+	}
+	-re ".*$gdb_prompt $" {
+	    pass "$test (no HWCAP)"
+	}
+    }
 
-if {[istarget powerpc64-*] && [is_lp64_target]} {
-    # With only minimal symbols GDB provides the function descriptors.  With
-    # full debug info the function code would be displayed.
-    set func_prefix {\.}
-} else {
-    set func_prefix {}
-}
+    gdb_test "p/x resolver_hwcap" "= $actual_hwcap" "resolver received HWCAP"
+
+    # Test GDB will skip the gnu_ifunc resolver on first call.
+
+    # Even if the resolver has debug info, stepping into an ifunc call
+    # should skip the resolver.
+    if {!$resolved_debug} {
+	# Make GDB stop stepping even if it steps into a function with
+	# no debug info.
+	gdb_test_no_output "set step-mode on"
+	gdb_test "step" "$hex in final \\\(\\\)"
+    } else {
+	gdb_test "step" "\r\nfinal .*"
+    }
+
+    # Test GDB will not break before the final chosen implementation.
+
+    # Also test a former patch regression:
+    # Continuing.
+    # Error in testing breakpoint condition:
+    # Attempt to take address of value not located in memory.
+    #
+    # Breakpoint 2, main () at ./gdb.base/gnu-ifunc.c:33
+
+    gdb_test "continue" \
+	"Continuing.\r\n\r\nBreakpoint .* (at|in) .*break-at-nextcall.*" \
+	"continue to break-at-nextcall"
+
+    gdb_breakpoint "gnu_ifunc"
+
+    gdb_continue_to_breakpoint "nextcall gnu_ifunc"
+
+    gdb_test "frame" \
+	"#0 +(0x\[0-9a-f\]+ in +)?final \\(.*" "nextcall gnu_ifunc skipped"
+
+
+    # Check any commands not doing an inferior call access the address of the
+    # STT_GNU_IFUNC resolver, not the target function.
 
-gdb_test "p gnu_ifunc" " = {<text gnu-indirect-function variable, no debug info>} 0x\[0-9a-f\]+ <${func_prefix}gnu_ifunc>" "p gnu_ifunc executing"
-gdb_test "info sym gnu_ifunc" "gnu_ifunc in section .*" "info sym gnu_ifunc executing"
+    if {[istarget powerpc64-*] && [is_lp64_target]} {
+	# With only minimal symbols GDB provides the function descriptors.  With
+	# full debug info the function code would be displayed.
+	set func_prefix {\.}
+    } else {
+	set func_prefix {}
+    }
 
-set test "info addr gnu_ifunc"
-gdb_test_multiple $test $test {
-    -re "Symbol \"gnu_ifunc\" is at (0x\[0-9a-f\]+) in .*$gdb_prompt $" {
-	pass $test
+    gdb_test "p gnu_ifunc" \
+	" = {<text gnu-indirect-function variable, no debug info>} 0x\[0-9a-f\]+ <${func_prefix}${gnu_ifunc_resolver}>" \
+	"p gnu_ifunc executing"
+    gdb_test "info sym gnu_ifunc" \
+	"${gnu_ifunc_resolver} in section .*" \
+	"info sym gnu_ifunc executing"
+
+    set test "info addr gnu_ifunc"
+    if {!$resolver_attr && $resolver_debug} {
+	gdb_test_multiple $test $test {
+	    -re "Symbol \"gnu_ifunc\" is a function at address (0x\[0-9a-f\]+).*$gdb_prompt $" {
+		pass $test
+	    }
+	}
+    } else {
+	gdb_test_multiple $test $test {
+	    -re "Symbol \"gnu_ifunc\" is at (0x\[0-9a-f\]+) in .*$gdb_prompt $" {
+		pass $test
+	    }
+	}
+    }
+    gdb_test "info sym $expect_out(1,string)" \
+	"${gnu_ifunc_resolver} in section .*" \
+	"info sym <gnu_ifunc-address>"
+
+    # Test calling the resolver directly instead of the ifunc symbol.
+    # Can only do that if the ifunc and the ifunc resolver have
+    # different names.
+    if {$resolver_attr} {
+	if {$resolver_debug} {
+	    gdb_test "p gnu_ifunc_resolver(0)" \
+		" = \\(int \\(\\*\\)\\(int\\)\\) $hex <final>"
+	} else {
+	    gdb_test "p gnu_ifunc_resolver(0)" \
+		"'gnu_ifunc_resolver' has unknown return type; cast the call to its declared return type"
+	    gdb_test "p (void *) gnu_ifunc_resolver(0)" \
+		" = \\(void \\*\\) $hex <final>"
+	}
     }
 }
-gdb_test "info sym $expect_out(1,string)" "gnu_ifunc in section .*" "info sym <gnu_ifunc-address>"
 
+# Test all the combinations of:
+#
+# - An ifunc resolver with the same name as the ifunc symbol vs an
+#   ifunc resolver with a different name as the ifunc symbol.
+#
+# - ifunc resolver compiled with and without debug info.  This ensures
+#   that GDB understands that a function not a regular function by
+#   looking at the STT_GNU_IFUNC type in the elf symbols.  DWARF has
+#   no way to express the STT_GNU_IFUNC type.
+#
+# - ifunc target function (resolved) compiled with and without debug
+#   info.
+foreach_with_prefix resolver_attr {0 1} {
+    foreach_with_prefix resolver_debug {0 1} {
+	foreach_with_prefix resolved_debug {0 1} {
+	    build $resolver_attr $resolver_debug $resolved_debug
+	    misc_tests $resolver_attr $resolver_debug $resolved_debug
+	    set-break $resolver_attr $resolver_debug $resolved_debug
+	}
+    }
+}
 
 # Test statically linked ifunc resolving during inferior start.
 # https://bugzilla.redhat.com/show_bug.cgi?id=624967
 
-# Compile $staticbinfile separately as it may exit on error (ld/12595).
-
-if { [gdb_compile ${srcdir}/${subdir}/$libsrc $lib_o object {}] != ""
-     || [gdb_compile "${srcdir}/${subdir}/$srcfile $lib_o" $staticbinfile executable {debug}] != "" } {
-    untested "failed to compile second testcase"
-    return -1
-}
+with_test_prefix "static" {
+    # Compile $staticbinfile separately as it may exit on error
+    # (ld/12595).
+
+    set lib_o [standard_output_file ${libfile}.o]
+    set resd_o [standard_output_file ${resdfile}.o]
+    if { [gdb_compile ${srcdir}/${subdir}/$libsrc $lib_o object {}] != ""
+	 || [gdb_compile ${srcdir}/${subdir}/$resdsrc $resd_o object {}] != ""
+	 || [gdb_compile "${srcdir}/${subdir}/$srcfile $lib_o $resd_o" \
+		 $staticbinfile executable {debug}] != "" } {
+	untested "failed to compile second testcase"
+	return -1
+    }
 
-clean_restart $staticexecutable
+    clean_restart $staticexecutable
 
-gdb_breakpoint "gnu_ifunc"
-gdb_breakpoint "main"
-gdb_run_cmd
-gdb_test "" "Breakpoint \[0-9\]*, main .*" "static gnu_ifunc"
+    gdb_breakpoint "gnu_ifunc"
+    gdb_breakpoint "main"
+    gdb_run_cmd
+    gdb_test "" "Breakpoint \[0-9\]*, main .*" "static gnu_ifunc"
+}
-- 
2.14.3

[PATCH 09/11] Factor out minsym_found/find_function_start_sal overload

[Pedro Alves]

I need to make the ifunc resolving code in elfread.c skip the target
function's prologue like minsym_found does.  I thought of factoring
that out to a separate function, but turns out there's already a
comment in find_function_start_sal that says that should agree with
minsym_found...

Instead of making sure the code agrees with a comment, factor out the
common code to a separate function and use it from both places.

Note that the current find_function_start_sal does a bit more than
minsym_found's equivalent (the "We always should ..." bit), though
that's probably a latent bug.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* linespec.c (minsym_found): Use find_function_start_sal CORE_ADDR
	overload.
	* symtab.c (find_function_start_sal(CORE_ADDR, obj_section *,bool)):
	New, factored out from ...
	(find_function_start_sal(symbol *, int)): ... this.  Reimplement
	and use bool.
	* symtab.h (find_function_start_sal(CORE_ADDR, obj_section *,bool)):
	New.
	(find_function_start_sal(symbol *, int)): Change boolean parameter
	type to bool.
---
 gdb/linespec.c | 20 +-------------------
 gdb/symtab.c   | 45 +++++++++++++++++++++++++--------------------
 gdb/symtab.h   | 13 +++++++++++--
 3 files changed, 37 insertions(+), 41 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index abf31559f54..8154f62858e 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -4426,25 +4426,7 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
     is_function = msymbol_is_function (objfile, msymbol, &func_addr);
 
   if (is_function)
-    {
-      sal = find_pc_sect_line (func_addr, NULL, 0);
-
-      if (self->funfirstline)
-	{
-	  if (sal.symtab != NULL
-	      && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab))
-		  || SYMTAB_LANGUAGE (sal.symtab) == language_asm))
-	    {
-	      struct gdbarch *gdbarch = get_objfile_arch (objfile);
-
-	      sal.pc = func_addr;
-	      if (gdbarch_skip_entrypoint_p (gdbarch))
-		sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
-	    }
-	  else
-	    skip_prologue_sal (&sal);
-	}
-    }
+    sal = find_function_start_sal (func_addr, NULL, self->funfirstline);
   else
     {
       sal.objfile = objfile;
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 567195304f4..13f67639da8 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3575,46 +3575,36 @@ find_pc_line_pc_range (CORE_ADDR pc, CORE_ADDR *startptr, CORE_ADDR *endptr)
   return sal.symtab != 0;
 }
 
-/* Given a function symbol SYM, find the symtab and line for the start
-   of the function.
-   If the argument FUNFIRSTLINE is nonzero, we want the first line
-   of real code inside the function.
-   This function should return SALs matching those from minsym_found,
-   otherwise false multiple-locations breakpoints could be placed.  */
+/* See symtab.h.  */
 
-struct symtab_and_line
-find_function_start_sal (struct symbol *sym, int funfirstline)
+symtab_and_line
+find_function_start_sal (CORE_ADDR func_addr, obj_section *section,
+			 bool funfirstline)
 {
-  fixup_symbol_section (sym, NULL);
-
-  obj_section *section = SYMBOL_OBJ_SECTION (symbol_objfile (sym), sym);
-  symtab_and_line sal
-    = find_pc_sect_line (BLOCK_START (SYMBOL_BLOCK_VALUE (sym)), section, 0);
-  sal.symbol = sym;
+  symtab_and_line sal = find_pc_sect_line (func_addr, section, 0);
 
   if (funfirstline && sal.symtab != NULL
       && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab))
 	  || SYMTAB_LANGUAGE (sal.symtab) == language_asm))
     {
-      struct gdbarch *gdbarch = symbol_arch (sym);
+      struct gdbarch *gdbarch = get_objfile_arch (sal.objfile);
 
-      sal.pc = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      sal.pc = func_addr;
       if (gdbarch_skip_entrypoint_p (gdbarch))
 	sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
       return sal;
     }
 
   /* We always should have a line for the function start address.
-     If we don't, something is odd.  Create a plain SAL refering
+     If we don't, something is odd.  Create a plain SAL referring
      just the PC and hope that skip_prologue_sal (if requested)
      can find a line number for after the prologue.  */
-  if (sal.pc < BLOCK_START (SYMBOL_BLOCK_VALUE (sym)))
+  if (sal.pc < func_addr)
     {
       sal = {};
       sal.pspace = current_program_space;
-      sal.pc = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      sal.pc = func_addr;
       sal.section = section;
-      sal.symbol = sym;
     }
 
   if (funfirstline)
@@ -3623,6 +3613,21 @@ find_function_start_sal (struct symbol *sym, int funfirstline)
   return sal;
 }
 
+/* See symtab.h.  */
+
+symtab_and_line
+find_function_start_sal (symbol *sym, bool funfirstline)
+{
+  fixup_symbol_section (sym, NULL);
+  symtab_and_line sal
+    = find_function_start_sal (BLOCK_START (SYMBOL_BLOCK_VALUE (sym)),
+			       SYMBOL_OBJ_SECTION (symbol_objfile (sym), sym),
+			       funfirstline);
+  sal.symbol = sym;
+  return sal;
+}
+
+
 /* Given a function start address FUNC_ADDR and SYMTAB, find the first
    address for that function that has an entry in SYMTAB's line info
    table.  If such an entry cannot be found, return FUNC_ADDR
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 483e643ebdb..e1f8f566135 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1895,8 +1895,17 @@ int matching_obj_sections (struct obj_section *, struct obj_section *);
 
 extern struct symtab *find_line_symtab (struct symtab *, int, int *, int *);
 
-extern struct symtab_and_line find_function_start_sal (struct symbol *sym,
-						       int);
+/* Given a function symbol SYM, find the symtab and line for the start
+   of the function.  If FUNFIRSTLINE is true, we want the first line
+   of real code inside the function.  */
+extern symtab_and_line find_function_start_sal (symbol *sym, bool
+						funfirstline);
+
+/* Same, but start with a function address/section instead of a
+   symbol.  */
+extern symtab_and_line find_function_start_sal (CORE_ADDR func_addr,
+						obj_section *section,
+						bool funfirstline);
 
 extern void skip_prologue_sal (struct symtab_and_line *);
 
-- 
2.14.3

[PATCH 07/11] Breakpoints, don't skip prologue of ifunc resolvers with debug info

[Pedro Alves]

Without this patch, some of the tests added to gdb.base/gnu-ifunc.exp
by a following patch fail like so:

  FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: resolved_debug=0: set-break: before resolving: break gnu_ifunc
  FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: resolved_debug=0: set-break: before resolving: info breakpoints
  FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: resolved_debug=0: set-break: after resolving: break gnu_ifunc
  FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: resolved_debug=0: set-break: after resolving: info breakpoints
  FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: resolved_debug=1: set-break: before resolving: break gnu_ifunc
  FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: resolved_debug=1: set-break: before resolving: info breakpoints
  FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: resolved_debug=1: set-break: after resolving: break gnu_ifunc
  FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: resolved_debug=1: set-break: after resolving: info breakpoints

All of them trigger iff:

 - you have debug info for the ifunc resolver.
 - the resolver and the user-visible symbol have the same name.

If you have an ifunc that has a resolver with the same name as the
user visible symbol, debug info for the resolver masks out the ifunc
minsym.  When you set a breakpoint by name on the user visible symbol,
GDB finds the DWARF symbol for the resolver, and thinking that it's a
regular function, sets a breakpoint location past its prologue.

Like so, location 1.2, before the ifunc is resolved by the inferior:

  (gdb) break gnu_ifunc
  Breakpoint 2 at 0x7ffff7bd36ea (2 locations)
  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  1       breakpoint     keep y   <MULTIPLE>
  1.1                         y     0x00007ffff7bd36ea <gnu_ifunc>
  1.2                         y     0x00007ffff7bd36f2 in gnu_ifunc at src/gdb/testsuite/gdb.base/gnu-ifunc-lib.c:34
  (gdb)

And like so, location 2.2, if you set the breakpoint after the ifunc
is resolved by the inferior (to "final"):

  (gdb) break gnu_ifunc
  Breakpoint 5 at 0x400757 (2 locations)
  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  2       breakpoint     keep y   <MULTIPLE>
  2.1                         y     0x000000000040075a in final at src/gdb/testsuite/gdb.base/gnu-ifunc-resd.c:21
  2.2                         y     0x00007ffff7bd36f2 in gnu_ifunc at src/gdb/testsuite/gdb.base/gnu-ifunc-lib.c:34
  (gdb)

I don't think this is right because when users set a breakpoint at an
ifunc, they don't care about debugging the resolver.  Instead what you
should is a single location for the ifunc in the first case, and a
single location of the ifunc target in the second case.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* linespec.c (struct bound_minimal_symbol_search_key): New.
	(convert_linespec_to_sals): Sort minimal symbols earlier.  Don't
	skip first line if we found a GNU ifunc minimal symbol by name.
	(compare_msymbols): Change parameters to work with a destructured
	lhs minsym.
	(compare_msymbols_for_qsort, compare_msymbols_for_bsearch): New
	functions.
---
 gdb/linespec.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 101 insertions(+), 20 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 1f1fb695c14..abf31559f54 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -402,7 +402,8 @@ static void minsym_found (struct linespec_state *self, struct objfile *objfile,
 
 static int compare_symbols (const void *a, const void *b);
 
-static int compare_msymbols (const void *a, const void *b);
+static int compare_msymbols_for_qsort (const void *a, const void *b);
+static int compare_msymbols_for_bsearch (const void *a, const void *b);
 
 /* Permitted quote characters for the parser.  This is different from the
    completer's quote characters to allow backward compatibility with the
@@ -2281,6 +2282,20 @@ convert_address_location_to_sals (struct linespec_state *self,
   return sals;
 }
 
+/* Search key passed to compare_msymbols_for_bsearch, a bsearch
+   callback.  */
+struct bound_minimal_symbol_search_key
+{
+  /* The program space.  */
+  program_space *pspace;
+
+  /* The minimal symbol's address.  */
+  CORE_ADDR address;
+
+  /* The minimal symbol's type.  */
+  enum minimal_symbol_type type;
+};
+
 /* Create and return SALs from the linespec LS.  */
 
 static std::vector<symtab_and_line>
@@ -2321,12 +2336,52 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 	  qsort (VEC_address (symbolp, ls->function_symbols),
 		 VEC_length (symbolp, ls->function_symbols),
 		 sizeof (symbolp), compare_symbols);
+	}
+
+      if (ls->minimal_symbols != NULL)
+	{
+	  /* Sort minimal symbols by program space, address, and type.
+	     Do this before the minsym bsearch, below.  */
+	  qsort (VEC_address (bound_minimal_symbol_d, ls->minimal_symbols),
+		 VEC_length (bound_minimal_symbol_d, ls->minimal_symbols),
+		 sizeof (bound_minimal_symbol_d), compare_msymbols_for_qsort);
+	}
 
+      if (ls->function_symbols != NULL)
+	{
 	  for (i = 0; VEC_iterate (symbolp, ls->function_symbols, i, sym); ++i)
 	    {
 	      pspace = SYMTAB_PSPACE (symbol_symtab (sym));
 	      set_current_program_space (pspace);
-	      if (symbol_to_sal (&sal, state->funfirstline, sym)
+
+	      /* Don't skip to the first line of the function if we
+		 had found an ifunc minimal symbol for this function,
+		 because that means that this function is an ifunc
+		 resolver with the same name as the ifunc itself.  */
+	      bound_minimal_symbol *ifunc = NULL;
+
+	      if (state->funfirstline
+		   && ls->minimal_symbols != NULL
+		   && SYMBOL_CLASS (sym) == LOC_BLOCK)
+		{
+		  const CORE_ADDR addr
+		    = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+		  const bound_minimal_symbol_search_key key
+		    = {pspace, addr, mst_text_gnu_ifunc};
+
+		  const bound_minimal_symbol *begin
+		    = VEC_address (bound_minimal_symbol_d, ls->minimal_symbols);
+		  ifunc
+		    = ((bound_minimal_symbol *)
+		       bsearch (&key, begin,
+				VEC_length (bound_minimal_symbol_d,
+					    ls->minimal_symbols),
+				sizeof (bound_minimal_symbol_d),
+				compare_msymbols_for_bsearch));
+		}
+
+	      if (ifunc == NULL
+		  && symbol_to_sal (&sal, state->funfirstline, sym)
 		  && maybe_add_address (state->addr_set, pspace, sal.pc))
 		add_sal_to_sals (state, &sals, &sal,
 				 SYMBOL_NATURAL_NAME (sym), 0);
@@ -2335,11 +2390,6 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 
       if (ls->minimal_symbols != NULL)
 	{
-	  /* Sort minimal symbols by program space, too.  */
-	  qsort (VEC_address (bound_minimal_symbol_d, ls->minimal_symbols),
-		 VEC_length (bound_minimal_symbol_d, ls->minimal_symbols),
-		 sizeof (bound_minimal_symbol_d), compare_msymbols);
-
 	  for (i = 0;
 	       VEC_iterate (bound_minimal_symbol_d, ls->minimal_symbols,
 			    i, elem);
@@ -3662,36 +3712,67 @@ compare_symbols (const void *a, const void *b)
   return 0;
 }
 
-/* Like compare_symbols but for minimal symbols.  */
+/* Helper for compare_symbols_for_qsort and
+   compare_symbols_for_bsearch.  Sorts msymbols by pspace, address and
+   then type.  */
 
 static int
-compare_msymbols (const void *a, const void *b)
+compare_msymbols (const program_space *l_pspace,
+		  CORE_ADDR l_address,
+		  enum minimal_symbol_type l_type,
+		  const bound_minimal_symbol *r_minsym)
 {
-  const struct bound_minimal_symbol *sa
-    = (const struct bound_minimal_symbol *) a;
-  const struct bound_minimal_symbol *sb
-    = (const struct bound_minimal_symbol *) b;
-  uintptr_t uia, uib;
+  const struct bound_minimal_symbol *sb = r_minsym;
 
-  uia = (uintptr_t) sa->objfile->pspace;
-  uib = (uintptr_t) sa->objfile->pspace;
+  uintptr_t uia = (uintptr_t) l_pspace;
+  uintptr_t uib = (uintptr_t) sb->objfile->pspace;
 
   if (uia < uib)
     return -1;
   if (uia > uib)
     return 1;
 
-  uia = (uintptr_t) sa->minsym;
-  uib = (uintptr_t) sb->minsym;
+  if (l_address < BMSYMBOL_VALUE_ADDRESS (*sb))
+    return -1;
+  if (l_address > BMSYMBOL_VALUE_ADDRESS (*sb))
+    return 1;
 
-  if (uia < uib)
+  if (l_type < MSYMBOL_TYPE (sb->minsym))
     return -1;
-  if (uia > uib)
+  if (l_type > MSYMBOL_TYPE (sb->minsym))
     return 1;
 
   return 0;
 }
 
+/* Like compare_symbols but for minimal symbols.  Version suitable for
+   use with qsort.  */
+
+static int
+compare_msymbols_for_qsort (const void *a, const void *b)
+{
+  const auto *sa = (bound_minimal_symbol *) a;
+  const auto *sb = (bound_minimal_symbol *) b;
+
+  return compare_msymbols (sa->objfile->pspace,
+			   BMSYMBOL_VALUE_ADDRESS (*sa),
+			   MSYMBOL_TYPE (sa->minsym),
+			   sb);
+}
+
+/* Like compare_symbols but for minimal symbols.  This version is
+   suitable for use with bsearch with bound_minimal_symbol_search_key
+   as key type.  */
+
+static int
+compare_msymbols_for_bsearch (const void *key, const void *bmsym)
+{
+  const auto *k = (bound_minimal_symbol_search_key *) key;
+  const auto *m = (bound_minimal_symbol *) bmsym;
+
+  return compare_msymbols (k->pspace, k->address, k->type, m);
+}
+
 /* Look for all the matching instances of each symbol in NAMES.  Only
    instances from PSPACE are considered; other program spaces are
    handled by our caller.  If PSPACE is NULL, then all program spaces
-- 
2.14.3

[PATCH 08/11] Eliminate find_pc_partial_function_gnu_ifunc

[Pedro Alves]

Not used anywhere any longer.

If this is ever reinstated, note that this case:

	  cache_pc_function_is_gnu_ifunc = TYPE_GNU_IFUNC (SYMBOL_TYPE (f));

was incorrect in that regular symbols never have type marked as GNU
ifunc type, only minimal symbols.  At some point I had some fix that
checking the matching minsym here.  But in the end I ended up just
eliminating need for this function, so that fix was not necessary.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* blockframe.c (cache_pc_function_is_gnu_ifunc): Delete.  Remove
	all references.
	(find_pc_partial_function_gnu_ifunc): Rename to ...
	(find_pc_partial_function): ... this, and remove references to
	'is_gnu_ifunc_p'.
	(find_pc_partial_function): Delete old implementation.
	* symtab.h (find_pc_partial_function_gnu_ifunc): Delete.
---
 gdb/blockframe.c | 36 +++++++-----------------------------
 gdb/symtab.h     |  5 -----
 2 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index db02b35742d..1a28b3c8b5f 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -159,7 +159,6 @@ static CORE_ADDR cache_pc_function_low = 0;
 static CORE_ADDR cache_pc_function_high = 0;
 static const char *cache_pc_function_name = 0;
 static struct obj_section *cache_pc_function_section = NULL;
-static int cache_pc_function_is_gnu_ifunc = 0;
 
 /* Clear cache, e.g. when symbol table is discarded.  */
 
@@ -170,7 +169,6 @@ clear_pc_function_cache (void)
   cache_pc_function_high = 0;
   cache_pc_function_name = (char *) 0;
   cache_pc_function_section = NULL;
-  cache_pc_function_is_gnu_ifunc = 0;
 }
 
 /* Finds the "function" (text symbol) that is smaller than PC but
@@ -178,19 +176,17 @@ clear_pc_function_cache (void)
    *NAME and/or *ADDRESS conditionally if that pointer is non-null.
    If ENDADDR is non-null, then set *ENDADDR to be the end of the
    function (exclusive), but passing ENDADDR as non-null means that
-   the function might cause symbols to be read.  If IS_GNU_IFUNC_P is provided
-   *IS_GNU_IFUNC_P is set to 1 on return if the function is STT_GNU_IFUNC.
-   This function either succeeds or fails (not halfway succeeds).  If it
-   succeeds, it sets *NAME, *ADDRESS, and *ENDADDR to real information and
-   returns 1.  If it fails, it sets *NAME, *ADDRESS, *ENDADDR and
-   *IS_GNU_IFUNC_P to zero and returns 0.  */
+   the function might cause symbols to be read.  This function either
+   succeeds or fails (not halfway succeeds).  If it succeeds, it sets
+   *NAME, *ADDRESS, and *ENDADDR to real information and returns 1.
+   If it fails, it sets *NAME, *ADDRESS and *ENDADDR to zero and
+   returns 0.  */
 
 /* Backward compatibility, no section argument.  */
 
 int
-find_pc_partial_function_gnu_ifunc (CORE_ADDR pc, const char **name,
-				    CORE_ADDR *address, CORE_ADDR *endaddr,
-				    int *is_gnu_ifunc_p)
+find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
+			  CORE_ADDR *endaddr)
 {
   struct obj_section *section;
   struct symbol *f;
@@ -243,7 +239,6 @@ find_pc_partial_function_gnu_ifunc (CORE_ADDR pc, const char **name,
 	  cache_pc_function_high = BLOCK_END (SYMBOL_BLOCK_VALUE (f));
 	  cache_pc_function_name = SYMBOL_LINKAGE_NAME (f);
 	  cache_pc_function_section = section;
-	  cache_pc_function_is_gnu_ifunc = TYPE_GNU_IFUNC (SYMBOL_TYPE (f));
 	  goto return_cached_value;
 	}
     }
@@ -266,16 +261,12 @@ find_pc_partial_function_gnu_ifunc (CORE_ADDR pc, const char **name,
 	*address = 0;
       if (endaddr != NULL)
 	*endaddr = 0;
-      if (is_gnu_ifunc_p != NULL)
-	*is_gnu_ifunc_p = 0;
       return 0;
     }
 
   cache_pc_function_low = BMSYMBOL_VALUE_ADDRESS (msymbol);
   cache_pc_function_name = MSYMBOL_LINKAGE_NAME (msymbol.minsym);
   cache_pc_function_section = section;
-  cache_pc_function_is_gnu_ifunc = (MSYMBOL_TYPE (msymbol.minsym)
-				    == mst_text_gnu_ifunc);
   cache_pc_function_high = minimal_symbol_upper_bound (msymbol);
 
  return_cached_value:
@@ -307,22 +298,9 @@ find_pc_partial_function_gnu_ifunc (CORE_ADDR pc, const char **name,
 	*endaddr = cache_pc_function_high;
     }
 
-  if (is_gnu_ifunc_p)
-    *is_gnu_ifunc_p = cache_pc_function_is_gnu_ifunc;
-
   return 1;
 }
 
-/* See find_pc_partial_function_gnu_ifunc, only the IS_GNU_IFUNC_P parameter
-   is omitted here for backward API compatibility.  */
-
-int
-find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
-			  CORE_ADDR *endaddr)
-{
-  return find_pc_partial_function_gnu_ifunc (pc, name, address, endaddr, NULL);
-}
-
 /* See symtab.h.  */
 
 struct type *
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 52ab1357300..483e643ebdb 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1665,11 +1665,6 @@ extern struct symbol *find_pc_sect_function (CORE_ADDR, struct obj_section *);
 
 extern struct symbol *find_symbol_at_address (CORE_ADDR);
 
-extern int find_pc_partial_function_gnu_ifunc (CORE_ADDR pc, const char **name,
-					       CORE_ADDR *address,
-					       CORE_ADDR *endaddr,
-					       int *is_gnu_ifunc_p);
-
 /* lookup function from address, return name, start addr and end addr.  */
 
 extern int find_pc_partial_function (CORE_ADDR, const char **, CORE_ADDR *,
-- 
2.14.3

[PATCH 03/11] Fix calling ifunc functions when resolver has debug info and different name

[Pedro Alves]

Currently, on Fedora 27 (glibc 2.26), if you try to call strlen in the
inferior you get:

 (gdb) p strlen ("hello")
 $1 = (size_t (*)(const char *)) 0x7ffff554aac0 <__strlen_avx2>

strlen is an ifunc function, and what we see above is the result of
calling the ifunc resolver in the inferior.  That returns a pointer to
the actual target function that implements strlen on my machine.  GDB
should have turned around and called the resolver automatically
without the user noticing.

This is was caused by commit:

  commit bf223d3e808e6fec9ee165d3d48beb74837796de
  Date: Mon Aug 21 11:34:32 2017 +0100

      Handle function aliases better (PR gdb/19487, errno printing)

which added the find_function_alias_target call to c-exp.y, to try to
find an alias with debug info for a minsym.  For ifunc symbols, that
finds the ifunc's resolver if it has debug info (in the example it's
called "strlen_ifunc"), with the result that GDB calls that as a
regular function.

After this commit, we get now get:

  (top-gdb) p strlen ("hello")
  '__strlen_avx2' has unknown return type; cast the call to its declared return type

Which is correct, because __strlen_avx2 is written in assembly.
That'll be improved in a following patch, though.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* c-exp.y (variable production): Skip finding an alias for ifunc
	symbols.
---
 gdb/c-exp.y | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 8dc3c068a5e..e2ea07cd792 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1081,7 +1081,9 @@ variable:	name_not_typename
 				 is important for example for "p
 				 *__errno_location()".  */
 			      symbol *alias_target
-				= find_function_alias_target (msymbol);
+				= (msymbol.minsym->type != mst_text_gnu_ifunc
+				   ? find_function_alias_target (msymbol)
+				   : NULL);
 			      if (alias_target != NULL)
 				{
 				  write_exp_elt_opcode (pstate, OP_VAR_VALUE);
-- 
2.14.3

[PATCH 00/11] Fixing GNU ifunc support

[Pedro Alves]

Jakub Jelinek noticed that on Fedora 28, GDB can't call strlen:

  (top-gdb) p strlen("hello")
  $1 = (size_t (*)(const char *)) 0x7ffff554aac0 <__strlen_avx2>

That's clearly GDB printing the pointer to the ifunc target function
that implements strlen, instead of calling that function and printing
the result...

Suspecting that that might have been caused by my earlier improvements
to calling functions with no debug info, and improved support for
function aliases, I took a look.  And then I started writing a test,
which then uncovered a ton of problems...  All fixed by this series.

The main issue is that GDB's current ifunc support assumes that (and
the testcase exercises that) the resolver must be compiled without
debug info, and that the resolver has the same name as the user
visible function.

However, glibc nowadays implements ifunc resolvers in C using GCC's
__attribute__((ifunc)), and compiles them with debug info.
With __attribute__((ifunc)), the ifunc symbol has the user visible
name, and the resolver gets a regular function symbol with a different
name (what is passed to the attribute).

While fixing that, I thought I'd extend the existing testcase to
exercise all combination of

 - An ifunc set with __attribute__(ifunc) [different name as the
   user-visible symbol], vs set with

     asm (".type gnu_ifunc, %gnu_indirect_function");

   i.e., with the same name as the user-visible symbol.

 - ifunc resolver compiled with and without debug info.

 - ifunc target function compiled with and without debug info.

Of course that uncovered a whole slew of problems...

And then along the way noticed several other issues and added several
tests for them.  The testcase patch is added torward the end of the
series, because I honestly don't think I can effectively split it down
and split chunks into the patches that implement the fix.  Most of the
testcase changes need all the fixes in place to do any meaningful
testing.  The exception is the last patch in the series.

Pedro Alves (11):
  eval.c: reverse minsym and sym
  Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol
    creation)
  Fix calling ifunc functions when resolver has debug info and different
    name
  Calling ifunc functions when target has no debug info but resolver has
  Calling ifunc functions when resolver has debug info, user symbol same
    name
  Fix setting breakpoints on ifunc functions after they're already
    resolved
  Breakpoints, don't skip prologue of ifunc resolvers with debug info
  Eliminate find_pc_partial_function_gnu_ifunc
  Factor out minsym_found/find_function_start_sal overload
  Extend GNU ifunc testcases
  Fix resolving GNU ifunc bp locations when inferior runs resolver

 gdb/blockframe.c                        |  62 +++--
 gdb/breakpoint.c                        |  31 ++-
 gdb/breakpoint.h                        |   8 +
 gdb/c-exp.y                             |  39 +++-
 gdb/elfread.c                           |  15 +-
 gdb/eval.c                              |  33 +--
 gdb/gdbtypes.c                          |   4 -
 gdb/infcall.c                           |  58 +++--
 gdb/infcall.h                           |   9 +-
 gdb/linespec.c                          | 151 ++++++++----
 gdb/minsyms.c                           |  91 ++++----
 gdb/minsyms.h                           |  26 ++-
 gdb/symtab.c                            |  45 ++--
 gdb/symtab.h                            |  29 ++-
 gdb/testsuite/gdb.base/gnu-ifunc-lib.c  |  12 +-
 gdb/testsuite/gdb.base/gnu-ifunc-resd.c |  22 ++
 gdb/testsuite/gdb.base/gnu-ifunc.c      |   6 -
 gdb/testsuite/gdb.base/gnu-ifunc.exp    | 394 +++++++++++++++++++++++++-------
 18 files changed, 728 insertions(+), 307 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/gnu-ifunc-resd.c

-- 
2.14.3

Re: [PATCH 00/11] Fixing GNU ifunc support

[Pedro Alves]

On 03/09/2018 09:16 PM, Pedro Alves wrote:

> Pedro Alves (11):
>   eval.c: reverse minsym and sym
>   Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol
>     creation)
>   Fix calling ifunc functions when resolver has debug info and different
>     name
>   Calling ifunc functions when target has no debug info but resolver has
>   Calling ifunc functions when resolver has debug info, user symbol same
>     name
>   Fix setting breakpoints on ifunc functions after they're already
>     resolved
>   Breakpoints, don't skip prologue of ifunc resolvers with debug info
>   Eliminate find_pc_partial_function_gnu_ifunc
>   Factor out minsym_found/find_function_start_sal overload
>   Extend GNU ifunc testcases
>   Fix resolving GNU ifunc bp locations when inferior runs resolver

As always, I forgot to say that I pushed this to the
"users/palves/ifunc" branch on sourceware.org.

Thanks,
Pedro Alves

Re: [PATCH 10/11] Extend GNU ifunc testcases

[Pedro Alves]

For convenience, here's the same patch with "git diff -w",
easier to read because the patch moves code at global
scope to functions.

diff --git c/gdb/testsuite/gdb.base/gnu-ifunc-lib.c w/gdb/testsuite/gdb.base/gnu-ifunc-lib.c
index b9d446c92fa..7aac81faec6 100644
--- c/gdb/testsuite/gdb.base/gnu-ifunc-lib.c
+++ w/gdb/testsuite/gdb.base/gnu-ifunc-lib.c
@@ -22,10 +22,14 @@ extern int final (int arg);
 
 typedef int (*final_t) (int arg);
 
+#ifndef IFUNC_RESOLVER_ATTR
 asm (".type gnu_ifunc, %gnu_indirect_function");
-
 final_t
 gnu_ifunc (unsigned long hwcap)
+#else
+final_t
+gnu_ifunc_resolver (unsigned long hwcap)
+#endif
 {
   resolver_hwcap = hwcap;
   if (! gnu_ifunc_initialized)
@@ -33,3 +37,9 @@ gnu_ifunc (unsigned long hwcap)
   else
     return final;
 }
+
+#ifdef IFUNC_RESOLVER_ATTR
+extern int gnu_ifunc (int arg);
+
+__typeof (gnu_ifunc) gnu_ifunc __attribute__ ((ifunc ("gnu_ifunc_resolver")));
+#endif
diff --git c/gdb/testsuite/gdb.base/gnu-ifunc-resd.c w/gdb/testsuite/gdb.base/gnu-ifunc-resd.c
new file mode 100644
index 00000000000..ef19fc98fab
--- /dev/null
+++ w/gdb/testsuite/gdb.base/gnu-ifunc-resd.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2009-2018 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/>.  */
+
+int
+final (int arg)
+{
+  return arg + 1;
+}
diff --git c/gdb/testsuite/gdb.base/gnu-ifunc.c w/gdb/testsuite/gdb.base/gnu-ifunc.c
index 78fd30d9fd2..a4abacaccee 100644
--- c/gdb/testsuite/gdb.base/gnu-ifunc.c
+++ w/gdb/testsuite/gdb.base/gnu-ifunc.c
@@ -23,12 +23,6 @@ init_stub (int arg)
   return 0;
 }
 
-int
-final (int arg)
-{
-  return arg + 1;
-}
-
 /* Make differentiation of how the gnu_ifunc call resolves before and after
    calling gnu_ifunc_pre.  This ensures the resolved function address is not
    being cached anywhere for the debugging purposes.  */
diff --git c/gdb/testsuite/gdb.base/gnu-ifunc.exp w/gdb/testsuite/gdb.base/gnu-ifunc.exp
index 1d0d0401c37..fa1464bec51 100644
--- c/gdb/testsuite/gdb.base/gnu-ifunc.exp
+++ w/gdb/testsuite/gdb.base/gnu-ifunc.exp
@@ -18,47 +18,177 @@ if {[skip_shlib_tests]} {
 }
 
 standard_testfile .c
-set executable ${testfile}
-set staticexecutable ${executable}-static
+set staticexecutable ${testfile}-static
 set staticbinfile [standard_output_file ${staticexecutable}]
 
 set libfile "${testfile}-lib"
 set libsrc ${libfile}.c
-set lib_so [standard_output_file ${libfile}.so]
+
+set resdfile "${testfile}-resd"
+set resdsrc ${resdfile}.c
+
+if [get_compiler_info] {
+    return -1
+}
+
+# Return the binary suffix appended to program and library names to
+# make each testcase variant unique.
+proc make_binsuffix {resolver_attr resolver_debug resolved_debug} {
+    return "$resolver_attr-$resolver_debug-$resolved_debug"
+}
+
+# Compile the testcase.  RESOLVER_ATTR is true if we're testing with
+# an ifunc resolver that has a different name from the user symbol,
+# specified with GCC's __attribute__ ifunc.  RESOLVER_DEBUG is true
+# iff the resolver was compiled with debug info.  RESOLVED_DEBUG is
+# true iff the target function was compiled with debug info.
+proc build {resolver_attr resolver_debug resolved_debug} {
+    global srcdir subdir srcfile binfile
+    global libsrc lib_so libfile
+    global exec_opts executable
+    global hex gdb_prompt
+    global resdfile resdsrc
+
+    set suffix [make_binsuffix $resolver_attr $resolver_debug $resolved_debug]
+
+    set lib_so [standard_output_file ${libfile}-$suffix.so]
     # $lib_o must not have {debug}, it would override the STT_GNU_IFUNC ELF markers.
-set lib_o [standard_output_file ${libfile}.o]
+    set lib_o [standard_output_file ${libfile}-$suffix.o]
 
-# We need DWARF for the "final" function as we "step" into the function and GDB
-# would step-over the "final" function if there would be no line number debug
-# information (DWARF) available.
-#
-# We must not have DWARF for the "gnu_ifunc" function as DWARF has no way to
-# express the STT_GNU_IFUNC type and it would be considered as a regular
-# function due to DWARF by GDB.  In ELF gnu-ifunc is expressed by the
-# STT_GNU_IFUNC type.
-#
-# Both functions need to be in the same shared library file but
-# gdb_compile_shlib has no way to specify source-specific compilation options.
-#
-# Therefore $libfile contains only the STT_GNU_IFUNC function with no DWARF
-# referencing all the other parts from the main executable with DWARF.
+    set exec_opts [list debug shlib=$lib_so]
 
     set lib_opts {}
-set exec_opts [list debug shlib=$lib_so]
+    set resd_opts {}
 
-if [get_compiler_info] {
-    return -1
+    if {$resolver_attr} {
+	lappend lib_opts "additional_flags=-DIFUNC_RESOLVER_ATTR"
     }
 
-if { [gdb_compile_shlib ${srcdir}/${subdir}/$libsrc $lib_so $lib_opts] != ""
-     || [gdb_compile ${srcdir}/${subdir}/$srcfile $binfile executable $exec_opts] != ""} {
-    untested "failed to compile first testcase"
-    return -1
+    if {$resolver_debug} {
+	lappend lib_opts "debug"
+    }
+
+    if {$resolved_debug} {
+	lappend resd_opts "debug"
+    }
+
+    set resd_o $resdfile-$suffix.o
+
+    if { [gdb_compile_shlib ${srcdir}/${subdir}/$libsrc \
+	      $lib_so $lib_opts] != ""
+	 || [gdb_compile ${srcdir}/${subdir}/$resdsrc \
+		 $resd_o object $resd_opts] != ""
+	 || [gdb_compile [list ${srcdir}/${subdir}/$srcfile $resd_o] \
+		 $binfile-$suffix executable $exec_opts] != ""} {
+	untested "failed to compile testcase"
+	return 0
+    }
+
+    return 1
+}
+
+# Test setting a breakpoint on a ifunc function before and after the
+# ifunc is resolved.  RESOLVER_ATTR is true if we're testing with a
+# resolver specified with __attribute__ ifunc.  RESOLVER_DEBUG is true
+# iff the resolver was compiled with debug info.  RESOLVED_DEBUG is
+# true iff the target function was compiled with debug info.
+proc_with_prefix set-break {resolver_attr resolver_debug resolved_debug} {
+    global binfile libfile lib_so
+    global hex decimal
+    global gdb_prompt
+
+    set suffix [make_binsuffix $resolver_attr $resolver_debug $resolved_debug]
+
+    set lib_so [standard_output_file ${libfile}-$suffix.so]
+    clean_restart $binfile-$suffix
+    gdb_load_shlib ${lib_so}
+
+    if ![runto_main] then {
+	fail "can't run to main"
+	return 1
+    }
+
+    set ws "\[ \t\]+"
+
+    if {$resolver_attr} {
+	set gnu_ifunc_resolver "gnu_ifunc_resolver"
+    } else {
+	set gnu_ifunc_resolver "gnu_ifunc"
+    }
+
+    with_test_prefix "before resolving" {
+	delete_breakpoints
+	gdb_test "break gnu_ifunc" \
+	    "Breakpoint $decimal at gnu-indirect-function resolver at $hex"
+	gdb_test "info breakpoints" \
+	    "$decimal${ws}STT_GNU_IFUNC resolver${ws}keep${ws}y${ws}$hex <${gnu_ifunc_resolver}>"
+    }
+
+    global resdsrc
+
+    with_test_prefix "resolve" {
+	delete_breakpoints
+	gdb_breakpoint [gdb_get_line_number "break-at-exit"]
+	gdb_continue_to_breakpoint "break-at-exit" ".*break-at-exit.*"
+    }
+
+    with_test_prefix "after resolving" {
+	delete_breakpoints
+
+	if {!$resolved_debug} {
+	    # Set a breakpoint both at the ifunc, and at the ifunc's
+	    # target.  GDB should resolve both to the same address.
+	    # Start with the ifunc's target.
+	    set addr "-"
+	    set test "break final"
+	    # Extract the address without the leading "0x", because
+	    # addresses in "info break" output include leading 0s
+	    # (like "0x0000ADDR").
+	    set hex_number {[0-9a-fA-F][0-9a-fA-F]*}
+	    gdb_test_multiple $test $test {
+		-re "Breakpoint .* at 0x($hex_number)\r\n$gdb_prompt $" {
+		    set addr $expect_out(1,string)
+		    pass $test
+		}
+	    }
+
+	    # Now set a break at the ifunc.
+	    gdb_test "break gnu_ifunc" "Breakpoint .* at 0x$addr"
+	    set location "$decimal${ws}breakpoint${ws}keep${ws}y${ws}0x0*$addr${ws}<final\\+.*>"
+	} else {
+	    set lineno -1
+	    set test "break final"
+	    gdb_test_multiple $test $test {
+		-re "Breakpoint .* at $hex: file .*$resdsrc, line ($decimal)\\.\r\n$gdb_prompt $" {
+		    set lineno $expect_out(1,string)
+		    pass $test
+		}
+	    }
+	    gdb_test "break gnu_ifunc" "Breakpoint .* at $hex: file .*$resdsrc, line $lineno\\."
+	    set location "$decimal${ws}breakpoint${ws}keep${ws}y${ws}$hex in final at .*$resdsrc:$lineno"
+	}
+	gdb_test "info breakpoints" "$location\r\n$location"
+    }
+}
+
+proc misc_tests {resolver_attr resolver_debug resolved_debug} {
+    global srcdir subdir srcfile binfile
+    global libsrc lib_so libfile
+    global exec_opts executable
+    global hex gdb_prompt
+    global resdfile resdsrc
+
+    set suffix [make_binsuffix $resolver_attr $resolver_debug $resolved_debug]
+
+    if {$resolver_attr} {
+	set gnu_ifunc_resolver "gnu_ifunc_resolver"
+    } else {
+	set gnu_ifunc_resolver "gnu_ifunc"
     }
 
     # Start with a fresh gdb.
 
-clean_restart $executable
+    clean_restart $binfile-$suffix
     gdb_load_shlib ${lib_so}
 
     if ![runto_main] then {
@@ -67,14 +197,23 @@ if ![runto_main] then {
     }
 
     # The "if" condition is artifical to test regression of a former patch.
-gdb_breakpoint "[gdb_get_line_number "break-at-nextcall"] if i && gnu_ifunc (i) != 42"
+    gdb_breakpoint "[gdb_get_line_number "break-at-nextcall"] if i && (int) gnu_ifunc (i) != 42"
 
     gdb_breakpoint [gdb_get_line_number "break-at-call"]
     gdb_continue_to_breakpoint "break-at-call" ".*break-at-call.*"
 
     # Test GDB will automatically indirect the call.
 
+    if {!$resolver_debug && !$resolved_debug} {
+	gdb_test "p gnu_ifunc()" \
+	    "'final' has unknown return type; cast the call to its declared return type"
+	gdb_test "p gnu_ifunc (3)" \
+	    "'final' has unknown return type; cast the call to its declared return type"
+	gdb_test "p (int) gnu_ifunc (3)" " = 4"
+    } else {
+	gdb_test "p gnu_ifunc()" "Too few arguments in function call\\."
 	gdb_test "p gnu_ifunc (3)" " = 4"
+    }
 
     # Test that the resolver received its argument.
 
@@ -93,7 +232,16 @@ gdb_test "p/x resolver_hwcap" "= $actual_hwcap" "resolver received HWCAP"
 
     # Test GDB will skip the gnu_ifunc resolver on first call.
 
+    # Even if the resolver has debug info, stepping into an ifunc call
+    # should skip the resolver.
+    if {!$resolved_debug} {
+	# Make GDB stop stepping even if it steps into a function with
+	# no debug info.
+	gdb_test_no_output "set step-mode on"
+	gdb_test "step" "$hex in final \\\(\\\)"
+    } else {
 	gdb_test "step" "\r\nfinal .*"
+    }
 
     # Test GDB will not break before the final chosen implementation.
 
@@ -104,14 +252,16 @@ gdb_test "step" "\r\nfinal .*"
     #
     # Breakpoint 2, main () at ./gdb.base/gnu-ifunc.c:33
 
-gdb_test "continue" "Continuing.\r\n\r\nBreakpoint .* (at|in) .*break-at-nextcall.*" \
+    gdb_test "continue" \
+	"Continuing.\r\n\r\nBreakpoint .* (at|in) .*break-at-nextcall.*" \
 	"continue to break-at-nextcall"
 
     gdb_breakpoint "gnu_ifunc"
 
     gdb_continue_to_breakpoint "nextcall gnu_ifunc"
 
-gdb_test "frame" "#0 +(0x\[0-9a-f\]+ in +)?final \\(.*" "nextcall gnu_ifunc skipped"
+    gdb_test "frame" \
+	"#0 +(0x\[0-9a-f\]+ in +)?final \\(.*" "nextcall gnu_ifunc skipped"
 
 
     # Check any commands not doing an inferior call access the address of the
@@ -125,25 +275,82 @@ if {[istarget powerpc64-*] && [is_lp64_target]} {
 	set func_prefix {}
     }
 
-gdb_test "p gnu_ifunc" " = {<text gnu-indirect-function variable, no debug info>} 0x\[0-9a-f\]+ <${func_prefix}gnu_ifunc>" "p gnu_ifunc executing"
-gdb_test "info sym gnu_ifunc" "gnu_ifunc in section .*" "info sym gnu_ifunc executing"
+    gdb_test "p gnu_ifunc" \
+	" = {<text gnu-indirect-function variable, no debug info>} 0x\[0-9a-f\]+ <${func_prefix}${gnu_ifunc_resolver}>" \
+	"p gnu_ifunc executing"
+    gdb_test "info sym gnu_ifunc" \
+	"${gnu_ifunc_resolver} in section .*" \
+	"info sym gnu_ifunc executing"
 
     set test "info addr gnu_ifunc"
+    if {!$resolver_attr && $resolver_debug} {
+	gdb_test_multiple $test $test {
+	    -re "Symbol \"gnu_ifunc\" is a function at address (0x\[0-9a-f\]+).*$gdb_prompt $" {
+		pass $test
+	    }
+	}
+    } else {
 	gdb_test_multiple $test $test {
 	    -re "Symbol \"gnu_ifunc\" is at (0x\[0-9a-f\]+) in .*$gdb_prompt $" {
 		pass $test
 	    }
 	}
-gdb_test "info sym $expect_out(1,string)" "gnu_ifunc in section .*" "info sym <gnu_ifunc-address>"
+    }
+    gdb_test "info sym $expect_out(1,string)" \
+	"${gnu_ifunc_resolver} in section .*" \
+	"info sym <gnu_ifunc-address>"
+
+    # Test calling the resolver directly instead of the ifunc symbol.
+    # Can only do that if the ifunc and the ifunc resolver have
+    # different names.
+    if {$resolver_attr} {
+	if {$resolver_debug} {
+	    gdb_test "p gnu_ifunc_resolver(0)" \
+		" = \\(int \\(\\*\\)\\(int\\)\\) $hex <final>"
+	} else {
+	    gdb_test "p gnu_ifunc_resolver(0)" \
+		"'gnu_ifunc_resolver' has unknown return type; cast the call to its declared return type"
+	    gdb_test "p (void *) gnu_ifunc_resolver(0)" \
+		" = \\(void \\*\\) $hex <final>"
+	}
+    }
+}
 
+# Test all the combinations of:
+#
+# - An ifunc resolver with the same name as the ifunc symbol vs an
+#   ifunc resolver with a different name as the ifunc symbol.
+#
+# - ifunc resolver compiled with and without debug info.  This ensures
+#   that GDB understands that a function not a regular function by
+#   looking at the STT_GNU_IFUNC type in the elf symbols.  DWARF has
+#   no way to express the STT_GNU_IFUNC type.
+#
+# - ifunc target function (resolved) compiled with and without debug
+#   info.
+foreach_with_prefix resolver_attr {0 1} {
+    foreach_with_prefix resolver_debug {0 1} {
+	foreach_with_prefix resolved_debug {0 1} {
+	    build $resolver_attr $resolver_debug $resolved_debug
+	    misc_tests $resolver_attr $resolver_debug $resolved_debug
+	    set-break $resolver_attr $resolver_debug $resolved_debug
+	}
+    }
+}
 
 # Test statically linked ifunc resolving during inferior start.
 # https://bugzilla.redhat.com/show_bug.cgi?id=624967
 
-# Compile $staticbinfile separately as it may exit on error (ld/12595).
+with_test_prefix "static" {
+    # Compile $staticbinfile separately as it may exit on error
+    # (ld/12595).
 
+    set lib_o [standard_output_file ${libfile}.o]
+    set resd_o [standard_output_file ${resdfile}.o]
     if { [gdb_compile ${srcdir}/${subdir}/$libsrc $lib_o object {}] != ""
-     || [gdb_compile "${srcdir}/${subdir}/$srcfile $lib_o" $staticbinfile executable {debug}] != "" } {
+	 || [gdb_compile ${srcdir}/${subdir}/$resdsrc $resd_o object {}] != ""
+	 || [gdb_compile "${srcdir}/${subdir}/$srcfile $lib_o $resd_o" \
+		 $staticbinfile executable {debug}] != "" } {
 	untested "failed to compile second testcase"
 	return -1
     }
@@ -154,3 +361,4 @@ gdb_breakpoint "gnu_ifunc"
     gdb_breakpoint "main"
     gdb_run_cmd
     gdb_test "" "Breakpoint \[0-9\]*, main .*" "static gnu_ifunc"
+}

[PATCH 11/11] Fix resolving GNU ifunc bp locations when inferior runs resolver

[Pedro Alves]

I noticed that if you set a breakpoint on an ifunc before the ifunc is
resolved, and then let the program call the ifunc, thus resolving it,
GDB end up with a location for that original breakpoint that is
pointing to the ifunc target, but it is left pointing to the first
address of the function, instead of after its prologue.  After
prologue is what you get if you create a new breakpoint at that point.

1) With no debug info for the target function:

  1.a) Set before resolving, and then program continued passed resolving:

    Num     Type           Disp Enb Address            What
    1       breakpoint     keep y   0x0000000000400753 <final>

  1.b) Breakpoint set after inferior resolved ifunc:

    Num     Type           Disp Enb Address            What
    2       breakpoint     keep y   0x0000000000400757 <final+4>


2) With debug info for the target function:

   1.a) Set before resolving, and then program continued passed resolving:

     Num     Type           Disp Enb Address            What
     1       breakpoint     keep y   0x0000000000400753 in final at gdb/testsuite/gdb.base/gnu-ifunc-resd.c:20

   1.b) Breakpoint set after inferior resolved ifunc:

     Num     Type           Disp Enb Address            What
     2       breakpoint     keep y   0x000000000040075a in final at gdb/testsuite/gdb.base/gnu-ifunc-resd.c:21

The problem is that elf_gnu_ifunc_resolver_return_stop (called by the
internal breakpoint that traps the resolver returning) does not agree
with linespec.c:minsym_found.  It does not skip to the function's
start line (i.e., past the prologue).  We can now use the
find_function_start_sal overload added by the previous commmit to fix
this.

New tests included, which fail before the patch, and pass afterwards.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* elfread.c (elf_gnu_ifunc_resolver_return_stop): Use
	find_function_start_sal instead of find_pc_line.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/gnu-ifunc.exp (set-break): Test that GDB resolves
	ifunc breakpoint locations correctly of ifunc breakpoints set
	while the program resolves the ifunc.
---
 gdb/elfread.c                        |  3 ++-
 gdb/testsuite/gdb.base/gnu-ifunc.exp | 18 ++++++++++++++----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 454b77e9bdc..0cb404186ff 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -990,7 +990,8 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
 
   b->type = bp_breakpoint;
   update_breakpoint_locations (b, current_program_space,
-			       find_pc_line (resolved_pc, 0), {});
+			       find_function_start_sal (resolved_pc, NULL, true),
+			       {});
 }
 
 /* A helper function for elf_symfile_read that reads the minimal
diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
index fa1464bec51..c936aba7c14 100644
--- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
+++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
@@ -108,6 +108,9 @@ proc_with_prefix set-break {resolver_attr resolver_debug resolved_debug} {
 	return 1
     }
 
+    gdb_breakpoint [gdb_get_line_number "break-at-call"]
+    gdb_continue_to_breakpoint "break-at-call" ".*break-at-call.*"
+
     set ws "\[ \t\]+"
 
     if {$resolver_attr} {
@@ -122,19 +125,21 @@ proc_with_prefix set-break {resolver_attr resolver_debug resolved_debug} {
 	    "Breakpoint $decimal at gnu-indirect-function resolver at $hex"
 	gdb_test "info breakpoints" \
 	    "$decimal${ws}STT_GNU_IFUNC resolver${ws}keep${ws}y${ws}$hex <${gnu_ifunc_resolver}>"
+
+	# Make the breakpoint conditional on a condition that always
+	# fails.  This is so that when the ifunc-resolver breakpoint
+	# triggers, GDB resumes the program immediately.
+	gdb_test_no_output "condition \$bpnum 0"
     }
 
     global resdsrc
 
     with_test_prefix "resolve" {
-	delete_breakpoints
 	gdb_breakpoint [gdb_get_line_number "break-at-exit"]
 	gdb_continue_to_breakpoint "break-at-exit" ".*break-at-exit.*"
     }
 
     with_test_prefix "after resolving" {
-	delete_breakpoints
-
 	if {!$resolved_debug} {
 	    # Set a breakpoint both at the ifunc, and at the ifunc's
 	    # target.  GDB should resolve both to the same address.
@@ -167,7 +172,12 @@ proc_with_prefix set-break {resolver_attr resolver_debug resolved_debug} {
 	    gdb_test "break gnu_ifunc" "Breakpoint .* at $hex: file .*$resdsrc, line $lineno\\."
 	    set location "$decimal${ws}breakpoint${ws}keep${ws}y${ws}$hex in final at .*$resdsrc:$lineno"
 	}
-	gdb_test "info breakpoints" "$location\r\n$location"
+
+	# The first location here is for the breakpoint that was set
+	# before the ifunc was resolved.  It should be resolved by
+	# now, and it should have the exact same address/line as the
+	# other two locations.
+	gdb_test "info breakpoints" "$location\r\n.*$location\r\n$location"
     }
 }
 
-- 
2.14.3

[PATCH 05/11] Calling ifunc functions when resolver has debug info, user symbol same name

[Pedro Alves]

If the GNU ifunc resolver has the same name as the user visible
symbol, and the resolver has debug info, then the DWARF info for the
resolver masks the ifunc minsym.  In that scenario, if you try calling
the ifunc from GDB, you call the resolver instead.  With the
gnu-ifunc.exp testcase added in a following patch, you'd see:

  (gdb) p gnu_ifunc (3)
  $1 = (int (*)(int)) 0x400753 <final>
  (gdb) FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: resolved_debug=0: p gnu_ifunc (3)
                                                       ^^^^^^^^^^^^^^^^

That is, we called the ifunc resolver manually, which returned a
pointer to the ifunc target function ("final").  The "final" symbol is
the function that GDB should have called automatically,

  ~~~~~~~~~~~~
  int
  final (int arg)
  {
    return arg + 1;
  }
  ~~~~~~~~~

which is what happens if you don't have debug info for the resolver:

  (gdb) p gnu_ifunc (3)
  $1 = 4
  (gdb) PASS: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0: resolved_debug=1: p gnu_ifunc (3)
                                                       ^^^^^^^^^^^^^^^^

or if the resolver's symbol has a different name from the ifunc (as is
the case with modern uses of ifunc via __attribute__ ifunc, such as
glibc uses):

  (gdb) p gnu_ifunc (3)
  $1 = 4
  (gdb) PASS: gdb.base/gnu-ifunc.exp: resolver_attr=1: resolver_debug=1: resolved_debug=0: p gnu_ifunc (3)
                                      ^^^^^^^^^^^^^^^

in which case after this patch, you can still call the resolver
directly if you want:

  (gdb) p gnu_ifunc_resolver (3)
  $1 = (int (*)(int)) 0x400753 <final>

We need to tweak lookup_minimal_symbol_by_pc_section to make it
possible to prefer the mst_text_gnu_ifunc symbol over the regular
mst_text symbol, which is the ifunc resolver in this case.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* c-exp.y (variable production): Prefer ifunc minsyms over
	regular function symbols.
	* minsyms.c (lookup_minimal_symbol_by_pc_section_1): Rename to ...
	(lookup_minimal_symbol_by_pc_section): ... this.  Replace
	'want_trampoline' parameter by a lookup_msym_prefer parameter.
	Handle it.
	(lookup_minimal_symbol_by_pc_section): Delete old implementation.
	(lookup_minimal_symbol_by_pc): Adjust.
	(in_gnu_ifunc_stub): Prefer GNU ifunc symbols.
	(find_gnu_ifunc): New function.
	(lookup_solib_trampoline_symbol_by_pc): Adjust.
	* minsyms.h (lookup_msym_prefer): New enum.
	(lookup_minimal_symbol_by_pc_section): Replace 'want_trampoline'
	parameter by a lookup_msym_prefer parameter.
	* symtab.h (find_gnu_ifunc): New declaration.
---
 gdb/c-exp.y   | 35 ++++++++++++++++++++---
 gdb/minsyms.c | 91 +++++++++++++++++++++++++++++------------------------------
 gdb/minsyms.h | 26 +++++++++++++++--
 gdb/symtab.h  |  4 +++
 4 files changed, 103 insertions(+), 53 deletions(-)

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index e2ea07cd792..8d27887849d 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1038,13 +1038,40 @@ variable:	name_not_typename
 
 			  if (sym.symbol)
 			    {
+			      bool skip = false;
+
 			      if (symbol_read_needs_frame (sym.symbol))
 				innermost_block.update (sym);
 
-			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
-			      write_exp_elt_block (pstate, sym.block);
-			      write_exp_elt_sym (pstate, sym.symbol);
-			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
+			      /* If we found a function, see if it's
+				 an ifunc resolver that has the same
+				 address as the ifunc symbol itself.
+				 If so, prefer the ifunc symbol.  */
+			      if (SYMBOL_CLASS (sym.symbol) == LOC_BLOCK)
+				{
+				  CORE_ADDR pc = BLOCK_START (SYMBOL_BLOCK_VALUE
+							      (sym.symbol));
+
+				  bound_minimal_symbol msymbol
+				    = find_gnu_ifunc (pc);
+				  if (msymbol.minsym != NULL
+				      && strcmp (MSYMBOL_LINKAGE_NAME
+						 (msymbol.minsym),
+						 SYMBOL_LINKAGE_NAME
+						 (sym.symbol)) == 0)
+				    {
+				      write_exp_msymbol (pstate, msymbol);
+				      skip = true;
+				    }
+				}
+
+			      if (!skip)
+				{
+				  write_exp_elt_opcode (pstate, OP_VAR_VALUE);
+				  write_exp_elt_block (pstate, sym.block);
+				  write_exp_elt_sym (pstate, sym.symbol);
+				  write_exp_elt_opcode (pstate, OP_VAR_VALUE);
+				}
 			    }
 			  else if ($1.is_a_field_of_this)
 			    {
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index a55c0718fcb..b88550351fb 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -670,10 +670,9 @@ frob_address (struct objfile *objfile, CORE_ADDR *pc)
    there are text and trampoline symbols at the same address.
    Otherwise prefer mst_text symbols.  */
 
-static struct bound_minimal_symbol
-lookup_minimal_symbol_by_pc_section_1 (CORE_ADDR pc_in,
-				       struct obj_section *section,
-				       int want_trampoline)
+bound_minimal_symbol
+lookup_minimal_symbol_by_pc_section (CORE_ADDR pc_in, struct obj_section *section,
+				     lookup_msym_prefer prefer)
 {
   int lo;
   int hi;
@@ -683,10 +682,27 @@ lookup_minimal_symbol_by_pc_section_1 (CORE_ADDR pc_in,
   struct minimal_symbol *best_symbol = NULL;
   struct objfile *best_objfile = NULL;
   struct bound_minimal_symbol result;
-  enum minimal_symbol_type want_type, other_type;
+  enum minimal_symbol_type want_type;
 
-  want_type = want_trampoline ? mst_solib_trampoline : mst_text;
-  other_type = want_trampoline ? mst_text : mst_solib_trampoline;
+  if (section == NULL)
+    {
+      section = find_pc_section (pc_in);
+      if (section == NULL)
+	return {};
+    }
+
+  switch (prefer)
+    {
+    case lookup_msym_prefer::TEXT:
+      want_type = mst_text;
+      break;
+    case lookup_msym_prefer::TRAMPOLINE:
+      want_type = mst_solib_trampoline;
+      break;
+    case lookup_msym_prefer::GNU_IFUNC:
+      want_type = mst_text_gnu_ifunc;
+      break;
+    }
 
   /* We can not require the symbol found to be in section, because
      e.g. IRIX 6.5 mdebug relies on this code returning an absolute
@@ -805,7 +821,7 @@ lookup_minimal_symbol_by_pc_section_1 (CORE_ADDR pc_in,
 		     preceding symbol too.  If they are otherwise
 		     identical prefer that one.  */
 		  if (hi > 0
-		      && MSYMBOL_TYPE (&msymbol[hi]) == other_type
+		      && MSYMBOL_TYPE (&msymbol[hi]) != want_type
 		      && MSYMBOL_TYPE (&msymbol[hi - 1]) == want_type
 		      && (MSYMBOL_SIZE (&msymbol[hi])
 			  == MSYMBOL_SIZE (&msymbol[hi - 1]))
@@ -902,41 +918,12 @@ lookup_minimal_symbol_by_pc_section_1 (CORE_ADDR pc_in,
   return result;
 }
 
-struct bound_minimal_symbol
-lookup_minimal_symbol_by_pc_section (CORE_ADDR pc, struct obj_section *section)
-{
-  if (section == NULL)
-    {
-      /* NOTE: cagney/2004-01-27: This was using find_pc_mapped_section to
-	 force the section but that (well unless you're doing overlay
-	 debugging) always returns NULL making the call somewhat useless.  */
-      section = find_pc_section (pc);
-      if (section == NULL)
-	{
-	  struct bound_minimal_symbol result;
-
-	  memset (&result, 0, sizeof (result));
-	  return result;
-	}
-    }
-  return lookup_minimal_symbol_by_pc_section_1 (pc, section, 0);
-}
-
 /* See minsyms.h.  */
 
 struct bound_minimal_symbol
 lookup_minimal_symbol_by_pc (CORE_ADDR pc)
 {
-  struct obj_section *section = find_pc_section (pc);
-
-  if (section == NULL)
-    {
-      struct bound_minimal_symbol result;
-
-      memset (&result, 0, sizeof (result));
-      return result;
-    }
-  return lookup_minimal_symbol_by_pc_section_1 (pc, section, 0);
+  return lookup_minimal_symbol_by_pc_section (pc, NULL);
 }
 
 /* Return non-zero iff PC is in an STT_GNU_IFUNC function resolver.  */
@@ -944,11 +931,26 @@ lookup_minimal_symbol_by_pc (CORE_ADDR pc)
 int
 in_gnu_ifunc_stub (CORE_ADDR pc)
 {
-  struct bound_minimal_symbol msymbol = lookup_minimal_symbol_by_pc (pc);
-
+  bound_minimal_symbol msymbol
+    = lookup_minimal_symbol_by_pc_section (pc, NULL,
+					   lookup_msym_prefer::GNU_IFUNC);
   return msymbol.minsym && MSYMBOL_TYPE (msymbol.minsym) == mst_text_gnu_ifunc;
 }
 
+/* See symtab.h.  */
+
+bound_minimal_symbol
+find_gnu_ifunc (CORE_ADDR pc)
+{
+  bound_minimal_symbol msymbol
+    = lookup_minimal_symbol_by_pc_section (pc, NULL,
+					   lookup_msym_prefer::GNU_IFUNC);
+  if (MSYMBOL_TYPE (msymbol.minsym) == mst_text_gnu_ifunc
+      && BMSYMBOL_VALUE_ADDRESS (msymbol) == pc)
+    return msymbol;
+  return {};
+}
+
 /* See elf_gnu_ifunc_resolve_addr for its real implementation.  */
 
 static CORE_ADDR
@@ -1465,12 +1467,9 @@ terminate_minimal_symbol_table (struct objfile *objfile)
 static struct minimal_symbol *
 lookup_solib_trampoline_symbol_by_pc (CORE_ADDR pc)
 {
-  struct obj_section *section = find_pc_section (pc);
-  struct bound_minimal_symbol msymbol;
-
-  if (section == NULL)
-    return NULL;
-  msymbol = lookup_minimal_symbol_by_pc_section_1 (pc, section, 1);
+  bound_minimal_symbol msymbol
+    = lookup_minimal_symbol_by_pc_section (pc, NULL,
+					   lookup_msym_prefer::TRAMPOLINE);
 
   if (msymbol.minsym != NULL
       && MSYMBOL_TYPE (msymbol.minsym) == mst_solib_trampoline)
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index 78b32e8d1c4..ddcb5d01e23 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -238,6 +238,22 @@ struct bound_minimal_symbol lookup_minimal_symbol_solib_trampoline
 struct minimal_symbol *lookup_minimal_symbol_by_pc_name
     (CORE_ADDR, const char *, struct objfile *);
 
+enum class lookup_msym_prefer
+{
+  /* Prefer mst_text symbols.  */
+  TEXT,
+
+  /* Prefer mst_solib_trampoline symbols when there are text and
+     trampoline symbols at the same address.  Otherwise prefer
+     mst_text symbols.  */
+  TRAMPOLINE,
+
+  /* Prefer mst_text_gnu_ifunc symbols when there are text and ifunc
+     symbols at the same address.  Otherwise prefer mst_text
+     symbols.  */
+  GNU_IFUNC,
+};
+
 /* Search through the minimal symbol table for each objfile and find
    the symbol whose address is the largest address that is still less
    than or equal to PC, and which matches SECTION.
@@ -246,11 +262,15 @@ struct minimal_symbol *lookup_minimal_symbol_by_pc_name
    instead.
 
    The result has a non-NULL 'minsym' member if such a symbol is
-   found, or NULL if PC is not in a suitable range.  */
+   found, or NULL if PC is not in a suitable range.
+
+   See definition of lookup_msym_prefer for description of PREFER.  By
+   default mst_text symbols are preferred.  */
 
 struct bound_minimal_symbol lookup_minimal_symbol_by_pc_section
-    (CORE_ADDR,
-     struct obj_section *);
+  (CORE_ADDR,
+   struct obj_section *,
+   lookup_msym_prefer prefer = lookup_msym_prefer::TEXT);
 
 /* Backward compatibility: search through the minimal symbol table 
    for a matching PC (no section given).
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 22b52019ee3..5dc52c58e0e 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1720,6 +1720,10 @@ extern struct type *basic_lookup_transparent_type (const char *);
 
 extern int in_gnu_ifunc_stub (CORE_ADDR pc);
 
+/* Look for a STT_GNU_IFUNC symbol at PC.  */
+
+extern bound_minimal_symbol find_gnu_ifunc (CORE_ADDR pc);
+
 /* Functions for resolving STT_GNU_IFUNC symbols which are implemented only
    for ELF symbol files.  */
 
-- 
2.14.3

[PATCH 04/11] Calling ifunc functions when target has no debug info but resolver has

[Pedro Alves]

After the previous patch, on Fedora 27 (glibc 2.26), if you try
calling strlen in the inferior, you now get:

  (top-gdb) p strlen ("hello")
  '__strlen_avx2' has unknown return type; cast the call to its declared return type

This is correct, because __strlen_avx2 is written in assembly.

We can improve on this though -- if the final ifunc resolved/target
function has no debug info, but the ifunc _resolver_ does have debug
info, we can try extracting the final function's type from the type
that the resolver returns.  E.g.,:

  typedef size_t (*strlen_t) (const char*);

  size_t my_strlen (const char *) { /* some implementation */ }
  strlen_t strlen_resolver (unsigned long hwcap) { return my_strlen; }

  extern size_t strlen (const char *s);
  __typeof (strlen) strlen __attribute__ ((ifunc ("strlen_resolver")));

In the strlen example above, the resolver returns strlen_t, which is a
typedef for pointer to a function that returns size_t.  "strlen_t" is
the type of both the user-visible "strlen", and of the the target
function that implements it.

This patch teaches GDB to extract that type.

This is done for actual inferior function calls (in infcall.c), and
for ptype (in eval_call).  By the time we get to either of these
places, we've already lost the original symbol/minsym, and only have
values and types to work with.  Hence the changes to c-exp.y and
evaluate_var_msym_value, to ensure that we propagate the ifunc
minsymbol's info.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* blockframe.c (gnu_ifunc_target_type): New function.
	* eval.c (evaluate_var_msym_value): For GNU ifunc types, always
	return a value with a memory address.
	(eval_call): For calls to GNU ifunc functions, try to find the
	type of the target function from the type that the resolver
	returns.
	* gdbtypes.c (objfile_type): Don't install a return type for ifunc
	symbols.
	* infcall.c (find_function_return_type): Rename to ...
	(find_function_type): ... this, and return the function's type
	instead of the function's return type.
	(find_function_addr): Add 'function_type' parameter.  For calls to
	GNU ifunc functions, try to find the type of the target function
	from the type that the resolver returns, and return it via
	FUNCTION_TYPE.
	(call_function_by_hand_dummy): Adjust to use the function type
	returned by find_function_addr.
	(find_function_addr): Add 'function_type' parameter and move
	description here.
	* symtab.h (find_gnu_ifunc_target_type): New declaration.
---
 gdb/blockframe.c | 34 +++++++++++++++++++++++++++++++++
 gdb/eval.c       | 25 ++++++++++++++----------
 gdb/gdbtypes.c   |  4 ----
 gdb/infcall.c    | 58 +++++++++++++++++++++++++++++++++++++-------------------
 gdb/infcall.h    |  9 ++++++++-
 gdb/symtab.h     |  6 ++++++
 6 files changed, 101 insertions(+), 35 deletions(-)

diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index 9be8871f756..db02b35742d 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -323,6 +323,40 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
   return find_pc_partial_function_gnu_ifunc (pc, name, address, endaddr, NULL);
 }
 
+/* See symtab.h.  */
+
+struct type *
+find_gnu_ifunc_target_type (CORE_ADDR resolver_funaddr)
+{
+  /* See if we can figure out the function's return type from the type
+     that the resolver returns.  */
+  symbol *sym = find_pc_function (resolver_funaddr);
+  if (sym != NULL
+      && SYMBOL_CLASS (sym) == LOC_BLOCK
+      && BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) == resolver_funaddr)
+    {
+      struct type *resolver_type = SYMBOL_TYPE (sym);
+      if (TYPE_CODE (resolver_type) == TYPE_CODE_FUNC)
+	{
+	  /* Get the return type of the resolver.  */
+	  struct type *resolver_ret_type
+	    = check_typedef (TYPE_TARGET_TYPE (resolver_type));
+
+	  /* If we found a pointer to function, then the resolved type
+	     is the type of the pointed-to function.  */
+	  if (TYPE_CODE (resolver_ret_type) == TYPE_CODE_PTR)
+	    {
+	      struct type *resolved_type
+		= TYPE_TARGET_TYPE (resolver_ret_type);
+	      if (TYPE_CODE (check_typedef (resolved_type)) == TYPE_CODE_FUNC)
+		return resolved_type;
+	    }
+	}
+    }
+
+  return NULL;
+}
+
 /* Return the innermost stack frame that is executing inside of BLOCK and is
    at least as old as the selected frame. Return NULL if there is no
    such frame.  If BLOCK is NULL, just return NULL.  */
diff --git a/gdb/eval.c b/gdb/eval.c
index a50299cbfdb..79f4d866e98 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -742,17 +742,13 @@ value *
 evaluate_var_msym_value (enum noside noside,
 			 struct objfile *objfile, minimal_symbol *msymbol)
 {
-  if (noside == EVAL_AVOID_SIDE_EFFECTS)
-    {
-      type *the_type = find_minsym_type_and_address (msymbol, objfile, NULL);
-      return value_zero (the_type, not_lval);
-    }
+  CORE_ADDR address;
+  type *the_type = find_minsym_type_and_address (msymbol, objfile, &address);
+
+  if (noside == EVAL_AVOID_SIDE_EFFECTS && !TYPE_GNU_IFUNC (the_type))
+    return value_zero (the_type, not_lval);
   else
-    {
-      CORE_ADDR address;
-      type *the_type = find_minsym_type_and_address (msymbol, objfile, &address);
-      return value_at_lazy (the_type, address);
-    }
+    return value_at_lazy (the_type, address);
 }
 
 /* Helper for returning a value when handling EVAL_SKIP.  */
@@ -805,6 +801,15 @@ eval_call (expression *exp, enum noside noside,
       else if (TYPE_CODE (ftype) == TYPE_CODE_FUNC
 	       || TYPE_CODE (ftype) == TYPE_CODE_METHOD)
 	{
+	  if (TYPE_GNU_IFUNC (ftype))
+	    {
+	      CORE_ADDR address = value_address (argvec[0]);
+	      type *resolved_type = find_gnu_ifunc_target_type (address);
+
+	      if (resolved_type != NULL)
+		ftype = resolved_type;
+	    }
+
 	  type *return_type = TYPE_TARGET_TYPE (ftype);
 
 	  if (return_type == NULL)
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index b3a037971e0..2efd1264ff8 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5443,10 +5443,6 @@ objfile_type (struct objfile *objfile)
   objfile_type->nodebug_text_gnu_ifunc_symbol
     = init_type (objfile, TYPE_CODE_FUNC, TARGET_CHAR_BIT,
 		 "<text gnu-indirect-function variable, no debug info>");
-  /* Ifunc resolvers return a function address.  */
-  TYPE_TARGET_TYPE (objfile_type->nodebug_text_gnu_ifunc_symbol)
-    = init_integer_type (objfile, gdbarch_addr_bit (gdbarch), 1,
-			 "__IFUNC_RESOLVER_RET");
   TYPE_GNU_IFUNC (objfile_type->nodebug_text_gnu_ifunc_symbol) = 1;
   objfile_type->nodebug_got_plt_symbol
     = init_pointer_type (objfile, gdbarch_addr_bit (gdbarch),
diff --git a/gdb/infcall.c b/gdb/infcall.c
index b7f4a176db5..c2710dd5519 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -229,26 +229,26 @@ value_arg_coerce (struct gdbarch *gdbarch, struct value *arg,
   return value_cast (type, arg);
 }
 
-/* Return the return type of a function with its first instruction exactly at
+/* Return the type of a function with its first instruction exactly at
    the PC address.  Return NULL otherwise.  */
 
 static struct type *
-find_function_return_type (CORE_ADDR pc)
+find_function_type (CORE_ADDR pc)
 {
   struct symbol *sym = find_pc_function (pc);
 
-  if (sym != NULL && BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) == pc
-      && SYMBOL_TYPE (sym) != NULL)
-    return TYPE_TARGET_TYPE (SYMBOL_TYPE (sym));
+  if (sym != NULL && BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) == pc)
+    return SYMBOL_TYPE (sym);
 
   return NULL;
 }
 
-/* Determine a function's address and its return type from its value.
-   Calls error() if the function is not valid for calling.  */
+/* See infcall.h.  */
 
 CORE_ADDR
-find_function_addr (struct value *function, struct type **retval_type)
+find_function_addr (struct value *function,
+		    struct type **retval_type,
+		    struct type **function_type)
 {
   struct type *ftype = check_typedef (value_type (function));
   struct gdbarch *gdbarch = get_type_arch (ftype);
@@ -275,17 +275,33 @@ find_function_addr (struct value *function, struct type **retval_type)
   if (TYPE_CODE (ftype) == TYPE_CODE_FUNC
       || TYPE_CODE (ftype) == TYPE_CODE_METHOD)
     {
-      value_type = TYPE_TARGET_TYPE (ftype);
-
       if (TYPE_GNU_IFUNC (ftype))
 	{
-	  funaddr = gnu_ifunc_resolve_addr (gdbarch, funaddr);
+	  CORE_ADDR resolver_addr = funaddr;
 
-	  /* Skip querying the function symbol if no RETVAL_TYPE has been
-	     asked for.  */
-	  if (retval_type)
-	    value_type = find_function_return_type (funaddr);
+	  /* Resolve the ifunc.  Note this may call the resolver
+	     function in the inferior.  */
+	  funaddr = gnu_ifunc_resolve_addr (gdbarch, resolver_addr);
+
+	  /* Skip querying the function symbol if no RETVAL_TYPE or
+	     FUNCTION_TYPE have been asked for.  */
+	  if (retval_type != NULL || function_type != NULL)
+	    {
+	      type *target_ftype = find_function_type (funaddr);
+	      /* If we don't have debug info for the target function,
+		 see if we can instead extract the target function's
+		 type from the type that the resolver returns.  */
+	      if (target_ftype == NULL)
+		target_ftype = find_gnu_ifunc_target_type (resolver_addr);
+	      if (target_ftype != NULL)
+		{
+		  value_type = TYPE_TARGET_TYPE (check_typedef (target_ftype));
+		  ftype = target_ftype;
+		}
+	    }
 	}
+      else
+	value_type = TYPE_TARGET_TYPE (ftype);
     }
   else if (TYPE_CODE (ftype) == TYPE_CODE_INT)
     {
@@ -320,6 +336,8 @@ find_function_addr (struct value *function, struct type **retval_type)
 
   if (retval_type != NULL)
     *retval_type = value_type;
+  if (function_type != NULL)
+    *function_type = ftype;
   return funaddr + gdbarch_deprecated_function_start_offset (gdbarch);
 }
 
@@ -730,7 +748,6 @@ call_function_by_hand_dummy (struct value *function,
   struct infcall_suspend_state *caller_state;
   CORE_ADDR funaddr;
   CORE_ADDR real_pc;
-  struct type *ftype = check_typedef (value_type (function));
   CORE_ADDR bp_addr;
   struct frame_id dummy_id;
   struct frame_info *frame;
@@ -741,9 +758,6 @@ call_function_by_hand_dummy (struct value *function,
   char name_buf[RAW_FUNCTION_ADDRESS_SIZE];
   int stack_temporaries = thread_stack_temporaries_enabled_p (inferior_ptid);
 
-  if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
-    ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
-
   if (!target_has_execution)
     noprocess ();
 
@@ -867,7 +881,11 @@ call_function_by_hand_dummy (struct value *function,
       }
   }
 
-  funaddr = find_function_addr (function, &values_type);
+  struct type *ftype = check_typedef (value_type (function));
+  if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
+    ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
+
+  funaddr = find_function_addr (function, &values_type, &ftype);
   if (values_type == NULL)
     values_type = default_return_type;
   if (values_type == NULL)
diff --git a/gdb/infcall.h b/gdb/infcall.h
index a3861fb1bf3..bea1494b50d 100644
--- a/gdb/infcall.h
+++ b/gdb/infcall.h
@@ -25,8 +25,15 @@
 struct value;
 struct type;
 
+/* Determine a function's address and its return type from its value.
+   If the function is a GNU ifunc, then return the address of the
+   target function, and set *FUNCTION_TYPE to the target function's
+   type, and *RETVAL_TYPE to the target function's return type..
+   Calls error() if the function is not valid for calling.  */
+
 extern CORE_ADDR find_function_addr (struct value *function, 
-				     struct type **retval_type);
+				     struct type **retval_type,
+				     struct type **function_type = NULL);
 
 /* Perform a function call in the inferior.
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index f9d52e76979..22b52019ee3 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1675,6 +1675,12 @@ extern int find_pc_partial_function_gnu_ifunc (CORE_ADDR pc, const char **name,
 extern int find_pc_partial_function (CORE_ADDR, const char **, CORE_ADDR *,
 				     CORE_ADDR *);
 
+/* See if we can figure out the function's actual type from the type
+   that the resolver returns.  RESOLVER_FUNADDR is the address of the
+   ifunc resolver.  */
+
+extern struct type *find_gnu_ifunc_target_type (CORE_ADDR resolver_funaddr);
+
 extern void clear_pc_function_cache (void);
 
 /* Expand symtab containing PC, SECTION if not already expanded.  */
-- 
2.14.3

[PATCH 06/11] Fix setting breakpoints on ifunc functions after they're already resolved

[Pedro Alves]

This fixes setting breakpoints on ifunc functions by name after the
ifunc has already been resolved.

In that case, if you have debug info for the ifunc resolver, without
the fix, then gdb puts a breakpoint past the prologue of the resolver,
instead of setting a breakpoint at the ifunc target:

  break gnu_ifunc
  Breakpoint 4 at 0x7ffff7bd36f2: file src/gdb/testsuite/gdb.base/gnu-ifunc-lib.c, line 34.
  (gdb) continue
  Continuing.
  [Inferior 1 (process 13300) exited normally]
  (gdb)

above we should have stopped at "final", but didn't because we never
resolved the ifunc to the final location.

If you don't have debug info for the resolver, GDB manages to resolve
the ifunc target, but, it should be setting a breakpoint after the
prologue of the final function, and instead what you get is that GDB
sets a breakpoint on the first address of the target function.  With
the gnu-ifunc.exp tests added by a later patch, we get, without the
fix:

  (gdb) break gnu_ifunc
  Breakpoint 4 at 0x400753
  (gdb) continue
  Continuing.

  Breakpoint 4, final (arg=1) at src/gdb/testsuite/gdb.base/gnu-ifunc-resd.c:20
  20	{

vs, fixed:

  (gdb) break gnu_ifunc
  Breakpoint 4 at 0x40075a: file src/gdb/testsuite/gdb.base/gnu-ifunc-resd.c, line 21.
  (gdb) continue
  Continuing.

  Breakpoint 4, final (arg=2) at src/gdb/testsuite/gdb.base/gnu-ifunc-resd.c:21
  21	  return arg + 1;
  (gdb)

Fix the problems above by moving the ifunc target resolving to
linespec.c, before we skip a function's prologue.  We need to save
something in the sal, so that set_breakpoint_location_function knows
that it needs to create a bp_gnu_ifunc_resolver bp_location.  Might as
well just save a pointer to the minsym.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (set_breakpoint_location_function): Don't resolve
	ifunc targets here.  Instead, if we have an ifunc minsym, use its
	address/name.
	(add_location_to_breakpoint): Store the minsym and the objfile in
	the breakpoint location.
	* breakpoint.h (bp_location) <msymbol, objfile>: New fields.
	* linespec.c (minsym_found): Resolve GNU ifunc targets here.
	Record the minsym in the sal.
	* symtab.h (symtab_and_line) <msymbol>: New field.
---
 gdb/breakpoint.c | 31 +++++++++++++------------------
 gdb/breakpoint.h |  8 ++++++++
 gdb/linespec.c   | 14 +++++++++++---
 gdb/symtab.h     |  1 +
 4 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 454fda7684a..928411149e4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7180,37 +7180,30 @@ set_breakpoint_location_function (struct bp_location *loc, int explicit_loc)
       || loc->owner->type == bp_hardware_breakpoint
       || is_tracepoint (loc->owner))
     {
-      int is_gnu_ifunc;
       const char *function_name;
-      CORE_ADDR func_addr;
 
-      find_pc_partial_function_gnu_ifunc (loc->address, &function_name,
-					  &func_addr, NULL, &is_gnu_ifunc);
-
-      if (is_gnu_ifunc && !explicit_loc)
+      if (loc->msymbol != NULL
+	  && MSYMBOL_TYPE (loc->msymbol) == mst_text_gnu_ifunc
+	  && !explicit_loc)
 	{
 	  struct breakpoint *b = loc->owner;
 
-	  gdb_assert (loc->pspace == current_program_space);
-	  if (gnu_ifunc_resolve_name (function_name,
-				      &loc->requested_address))
-	    {
-	      /* Recalculate ADDRESS based on new REQUESTED_ADDRESS.  */
-	      loc->address = adjust_breakpoint_address (loc->gdbarch,
-							loc->requested_address,
-							b->type);
-	    }
-	  else if (b->type == bp_breakpoint && b->loc == loc
-	           && loc->next == NULL && b->related_breakpoint == b)
+	  function_name = MSYMBOL_LINKAGE_NAME (loc->msymbol);
+
+	  if (b->type == bp_breakpoint && b->loc == loc
+	      && loc->next == NULL && b->related_breakpoint == b)
 	    {
 	      /* Create only the whole new breakpoint of this type but do not
 		 mess more complicated breakpoints with multiple locations.  */
 	      b->type = bp_gnu_ifunc_resolver;
 	      /* Remember the resolver's address for use by the return
 	         breakpoint.  */
-	      loc->related_address = func_addr;
+	      loc->related_address
+		= MSYMBOL_VALUE_ADDRESS (loc->objfile, loc->msymbol);
 	    }
 	}
+      else
+	find_pc_partial_function (loc->address, &function_name, NULL, NULL);
 
       if (function_name)
 	loc->function_name = xstrdup (function_name);
@@ -8674,6 +8667,8 @@ add_location_to_breakpoint (struct breakpoint *b,
   loc->line_number = sal->line;
   loc->symtab = sal->symtab;
   loc->symbol = sal->symbol;
+  loc->msymbol = sal->msymbol;
+  loc->objfile = sal->objfile;
 
   set_breakpoint_location_function (loc,
 				    sal->explicit_pc || sal->explicit_line);
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 8bb81d8d17e..0aeca3b7f95 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -491,6 +491,14 @@ public:
      ascertain when an event location was set at a different location than
      the one originally selected by parsing, e.g., inlined symbols.  */
   const struct symbol *symbol = NULL;
+
+  /* Similarly, the minimal symbol found by the location parser, if
+     any.  This may be used to ascertain if the location was
+     originally set on a GNU ifunc symbol.  */
+  const minimal_symbol *msymbol = NULL;
+
+  /* The objfile the symbol or minimal symbol were found in.  */
+  const struct objfile *objfile = NULL;
 };
 
 /* The possible return values for print_bpstat, print_it_normal,
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 1236b3f4754..1f1fb695c14 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -4334,10 +4334,17 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
 	      struct minimal_symbol *msymbol,
 	      std::vector<symtab_and_line> *result)
 {
-  struct symtab_and_line sal;
-
+  symtab_and_line sal;
   CORE_ADDR func_addr;
-  if (msymbol_is_function (objfile, msymbol, &func_addr))
+  bool is_function;
+
+  if (MSYMBOL_TYPE (msymbol) == mst_text_gnu_ifunc)
+    is_function = gnu_ifunc_resolve_name (MSYMBOL_LINKAGE_NAME (msymbol),
+					  &func_addr);
+  else
+    is_function = msymbol_is_function (objfile, msymbol, &func_addr);
+
+  if (is_function)
     {
       sal = find_pc_sect_line (func_addr, NULL, 0);
 
@@ -4360,6 +4367,7 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
   else
     {
       sal.objfile = objfile;
+      sal.msymbol = msymbol;
       sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
       sal.pspace = current_program_space;
     }
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 5dc52c58e0e..52ab1357300 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1761,6 +1761,7 @@ struct symtab_and_line
   struct symtab *symtab = NULL;
   struct symbol *symbol = NULL;
   struct obj_section *section = NULL;
+  struct minimal_symbol *msymbol = NULL;
   /* Line number.  Line numbers start at 1 and proceed through symtab->nlines.
      0 is never a valid line number; it is used to indicate that line number
      information is not available.  */
-- 
2.14.3

Re: [PATCH 01/11] eval.c: reverse minsym and sym

[Joel Brobecker]

> I noticed that in evaluate_funcall, where we handle
> OP_VAR_MSYM_VALUE/OP_VAR_VALUE to figure out the symbol's name gets
> the minimal_symbol/symbol backwards.  Happens to be harmless in
> practice because the symbol name is recorded in the common initial
> sequence (in the general_symbol_info field).
> 
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
> 
> 	* eval.c (evaluate_funcall): Swap OP_VAR_MSYM_VALUE/OP_VAR_VALUE
> 	if then/else bodies in var_func_name extraction.

Hah! Looks obvious and can be pushed independently of the series...

> ---
>  gdb/eval.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/eval.c b/gdb/eval.c
> index 4899011a58f..a50299cbfdb 100644
> --- a/gdb/eval.c
> +++ b/gdb/eval.c
> @@ -1046,13 +1046,13 @@ evaluate_funcall (type *expect_type, expression *exp, int *pos,
>  	{
>  	  if (op == OP_VAR_MSYM_VALUE)
>  	    {
> -	      symbol *sym = exp->elts[*pos + 2].symbol;
> -	      var_func_name = SYMBOL_PRINT_NAME (sym);
> +	      minimal_symbol *msym = exp->elts[*pos + 2].msymbol;
> +	      var_func_name = MSYMBOL_PRINT_NAME (msym);
>  	    }
>  	  else if (op == OP_VAR_VALUE)
>  	    {
> -	      minimal_symbol *msym = exp->elts[*pos + 2].msymbol;
> -	      var_func_name = MSYMBOL_PRINT_NAME (msym);
> +	      symbol *sym = exp->elts[*pos + 2].symbol;
> +	      var_func_name = SYMBOL_PRINT_NAME (sym);
>  	    }
>  
>  	  argvec[0] = evaluate_subexp_with_coercion (exp, pos, noside);
> -- 
> 2.14.3

-- 
Joel

Re: [PATCH 02/11] Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol creation)

[Simon Marchi]

On 2018-03-09 04:16 PM, Pedro Alves wrote:
> Setting a breakpoint on an ifunc symbol after the ifunc has already
> been resolved by the inferior should result in creating a breakpoint
> location at the ifunc target.  However, that's not what happens today:
> 
>   (gdb) n
>   53        i = gnu_ifunc (1);    /* break-at-call */
>   (gdb)
>   54        assert (i == 2);
>   (gdb) b gnu_ifunc
>   Breakpoint 2 at gnu-indirect-function resolver at 0x7ffff7bd36ee
>   (gdb) info breakpoints
>   Num     Type                   Disp Enb Address            What
>   2       STT_GNU_IFUNC resolver keep y   0x00007ffff7bd36ee <gnu_ifunc+4>
> 
> The problem is that elf_gnu_ifunc_resolve_by_got never manages to
> revolve an ifunc target.  The reason is that GDB never actually

revolve -> resolve

> creates the internal got.plt symbols:
> 
>  (gdb) p 'gnu_ifunc@got.plt'
>  No symbol "gnu_ifunc@got.plt" in current context.
> 
> and this is because GDB expects that rela.plt has relocations for
> .plt, while it actually has relocations for .got.plt:

Was it ever the case that rela.plt contained relocations for .plt, or
has it always been a mistake?

Simon

Re: [PATCH 00/11] Fixing GNU ifunc support

[Alan Hayward]

> On 9 Mar 2018, at 21:18, Pedro Alves <palves@redhat.com> wrote:
> 
> On 03/09/2018 09:16 PM, Pedro Alves wrote:
> 
>> Pedro Alves (11):
>>  eval.c: reverse minsym and sym
>>  Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol
>>    creation)
>>  Fix calling ifunc functions when resolver has debug info and different
>>    name
>>  Calling ifunc functions when target has no debug info but resolver has
>>  Calling ifunc functions when resolver has debug info, user symbol same
>>    name
>>  Fix setting breakpoints on ifunc functions after they're already
>>    resolved
>>  Breakpoints, don't skip prologue of ifunc resolvers with debug info
>>  Eliminate find_pc_partial_function_gnu_ifunc
>>  Factor out minsym_found/find_function_start_sal overload
>>  Extend GNU ifunc testcases
>>  Fix resolving GNU ifunc bp locations when inferior runs resolver
> 
> As always, I forgot to say that I pushed this to the
> "users/palves/ifunc" branch on sourceware.org.
> 

I was using your branch to try a few things, and debugging gdb gave me a segfault:

$ ./gdb/gdb ./gdb/gdb

(gdb) b amd64_push_dummy_call
Segmentation fault (core dumped)

Backtrace of core file gives me:

#0  get_objfile_arch (objfile=0x0) at ../../src/binutils-gdb/gdb/objfiles.c:448
#1  0x000000000067a889 in find_function_start_sal (func_addr=4335952, section=0x1a4eb20, funfirstline=<optimised out>)
    at ../../src/binutils-gdb/gdb/symtab.c:3590
#2  0x000000000067a934 in find_function_start_sal (sym=sym@entry=0x2b78980, funfirstline=<optimised out>)
    at ../../src/binutils-gdb/gdb/symtab.c:3625
#3  0x00000000005e77f2 in symbol_to_sal (result=result@entry=0x7fff8467b1a0, funfirstline=<optimised out>, sym=sym@entry=0x2b78980)
    at ../../src/binutils-gdb/gdb/linespec.c:4679
#4  0x00000000005ebbd0 in convert_linespec_to_sals (state=state@entry=0x7fff8467b560, ls=ls@entry=0x7fff8467b5b0)
    at ../../src/binutils-gdb/gdb/linespec.c:2384
#5  0x00000000005ed26d in parse_linespec (parser=parser@entry=0x7fff8467b530, arg=<optimised out>, match_type=<optimised out>)
    at ../../src/binutils-gdb/gdb/linespec.c:2771

This was running on Ubuntu 14.04.5 LTS on x86, gdb built with gcc 5.4.0.
Using gdb git HEAD 0dec80227990d5cc2fddd5e5e5bce92fd6629260

Alan.

Re: [PATCH 00/11] Fixing GNU ifunc support

[Pedro Alves]

On 03/12/2018 11:16 AM, Alan Hayward wrote:
> 
> 
>> On 9 Mar 2018, at 21:18, Pedro Alves <palves@redhat.com> wrote:
>>
>> On 03/09/2018 09:16 PM, Pedro Alves wrote:
>>
>>> Pedro Alves (11):
>>>  eval.c: reverse minsym and sym
>>>  Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol
>>>    creation)
>>>  Fix calling ifunc functions when resolver has debug info and different
>>>    name
>>>  Calling ifunc functions when target has no debug info but resolver has
>>>  Calling ifunc functions when resolver has debug info, user symbol same
>>>    name
>>>  Fix setting breakpoints on ifunc functions after they're already
>>>    resolved
>>>  Breakpoints, don't skip prologue of ifunc resolvers with debug info
>>>  Eliminate find_pc_partial_function_gnu_ifunc
>>>  Factor out minsym_found/find_function_start_sal overload
>>>  Extend GNU ifunc testcases
>>>  Fix resolving GNU ifunc bp locations when inferior runs resolver
>>
>> As always, I forgot to say that I pushed this to the
>> "users/palves/ifunc" branch on sourceware.org.
>>
> 
> I was using your branch to try a few things, and debugging gdb gave me a segfault:
> 
> $ ./gdb/gdb ./gdb/gdb
> 
> (gdb) b amd64_push_dummy_call
> Segmentation fault (core dumped)

Thanks, I can reproduce this.  Looks like the sal has a symtab, but sal.objfile
is left NULL.

Pedro Alves