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

[Simon Marchi <simark@simark.ca>]

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