[v2,01/15] Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol creation)

Message ID 20180325191943.8246-2-palves@redhat.com
State New
Headers show
Series
  • Fixing GNU ifunc support
Related show

Commit Message

Pedro Alves March 25, 2018, 7:19 p.m.
In v2: - reworked to keep supporting rel.plt sections for .plt, and
         to handle jump slot offsets pointing to .plt.

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 on
current Fedora:

  (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
resolve 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


Using an older system on the GCC compile farm (machine gcc15, an
x86-64 running Debian 6.0.8, with GNU ld 2.20.1), we see that it used
to be that we'd get a .rela.plt section for .plt:

 Relocation section [ 9] '.rela.plt' for section [11] '.plt' at offset 0x578 contains 3 entries:
   Offset              Type            Value               Addend Name
   0x0000000000600cc0  X86_64_JUMP_SLOT 000000000000000000      +0 __assert_fail
   0x0000000000600cc8  X86_64_JUMP_SLOT 000000000000000000      +0 __libc_start_main
   0x0000000000600cd0  X86_64_JUMP_SLOT 000000000000000000      +0 gnu_ifunc

Those offsets did point into .got.plt, as seen with objdump -h:

  20 .got.plt      00000030  0000000000600ca8  0000000000600ca8  00000ca8  2**3
     		   CONTENTS, ALLOC, LOAD, DATA

I also tested on gcc110 on the compile farm (PPC64 running CentOS
7.4.1708, with GNU ld 2.25.1), and there we see instead:

 Relocation section [ 9] '.rela.plt' for section [23] '.plt' at offset 0x5d0 contains 4 entries:
   Offset              Type            Value               Addend Name
   0x0000000010020148  PPC64_JMP_SLOT  000000000000000000      +0 __libc_start_main
   0x0000000010020160  PPC64_JMP_SLOT  000000000000000000      +0 __gmon_start__
   0x0000000010020178  PPC64_JMP_SLOT  000000000000000000      +0 __assert_fail
   0x0000000010020190  PPC64_JMP_SLOT  000000000000000000      +0 gnu_ifunc

But note that those offsets point into .plt, not .got.plt, as seen
with objdump -h:

 22 .plt          00000078  0000000010020130  0000000010020130  00010130  2**3
                  ALLOC

This commit makes us support all the different combinations above.

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 relocations for .got.plt too.
	not .plt.
---
 gdb/elfread.c | 64 +++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 47 insertions(+), 17 deletions(-)

-- 
2.14.3

Comments

Simon Marchi April 1, 2018, 3:35 a.m. | #1
On 2018-03-25 03:19 PM, Pedro Alves wrote:
> In v2: - reworked to keep supporting rel.plt sections for .plt, and

>          to handle jump slot offsets pointing to .plt.

> 

> 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 on

> current Fedora:

> 

>   (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

> resolve 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

> 

> 

> Using an older system on the GCC compile farm (machine gcc15, an

> x86-64 running Debian 6.0.8, with GNU ld 2.20.1), we see that it used

> to be that we'd get a .rela.plt section for .plt:

> 

>  Relocation section [ 9] '.rela.plt' for section [11] '.plt' at offset 0x578 contains 3 entries:

>    Offset              Type            Value               Addend Name

>    0x0000000000600cc0  X86_64_JUMP_SLOT 000000000000000000      +0 __assert_fail

>    0x0000000000600cc8  X86_64_JUMP_SLOT 000000000000000000      +0 __libc_start_main

>    0x0000000000600cd0  X86_64_JUMP_SLOT 000000000000000000      +0 gnu_ifunc

> 

> Those offsets did point into .got.plt, as seen with objdump -h:

> 

>   20 .got.plt      00000030  0000000000600ca8  0000000000600ca8  00000ca8  2**3

>      		   CONTENTS, ALLOC, LOAD, DATA

> 

> I also tested on gcc110 on the compile farm (PPC64 running CentOS

> 7.4.1708, with GNU ld 2.25.1), and there we see instead:

> 

>  Relocation section [ 9] '.rela.plt' for section [23] '.plt' at offset 0x5d0 contains 4 entries:

>    Offset              Type            Value               Addend Name

>    0x0000000010020148  PPC64_JMP_SLOT  000000000000000000      +0 __libc_start_main

>    0x0000000010020160  PPC64_JMP_SLOT  000000000000000000      +0 __gmon_start__

>    0x0000000010020178  PPC64_JMP_SLOT  000000000000000000      +0 __assert_fail

>    0x0000000010020190  PPC64_JMP_SLOT  000000000000000000      +0 gnu_ifunc

> 

> But note that those offsets point into .plt, not .got.plt, as seen

> with objdump -h:

> 

>  22 .plt          00000078  0000000010020130  0000000010020130  00010130  2**3

>                   ALLOC

> 

> This commit makes us support all the different combinations above.

> 

> 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 relocations for .got.plt too.

> 	not .plt.

> ---

>  gdb/elfread.c | 64 +++++++++++++++++++++++++++++++++++++++++++----------------

>  1 file changed, 47 insertions(+), 17 deletions(-)

> 

> diff --git a/gdb/elfread.c b/gdb/elfread.c

> index 260789062d0..9ffbf99db6e 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,12 +553,31 @@ elf_rel_plt_read (minimal_symbol_reader &reader,

>  	return;

>      }

>  

> +  /* Depending on system, we may find jump slots in a relocation

> +     section for either .got.plt or .plt.  */

> +  asection *plt = bfd_get_section_by_name (obfd, ".plt");

> +  int plt_elf_idx = (plt != NULL) ? elf_section_data (plt)->this_idx : -1;

> +

> +  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

> -	&& (elf_section_data (relplt)->this_hdr.sh_type == SHT_REL

> -	    || elf_section_data (relplt)->this_hdr.sh_type == SHT_RELA))

> -      break;

> +    {

> +      const auto &this_hdr = elf_section_data (relplt)->this_hdr;

> +

> +      if (this_hdr.sh_type == SHT_REL || this_hdr.sh_type == SHT_RELA)

> +	{

> +	  asection *target_section = NULL;

> +

> +	  if (this_hdr.sh_info == plt_elf_idx)

> +	    target_section = plt;

> +	  else if (this_hdr.sh_info == got_plt_elf_idx)

> +	    target_section = got_plt;

> +

> +	  if (target_section != NULL)

> +	    break;


Is it really useful to have/set target_section?  Couldn't we just break out of the
loop like this?

  if (this_hdr.sh_info == plt_elf_idx
      || this_hdr.sh_info == got_plt_elf_idx)
    break;

> +	}

> +    }

>    if (relplt == NULL)

>      return;

>  

> @@ -573,6 +586,18 @@ elf_rel_plt_read (minimal_symbol_reader &reader,

>  

>    std::string string_buffer;

>  

> +  /* Does ADDRESS reside in SECTION of OBFD?  */

> +  auto within_section = [obfd] (asection *section, CORE_ADDR address)

> +    {

> +      if (section == NULL)

> +	return false;

> +

> +      /* Does the pointer reside in the .got.plt section?  */


That comment should change, since it's not stricly .got.plt.

> +      return (bfd_get_section_vma (obfd, section) <= address

> +	      && (address < bfd_get_section_vma (obfd, section)

> +		  + bfd_get_section_size (section)));

> +    };

> +

>    reloc_count = relplt->size / elf_section_data (relplt)->this_hdr.sh_entsize;

>    for (reloc = 0; reloc < reloc_count; reloc++)

>      {

> @@ -585,10 +610,15 @@ elf_rel_plt_read (minimal_symbol_reader &reader,

>        name = bfd_asymbol_name (*relplt->relocation[reloc].sym_ptr_ptr);

>        address = relplt->relocation[reloc].address;

>  

> -      /* Does the pointer reside in the .got.plt section?  */

> -      if (!(bfd_get_section_vma (obfd, got_plt) <= address

> -            && address < bfd_get_section_vma (obfd, got_plt)

> -			 + bfd_get_section_size (got_plt)))

> +      asection *msym_section;

> +

> +      /* Does the pointer reside in either the .got.plt or .plt

> +	 sections?  */

> +      if (within_section (got_plt, address))

> +	msym_section = got_plt;

> +      else if (within_section (plt, address))

> +	msym_section = plt;

> +      else

>  	continue;


Or maybe you intended to use target_section here at some point?  Is there a
relationship between the section that matched in the for loop above and the
section that will contain the address?  In other words, could we save the
target_section from above and do

  if (!within_section (target_section, address))
    continue;

>  

>        /* We cannot check if NAME is a reference to mst_text_gnu_ifunc as in

> @@ -600,8 +630,8 @@ elf_rel_plt_read (minimal_symbol_reader &reader,

>  

>        msym = record_minimal_symbol (reader, string_buffer.c_str (),

>  				    string_buffer.size (),

> -                                    true, address, mst_slot_got_plt, got_plt,

> -				    objfile);

> +				    true, address, mst_slot_got_plt,

> +				    msym_section, objfile);

>        if (msym)

>  	SET_MSYMBOL_SIZE (msym, ptr_size);

>      }

> 


Simon
Pedro Alves April 10, 2018, 9:19 p.m. | #2
On 04/01/2018 04:35 AM, Simon Marchi wrote:
> On 2018-03-25 03:19 PM, Pedro Alves wrote:


>>    /* 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

>> -	&& (elf_section_data (relplt)->this_hdr.sh_type == SHT_REL

>> -	    || elf_section_data (relplt)->this_hdr.sh_type == SHT_RELA))

>> -      break;

>> +    {

>> +      const auto &this_hdr = elf_section_data (relplt)->this_hdr;

>> +

>> +      if (this_hdr.sh_type == SHT_REL || this_hdr.sh_type == SHT_RELA)

>> +	{

>> +	  asection *target_section = NULL;

>> +

>> +	  if (this_hdr.sh_info == plt_elf_idx)

>> +	    target_section = plt;

>> +	  else if (this_hdr.sh_info == got_plt_elf_idx)

>> +	    target_section = got_plt;

>> +

>> +	  if (target_section != NULL)

>> +	    break;

> 

> Is it really useful to have/set target_section?  Couldn't we just break out of the

> loop like this?

> 

>   if (this_hdr.sh_info == plt_elf_idx

>       || this_hdr.sh_info == got_plt_elf_idx)

>     break;

> 


Hmm, the original intention was to use target_section in the other
loop, but that didn't work, so I reverted it, but somehow not that
part.  :-P

>>  

>> @@ -573,6 +586,18 @@ elf_rel_plt_read (minimal_symbol_reader &reader,

>>  

>>    std::string string_buffer;

>>  

>> +  /* Does ADDRESS reside in SECTION of OBFD?  */

>> +  auto within_section = [obfd] (asection *section, CORE_ADDR address)

>> +    {

>> +      if (section == NULL)

>> +	return false;

>> +

>> +      /* Does the pointer reside in the .got.plt section?  */

> 

> That comment should change, since it's not stricly .got.plt.

> 


I've removed it, since the intro comment already says it all.

> Or maybe you intended to use target_section here at some point?  Is there a

> relationship between the section that matched in the for loop above and the

> section that will contain the address?  In other words, could we save the

> target_section from above and do


Yeah, in an earlier version I tried doing that, but then testing on the
different systems found out that there's no relation between the
two sections.

Here's the updated patch.  WDYT?

From 0e91b0c40141326243dbd1dd735ca1e1fe5c78ce Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>

Date: Tue, 10 Apr 2018 18:11:27 +0100
Subject: [PATCH] Fix breakpoints in ifunc after inferior resolved it (@got.plt
 symbol creation)

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 on
current Fedora:

  (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
resolve 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


Using an older system on the GCC compile farm (machine gcc15, an
x86-64 running Debian 6.0.8, with GNU ld 2.20.1), we see that it used
to be that we'd get a .rela.plt section for .plt:

 Relocation section [ 9] '.rela.plt' for section [11] '.plt' at offset 0x578 contains 3 entries:
   Offset              Type            Value               Addend Name
   0x0000000000600cc0  X86_64_JUMP_SLOT 000000000000000000      +0 __assert_fail
   0x0000000000600cc8  X86_64_JUMP_SLOT 000000000000000000      +0 __libc_start_main
   0x0000000000600cd0  X86_64_JUMP_SLOT 000000000000000000      +0 gnu_ifunc

Those offsets did point into .got.plt, as seen with objdump -h:

  20 .got.plt      00000030  0000000000600ca8  0000000000600ca8  00000ca8  2**3
     		   CONTENTS, ALLOC, LOAD, DATA

I also tested on gcc110 on the compile farm (PPC64 running CentOS
7.4.1708, with GNU ld 2.25.1), and there we see instead:

 Relocation section [ 9] '.rela.plt' for section [23] '.plt' at offset 0x5d0 contains 4 entries:
   Offset              Type            Value               Addend Name
   0x0000000010020148  PPC64_JMP_SLOT  000000000000000000      +0 __libc_start_main
   0x0000000010020160  PPC64_JMP_SLOT  000000000000000000      +0 __gmon_start__
   0x0000000010020178  PPC64_JMP_SLOT  000000000000000000      +0 __assert_fail
   0x0000000010020190  PPC64_JMP_SLOT  000000000000000000      +0 gnu_ifunc

But note that those offsets point into .plt, not .got.plt, as seen
with objdump -h:

 22 .plt          00000078  0000000010020130  0000000010020130  00010130  2**3
                  ALLOC

This commit makes us support all the different combinations above.

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 relocations for .got.plt too.
	not .plt.
---
 gdb/elfread.c | 57 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 260789062d0..16a692d3713 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,12 +553,25 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
 	return;
     }
 
+  /* Depending on system, we may find jump slots in a relocation
+     section for either .got.plt or .plt.  */
+  asection *plt = bfd_get_section_by_name (obfd, ".plt");
+  int plt_elf_idx = (plt != NULL) ? elf_section_data (plt)->this_idx : -1;
+
+  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
-	&& (elf_section_data (relplt)->this_hdr.sh_type == SHT_REL
-	    || elf_section_data (relplt)->this_hdr.sh_type == SHT_RELA))
-      break;
+    {
+      const auto &this_hdr = elf_section_data (relplt)->this_hdr;
+
+      if (this_hdr.sh_type == SHT_REL || this_hdr.sh_type == SHT_RELA)
+	{
+	  if (this_hdr.sh_info == plt_elf_idx
+	      || this_hdr.sh_info == got_plt_elf_idx)
+	    break;
+	}
+    }
   if (relplt == NULL)
     return;
 
@@ -573,6 +580,17 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
 
   std::string string_buffer;
 
+  /* Does ADDRESS reside in SECTION of OBFD?  */
+  auto within_section = [obfd] (asection *section, CORE_ADDR address)
+    {
+      if (section == NULL)
+	return false;
+
+      return (bfd_get_section_vma (obfd, section) <= address
+	      && (address < bfd_get_section_vma (obfd, section)
+		  + bfd_get_section_size (section)));
+    };
+
   reloc_count = relplt->size / elf_section_data (relplt)->this_hdr.sh_entsize;
   for (reloc = 0; reloc < reloc_count; reloc++)
     {
@@ -585,10 +603,15 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
       name = bfd_asymbol_name (*relplt->relocation[reloc].sym_ptr_ptr);
       address = relplt->relocation[reloc].address;
 
-      /* Does the pointer reside in the .got.plt section?  */
-      if (!(bfd_get_section_vma (obfd, got_plt) <= address
-            && address < bfd_get_section_vma (obfd, got_plt)
-			 + bfd_get_section_size (got_plt)))
+      asection *msym_section;
+
+      /* Does the pointer reside in either the .got.plt or .plt
+	 sections?  */
+      if (within_section (got_plt, address))
+	msym_section = got_plt;
+      else if (within_section (plt, address))
+	msym_section = plt;
+      else
 	continue;
 
       /* We cannot check if NAME is a reference to mst_text_gnu_ifunc as in
@@ -600,8 +623,8 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
 
       msym = record_minimal_symbol (reader, string_buffer.c_str (),
 				    string_buffer.size (),
-                                    true, address, mst_slot_got_plt, got_plt,
-				    objfile);
+				    true, address, mst_slot_got_plt,
+				    msym_section, objfile);
       if (msym)
 	SET_MSYMBOL_SIZE (msym, ptr_size);
     }
-- 
2.14.3
Simon Marchi April 14, 2018, 4:36 p.m. | #3
On 2018-04-10 17:19, Pedro Alves wrote:
> On 04/01/2018 04:35 AM, Simon Marchi wrote:

>> On 2018-03-25 03:19 PM, Pedro Alves wrote:

> 

>>>    /* 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

>>> -	&& (elf_section_data (relplt)->this_hdr.sh_type == SHT_REL

>>> -	    || elf_section_data (relplt)->this_hdr.sh_type == SHT_RELA))

>>> -      break;

>>> +    {

>>> +      const auto &this_hdr = elf_section_data (relplt)->this_hdr;

>>> +

>>> +      if (this_hdr.sh_type == SHT_REL || this_hdr.sh_type == 

>>> SHT_RELA)

>>> +	{

>>> +	  asection *target_section = NULL;

>>> +

>>> +	  if (this_hdr.sh_info == plt_elf_idx)

>>> +	    target_section = plt;

>>> +	  else if (this_hdr.sh_info == got_plt_elf_idx)

>>> +	    target_section = got_plt;

>>> +

>>> +	  if (target_section != NULL)

>>> +	    break;

>> 

>> Is it really useful to have/set target_section?  Couldn't we just 

>> break out of the

>> loop like this?

>> 

>>   if (this_hdr.sh_info == plt_elf_idx

>>       || this_hdr.sh_info == got_plt_elf_idx)

>>     break;

>> 

> 

> Hmm, the original intention was to use target_section in the other

> loop, but that didn't work, so I reverted it, but somehow not that

> part.  :-P

> 

>>> 

>>> @@ -573,6 +586,18 @@ elf_rel_plt_read (minimal_symbol_reader &reader,

>>> 

>>>    std::string string_buffer;

>>> 

>>> +  /* Does ADDRESS reside in SECTION of OBFD?  */

>>> +  auto within_section = [obfd] (asection *section, CORE_ADDR 

>>> address)

>>> +    {

>>> +      if (section == NULL)

>>> +	return false;

>>> +

>>> +      /* Does the pointer reside in the .got.plt section?  */

>> 

>> That comment should change, since it's not stricly .got.plt.

>> 

> 

> I've removed it, since the intro comment already says it all.

> 

>> Or maybe you intended to use target_section here at some point?  Is 

>> there a

>> relationship between the section that matched in the for loop above 

>> and the

>> section that will contain the address?  In other words, could we save 

>> the

>> target_section from above and do

> 

> Yeah, in an earlier version I tried doing that, but then testing on the

> different systems found out that there's no relation between the

> two sections.

> 

> Here's the updated patch.  WDYT?


LGTM (though I trust the passing tests more than I trust myself).

Simon

Patch

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 260789062d0..9ffbf99db6e 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,12 +553,31 @@  elf_rel_plt_read (minimal_symbol_reader &reader,
 	return;
     }
 
+  /* Depending on system, we may find jump slots in a relocation
+     section for either .got.plt or .plt.  */
+  asection *plt = bfd_get_section_by_name (obfd, ".plt");
+  int plt_elf_idx = (plt != NULL) ? elf_section_data (plt)->this_idx : -1;
+
+  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
-	&& (elf_section_data (relplt)->this_hdr.sh_type == SHT_REL
-	    || elf_section_data (relplt)->this_hdr.sh_type == SHT_RELA))
-      break;
+    {
+      const auto &this_hdr = elf_section_data (relplt)->this_hdr;
+
+      if (this_hdr.sh_type == SHT_REL || this_hdr.sh_type == SHT_RELA)
+	{
+	  asection *target_section = NULL;
+
+	  if (this_hdr.sh_info == plt_elf_idx)
+	    target_section = plt;
+	  else if (this_hdr.sh_info == got_plt_elf_idx)
+	    target_section = got_plt;
+
+	  if (target_section != NULL)
+	    break;
+	}
+    }
   if (relplt == NULL)
     return;
 
@@ -573,6 +586,18 @@  elf_rel_plt_read (minimal_symbol_reader &reader,
 
   std::string string_buffer;
 
+  /* Does ADDRESS reside in SECTION of OBFD?  */
+  auto within_section = [obfd] (asection *section, CORE_ADDR address)
+    {
+      if (section == NULL)
+	return false;
+
+      /* Does the pointer reside in the .got.plt section?  */
+      return (bfd_get_section_vma (obfd, section) <= address
+	      && (address < bfd_get_section_vma (obfd, section)
+		  + bfd_get_section_size (section)));
+    };
+
   reloc_count = relplt->size / elf_section_data (relplt)->this_hdr.sh_entsize;
   for (reloc = 0; reloc < reloc_count; reloc++)
     {
@@ -585,10 +610,15 @@  elf_rel_plt_read (minimal_symbol_reader &reader,
       name = bfd_asymbol_name (*relplt->relocation[reloc].sym_ptr_ptr);
       address = relplt->relocation[reloc].address;
 
-      /* Does the pointer reside in the .got.plt section?  */
-      if (!(bfd_get_section_vma (obfd, got_plt) <= address
-            && address < bfd_get_section_vma (obfd, got_plt)
-			 + bfd_get_section_size (got_plt)))
+      asection *msym_section;
+
+      /* Does the pointer reside in either the .got.plt or .plt
+	 sections?  */
+      if (within_section (got_plt, address))
+	msym_section = got_plt;
+      else if (within_section (plt, address))
+	msym_section = plt;
+      else
 	continue;
 
       /* We cannot check if NAME is a reference to mst_text_gnu_ifunc as in
@@ -600,8 +630,8 @@  elf_rel_plt_read (minimal_symbol_reader &reader,
 
       msym = record_minimal_symbol (reader, string_buffer.c_str (),
 				    string_buffer.size (),
-                                    true, address, mst_slot_got_plt, got_plt,
-				    objfile);
+				    true, address, mst_slot_got_plt,
+				    msym_section, objfile);
       if (msym)
 	SET_MSYMBOL_SIZE (msym, ptr_size);
     }