[v2] hppa: Handle __gmon_start__ as undefined weak or hidden on hppa

Message ID C82C1C5B-94AF-47DE-A588-0246C39778AD@bell.net
State New, archived
Headers

Commit Message

John David Anglin Nov. 15, 2015, 8:05 p.m. UTC
  On 2015-10-25, at 3:32 PM, John David Anglin wrote:

> The attached change fixes libc/19170.  It is identical to the approach used on ia64 aside from
> the hppa assembler code.


In testing, an issue was found with the previous change.  Final executables would generate a segmentation
fault in _dl_fixup (DL_FIXUP_MAKE_VALUE) when all instances of __gmon_start__ were removed from the
executable's dependent shared libraries.  Generally, this was easily fixed by rebuilding the executable but
this is problematic for tool-chain executables needing recursive builds.

The attached change facilitates the removal of the current weak definition of __gmon_start__.

The DL_FIXUP_MAKE_VALUE macro is updated so that it doesn't cause a segmentation fault when its
MAP argument is null.  It should be noted that the LOOKUP_VALUE_ADDRESS macro which also has
a MAP argument checks for a null value.  It now generates a null descriptor when MAP is null.
__canonicalize_funcptr_for_compare will now return null in a final executable if a function is undefined
due to library rebuilds.

Secondly, we now check in _dl_runtime_resolve whether _dl_fixup found the requested function and
if not it returns directly to the previous caller.  Of course, this only works once but that's all that's needed.
Old shared executables can resolve to their internal instance of __gmon_start__.  New executables
have checks that allow gmon_initializer to determine whether __gmon_start__ is defined or not.

The comment for gmon_initializer is updated to explain the treatment of PLABEL32 relocations in shared
and final executables.  gmon_initializer is updated to call __canonicalize_funcptr_for_compare.  This
doesn't significantly affect the fast path when __gmon_start__ is not defined, and resolution needs
doing when it is defined.  Also, function pointers are handled at runtime in shared executables.

There are significant advantages in having a weak undefined __gmon_start__.  The --as-needed processing
by ld works correctly and we don't end up with unnecessary libraries bound to an application.  Lazy
binding is also improved.

Okay?

Dave 
--
John David Anglin	dave.anglin@bell.net
2015-11-15  Helge Deller  <deller@gmx.de>
	    John David Anglin  <danglin@gcc.gnu.org>

	PR libc/19170
	* sysdeps/hppa/crti.S (gmon_initializer): New.  Use .init_array support
	to call gmon_initializer.  Only call __gmon_start__ when symbol is
	defined at runtime.
	* sysdeps/hppa/crtn.S (__gmon_start__): Delete.
	* sysdeps/hppa/dl-lookupcfg.h (DL_FIXUP_MAKE_VALUE): Provide null
	function descriptor when MAP argument is null.
	* sysdeps/hppa/dl-trampoline.S (_dl_runtime_resolve): Return directly
	to previous function when function is undefined.
  

Patch

diff --git a/sysdeps/hppa/crti.S b/sysdeps/hppa/crti.S
index 98f1af5..568a999 100644
--- a/sysdeps/hppa/crti.S
+++ b/sysdeps/hppa/crti.S
@@ -49,6 +49,95 @@ 
 # define PREINIT_FUNCTION_WEAK 1
 #endif
 
+#if PREINIT_FUNCTION_WEAK
+	weak_extern (PREINIT_FUNCTION)
+#else
+	.hidden PREINIT_FUNCTION
+#endif
+
+
+/* If we have working .init_array support, we want to keep the .init
+   section empty (apart from the mandatory prologue/epilogue.  This
+   ensures that the default unwind conventions (return-pointer in b0,
+   frame state in ar.pfs, etc.)  will do the Right Thing.  To ensure
+   an empty .init section, we register gmon_initializer() via the
+   .init_array.
+
+    --davidm 02/10/29 */
+
+#if PREINIT_FUNCTION_WEAK
+/* This blob of assembly code is one simple C function:
+
+static void
+__attribute__ ((used))
+gmon_initializer (void)
+{
+  extern void weak_function __gmon_start__ (void);
+
+  if (__gmon_start__)
+    (*__gmon_start__)();
+}
+
+In a final executable, PLABEL32 relocations for function pointers are
+resolved at link time.  Typically, binutils/ld resolves __gmon_start__
+using an external shared library.  __gmon_start__ is always called if
+it is found at link time.  If __gmon_start__ is not found at runtime
+due to a library update, then the function pointer will point at a null
+function descriptor and calling it will cause a segmentation fault.
+So, we call __canonicalize_funcptr_for_compare to obtain the canonicalized
+address of __gmon_start__ and skip calling __gmon_start__ if it is zero.
+
+ */
+	.type __canonicalize_funcptr_for_compare,@function
+	.type $$dyncall,@function
+
+	.section .data.rel.ro,"aw",@progbits
+	.align 4
+.LC0:
+	.type __gmon_start__,@function
+	.word P%__gmon_start__
+
+	.text
+	.align 4
+	.type gmon_initializer,@function
+gmon_initializer:
+	.PROC
+	.CALLINFO FRAME=64,CALLS,SAVE_RP,ENTRY_GR=4
+	.ENTRY
+	stw %r2,-20(%r30)
+	stwm %r4,64(%r30)
+	stw %r3,-60(%r30)
+	addil LT'.LC0,%r19
+	ldw RT'.LC0(%r1),%r28
+	ldw 0(%r28),%r3
+	comib,= 0,%r3,1f
+	copy %r19,%r4
+	stw %r19,-32(%r30)
+	bl __canonicalize_funcptr_for_compare,%r2
+	copy %r3,%r26
+	comib,= 0,%r28,1f
+	copy %r4,%r19
+	copy %r3,%r22
+	.CALL ARGW0=GR
+	bl $$dyncall,%r31
+	copy %r31,%r2
+1:
+	ldw -84(%r30),%r2
+	ldw -60(%r30),%r3
+	bv %r0(%r2)
+	ldwm -64(%r30),%r4
+	.EXIT
+	.PROCEND
+	.size gmon_initializer, .-gmon_initializer
+
+# undef PREINIT_FUNCTION
+# define PREINIT_FUNCTION gmon_initializer
+#endif
+
+	.section .init_array, "aw"
+	.word P% PREINIT_FUNCTION
+
+
 /* _init prologue.  */
 	.section .init, "ax", %progbits
 	.align 4
@@ -58,14 +147,6 @@  _init:
 	stw	%rp,-20(%sp)
 	stwm	%r4,64(%sp)
 	stw	%r19,-32(%sp)
-#if PREINIT_FUNCTION_WEAK
-	bl	PREINIT_FUNCTION,%rp
-	copy	%r19,%r4	/* delay slot */
-#else
-	bl	PREINIT_FUNCTION,%rp
-	copy	%r19,%r4	/* delay slot */
-#endif
-	copy	%r4,%r19
 
 /* _fini prologue.  */
         .section .fini,"ax",%progbits
diff --git a/sysdeps/hppa/crtn.S b/sysdeps/hppa/crtn.S
index e0d345f..a410425 100644
--- a/sysdeps/hppa/crtn.S
+++ b/sysdeps/hppa/crtn.S
@@ -38,27 +38,6 @@ 
 /* crtn.S puts function epilogues in the .init and .fini sections
    corresponding to the prologues in crti.S. */
 
-/* Note that we cannot have a weak undefined __gmon_start__, because
-   that would require this to be PIC, and the linker is currently not
-   able to generate a proper procedure descriptor for _init.  Sad but
-   true.  Anyway, HPPA is one of those horrible architectures where
-   making the comparison and indirect call is quite expensive (see the
-   comment in sysdeps/generic/initfini.c). */
-        .text
-        .align 4
-        .weak   __gmon_start__
-        .type    __gmon_start__,@function
-__gmon_start__:
-	.proc
-	.callinfo
-	.entry
-        bv,n %r0(%r2)
-	.exit
-	.procend
-
-/* Here is the tail end of _init.  We put __gmon_start before this so
-   that the assembler creates the .PARISC.unwind section for us, ie.
-   with the right attributes.  */
 	.section .init, "ax", @progbits
 	ldw	-84(%sp),%rp
 	copy	%r4,%r19
diff --git a/sysdeps/hppa/dl-lookupcfg.h b/sysdeps/hppa/dl-lookupcfg.h
index c36928c..dea1809 100644
--- a/sysdeps/hppa/dl-lookupcfg.h
+++ b/sysdeps/hppa/dl-lookupcfg.h
@@ -75,7 +75,9 @@  void attribute_hidden _dl_unmap (struct link_map *map);
 
 /* Construct a fixup value from the address and linkmap */
 #define DL_FIXUP_MAKE_VALUE(map, addr) \
-   ((struct fdesc) { (addr), (map)->l_info[DT_PLTGOT]->d_un.d_ptr })
+  (map) ? ((struct fdesc) { (addr), (map)->l_info[DT_PLTGOT]->d_un.d_ptr }) \
+	: ((struct fdesc) { 0, 0 })
+
 
 /* Extract the code address from a fixup value */
 #define DL_FIXUP_VALUE_CODE_ADDR(value) ((value).ip)
diff --git a/sysdeps/hppa/dl-trampoline.S b/sysdeps/hppa/dl-trampoline.S
index 22f484a..64f0966 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