Fix dladdr on hppa

The attached patch fixes dladdr on hppa.

Instead of using the generic version of _dl_lookup_address, we use an implementation more or less modelled
after __canonicalize_funcptr_for_compare() in gcc.  The function pointer is analyzed and if it points to the trampoline
used to call _dl_runtime_resolve just before the global offset table, then we call _dl_fixup to resolve the function
pointer.  Then, we return the instruction pointer from the first word of the descriptor.

The change fixes the testcase provided in [BZ #19415] and the Debian nss package now builds successfully.

Please install if okay.

Thanks,
Dave
--
John David Anglin	dave.anglin@bell.net
2016-01-02  John David Anglin  <danglin@gcc.gnu.org>

	[BZ #19415]
	* sysdeps/hppa/dl-fptr.c (_dl_fixup): Declare.
	(elf_machine_resolve): New.  Return address of _dl_runtime_resolve.
	(_dl_lookup_address): Rewrite using function resolver trampoline.
	* sysdeps/hppa/dl-lookupcfg.h (DL_LOOKUP_ADDRESS): Don't clear bottom
	two bits in address.

On 02 Jan 2016 09:48, John David Anglin wrote:
> The change fixes the testcase provided in [BZ #19415] and the Debian nss package now builds successfully.

can we include that testcase in a form that does self checking ?
-mike

On 01/02/2016 09:48 AM, John David Anglin wrote:
> 2016-01-02  John David Anglin  <danglin@gcc.gnu.org>
> 
> 	[BZ #19415]
> 	* sysdeps/hppa/dl-fptr.c (_dl_fixup): Declare.
> 	(elf_machine_resolve): New.  Return address of _dl_runtime_resolve.
> 	(_dl_lookup_address): Rewrite using function resolver trampoline.
> 	* sysdeps/hppa/dl-lookupcfg.h (DL_LOOKUP_ADDRESS): Don't clear bottom
> 	two bits in address.

Looks good to me.

The existing code was not ia64 specific, but it did assume that all of
the function descriptors were resolved before being compared, and that's
not a very good assumption. There were similar problems on i386 and
x86_64 until the compiler started loading the final symbol address from
the GOT and passing that to dladdr, otherwise it passed the PLT entry
address and that obviously causes the same problems hppa had.

That's the reason there is probably no test case for this because I bet
it also doesn't work reliable well on all the targets we support. For
example dlfcn/tst-dladdr.c just makes sure the various entries are
non-NULL in the returned struct.

Mike could you hlep check this in?
 
> diff --git a/sysdeps/hppa/dl-fptr.c b/sysdeps/hppa/dl-fptr.c
> index bb12ab2..ddb9db7 100644
> --- a/sysdeps/hppa/dl-fptr.c
> +++ b/sysdeps/hppa/dl-fptr.c
> @@ -315,23 +315,54 @@ _dl_unmap (struct link_map *map)
>    map->l_mach.fptr_table = NULL;
>  }
>  
> +extern ElfW(Addr) _dl_fixup (struct link_map *, ElfW(Word)) attribute_hidden;
>  
> -ElfW(Addr)
> -_dl_lookup_address (const void *address)
> +static inline Elf32_Addr
> +elf_machine_resolve (void)
>  {
> -  ElfW(Addr) addr = (ElfW(Addr)) address;
> -  struct fdesc_table *t;
> -  unsigned long int i;
> +  Elf32_Addr addr;
>  
> -  for (t = local.root; t != NULL; t = t->next)
> -    {
> -      i = (struct fdesc *) addr - &t->fdesc[0];
> -      if (i < t->first_unused && addr == (ElfW(Addr)) &t->fdesc[i])
> -	{
> -	  addr = t->fdesc[i].ip;
> -	  break;
> -	}
> -    }
> +  asm ("b,l     1f,%0\n"
> +"	depi	0,31,2,%0\n"
> +"1:	addil	L'_dl_runtime_resolve - ($PIC_pcrel$0 - 8),%0\n"
> +"	ldo	R'_dl_runtime_resolve - ($PIC_pcrel$0 - 12)(%%r1),%0\n"
> +       : "=r" (addr) : : "r1");
>  
>    return addr;
>  }
> +
> +ElfW(Addr)
> +_dl_lookup_address (const void *address)
> +{
> +  ElfW(Addr) addr = (ElfW(Addr)) address;
> +  unsigned int *desc, *gptr;
> +
> +  /* Check for special cases.  */

When might we have -1 and 0-4096?

They are obviously not real fdescs, but when are they used?

> +  if ((int) addr == -1
> +      || (unsigned int) addr < 4096
> +      || !((unsigned int) addr & 2))
> +    return addr;

OK.

> +
> +  /* Clear least-significant two bits from descriptor address.  */
> +  desc = (unsigned int *) ((unsigned int) addr & ~3);

OK.

> +
> +  /* Check if descriptor requires resolution.  The following trampoline is
> +     used in each global offset table for function resolution:
> +
> +		ldw 0(r20),r22
> +		bv r0(r22)
> +		ldw 4(r20),r21
> +     tramp:	b,l .-12,r20
> +		depwi 0,31,2,r20
> +		.word _dl_runtime_resolve
> +		.word "_dl_runtime_resolve ltp"
> +     got:	.word _DYNAMIC
> +		.word "struct link map address" */
> +  gptr = (unsigned int *) desc[0];
> +  if (gptr[0] == 0xea9f1fdd			/* b,l .-12,r20     */
> +      && gptr[1] == 0xd6801c1e			/* depwi 0,31,2,r20 */
> +      && (ElfW(Addr)) gptr[2] == elf_machine_resolve ())

OK.

> +    _dl_fixup ((struct link_map *) gptr[5], (ElfW(Word)) desc[1]);

OK.

> +
> +  return (ElfW(Addr)) desc[0];
> +}
> diff --git a/sysdeps/hppa/dl-lookupcfg.h b/sysdeps/hppa/dl-lookupcfg.h
> index c36928c..0b4dc45 100644
> --- a/sysdeps/hppa/dl-lookupcfg.h
> +++ b/sysdeps/hppa/dl-lookupcfg.h
> @@ -31,9 +31,7 @@ rtld_hidden_proto (_dl_symbol_address)
>  
>  Elf32_Addr _dl_lookup_address (const void *address);
>  
> -/* Clear the bottom two bits so generic code can find the fdesc entry */
> -#define DL_LOOKUP_ADDRESS(addr) \
> -  (_dl_lookup_address ((void *)((unsigned long)addr & ~3)))
> +#define DL_LOOKUP_ADDRESS(addr) _dl_lookup_address ((const void *) addr)

OK.

>  
>  void attribute_hidden _dl_unmap (struct link_map *map);
>  

Cheers,
Carlos.

On 2016-01-07 6:22 AM, Carlos O'Donell wrote:
> When might we have -1 and 0-4096?
>
> They are obviously not real fdescs, but when are they used?
>
>> >+  if ((int) addr == -1
>> >+      || (unsigned int) addr < 4096
>> >+      || !((unsigned int) addr & 2))
>> >+    return addr;

-1 is used in gcc's crtstuff.c to mark the end of a list of function 
pointers.
0-4096 is page 0 and can't be accessed on PA-RISC.  Thus, these values 
can be
reserved for special uses.  For example, we have for signals:

/* Fake signal functions.  */
#define SIG_ERR ((__sighandler_t) -1)           /* Error return.  */
#define SIG_DFL ((__sighandler_t) 0)            /* Default action. */
#define SIG_IGN ((__sighandler_t) 1)            /* Ignore signal. */

#ifdef __USE_UNIX98
# define SIG_HOLD       ((__sighandler_t) 2)    /* Add signal to hold 
mask.  */
#endif

Dave

On 07 Jan 2016 06:22, Carlos O'Donell wrote:
> On 01/02/2016 09:48 AM, John David Anglin wrote:
> > 2016-01-02  John David Anglin  <danglin@gcc.gnu.org>
> > 
> > 	[BZ #19415]
> > 	* sysdeps/hppa/dl-fptr.c (_dl_fixup): Declare.
> > 	(elf_machine_resolve): New.  Return address of _dl_runtime_resolve.
> > 	(_dl_lookup_address): Rewrite using function resolver trampoline.
> > 	* sysdeps/hppa/dl-lookupcfg.h (DL_LOOKUP_ADDRESS): Don't clear bottom
> > 	two bits in address.
> 
> Looks good to me.
> 
> The existing code was not ia64 specific, but it did assume that all of
> the function descriptors were resolved before being compared, and that's
> not a very good assumption. There were similar problems on i386 and
> x86_64 until the compiler started loading the final symbol address from
> the GOT and passing that to dladdr, otherwise it passed the PLT entry
> address and that obviously causes the same problems hppa had.
> 
> That's the reason there is probably no test case for this because I bet
> it also doesn't work reliable well on all the targets we support. For
> example dlfcn/tst-dladdr.c just makes sure the various entries are
> non-NULL in the returned struct.

ia64 is why i was wondering about test cases -- i wanted to know if i
needed the same fix there
-mike

On 01/07/2016 02:43 PM, Mike Frysinger wrote:
> On 07 Jan 2016 06:22, Carlos O'Donell wrote:
>> On 01/02/2016 09:48 AM, John David Anglin wrote:
>>> 2016-01-02  John David Anglin  <danglin@gcc.gnu.org>
>>>
>>> 	[BZ #19415]
>>> 	* sysdeps/hppa/dl-fptr.c (_dl_fixup): Declare.
>>> 	(elf_machine_resolve): New.  Return address of _dl_runtime_resolve.
>>> 	(_dl_lookup_address): Rewrite using function resolver trampoline.
>>> 	* sysdeps/hppa/dl-lookupcfg.h (DL_LOOKUP_ADDRESS): Don't clear bottom
>>> 	two bits in address.
>>
>> Looks good to me.
>>
>> The existing code was not ia64 specific, but it did assume that all of
>> the function descriptors were resolved before being compared, and that's
>> not a very good assumption. There were similar problems on i386 and
>> x86_64 until the compiler started loading the final symbol address from
>> the GOT and passing that to dladdr, otherwise it passed the PLT entry
>> address and that obviously causes the same problems hppa had.
>>
>> That's the reason there is probably no test case for this because I bet
>> it also doesn't work reliable well on all the targets we support. For
>> example dlfcn/tst-dladdr.c just makes sure the various entries are
>> non-NULL in the returned struct.
> 
> ia64 is why i was wondering about test cases -- i wanted to know if i
> needed the same fix there

Extend dlfcn/dl-addr.c to actually test the result of the call :-)

c.

On 07 Jan 2016 06:22, Carlos O'Donell wrote:
> On 01/02/2016 09:48 AM, John David Anglin wrote:
> > 2016-01-02  John David Anglin  <danglin@gcc.gnu.org>
> > 
> > 	[BZ #19415]
> > 	* sysdeps/hppa/dl-fptr.c (_dl_fixup): Declare.
> > 	(elf_machine_resolve): New.  Return address of _dl_runtime_resolve.
> > 	(_dl_lookup_address): Rewrite using function resolver trampoline.
> > 	* sysdeps/hppa/dl-lookupcfg.h (DL_LOOKUP_ADDRESS): Don't clear bottom
> > 	two bits in address.
> 
> Looks good to me.
> 
> The existing code was not ia64 specific, but it did assume that all of
> the function descriptors were resolved before being compared, and that's
> not a very good assumption. There were similar problems on i386 and
> x86_64 until the compiler started loading the final symbol address from
> the GOT and passing that to dladdr, otherwise it passed the PLT entry
> address and that obviously causes the same problems hppa had.
> 
> That's the reason there is probably no test case for this because I bet
> it also doesn't work reliable well on all the targets we support. For
> example dlfcn/tst-dladdr.c just makes sure the various entries are
> non-NULL in the returned struct.
> 
> Mike could you hlep check this in?

i've pushed it now.  i checked the testcase in the bug and it works fine
on ia64, so i'll stop looking into it ;).
-mike