[committed] Return to caller when _dl_fixup fails on hppa

[committed] Return to caller when _dl_fixup fails on hppa

[John David Anglin]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

The attached change modifies _dl_runtime_resolve to return to its caller when _dl_fixup
fails.  This can occur on hppa when the caller attempts to call an undefined weak function.
The old code would fault when _dl_fixup fails.

The most problematic function in this regard is __gmon_start__.  Currently, it is linked into
every shared object.  This breaks the --as-needed ld option.

On Debian, we have an additional patch to treat __gmon_start__ as a true weak symbol.
I don't plan to apply this change as it becomes tricky to rebuild a distribution.  Rebuilding
a shared library removes __gmon_start__ and this may break a package using the library.
This isn't a problem after everything is rebuilt.

Dave
--
John David Anglin	dave.anglin@bell.net



[-- Attachment #2: dl-trampoline.S.d.txt --]
[-- Type: text/plain, Size: 989 bytes --]

2017-07-16  John David Anglin  <danglin@gcc.gnu.org>

	* sysdeps/hppa/dl-trampoline.S (_dl_runtime_resolve): Return to caller
	if _dl_fixup fails.

diff --git a/sysdeps/hppa/dl-trampoline.S b/sysdeps/hppa/dl-trampoline.S
index 856339bffe..f1294a931f 100644
--- a/sysdeps/hppa/dl-trampoline.S
+++ b/sysdeps/hppa/dl-trampoline.S
@@ -82,6 +82,11 @@ _dl_runtime_resolve:
 	bl	_dl_fixup,%rp
 	copy	%r21,%r19		/* set fixup func ltp */
 
+	/* Sometimes a final executable may attempt to call an undefined
+	   weak function (e.g., __gmon_start__).  Return if the function
+	   was not resolved by _dl_fixup */
+	comib,=	0,%r28,1f
+
 	/* Load up the returned func descriptor */
 	copy	%r28, %r22
 	copy	%r29, %r19
@@ -107,6 +112,13 @@ _dl_runtime_resolve:
 	/* Jump to new function, but return to previous function */
 	bv	%r0(%r22)
 	ldw	-20(%sp),%rp
+
+1:
+	/* Return to previous function */
+	ldw	-148(%sp),%rp
+	bv	%r0(%rp)
+	ldo	-128(%sp),%sp
+
         .EXIT
         .PROCEND
 	cfi_endproc

Re: [committed] Return to caller when _dl_fixup fails on hppa

[Andreas Schwab]

On Jul 16 2017, John David Anglin <dave.anglin@bell.net> wrote:

> The most problematic function in this regard is __gmon_start__.

The caller needs to check for NULL before calling it.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [committed] Return to caller when _dl_fixup fails on hppa

[John David Anglin]

On 2017-07-16, at 2:52 PM, Andreas Schwab wrote:

> On Jul 16 2017, John David Anglin <dave.anglin@bell.net> wrote:
> 
>> The most problematic function in this regard is __gmon_start__.
> 
> The caller needs to check for NULL before calling it.


That's assumed.  However, it doesn't work on hppa.  The caller has access to a non NULL pointer
to a function descriptor.  In the typical case with deferred binding, the function pointer is not resolved
until _dl_fixup is called.

Dave
--
John David Anglin	dave.anglin@bell.net

Re: [committed] Return to caller when _dl_fixup fails on hppa

[Andreas Schwab]

On Jul 16 2017, John David Anglin <dave.anglin@bell.net> wrote:

> That's assumed.  However, it doesn't work on hppa.  The caller has access to a non NULL pointer
> to a function descriptor.

It works well on ia64 and ppc64, why not on hppa?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [committed] Return to caller when _dl_fixup fails on hppa

[John David Anglin]

On 2017-07-16, at 5:31 PM, Andreas Schwab wrote:

>> That's assumed.  However, it doesn't work on hppa.  The caller has access to a non NULL pointer
>> to a function descriptor.
> 
> It works well on ia64 and ppc64, why not on hppa?

32-bit hppa doesn't have official procedure descriptors (OPDs).  Carlos at one time indicated that
he wanted to rework  binutils elf32-hppa.c to use OPDs but it never happened.  As a result, undefined
weak symbols are poorly supported.

It was a bad design and HP recognized it when they created the 64-bit hppa and ia64 runtimes.  The
32-bit hpux linker doesn't support undefined weak symbols at all.  We worked around it with one-only
support in gcc.

Anyway, one compare instruction checking the return of _dl_fixup doesn't affect performance much.
It could be removed if the problems with undefined weak functions are resolved.

Dave
--
John David Anglin	dave.anglin@bell.net

Re: [committed] Return to caller when _dl_fixup fails on hppa

[Andreas Schwab]

This information should be added in a comment.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [committed] Return to caller when _dl_fixup fails on hppa

[Carlos O'Donell]

On 07/17/2017 03:10 AM, Andreas Schwab wrote:
> This information should be added in a comment.

Dave, Thanks for all the fixes. I agree with Andreas, this should be
documented with a comment.

-- 
Cheers,
Carlos.

Re: [committed] Return to caller when _dl_fixup fails on hppa

[John David Anglin]

On 2017-07-17, at 7:11 AM, Carlos O'Donell wrote:

> Dave, Thanks for all the fixes. I agree with Andreas, this should be
> documented with a comment.

No problem.  Probably next weekend.

Dave
--
John David Anglin	dave.anglin@bell.net

Re: [committed] Return to caller when _dl_fixup fails on hppa

[John David Anglin]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

On 2017-07-17, at 7:11 AM, Carlos O'Donell wrote:

> On 07/17/2017 03:10 AM, Andreas Schwab wrote:
>> This information should be added in a comment.
> 
> Dave, Thanks for all the fixes. I agree with Andreas, this should be
> documented with a comment.

I committed the attached comment update.  I tied it to the BZ report so it's clear what we
are trying to fix.

At this time, Debian has a local patch which removes __gmon_start__ from crtn.S on hppa.

Dave
--
John David Anglin	dave.anglin@bell.net



[-- Attachment #2: dl-trampoline-v3.S.d.txt --]
[-- Type: text/plain, Size: 1497 bytes --]

2017-07-23  John David Anglin  <danglin@gcc.gnu.org>

	[BZ 19170]
	* sysdeps/hppa/dl-trampoline.S (_dl_runtime_resolve): Revise comment.

diff --git a/sysdeps/hppa/dl-trampoline.S b/sysdeps/hppa/dl-trampoline.S
index f1294a931f..3165c6f0e2 100644
--- a/sysdeps/hppa/dl-trampoline.S
+++ b/sysdeps/hppa/dl-trampoline.S
@@ -82,9 +82,19 @@ _dl_runtime_resolve:
 	bl	_dl_fixup,%rp
 	copy	%r21,%r19		/* set fixup func ltp */
 
-	/* Sometimes a final executable may attempt to call an undefined
-	   weak function (e.g., __gmon_start__).  Return if the function
-	   was not resolved by _dl_fixup */
+	/* While the linker will set a function pointer to NULL when it
+	   encounters an undefined weak function, we need to dynamically
+	   detect removed weak functions.  The issue arises because a weak
+	   __gmon_start__ function was added to shared executables to work
+	   around issues in _init that are now resolved.  The presence of
+	   __gmon_start__ in every shared library breaks the linker
+	   `--as-needed' option.  This __gmon_start__ function does nothing
+	   but removal is tricky.  Depending on the binding, removal can
+	   cause an application using it to fault.  The call to _dl_fixup
+	   returns NULL when a function isn't resolved.  In order to help
+	   with __gmon_start__ removal, we return directly to the caller
+	   when _dl_fixup returns NULL.  This check could be removed when
+	   BZ 19170 is fixed.  */
 	comib,=	0,%r28,1f
 
 	/* Load up the returned func descriptor */