diff mbox

[v2,05/15] Fix elf_gnu_ifunc_resolve_by_got buglet

Message ID 20180325191943.8246-6-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves March 25, 2018, 7:19 p.m. UTC 
  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(-)

Comments

Simon Marchi April 1, 2018, 4:32 a.m. UTC | #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. UTC | #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
diff mbox

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;