[v2,05/15] Fix elf_gnu_ifunc_resolve_by_got buglet

Message ID 20180325191943.8246-6-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.
The next patch will add a call to elf_gnu_ifunc_resolve_by_got that
trips on a latent buglet -- the function is writing to its output
parameter even if the address wasn't found, confusing the caller.  The
function's intro comment says:

  /* Try to find the target resolved function entry address of a STT_GNU_IFUNC
     function NAME.  If the address is found it is stored to *ADDR_P (if ADDR_P
     is not NULL) and the function returns 1.  It returns 0 otherwise.

So fix the function accordingly.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* elfread.c (elf_gnu_ifunc_resolve_by_got): Don't write to *ADDR_P
	unless we actually resolved the ifunc.
---
 gdb/elfread.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

-- 
2.14.3

Comments

Simon Marchi April 1, 2018, 4:32 a.m. | #1
On 2018-03-25 03:19 PM, Pedro Alves wrote:
> The next patch will add a call to elf_gnu_ifunc_resolve_by_got that

> trips on a latent buglet -- the function is writing to its output

> parameter even if the address wasn't found, confusing the caller.  The

> function's intro comment says:

> 

>   /* Try to find the target resolved function entry address of a STT_GNU_IFUNC

>      function NAME.  If the address is found it is stored to *ADDR_P (if ADDR_P

>      is not NULL) and the function returns 1.  It returns 0 otherwise.

> 

> So fix the function accordingly.

> 

> gdb/ChangeLog:

> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

> 

> 	* elfread.c (elf_gnu_ifunc_resolve_by_got): Don't write to *ADDR_P

> 	unless we actually resolved the ifunc.

> ---

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

>  1 file changed, 5 insertions(+), 3 deletions(-)

> 

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

> index 9ffbf99db6e..82ab3d891ce 100644

> --- a/gdb/elfread.c

> +++ b/gdb/elfread.c

> @@ -840,10 +840,12 @@ elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)

>  						 &current_target);

>        addr = gdbarch_addr_bits_remove (gdbarch, addr);

>  

> -      if (addr_p)

> -	*addr_p = addr;

>        if (elf_gnu_ifunc_record_cache (name, addr))

> -	return 1;

> +	{

> +	  if (addr_p)


addr_p != NULL ?

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


>> --- a/gdb/elfread.c

>> +++ b/gdb/elfread.c

>> @@ -840,10 +840,12 @@ elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)

>>  						 &current_target);

>>        addr = gdbarch_addr_bits_remove (gdbarch, addr);

>>  

>> -      if (addr_p)

>> -	*addr_p = addr;

>>        if (elf_gnu_ifunc_record_cache (name, addr))

>> -	return 1;

>> +	{

>> +	  if (addr_p)

> 

> addr_p != NULL ?

> 


Might as well.  I've done that locally.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 9ffbf99db6e..82ab3d891ce 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -840,10 +840,12 @@  elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
 						 &current_target);
       addr = gdbarch_addr_bits_remove (gdbarch, addr);
 
-      if (addr_p)
-	*addr_p = addr;
       if (elf_gnu_ifunc_record_cache (name, addr))
-	return 1;
+	{
+	  if (addr_p)
+	    *addr_p = addr;
+	  return 1;
+	}
     }
 
   return 0;