[v2,01/15] Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol creation)
Commit Message
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(-)
Comments
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
@@ -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);
}