[PATCH] gdb/elfread.c: Enable ifunc support on ARM.

[PATCH] gdb/elfread.c: Enable ifunc support on ARM.

[Will Newton]

There are two failures in the gnu-ifunc.exp test on ARM. These are
due to the failure to resolve the correct target function when
attempting to breakpoint a GNU ifunc resolved function:

(gdb) break gnu_ifunc
Breakpoint 4 at gnu-indirect-function resolver at 0x2aacb5a2

when gnu_ifunc has been resolved this should actually be:

(gdb) break gnu_ifunc
Breakpoint 4 at 0x868c

There are two reasons for this. The first is that ARM does not have a
separate .got.plt section so looking this up will always fail. The second
is that the Thumb bit needs to be stripped from the address to allow
it to be reliably compared when inserting into the ifunc cache.

Tested with no regressions on arm-linux-gnueabihf and
x86_64-unknown-linux-gnu.

gdb/ChangeLog:

2013-12-16  Will Newton  <will.newton@linaro.org>

	* elfread.c (elf_rel_plt_read): Look for a .got section if
	looking up .got.plt fails.
	(elf_gnu_ifunc_resolve_by_got): Call gdbarch_addr_bits_remove
	on address passed to elf_gnu_ifunc_record_cache.
	(elf_gnu_ifunc_resolve_addr): Likewise.
	(elf_gnu_ifunc_resolver_return_stop): Likewise.
---
 gdb/elfread.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 4a36927..30076e7 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -646,7 +646,12 @@ elf_rel_plt_read (struct objfile *objfile, asymbol **dyn_symbol_table)

   got_plt = bfd_get_section_by_name (obfd, ".got.plt");
   if (got_plt == NULL)
-    return;
+    {
+      /* For platforms where there is no separate .got.plt.  */
+      got_plt = bfd_get_section_by_name (obfd, ".got");
+      if (got_plt == NULL)
+	return;
+    }

   /* This search algorithm is from _bfd_elf_canonicalize_dynamic_reloc.  */
   for (relplt = obfd->sections; relplt != NULL; relplt = relplt->next)
@@ -899,6 +904,7 @@ elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
       addr = extract_typed_address (buf, ptr_type);
       addr = gdbarch_convert_from_func_ptr_addr (gdbarch, addr,
 						 &current_target);
+      addr = gdbarch_addr_bits_remove (gdbarch, addr);

       if (addr_p)
 	*addr_p = addr;
@@ -962,6 +968,7 @@ elf_gnu_ifunc_resolve_addr (struct gdbarch *gdbarch, CORE_ADDR pc)
   address = value_as_address (address_val);
   address = gdbarch_convert_from_func_ptr_addr (gdbarch, address,
 						&current_target);
+  address = gdbarch_addr_bits_remove (gdbarch, address);

   if (name_at_pc)
     elf_gnu_ifunc_record_cache (name_at_pc, address);
@@ -1070,6 +1077,7 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
   resolved_pc = gdbarch_convert_from_func_ptr_addr (gdbarch,
 						    resolved_address,
 						    &current_target);
+  resolved_pc = gdbarch_addr_bits_remove (gdbarch, resolved_pc);

   gdb_assert (current_program_space == b->pspace || b->pspace == NULL);
   elf_gnu_ifunc_record_cache (b->addr_string, resolved_pc);
-- 
1.8.1.4

Re: [PATCH] gdb/elfread.c: Enable ifunc support on ARM.

[Will Newton]

On 16 December 2013 18:24, Will Newton <will.newton@linaro.org> wrote:
>
> There are two failures in the gnu-ifunc.exp test on ARM. These are
> due to the failure to resolve the correct target function when
> attempting to breakpoint a GNU ifunc resolved function:
>
> (gdb) break gnu_ifunc
> Breakpoint 4 at gnu-indirect-function resolver at 0x2aacb5a2
>
> when gnu_ifunc has been resolved this should actually be:
>
> (gdb) break gnu_ifunc
> Breakpoint 4 at 0x868c
>
> There are two reasons for this. The first is that ARM does not have a
> separate .got.plt section so looking this up will always fail. The second
> is that the Thumb bit needs to be stripped from the address to allow
> it to be reliably compared when inserting into the ifunc cache.
>
> Tested with no regressions on arm-linux-gnueabihf and
> x86_64-unknown-linux-gnu.
>
> gdb/ChangeLog:
>
> 2013-12-16  Will Newton  <will.newton@linaro.org>
>
>         * elfread.c (elf_rel_plt_read): Look for a .got section if
>         looking up .got.plt fails.
>         (elf_gnu_ifunc_resolve_by_got): Call gdbarch_addr_bits_remove
>         on address passed to elf_gnu_ifunc_record_cache.
>         (elf_gnu_ifunc_resolve_addr): Likewise.
>         (elf_gnu_ifunc_resolver_return_stop): Likewise.
> ---
>  gdb/elfread.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)

Ping?

-- 
Will Newton
Toolchain Working Group, Linaro

Re: [PATCH] gdb/elfread.c: Enable ifunc support on ARM.

[Will Newton]

On 23 January 2014 15:21, Will Newton <will.newton@linaro.org> wrote:
> On 16 December 2013 18:24, Will Newton <will.newton@linaro.org> wrote:
>>
>> There are two failures in the gnu-ifunc.exp test on ARM. These are
>> due to the failure to resolve the correct target function when
>> attempting to breakpoint a GNU ifunc resolved function:
>>
>> (gdb) break gnu_ifunc
>> Breakpoint 4 at gnu-indirect-function resolver at 0x2aacb5a2
>>
>> when gnu_ifunc has been resolved this should actually be:
>>
>> (gdb) break gnu_ifunc
>> Breakpoint 4 at 0x868c
>>
>> There are two reasons for this. The first is that ARM does not have a
>> separate .got.plt section so looking this up will always fail. The second
>> is that the Thumb bit needs to be stripped from the address to allow
>> it to be reliably compared when inserting into the ifunc cache.
>>
>> Tested with no regressions on arm-linux-gnueabihf and
>> x86_64-unknown-linux-gnu.
>>
>> gdb/ChangeLog:
>>
>> 2013-12-16  Will Newton  <will.newton@linaro.org>
>>
>>         * elfread.c (elf_rel_plt_read): Look for a .got section if
>>         looking up .got.plt fails.
>>         (elf_gnu_ifunc_resolve_by_got): Call gdbarch_addr_bits_remove
>>         on address passed to elf_gnu_ifunc_record_cache.
>>         (elf_gnu_ifunc_resolve_addr): Likewise.
>>         (elf_gnu_ifunc_resolver_return_stop): Likewise.
>> ---
>>  gdb/elfread.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> Ping?

Ping?

Despite the subject the contents of this patch are not ARM specific
and change generic functionality.

-- 
Will Newton
Toolchain Working Group, Linaro

Re: [PATCH] gdb/elfread.c: Enable ifunc support on ARM.

[Pedro Alves]

On 12/16/2013 06:24 PM, Will Newton wrote:
> 
> There are two failures in the gnu-ifunc.exp test on ARM. These are
> due to the failure to resolve the correct target function when
> attempting to breakpoint a GNU ifunc resolved function:
> 
> (gdb) break gnu_ifunc
> Breakpoint 4 at gnu-indirect-function resolver at 0x2aacb5a2
> 
> when gnu_ifunc has been resolved this should actually be:
> 
> (gdb) break gnu_ifunc
> Breakpoint 4 at 0x868c
> 
> There are two reasons for this. The first is that ARM does not have a
> separate .got.plt section so looking this up will always fail. The second
> is that the Thumb bit needs to be stripped from the address to allow
> it to be reliably compared when inserting into the ifunc cache.
> 
> Tested with no regressions on arm-linux-gnueabihf and
> x86_64-unknown-linux-gnu.
> 
> gdb/ChangeLog:
> 
> 2013-12-16  Will Newton  <will.newton@linaro.org>
> 
> 	* elfread.c (elf_rel_plt_read): Look for a .got section if
> 	looking up .got.plt fails.
> 	(elf_gnu_ifunc_resolve_by_got): Call gdbarch_addr_bits_remove
> 	on address passed to elf_gnu_ifunc_record_cache.
> 	(elf_gnu_ifunc_resolve_addr): Likewise.
> 	(elf_gnu_ifunc_resolver_return_stop): Likewise.

Couple notes:

 - I think you can look at 'get_elf_backend_data (abfd)->want_got_plt'
   to decide whether to look up ".got" vs ".got.plt".

 - I'm also wondering whether got.plt in the symbol names
   (#define SYMBOL_GOT_PLT_SUFFIX "@got.plt") won't be a little confusing,
   and thus that prefix too should be decided on a want_got_plt basis,
   similarly.

-- 
Pedro Alves

Re: [PATCH] gdb/elfread.c: Enable ifunc support on ARM.

[Will Newton]

On 6 February 2014 17:42, Pedro Alves <palves@redhat.com> wrote:
> On 12/16/2013 06:24 PM, Will Newton wrote:
>>
>> There are two failures in the gnu-ifunc.exp test on ARM. These are
>> due to the failure to resolve the correct target function when
>> attempting to breakpoint a GNU ifunc resolved function:
>>
>> (gdb) break gnu_ifunc
>> Breakpoint 4 at gnu-indirect-function resolver at 0x2aacb5a2
>>
>> when gnu_ifunc has been resolved this should actually be:
>>
>> (gdb) break gnu_ifunc
>> Breakpoint 4 at 0x868c
>>
>> There are two reasons for this. The first is that ARM does not have a
>> separate .got.plt section so looking this up will always fail. The second
>> is that the Thumb bit needs to be stripped from the address to allow
>> it to be reliably compared when inserting into the ifunc cache.
>>
>> Tested with no regressions on arm-linux-gnueabihf and
>> x86_64-unknown-linux-gnu.
>>
>> gdb/ChangeLog:
>>
>> 2013-12-16  Will Newton  <will.newton@linaro.org>
>>
>>       * elfread.c (elf_rel_plt_read): Look for a .got section if
>>       looking up .got.plt fails.
>>       (elf_gnu_ifunc_resolve_by_got): Call gdbarch_addr_bits_remove
>>       on address passed to elf_gnu_ifunc_record_cache.
>>       (elf_gnu_ifunc_resolve_addr): Likewise.
>>       (elf_gnu_ifunc_resolver_return_stop): Likewise.

Thanks for taking a look.

> Couple notes:
>
>  - I think you can look at 'get_elf_backend_data (abfd)->want_got_plt'
>    to decide whether to look up ".got" vs ".got.plt".

I don't think that will work. We actually want to look at the value of
SEPARATE_GOTPLT I believe, which I don't think we can easily
determine. For example, ARM sets want_got_plt but not SEPARATE_GOTPLT.

-- 
Will Newton
Toolchain Working Group, Linaro

Re: [PATCH] gdb/elfread.c: Enable ifunc support on ARM.

[Pedro Alves]

On 02/10/2014 02:57 PM, Will Newton wrote:

> Thanks for taking a look.
> 
>> Couple notes:
>>
>>  - I think you can look at 'get_elf_backend_data (abfd)->want_got_plt'
>>    to decide whether to look up ".got" vs ".got.plt".
> 
> I don't think that will work. We actually want to look at the value of
> SEPARATE_GOTPLT I believe, which I don't think we can easily
> determine. For example, ARM sets want_got_plt but not SEPARATE_GOTPLT.

Alright.  Lets go with your original patch then.  Patch is OK.

Thanks,
-- 
Pedro Alves