For PPC64/ELFv1: Introduce mst_data_gnu_ifunc

Running the new tests added later in the series on PPC64 (ELFv1)
revealed that the current ifunc support needs a bit of a design rework
to work properly on PPC64/ELFv1, as most of the new tests fail.  The
ifunc support only kind of works today if the ifunc symbol and the
resolver have the same name, as is currently tested by the
gdb.base/gnu-ifunc.exp testcase, which is unlike how ifuncs are
written nowadays.

The crux of the problem is that ifunc symbols are really function
descriptors, not text symbols:

   44: 0000000000020060    104 FUNC    GLOBAL DEFAULT       18 gnu_ifunc_resolver
   54: 0000000000020060    104 GNU_IFUNC GLOBAL DEFAULT     18 gnu_ifunc

But, currently GDB only knows about ifunc symbols that are text
symbols.  GDB's support happens to work in practice for PPC64 when the
ifunc and resolver are one and only, like in the current
gdb.base/gnu-ifunc.exp testcase:

   15: 0000000000020060    104 GNU_IFUNC GLOBAL DEFAULT       18 gnu_ifunc

because in that case, the synthetic ".gnu_ifunc" entry point text
symbol that bfd creates from the actual GNU ifunc "gnu_ifunc" function
(descriptor) symbol ends up with the the "is a gnu ifunc" flag set /
copied over:

  (gdb) maint print msymbols
  ...
  [ 8] i 0x9c4 .gnu_ifunc section .text                <<< mst_text_gnu_ifunc
  ...
  [29] D 0x20060 gnu_ifunc section .opd  crtstuff.c    <<< mst_data

But, if the resolver gets a distinct symbol/name from the ifunc
symbol, then we end up with this:

  (gdb) maint print msymbols
  [ 8] T 0x9e4 .gnu_ifunc_resolver section .text               <<< mst_text
  ...
  [29] D 0x20060 gnu_ifunc section .opd  crtstuff.c            <<< mst_data
  [30] D 0x20060 gnu_ifunc_resolver section .opd  crtstuff.c   <<< mst_data

I have a follow up bfd patch that turns that into:

   (gdb) maint print msymbols
+  [ 8] i 0x9e4 .gnu_ifunc section .text               <<< mst_text_gnu_ifunc
   [ 8] T 0x9e4 .gnu_ifunc_resolver section .text      <<< mst_text
   ...
   [29] D 0x20060 gnu_ifunc section .opd  crtstuff.c
   [30] D 0x20060 gnu_ifunc_resolver section .opd  crtstuff.c

but that won't help everything.  We still need this patch.

Specifically, when we do a symbol lookup by name, like e.g., to call a
function (see c-exp.y hunk), e.g., "p gnu_ifunc()", then we need to
know that the found "gnu_ifunc" minimal symbol is an ifunc in order to
do some special processing.  But, on PPC, that lookup by name finds
the function descriptor symbol, which presently is just a mst_data
symbol, while at present, we look for mst_text_gnu_ifunc symbols to
decide whether to do special GNU ifunc processing.  In most of those
places, we could try to resolve the function descriptor with
gdbarch_convert_from_func_ptr_addr, and then lookup the minimal symbol
at the resolved PC, see if that finds a minimal symbol of type
mst_text_gnu_ifunc.  If so, then we could assume that the original
mst_dadta / function descriptor "gnu_ifunc" symbol was an ifunc.  I
tried it, and it mostly works, even if it's not the most efficient.

However, there's one case that can't work with such a design -- it's
that of the user calling the ifunc resolver directly to debug it, like
"p gnu_ifunc_resolver(0)", expecting that to return the function
pointer of the final function (which is exercised by the new tests
added later).  In this case, with the not-fully-working solution, we'd
resolve the function descriptor, find that there's an
mst_text_gnu_ifunc symbol for the resolved address, and proceed
calling the function as if we tried to call "gnu_ifunc", the
user-visible GNU ifunc symbol, instead of the resolver.  I.e., it'd be
impossible to call the resolver directly as a normal function.

Introducing mst_data_gnu_ifunc eliminates the need for several
gdbarch_convert_from_func_ptr_addr calls, and, fixes the "call
resolver directly" use case mentioned above too.  It's the cleanest
approach I could think of.

In sum, we make GNU ifunc function descriptor symbols get a new
"mst_data_gnu_ifunc" minimal symbol type instead of the bare mst_data
type.  So when symbol lookup by name finds such a minimal symbol, we
know we found an ifunc symbol, without resolving the entry/text
symbol.  If the user calls the the resolver symbol instead, like "p
gnu_ifunc_resolver(0)", then we'll find the regular mst_data symbol
for "gnu_ifunc_resolver", and we'll call the resolver function as just
another regular function.

With this, most of the GNU ifunc tests added by a later patch pass on
PPC64 too.  The following bfd patch fixes the remaining issues.

gdb/ChangeLog:
2018-04-26  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (set_breakpoint_location_function): Handle
	mst_data_gnu_ifunc.
	* c-exp.y (variable production): Handle mst_data_gnu_ifunc.
	* elfread.c (elf_symtab_read): Give data symbols with
	BSF_GNU_INDIRECT_FUNCTION set mst_data_gnu_ifunc type.
	(elf_rel_plt_read): Update comment.
	* linespec.c (convert_linespec_to_sals): Handle
	mst_data_gnu_ifunc.
	(minsym_found): Handle mst_data_gnu_ifunc.
	* minsyms.c (msymbol_is_function, minimal_symbol_reader::record)
	(find_solib_trampoline_target): Handle mst_data_gnu_ifunc.
	* parse.c (find_minsym_type_and_address): Handle
	mst_data_gnu_ifunc.
	* symmisc.c (dump_msymbols): Handle mst_data_gnu_ifunc.
	* symtab.c (find_gnu_ifunc): Handle mst_data_gnu_ifunc.
	* symtab.h (minimal_symbol_type) <mst_text_gnu_ifunc>: Update
	comment.
	<mst_data_gnu_ifunc>: New enumerator.

10 files changed, 116 insertions, 54 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b2b2191..5916889 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,26 @@
 2018-04-26  Pedro Alves  <palves@redhat.com>
 
+	* breakpoint.c (set_breakpoint_location_function): Handle
+	mst_data_gnu_ifunc.
+	* c-exp.y (variable production): Handle mst_data_gnu_ifunc.
+	* elfread.c (elf_symtab_read): Give data symbols with
+	BSF_GNU_INDIRECT_FUNCTION set mst_data_gnu_ifunc type.
+	(elf_rel_plt_read): Update comment.
+	* linespec.c (convert_linespec_to_sals): Handle
+	mst_data_gnu_ifunc.
+	(minsym_found): Handle mst_data_gnu_ifunc.
+	* minsyms.c (msymbol_is_function, minimal_symbol_reader::record)
+	(find_solib_trampoline_target): Handle mst_data_gnu_ifunc.
+	* parse.c (find_minsym_type_and_address): Handle
+	mst_data_gnu_ifunc.
+	* symmisc.c (dump_msymbols): Handle mst_data_gnu_ifunc.
+	* symtab.c (find_gnu_ifunc): Handle mst_data_gnu_ifunc.
+	* symtab.h (minimal_symbol_type) <mst_text_gnu_ifunc>: Update
+	comment.
+	<mst_data_gnu_ifunc>: New enumerator.
+
+2018-04-26  Pedro Alves  <palves@redhat.com>
+
 	* 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.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 4e514d2..b019610 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7159,7 +7159,8 @@ set_breakpoint_location_function (struct bp_location *loc, int explicit_loc)
       const char *function_name;
 
       if (loc->msymbol != NULL
-	  && MSYMBOL_TYPE (loc->msymbol) == mst_text_gnu_ifunc
+	  && (MSYMBOL_TYPE (loc->msymbol) == mst_text_gnu_ifunc
+	      || MSYMBOL_TYPE (loc->msymbol) == mst_data_gnu_ifunc)
 	  && !explicit_loc)
 	{
 	  struct breakpoint *b = loc->owner;
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 723249c..9e2f808 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1093,7 +1093,8 @@ variable:	name_not_typename
 				 is important for example for "p
 				 *__errno_location()".  */
 			      symbol *alias_target
-				= (msymbol.minsym->type != mst_text_gnu_ifunc
+				= ((msymbol.minsym->type != mst_text_gnu_ifunc
+				    && msymbol.minsym->type != mst_data_gnu_ifunc)
 				   ? find_function_alias_target (msymbol)
 				   : NULL);
 			      if (alias_target != NULL)
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 82437f8..e724f34 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -422,7 +422,11 @@ elf_symtab_read (minimal_symbol_reader &reader,
 	    {
 	      if (sym->flags & (BSF_GLOBAL | BSF_WEAK | BSF_GNU_UNIQUE))
 		{
-		  if (sym->section->flags & SEC_LOAD)
+		  if (sym->flags & BSF_GNU_INDIRECT_FUNCTION)
+		    {
+		      ms_type = mst_data_gnu_ifunc;
+		    }
+		  else if (sym->section->flags & SEC_LOAD)
 		    {
 		      ms_type = mst_data;
 		    }
@@ -614,9 +618,10 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
       else
 	continue;
 
-      /* We cannot check if NAME is a reference to mst_text_gnu_ifunc as in
-	 OBJFILE the symbol is undefined and the objfile having NAME defined
-	 may not yet have been loaded.  */
+      /* We cannot check if NAME is a reference to
+	 mst_text_gnu_ifunc/mst_data_gnu_ifunc as in OBJFILE the
+	 symbol is undefined and the objfile having NAME defined may
+	 not yet have been loaded.  */
 
       string_buffer.assign (name);
       string_buffer.append (got_suffix, got_suffix + got_suffix_len);
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 045c97a..194d50c 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2292,10 +2292,25 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 		       ++m)
 		    {
 		      if (MSYMBOL_TYPE (elem->minsym) == mst_text_gnu_ifunc
-			  && BMSYMBOL_VALUE_ADDRESS (*elem) == addr)
+			  || MSYMBOL_TYPE (elem->minsym) == mst_data_gnu_ifunc)
 			{
-			  found_ifunc = true;
-			  break;
+			  CORE_ADDR msym_addr = BMSYMBOL_VALUE_ADDRESS (*elem);
+			  if (MSYMBOL_TYPE (elem->minsym) == mst_data_gnu_ifunc)
+			    {
+			      struct gdbarch *gdbarch
+				= get_objfile_arch (elem->objfile);
+			      msym_addr
+				= (gdbarch_convert_from_func_ptr_addr
+				   (gdbarch,
+				    msym_addr,
+				    &current_target));
+			    }
+
+			  if (msym_addr == addr)
+			    {
+			      found_ifunc = true;
+			      break;
+			    }
 			}
 		    }
 		}
@@ -4283,7 +4298,8 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
     {
       const char *msym_name = MSYMBOL_LINKAGE_NAME (msymbol);
 
-      if (MSYMBOL_TYPE (msymbol) == mst_text_gnu_ifunc)
+      if (MSYMBOL_TYPE (msymbol) == mst_text_gnu_ifunc
+	  || MSYMBOL_TYPE (msymbol) == mst_data_gnu_ifunc)
 	want_start_sal = gnu_ifunc_resolve_name (msym_name, &func_addr);
       else
 	want_start_sal = true;
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 601baee..221404e 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -70,6 +70,7 @@ msymbol_is_function (struct objfile *objfile, minimal_symbol *minsym,
     case mst_abs:
     case mst_file_data:
     case mst_file_bss:
+    case mst_data_gnu_ifunc:
       {
 	struct gdbarch *gdbarch = get_objfile_arch (objfile);
 	CORE_ADDR pc = gdbarch_convert_from_func_ptr_addr (gdbarch, msym_addr,
@@ -1072,6 +1073,7 @@ minimal_symbol_reader::record (const char *name, CORE_ADDR address,
       section = SECT_OFF_TEXT (m_objfile);
       break;
     case mst_data:
+    case mst_data_gnu_ifunc:
     case mst_file_data:
       section = SECT_OFF_DATA (m_objfile);
       break;
@@ -1472,26 +1474,19 @@ find_solib_trampoline_target (struct frame_info *frame, CORE_ADDR pc)
     {
       ALL_MSYMBOLS (objfile, msymbol)
       {
-	if ((MSYMBOL_TYPE (msymbol) == mst_text
-	    || MSYMBOL_TYPE (msymbol) == mst_text_gnu_ifunc)
-	    && strcmp (MSYMBOL_LINKAGE_NAME (msymbol),
-		       MSYMBOL_LINKAGE_NAME (tsymbol)) == 0)
-	  return MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
-
 	/* Also handle minimal symbols pointing to function descriptors.  */
-	if (MSYMBOL_TYPE (msymbol) == mst_data
+	if ((MSYMBOL_TYPE (msymbol) == mst_text
+	     || MSYMBOL_TYPE (msymbol) == mst_text_gnu_ifunc
+	     || MSYMBOL_TYPE (msymbol) == mst_data
+	     || MSYMBOL_TYPE (msymbol) == mst_data_gnu_ifunc)
 	    && strcmp (MSYMBOL_LINKAGE_NAME (msymbol),
 		       MSYMBOL_LINKAGE_NAME (tsymbol)) == 0)
 	  {
 	    CORE_ADDR func;
 
-	    func = gdbarch_convert_from_func_ptr_addr
-		    (get_objfile_arch (objfile),
-		     MSYMBOL_VALUE_ADDRESS (objfile, msymbol),
-		     &current_target);
-
-	    /* Ignore data symbols that are not function descriptors.  */
-	    if (func != MSYMBOL_VALUE_ADDRESS (objfile, msymbol))
+	    /* Ignore data symbols that are not function
+	       descriptors.  */
+	    if (msymbol_is_function (objfile, msymbol, &func))
 	      return func;
 	  }
       }
diff --git a/gdb/parse.c b/gdb/parse.c
index 3abb9d4..1d53b5a 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -461,42 +461,35 @@ find_minsym_type_and_address (minimal_symbol *msymbol,
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   struct obj_section *section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
   enum minimal_symbol_type type = MSYMBOL_TYPE (msymbol);
-  CORE_ADDR pc;
 
   bool is_tls = (section != NULL
 		 && section->the_bfd_section->flags & SEC_THREAD_LOCAL);
 
-  /* Addresses of TLS symbols are really offsets into a
-     per-objfile/per-thread storage block.  */
-  CORE_ADDR addr = (is_tls
-		    ? MSYMBOL_VALUE_RAW_ADDRESS (bound_msym.minsym)
-		    : BMSYMBOL_VALUE_ADDRESS (bound_msym));
-
   /* The minimal symbol might point to a function descriptor;
      resolve it to the actual code address instead.  */
-  pc = gdbarch_convert_from_func_ptr_addr (gdbarch, addr, &current_target);
-  if (pc != addr)
+  CORE_ADDR addr;
+  if (is_tls)
     {
-      struct bound_minimal_symbol ifunc_msym = lookup_minimal_symbol_by_pc (pc);
-
-      /* In this case, assume we have a code symbol instead of
-	 a data symbol.  */
-
-      if (ifunc_msym.minsym != NULL
-	  && MSYMBOL_TYPE (ifunc_msym.minsym) == mst_text_gnu_ifunc
-	  && BMSYMBOL_VALUE_ADDRESS (ifunc_msym) == pc)
+      /* Addresses of TLS symbols are really offsets into a
+	 per-objfile/per-thread storage block.  */
+      addr = MSYMBOL_VALUE_RAW_ADDRESS (bound_msym.minsym);
+    }
+  else if (msymbol_is_function (objfile, msymbol, &addr))
+    {
+      if (addr != BMSYMBOL_VALUE_ADDRESS (bound_msym))
 	{
-	  /* A function descriptor has been resolved but PC is still in the
-	     STT_GNU_IFUNC resolver body (such as because inferior does not
-	     run to be able to call it).  */
-
-	  type = mst_text_gnu_ifunc;
+	  /* This means we resolved a function descriptor, and we now
+	     have an address for a code/text symbol instead of a data
+	     symbol.  */
+	  if (MSYMBOL_TYPE (msymbol) == mst_data_gnu_ifunc)
+	    type = mst_text_gnu_ifunc;
+	  else
+	    type = mst_text;
+	  section = NULL;
 	}
-      else
-	type = mst_text;
-      section = NULL;
-      addr = pc;
     }
+  else
+    addr = BMSYMBOL_VALUE_ADDRESS (bound_msym);
 
   if (overlay_debugging)
     addr = symbol_overlayed_address (addr, section);
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 9adde04..91ddc57 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -209,6 +209,7 @@ dump_msymbols (struct objfile *objfile, struct ui_file *outfile)
 	  ms_type = 'T';
 	  break;
 	case mst_text_gnu_ifunc:
+	case mst_data_gnu_ifunc:
 	  ms_type = 'i';
 	  break;
 	case mst_solib_trampoline:
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 13910c6..a24a083 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4977,10 +4977,21 @@ find_gnu_ifunc (const symbol *sym)
 				[&] (minimal_symbol *minsym)
     {
       if (MSYMBOL_TYPE (minsym) == mst_text_gnu_ifunc
-	  && MSYMBOL_VALUE_ADDRESS (objfile, minsym) == address)
+	  || MSYMBOL_TYPE (minsym) == mst_data_gnu_ifunc)
 	{
-	  ifunc = minsym;
-	  return true;
+	  CORE_ADDR msym_addr = MSYMBOL_VALUE_ADDRESS (objfile, minsym);
+	  if (MSYMBOL_TYPE (minsym) == mst_data_gnu_ifunc)
+	    {
+	      struct gdbarch *gdbarch = get_objfile_arch (objfile);
+	      msym_addr = gdbarch_convert_from_func_ptr_addr (gdbarch,
+							      msym_addr,
+							      &current_target);
+	    }
+	  if (msym_addr == address)
+	    {
+	      ifunc = minsym;
+	      return true;
+	    }
 	}
       return false;
     });
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 0bd95fa..84fc897 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -580,8 +580,26 @@ enum minimal_symbol_type
 {
   mst_unknown = 0,		/* Unknown type, the default */
   mst_text,			/* Generally executable instructions */
-  mst_text_gnu_ifunc,		/* Executable code returning address
+
+  /* A GNU ifunc symbol, in the .text section.  GDB uses to know
+     whether the user is setting a breakpoint on a GNU ifunc function,
+     and thus GDB needs to actually set the breakpoint on the target
+     function.  It is also used to know whether the program stepped
+     into an ifunc resolver -- the resolver may get a separate
+     symbol/alias under a different name, but it'll have the same
+     address as the ifunc symbol.  */
+  mst_text_gnu_ifunc,           /* Executable code returning address
+				   of executable code */
+
+  /* A GNU ifunc function descriptor symbol, in a data section
+     (typically ".opd").  Seen on architectures that use function
+     descriptors, like PPC64/ELFv1.  In this case, this symbol's value
+     is the address of the descriptor.  There'll be a corresponding
+     mst_text_gnu_ifunc synthetic symbol for the text/entry
+     address.  */
+  mst_data_gnu_ifunc,		/* Executable code returning address
 				   of executable code */
+
   mst_slot_got_plt,		/* GOT entries for .plt sections */
   mst_data,			/* Generally initialized data */
   mst_bss,			/* Generally uninitialized data */