[committed] hppa: Improve ordering of accesses during function pointer canonicalization

[John David Anglin]

The main fix here is to load the relocation offset before the function pointer during
function pointer canonicalization.  There is still a small race in multi-threaded applications
because the dynamic linker currently updates the relocation offset before the function pointer
when doing lazy binding.  When binutils is fixed to double word align function descriptors,
it will be possible to atomically update the descriptor using a floating point store.  Then,
this race will be eliminated.

Tested on hppa-unknown-linux-gnu with no regressions.  Committed to trunk and gcc-9 branch.

Dave

2019-10-15  John David Anglin  <danglin@gcc.gnu.org>

	* config/pa/fptr.c (_dl_read_access_allowed): Change argument to
	unsigned int.  Adjust callers.
	(__canonicalize_funcptr_for_compare): Change plabel type to volatile
	unsigned int *.  Load relocation offset before function pointer.
	Add barrier to ensure ordering.

Index: config/pa/fptr.c
===================================================================
--- config/pa/fptr.c	(revision 276964)
+++ config/pa/fptr.c	(working copy)
@@ -53,7 +53,7 @@
 extern unsigned int _GLOBAL_OFFSET_TABLE_;

 static inline int
-_dl_read_access_allowed (unsigned int *addr)
+_dl_read_access_allowed (unsigned int addr)
 {
   int result;

@@ -79,7 +79,8 @@
 {
   static unsigned int fixup_plabel[2] __attribute__((used));
   fixup_t fixup;
-  unsigned int *got, *iptr, *plabel;
+  volatile unsigned int *plabel;
+  unsigned int *got, *iptr, reloc_offset;
   int i;

   /* -1 and page 0 are special.  -1 is used in crtend to mark the end of
@@ -94,8 +95,8 @@
      to the entry of the PLT stub just before the global offset table.
      The second word in the plabel contains the relocation offset for the
      function.  */
-  plabel = (unsigned int *) ((unsigned int) fptr & ~3);
-  if (!_dl_read_access_allowed (plabel))
+  plabel = (volatile unsigned int *) ((unsigned int) fptr & ~3);
+  if (!_dl_read_access_allowed ((unsigned int)plabel))
     return (unsigned int) fptr;

   /* Load first word of candidate descriptor.  It should be a pointer
@@ -102,9 +103,12 @@
      with word alignment and point to memory that can be read.  */
   got = (unsigned int *) plabel[0];
   if (((unsigned int) got & 3) != 0
-      || !_dl_read_access_allowed (got))
+      || !_dl_read_access_allowed ((unsigned int)got))
     return (unsigned int) fptr;

+  /* We need to load the relocation offset before the function address.  */
+  reloc_offset = plabel[1];
+  __sync_synchronize();
   got = (unsigned int *) (plabel[0] + GOT_FROM_PLT_STUB);

   /* Return the address of the function if the plabel has been resolved.  */
@@ -140,7 +144,7 @@

   /* Call fixup to resolve the function address.  got[1] contains the
      link_map pointer and plabel[1] the relocation offset.  */
-  fixup ((struct link_map *) got[1], plabel[1]);
+  fixup ((struct link_map *) got[1], reloc_offset);

   return plabel[0];
 }