powerpc build failure with GCC mainline -fno-common change

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

powerpc build failure with GCC mainline -fno-common change

Joseph Myers
glibc for powerpc configurations is failing to build with GCC mainline
after the switch to -fno-common by default.  The error is multiple
definitions of __cache_line_size linking sln, in dl-sysdep.c and
libc-start.c.

https://sourceware.org/ml/libc-testresults/2019-q4/msg00237.html

As I see it, there are two obvious possible fixes: either use
__attribute__ ((common)) on both definitions, or change one of them to an
extern declaration if SHARED is not defined.  Any comments on what would
be the better fix?

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: powerpc build failure with GCC mainline -fno-common change

Andreas Schwab
On Nov 21 2019, Joseph Myers wrote:

> As I see it, there are two obvious possible fixes: either use
> __attribute__ ((common)) on both definitions, or change one of them to an
> extern declaration if SHARED is not defined.  Any comments on what would
> be the better fix?

Perhaps __cache_line_size should be part of _rtld_global?

Andreas.

--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: powerpc build failure with GCC mainline -fno-common change

Tulio Magno Quites Machado Filho-2
In reply to this post by Joseph Myers
Joseph Myers <[hidden email]> writes:

> glibc for powerpc configurations is failing to build with GCC mainline
> after the switch to -fno-common by default.  The error is multiple
> definitions of __cache_line_size linking sln, in dl-sysdep.c and
> libc-start.c.
>
> https://sourceware.org/ml/libc-testresults/2019-q4/msg00237.html
>
> As I see it, there are two obvious possible fixes: either use
> __attribute__ ((common)) on both definitions, or change one of them to an
> extern declaration if SHARED is not defined.  Any comments on what would
> be the better fix?

AFAICS, this variable is duplicated in the loader and in libc.so and I'm afraid
these suggestions would hide the real issue.

> Perhaps __cache_line_size should be part of _rtld_global?

Having this variable in a single place is ideal, but does it really need to
be in the loader?
AFAICS, libc is the only place using __cache_line_size.

I'm preparing a patch that leaves this variable only in libc (shared and
static), but I'm open to suggestions if you think this could variable should
have been available to the other libraries too.

--
Tulio Magno
Reply | Threaded
Open this post in threaded view
|

Re: powerpc build failure with GCC mainline -fno-common change

Florian Weimer
* Tulio Magno Quites Machado Filho:

>> Perhaps __cache_line_size should be part of _rtld_global?
>
> Having this variable in a single place is ideal, but does it really need to
> be in the loader?

Doesn't the loader need it for its implementation of memset?

The value is determined by parsing the auxiliary vector, so seems
reasonable to do this in the loader.
Reply | Threaded
Open this post in threaded view
|

Re: powerpc build failure with GCC mainline -fno-common change

Tulio Magno Quites Machado Filho-2
Florian Weimer <[hidden email]> writes:

> * Tulio Magno Quites Machado Filho:
>
>>> Perhaps __cache_line_size should be part of _rtld_global?
>>
>> Having this variable in a single place is ideal, but does it really need to
>> be in the loader?
>
> Doesn't the loader need it for its implementation of memset?

No, but sysdeps/powerpc/powerpc32/dl-machine.c does need __cache_line_size.

--
Tulio Magno
Reply | Threaded
Open this post in threaded view
|

[PATCH] powerpc: Move cache line size to rtld_global_ro

Tulio Magno Quites Machado Filho-2
In reply to this post by Andreas Schwab
GCC 10.0 enabled -fno-common by default and this started to point that
__cache_line_size had been implemented in 2 different places: loader and
libc.

In order to avoid this duplication, the libc variable has been removed
and the loader variable is moved to rtld_global_ro.

File sysdeps/unix/sysv/linux/powerpc/dl-auxv.h has been added in order
to reuse code for both static and dynamic linking scenarios.

Signed-off-by: Tulio Magno Quites Machado Filho <[hidden email]>
---
 sysdeps/powerpc/dl-procinfo.c                | 17 ++++++++++++
 sysdeps/powerpc/powerpc32/a2/memcpy.S        | 23 ++++++++--------
 sysdeps/powerpc/powerpc32/dl-machine.c       | 11 ++------
 sysdeps/powerpc/powerpc32/memset.S           | 29 +++++++++-----------
 sysdeps/powerpc/powerpc32/sysdep.h           | 26 ++++++++++++++++++
 sysdeps/powerpc/powerpc64/a2/memcpy.S        | 21 +++++++++++---
 sysdeps/powerpc/powerpc64/memset.S           | 24 ++++++++++++++--
 sysdeps/powerpc/rtld-global-offsets.sym      |  1 +
 sysdeps/unix/sysv/linux/powerpc/dl-auxv.h    | 24 ++++++++++++++++
 sysdeps/unix/sysv/linux/powerpc/dl-support.c | 23 ++++++++++++++++
 sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c  | 12 +-------
 sysdeps/unix/sysv/linux/powerpc/libc-start.c | 10 ++-----
 12 files changed, 160 insertions(+), 61 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-support.c

diff --git a/sysdeps/powerpc/dl-procinfo.c b/sysdeps/powerpc/dl-procinfo.c
index 20706a648a..94b33d0c16 100644
--- a/sysdeps/powerpc/dl-procinfo.c
+++ b/sysdeps/powerpc/dl-procinfo.c
@@ -89,5 +89,22 @@ PROCINFO_CLASS const char _dl_powerpc_cap_flags[64][15]
 ,
 #endif
 
+#if !IS_IN (ldconfig)
+# if !defined PROCINFO_DECL && defined SHARED
+     ._dl_cache_line_size
+# else
+PROCINFO_CLASS int _dl_cache_line_size
+# endif
+# ifndef PROCINFO_DECL
+     = 0
+# endif
+# if !defined SHARED || defined PROCINFO_DECL
+;
+# else
+,
+# endif
+#endif
+
+
 #undef PROCINFO_DECL
 #undef PROCINFO_CLASS
diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S
index 9bc91a8df1..ecfea5c832 100644
--- a/sysdeps/powerpc/powerpc32/a2/memcpy.S
+++ b/sysdeps/powerpc/powerpc32/a2/memcpy.S
@@ -18,6 +18,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
+#include <rtld-global-offsets.h>
 
 #define PREFETCH_AHEAD 4        /* no cache lines SRC prefetching ahead  */
 #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
@@ -106,25 +107,23 @@ EALIGN (memcpy, 5, 0)
 L(dst_aligned):
 
 
-#ifdef SHARED
+#ifdef PIC
  mflr    r0
-/* Establishes GOT addressability so we can load __cache_line_size
-   from static. This value was set from the aux vector during startup.  */
+/* Establishes GOT addressability so we can load the cache line size
+   from rtld_global_ro. This value was set from the aux vector during
+   startup.  */
  SETUP_GOT_ACCESS(r9,got_label)
- addis   r9,r9,__cache_line_size-got_label@ha
- lwz     r9,__cache_line_size-got_label@l(r9)
- mtlr    r0
-#else
-/* Load __cache_line_size from static. This value was set from the
-   aux vector during startup.  */
- lis     r9,__cache_line_size@ha
- lwz     r9,__cache_line_size@l(r9)
+ addis r9,r9,_GLOBAL_OFFSET_TABLE_-got_label@ha
+ addi r9,r9,_GLOBAL_OFFSET_TABLE_-got_label@l
+ mtlr r0
 #endif
+ __GLRO(r9, r9, _dl_cache_line_size,
+       RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)
 
  cmplwi  cr5, r9, 0
  bne+    cr5,L(cachelineset)
 
-/* __cache_line_size not set: generic byte copy without much optimization */
+/* Cache line size not set: generic byte copy without much optimization */
  andi. r0,r5,1 /* If length is odd copy one byte.  */
  beq L(cachelinenotset_align)
  lbz r7,0(r4) /* Read one byte from source.  */
diff --git a/sysdeps/powerpc/powerpc32/dl-machine.c b/sysdeps/powerpc/powerpc32/dl-machine.c
index a90cbc1ae3..740ff8f2ec 100644
--- a/sysdeps/powerpc/powerpc32/dl-machine.c
+++ b/sysdeps/powerpc/powerpc32/dl-machine.c
@@ -25,11 +25,6 @@
 #include <dl-machine.h>
 #include <_itoa.h>
 
-/* The value __cache_line_size is defined in dl-sysdep.c and is initialised
-   by _dl_sysdep_start via DL_PLATFORM_INIT.  */
-extern int __cache_line_size attribute_hidden;
-
-
 /* Stuff for the PLT.  */
 #define PLT_INITIAL_ENTRY_WORDS 18
 #define PLT_LONGBRANCH_ENTRY_WORDS 0
@@ -309,14 +304,14 @@ __elf_machine_runtime_setup (struct link_map *map, int lazy, int profile)
 
  Assumes that dcbst and icbi apply to lines of 16 bytes or
  more.  Current known line sizes are 16, 32, and 128 bytes.
- The following gets the __cache_line_size, when available.  */
+ The following gets the cache line size, when available.  */
 
       /* Default minimum 4 words per cache line.  */
       int line_size_words = 4;
 
-      if (lazy && __cache_line_size != 0)
+      if (lazy && GLRO(dl_cache_line_size) != 0)
  /* Convert bytes to words.  */
- line_size_words = __cache_line_size / 4;
+ line_size_words = GLRO(dl_cache_line_size) / 4;
 
       size_modified = lazy ? rel_offset_words : 6;
       for (i = 0; i < size_modified; i += line_size_words)
diff --git a/sysdeps/powerpc/powerpc32/memset.S b/sysdeps/powerpc/powerpc32/memset.S
index fc8b1a2547..ea96913e4e 100644
--- a/sysdeps/powerpc/powerpc32/memset.S
+++ b/sysdeps/powerpc/powerpc32/memset.S
@@ -17,12 +17,13 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
+#include <rtld-global-offsets.h>
 
 /* void * [r3] memset (void *s [r3], int c [r4], size_t n [r5]));
    Returns 's'.
 
    The memset is done in four sizes: byte (8 bits), word (32 bits),
-   32-byte blocks (256 bits) and __cache_line_size (128, 256, 1024 bits).
+   32-byte blocks (256 bits) and cache line size (128, 256, 1024 bits).
    There is a special case for setting whole cache lines to 0, which
    takes advantage of the dcbz instruction.  */
 
@@ -95,7 +96,7 @@ L(caligned):
 
 /* Check if we can use the special case for clearing memory using dcbz.
    This requires that we know the correct cache line size for this
-   processor.  Getting the __cache_line_size may require establishing GOT
+   processor.  Getting the cache line size may require establishing GOT
    addressability, so branch out of line to set this up.  */
  beq cr1, L(checklinesize)
 
@@ -230,26 +231,22 @@ L(medium_28t):
  blr
 
 L(checklinesize):
-#ifdef SHARED
- mflr rTMP
 /* If the remaining length is less the 32 bytes then don't bother getting
    the cache line size.  */
  beq L(medium)
-/* Establishes GOT addressability so we can load __cache_line_size
-   from static. This value was set from the aux vector during startup.  */
+#ifdef PIC
+ mflr rTMP
+/* Establishes GOT addressability so we can load the cache line size
+   from rtld_global_ro. This value was set from the aux vector during
+   startup.  */
  SETUP_GOT_ACCESS(rGOT,got_label)
- addis rGOT,rGOT,__cache_line_size-got_label@ha
- lwz rCLS,__cache_line_size-got_label@l(rGOT)
+ addis rGOT,rGOT,_GLOBAL_OFFSET_TABLE_-got_label@ha
+ addi rGOT,rGOT,_GLOBAL_OFFSET_TABLE_-got_label@l
  mtlr rTMP
-#else
-/* Load __cache_line_size from static. This value was set from the
-   aux vector during startup.  */
- lis rCLS,__cache_line_size@ha
-/* If the remaining length is less the 32 bytes then don't bother getting
-   the cache line size.  */
- beq L(medium)
- lwz rCLS,__cache_line_size@l(rCLS)
 #endif
+/* Load rtld_global_ro._dl_cache_line_size.  */
+ __GLRO(rCLS, rGOT, _dl_cache_line_size,
+       RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)
 
 /* If the cache line size was not set then goto to L(nondcbz), which is
    safe for any cache line size.  */
diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
index c21ea87f5a..4605276ebf 100644
--- a/sysdeps/powerpc/powerpc32/sysdep.h
+++ b/sysdeps/powerpc/powerpc32/sysdep.h
@@ -157,4 +157,30 @@ GOT_LABEL: ;      \
 /* Label in text section.  */
 #define C_TEXT(name) name
 
+/* Read the value of member from rtld_global_ro.  */
+#ifdef PIC
+# ifdef SHARED
+#  if IS_IN (rtld)
+/* Inside ld.so we use the local alias to avoid runtime GOT
+   relocations.  */
+#   define __GLRO(rOUT, rGOT, member, offset) \
+ lwz     rOUT,_rtld_local_ro@got(rGOT); \
+ lwz     rOUT,offset(rOUT)
+#  else
+#   define __GLRO(rOUT, rGOT, member, offset) \
+ lwz     rOUT,_rtld_global_ro@got(rGOT); \
+ lwz     rOUT,offset(rOUT)
+#  endif
+# else
+#  define __GLRO(rOUT, rGOT, member, offset) \
+ lwz     rOUT,member@got(rGOT); \
+ lwz     rOUT,0(rOUT)
+# endif
+#else
+/* Position-dependent code does not require access to the GOT.  */
+# define __GLRO(rOUT, rGOT, member, offset) \
+ lis     rOUT,(member+LOWORD)@ha \
+ lwz     rOUT,(member+LOWORD)@l(rOUT)
+#endif /* PIC */
+
 #endif /* __ASSEMBLER__ */
diff --git a/sysdeps/powerpc/powerpc64/a2/memcpy.S b/sysdeps/powerpc/powerpc64/a2/memcpy.S
index 6d5c5afddb..e515255126 100644
--- a/sysdeps/powerpc/powerpc64/a2/memcpy.S
+++ b/sysdeps/powerpc/powerpc64/a2/memcpy.S
@@ -18,6 +18,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
+#include <rtld-global-offsets.h>
 
 #ifndef MEMCPY
 # define MEMCPY memcpy
@@ -27,8 +28,19 @@
 #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
 
  .section        ".toc","aw"
-.LC0:
- .tc __cache_line_size[TC],__cache_line_size
+.LC__dl_cache_line_size:
+#ifdef SHARED
+# if IS_IN (rtld)
+ /* Inside ld.so we use the local alias to avoid runtime GOT
+   relocations.  */
+ .tc _rtld_local_ro[TC],_rtld_local_ro
+# else
+ .tc _rtld_global_ro[TC],_rtld_global_ro
+# endif
+#else
+ .tc _dl_cache_line_size[TC],_dl_cache_line_size
+#endif
+
  .section        ".text"
  .align 2
 
@@ -55,7 +67,8 @@ ENTRY (MEMCPY, 5)
  */
 
  neg     r8,r3           /* LS 4 bits = # bytes to 8-byte dest bdry  */
- ld      r9,.LC0@toc(r2) /* Get cache line size (part 1) */
+ /* Get cache line size (part 1) */
+ ld      r9,.LC__dl_cache_line_size@toc(r2)
  clrldi  r8,r8,64-4      /* align to 16byte boundary  */
  sub     r7,r4,r3        /* compute offset to src from dest */
  lwz     r9,0(r9)        /* Get cache line size (part 2) */
@@ -121,7 +134,7 @@ L(dst_aligned):
  cmpdi cr0,r9,0 /* Cache line size set? */
  bne+ cr0,L(cachelineset)
 
-/* __cache_line_size not set: generic byte copy without much optimization */
+/* Cache line size not set: generic byte copy without much optimization */
  clrldi. r0,r5,63 /* If length is odd copy one byte */
  beq L(cachelinenotset_align)
  lbz r7,0(r4) /* Read one byte from source */
diff --git a/sysdeps/powerpc/powerpc64/memset.S b/sysdeps/powerpc/powerpc64/memset.S
index 5d96696e87..d49ef102d4 100644
--- a/sysdeps/powerpc/powerpc64/memset.S
+++ b/sysdeps/powerpc/powerpc64/memset.S
@@ -17,10 +17,22 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
+#include <rtld-global-offsets.h>
 
  .section ".toc","aw"
-.LC0:
- .tc __cache_line_size[TC],__cache_line_size
+.LC__dl_cache_line_size:
+#ifdef SHARED
+# if IS_IN (rtld)
+ /* Inside ld.so we use the local alias to avoid runtime GOT
+   relocations.  */
+ .tc _rtld_local_ro[TC],_rtld_local_ro
+# else
+ .tc _rtld_global_ro[TC],_rtld_global_ro
+# endif
+#else
+ .tc _dl_cache_line_size[TC],_dl_cache_line_size
+#endif
+
  .section ".text"
  .align 2
 
@@ -146,8 +158,14 @@ L(zloopstart):
 /* If the remaining length is less the 32 bytes, don't bother getting
  the cache line size.  */
  beq L(medium)
- ld rCLS,.LC0@toc(r2)
+ ld rCLS,.LC__dl_cache_line_size@toc(r2)
+# ifdef SHARED
+ /* Load _rtld_global_ro._dl_cache_line_size.  */
+ lwz rCLS,RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET(rCLS)
+# else
+ /* Load _dl_cache_line_size.  */
  lwz rCLS,0(rCLS)
+# endif
 /* If the cache line size was not set just goto to L(nondcbz) which is
  safe for any cache line size.  */
  cmpldi cr1,rCLS,0
diff --git a/sysdeps/powerpc/rtld-global-offsets.sym b/sysdeps/powerpc/rtld-global-offsets.sym
index f5ea5a1466..6b348fd522 100644
--- a/sysdeps/powerpc/rtld-global-offsets.sym
+++ b/sysdeps/powerpc/rtld-global-offsets.sym
@@ -6,3 +6,4 @@
 
 RTLD_GLOBAL_RO_DL_HWCAP_OFFSET rtld_global_ro_offsetof (_dl_hwcap)
 RTLD_GLOBAL_RO_DL_HWCAP2_OFFSET rtld_global_ro_offsetof (_dl_hwcap2)
+RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET rtld_global_ro_offsetof (_dl_cache_line_size)
diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
new file mode 100644
index 0000000000..44bea09974
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
@@ -0,0 +1,24 @@
+/* Auxiliary vector processing.  Linux/PPC version.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+/* Scan the Aux Vector for the "Data Cache Block Size" entry and assign it
+   to dl_cache_line_size.  */
+#define DL_PLATFORM_AUXV      \
+      case AT_DCACHEBSIZE:      \
+ GLRO(dl_cache_line_size) = av->a_un.a_val;      \
+ break;
diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-support.c b/sysdeps/unix/sysv/linux/powerpc/dl-support.c
new file mode 100644
index 0000000000..2a792d7092
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/dl-support.c
@@ -0,0 +1,23 @@
+/* Support for dynamic linking code in static libc.  Linux/PPC version.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include "dl-auxv.h"
+#include <ldsodefs.h>
+int GLRO(dl_cache_line_size);
+
+#include <elf/dl-support.c>
diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
index fa19cc66c0..6d51d6061e 100644
--- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
+++ b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
@@ -18,16 +18,6 @@
 
 #include <config.h>
 #include <ldsodefs.h>
-
-int __cache_line_size attribute_hidden;
-
-/* Scan the Aux Vector for the "Data Cache Block Size" entry.  If found
-   verify that the static extern __cache_line_size is defined by checking
-   for not NULL.  If it is defined then assign the cache block size
-   value to __cache_line_size.  */
-#define DL_PLATFORM_AUXV      \
-      case AT_DCACHEBSIZE:      \
- __cache_line_size = av->a_un.a_val;      \
- break;
+#include <dl-auxv.h>
 
 #include <sysdeps/unix/sysv/linux/dl-sysdep.c>
diff --git a/sysdeps/unix/sysv/linux/powerpc/libc-start.c b/sysdeps/unix/sysv/linux/powerpc/libc-start.c
index a659a9144f..3a5eb0e952 100644
--- a/sysdeps/unix/sysv/linux/powerpc/libc-start.c
+++ b/sysdeps/unix/sysv/linux/powerpc/libc-start.c
@@ -24,7 +24,6 @@
 #include <hwcapinfo.h>
 #endif
 
-int __cache_line_size attribute_hidden;
 /* The main work is done in the generic function.  */
 #define LIBC_START_MAIN generic_start_main
 #define LIBC_START_DISABLE_INLINE
@@ -71,15 +70,12 @@ __libc_start_main (int argc, char **argv,
       rtld_fini = NULL;
     }
 
-  /* Initialize the __cache_line_size variable from the aux vector.  For the
-     static case, we also need _dl_hwcap, _dl_hwcap2 and _dl_platform, so we
-     can call __tcb_parse_hwcap_and_convert_at_platform ().  */
   for (ElfW (auxv_t) * av = auxvec; av->a_type != AT_NULL; ++av)
     switch (av->a_type)
       {
-      case AT_DCACHEBSIZE:
- __cache_line_size = av->a_un.a_val;
- break;
+      /* For the static case, we also need _dl_hwcap, _dl_hwcap2 and
+         _dl_platform, so we can call
+         __tcb_parse_hwcap_and_convert_at_platform ().  */
 #ifndef SHARED
       case AT_HWCAP:
  _dl_hwcap = (unsigned long int) av->a_un.a_val;
--
2.24.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Adhemerval Zanella-2


On 23/12/2019 15:45, Tulio Magno Quites Machado Filho wrote:

> GCC 10.0 enabled -fno-common by default and this started to point that
> __cache_line_size had been implemented in 2 different places: loader and
> libc.
>
> In order to avoid this duplication, the libc variable has been removed
> and the loader variable is moved to rtld_global_ro.
>
> File sysdeps/unix/sysv/linux/powerpc/dl-auxv.h has been added in order
> to reuse code for both static and dynamic linking scenarios.
>
> Signed-off-by: Tulio Magno Quites Machado Filho <[hidden email]>

We do not use signed-off.  

Although not strickly necessary since the memset.c implementation already
check for non set _dl_cache_line_size, you may also add the cache-line
initialization for the static dlopen on dl-static.c (as powerpc
already does for dl_pagesize).

I assume you have also checked on powerpc-linux-gnu and powerpc64-linux-gnu,
correct?

> ---
>  sysdeps/powerpc/dl-procinfo.c                | 17 ++++++++++++
>  sysdeps/powerpc/powerpc32/a2/memcpy.S        | 23 ++++++++--------
>  sysdeps/powerpc/powerpc32/dl-machine.c       | 11 ++------
>  sysdeps/powerpc/powerpc32/memset.S           | 29 +++++++++-----------
>  sysdeps/powerpc/powerpc32/sysdep.h           | 26 ++++++++++++++++++
>  sysdeps/powerpc/powerpc64/a2/memcpy.S        | 21 +++++++++++---
>  sysdeps/powerpc/powerpc64/memset.S           | 24 ++++++++++++++--
>  sysdeps/powerpc/rtld-global-offsets.sym      |  1 +
>  sysdeps/unix/sysv/linux/powerpc/dl-auxv.h    | 24 ++++++++++++++++
>  sysdeps/unix/sysv/linux/powerpc/dl-support.c | 23 ++++++++++++++++
>  sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c  | 12 +-------
>  sysdeps/unix/sysv/linux/powerpc/libc-start.c | 10 ++-----
>  12 files changed, 160 insertions(+), 61 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
>  create mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-support.c
>
> diff --git a/sysdeps/powerpc/dl-procinfo.c b/sysdeps/powerpc/dl-procinfo.c
> index 20706a648a..94b33d0c16 100644
> --- a/sysdeps/powerpc/dl-procinfo.c
> +++ b/sysdeps/powerpc/dl-procinfo.c
> @@ -89,5 +89,22 @@ PROCINFO_CLASS const char _dl_powerpc_cap_flags[64][15]
>  ,
>  #endif
>  
> +#if !IS_IN (ldconfig)
> +# if !defined PROCINFO_DECL && defined SHARED
> +     ._dl_cache_line_size
> +# else
> +PROCINFO_CLASS int _dl_cache_line_size
> +# endif
> +# ifndef PROCINFO_DECL
> +     = 0
> +# endif
> +# if !defined SHARED || defined PROCINFO_DECL
> +;
> +# else
> +,
> +# endif
> +#endif
> +
> +
>  #undef PROCINFO_DECL
>  #undef PROCINFO_CLASS

Ok.

> diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S
> index 9bc91a8df1..ecfea5c832 100644
> --- a/sysdeps/powerpc/powerpc32/a2/memcpy.S
> +++ b/sysdeps/powerpc/powerpc32/a2/memcpy.S
> @@ -18,6 +18,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <sysdep.h>
> +#include <rtld-global-offsets.h>
>  
>  #define PREFETCH_AHEAD 4        /* no cache lines SRC prefetching ahead  */
>  #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
> @@ -106,25 +107,23 @@ EALIGN (memcpy, 5, 0)
>  L(dst_aligned):
>  
>  
> -#ifdef SHARED
> +#ifdef PIC
>   mflr    r0
> -/* Establishes GOT addressability so we can load __cache_line_size
> -   from static. This value was set from the aux vector during startup.  */
> +/* Establishes GOT addressability so we can load the cache line size
> +   from rtld_global_ro. This value was set from the aux vector during
> +   startup.  */

My understanding is code guidelines states two spaces after the end of a
sentence in comments.

>   SETUP_GOT_ACCESS(r9,got_label)
> - addis   r9,r9,__cache_line_size-got_label@ha
> - lwz     r9,__cache_line_size-got_label@l(r9)
> - mtlr    r0
> -#else
> -/* Load __cache_line_size from static. This value was set from the
> -   aux vector during startup.  */
> - lis     r9,__cache_line_size@ha
> - lwz     r9,__cache_line_size@l(r9)
> + addis r9,r9,_GLOBAL_OFFSET_TABLE_-got_label@ha
> + addi r9,r9,_GLOBAL_OFFSET_TABLE_-got_label@l
> + mtlr r0
>  #endif
> + __GLRO(r9, r9, _dl_cache_line_size,
> +       RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)
>  
>   cmplwi  cr5, r9, 0
>   bne+    cr5,L(cachelineset)
>  
> -/* __cache_line_size not set: generic byte copy without much optimization */
> +/* Cache line size not set: generic byte copy without much optimization */
>   andi. r0,r5,1 /* If length is odd copy one byte.  */
>   beq L(cachelinenotset_align)
>   lbz r7,0(r4) /* Read one byte from source.  */

Ok.

> diff --git a/sysdeps/powerpc/powerpc32/dl-machine.c b/sysdeps/powerpc/powerpc32/dl-machine.c
> index a90cbc1ae3..740ff8f2ec 100644
> --- a/sysdeps/powerpc/powerpc32/dl-machine.c
> +++ b/sysdeps/powerpc/powerpc32/dl-machine.c
> @@ -25,11 +25,6 @@
>  #include <dl-machine.h>
>  #include <_itoa.h>
>  
> -/* The value __cache_line_size is defined in dl-sysdep.c and is initialised
> -   by _dl_sysdep_start via DL_PLATFORM_INIT.  */
> -extern int __cache_line_size attribute_hidden;
> -
> -
>  /* Stuff for the PLT.  */
>  #define PLT_INITIAL_ENTRY_WORDS 18
>  #define PLT_LONGBRANCH_ENTRY_WORDS 0
> @@ -309,14 +304,14 @@ __elf_machine_runtime_setup (struct link_map *map, int lazy, int profile)
>  
>   Assumes that dcbst and icbi apply to lines of 16 bytes or
>   more.  Current known line sizes are 16, 32, and 128 bytes.
> - The following gets the __cache_line_size, when available.  */
> + The following gets the cache line size, when available.  */
>  
>        /* Default minimum 4 words per cache line.  */
>        int line_size_words = 4;
>  
> -      if (lazy && __cache_line_size != 0)
> +      if (lazy && GLRO(dl_cache_line_size) != 0)
>   /* Convert bytes to words.  */
> - line_size_words = __cache_line_size / 4;
> + line_size_words = GLRO(dl_cache_line_size) / 4;
>  
>        size_modified = lazy ? rel_offset_words : 6;
>        for (i = 0; i < size_modified; i += line_size_words)

Ok.

> diff --git a/sysdeps/powerpc/powerpc32/memset.S b/sysdeps/powerpc/powerpc32/memset.S
> index fc8b1a2547..ea96913e4e 100644
> --- a/sysdeps/powerpc/powerpc32/memset.S
> +++ b/sysdeps/powerpc/powerpc32/memset.S
> @@ -17,12 +17,13 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <sysdep.h>
> +#include <rtld-global-offsets.h>
>  
>  /* void * [r3] memset (void *s [r3], int c [r4], size_t n [r5]));
>     Returns 's'.
>  
>     The memset is done in four sizes: byte (8 bits), word (32 bits),
> -   32-byte blocks (256 bits) and __cache_line_size (128, 256, 1024 bits).
> +   32-byte blocks (256 bits) and cache line size (128, 256, 1024 bits).
>     There is a special case for setting whole cache lines to 0, which
>     takes advantage of the dcbz instruction.  */
>  
> @@ -95,7 +96,7 @@ L(caligned):
>  
>  /* Check if we can use the special case for clearing memory using dcbz.
>     This requires that we know the correct cache line size for this
> -   processor.  Getting the __cache_line_size may require establishing GOT
> +   processor.  Getting the cache line size may require establishing GOT
>     addressability, so branch out of line to set this up.  */
>   beq cr1, L(checklinesize)
>  

Ok.

> @@ -230,26 +231,22 @@ L(medium_28t):
>   blr
>  
>  L(checklinesize):
> -#ifdef SHARED
> - mflr rTMP
>  /* If the remaining length is less the 32 bytes then don't bother getting
>     the cache line size.  */
>   beq L(medium)
> -/* Establishes GOT addressability so we can load __cache_line_size
> -   from static. This value was set from the aux vector during startup.  */
> +#ifdef PIC
> + mflr rTMP
> +/* Establishes GOT addressability so we can load the cache line size
> +   from rtld_global_ro. This value was set from the aux vector during
> +   startup.  */
>   SETUP_GOT_ACCESS(rGOT,got_label)
> - addis rGOT,rGOT,__cache_line_size-got_label@ha
> - lwz rCLS,__cache_line_size-got_label@l(rGOT)
> + addis rGOT,rGOT,_GLOBAL_OFFSET_TABLE_-got_label@ha
> + addi rGOT,rGOT,_GLOBAL_OFFSET_TABLE_-got_label@l
>   mtlr rTMP
> -#else
> -/* Load __cache_line_size from static. This value was set from the
> -   aux vector during startup.  */
> - lis rCLS,__cache_line_size@ha
> -/* If the remaining length is less the 32 bytes then don't bother getting
> -   the cache line size.  */
> - beq L(medium)
> - lwz rCLS,__cache_line_size@l(rCLS)
>  #endif
> +/* Load rtld_global_ro._dl_cache_line_size.  */
> + __GLRO(rCLS, rGOT, _dl_cache_line_size,
> +       RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)
>  
>  /* If the cache line size was not set then goto to L(nondcbz), which is
>     safe for any cache line size.  */

Ok.

> diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
> index c21ea87f5a..4605276ebf 100644
> --- a/sysdeps/powerpc/powerpc32/sysdep.h
> +++ b/sysdeps/powerpc/powerpc32/sysdep.h
> @@ -157,4 +157,30 @@ GOT_LABEL: ;      \
>  /* Label in text section.  */
>  #define C_TEXT(name) name
>  
> +/* Read the value of member from rtld_global_ro.  */
> +#ifdef PIC
> +# ifdef SHARED
> +#  if IS_IN (rtld)
> +/* Inside ld.so we use the local alias to avoid runtime GOT
> +   relocations.  */
> +#   define __GLRO(rOUT, rGOT, member, offset) \
> + lwz     rOUT,_rtld_local_ro@got(rGOT); \
> + lwz     rOUT,offset(rOUT)
> +#  else
> +#   define __GLRO(rOUT, rGOT, member, offset) \
> + lwz     rOUT,_rtld_global_ro@got(rGOT); \
> + lwz     rOUT,offset(rOUT)
> +#  endif
> +# else
> +#  define __GLRO(rOUT, rGOT, member, offset) \
> + lwz     rOUT,member@got(rGOT); \
> + lwz     rOUT,0(rOUT)
> +# endif
> +#else
> +/* Position-dependent code does not require access to the GOT.  */
> +# define __GLRO(rOUT, rGOT, member, offset) \
> + lis     rOUT,(member+LOWORD)@ha \
> + lwz     rOUT,(member+LOWORD)@l(rOUT)
> +#endif /* PIC */
> +
>  #endif /* __ASSEMBLER__ */

Ok.

> diff --git a/sysdeps/powerpc/powerpc64/a2/memcpy.S b/sysdeps/powerpc/powerpc64/a2/memcpy.S
> index 6d5c5afddb..e515255126 100644
> --- a/sysdeps/powerpc/powerpc64/a2/memcpy.S
> +++ b/sysdeps/powerpc/powerpc64/a2/memcpy.S
> @@ -18,6 +18,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <sysdep.h>
> +#include <rtld-global-offsets.h>
>  
>  #ifndef MEMCPY
>  # define MEMCPY memcpy
> @@ -27,8 +28,19 @@
>  #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
>  
>   .section        ".toc","aw"
> -.LC0:
> - .tc __cache_line_size[TC],__cache_line_size
> +.LC__dl_cache_line_size:
> +#ifdef SHARED
> +# if IS_IN (rtld)
> + /* Inside ld.so we use the local alias to avoid runtime GOT
> +   relocations.  */
> + .tc _rtld_local_ro[TC],_rtld_local_ro
> +# else
> + .tc _rtld_global_ro[TC],_rtld_global_ro
> +# endif
> +#else
> + .tc _dl_cache_line_size[TC],_dl_cache_line_size
> +#endif
> +
>   .section        ".text"
>   .align 2
>  

Ok. Maybe add a similar macro for powerpc64 as you did for powerpc32 with
__GLRO ?

> @@ -55,7 +67,8 @@ ENTRY (MEMCPY, 5)
>   */
>  
>   neg     r8,r3           /* LS 4 bits = # bytes to 8-byte dest bdry  */
> - ld      r9,.LC0@toc(r2) /* Get cache line size (part 1) */
> + /* Get cache line size (part 1) */
> + ld      r9,.LC__dl_cache_line_size@toc(r2)
>   clrldi  r8,r8,64-4      /* align to 16byte boundary  */
>   sub     r7,r4,r3        /* compute offset to src from dest */
>   lwz     r9,0(r9)        /* Get cache line size (part 2) */

Ok.

> @@ -121,7 +134,7 @@ L(dst_aligned):
>   cmpdi cr0,r9,0 /* Cache line size set? */
>   bne+ cr0,L(cachelineset)
>  
> -/* __cache_line_size not set: generic byte copy without much optimization */
> +/* Cache line size not set: generic byte copy without much optimization */
>   clrldi. r0,r5,63 /* If length is odd copy one byte */
>   beq L(cachelinenotset_align)
>   lbz r7,0(r4) /* Read one byte from source */

Ok.

> diff --git a/sysdeps/powerpc/powerpc64/memset.S b/sysdeps/powerpc/powerpc64/memset.S
> index 5d96696e87..d49ef102d4 100644
> --- a/sysdeps/powerpc/powerpc64/memset.S
> +++ b/sysdeps/powerpc/powerpc64/memset.S
> @@ -17,10 +17,22 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <sysdep.h>
> +#include <rtld-global-offsets.h>
>  
>   .section ".toc","aw"
> -.LC0:
> - .tc __cache_line_size[TC],__cache_line_size
> +.LC__dl_cache_line_size:
> +#ifdef SHARED
> +# if IS_IN (rtld)
> + /* Inside ld.so we use the local alias to avoid runtime GOT
> +   relocations.  */
> + .tc _rtld_local_ro[TC],_rtld_local_ro
> +# else
> + .tc _rtld_global_ro[TC],_rtld_global_ro
> +# endif
> +#else
> + .tc _dl_cache_line_size[TC],_dl_cache_line_size
> +#endif
> +
>   .section ".text"
>   .align 2
>  

Ok.

> @@ -146,8 +158,14 @@ L(zloopstart):
>  /* If the remaining length is less the 32 bytes, don't bother getting
>   the cache line size.  */
>   beq L(medium)
> - ld rCLS,.LC0@toc(r2)
> + ld rCLS,.LC__dl_cache_line_size@toc(r2)
> +# ifdef SHARED
> + /* Load _rtld_global_ro._dl_cache_line_size.  */
> + lwz rCLS,RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET(rCLS)
> +# else
> + /* Load _dl_cache_line_size.  */
>   lwz rCLS,0(rCLS)
> +# endif
>  /* If the cache line size was not set just goto to L(nondcbz) which is
>   safe for any cache line size.  */
>   cmpldi cr1,rCLS,0

Ok.

> diff --git a/sysdeps/powerpc/rtld-global-offsets.sym b/sysdeps/powerpc/rtld-global-offsets.sym
> index f5ea5a1466..6b348fd522 100644
> --- a/sysdeps/powerpc/rtld-global-offsets.sym
> +++ b/sysdeps/powerpc/rtld-global-offsets.sym
> @@ -6,3 +6,4 @@
>  
>  RTLD_GLOBAL_RO_DL_HWCAP_OFFSET rtld_global_ro_offsetof (_dl_hwcap)
>  RTLD_GLOBAL_RO_DL_HWCAP2_OFFSET rtld_global_ro_offsetof (_dl_hwcap2)
> +RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET rtld_global_ro_offsetof (_dl_cache_line_size)

Ok.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
> new file mode 100644
> index 0000000000..44bea09974
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
> @@ -0,0 +1,24 @@
> +/* Auxiliary vector processing.  Linux/PPC version.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* Scan the Aux Vector for the "Data Cache Block Size" entry and assign it
> +   to dl_cache_line_size.  */
> +#define DL_PLATFORM_AUXV      \
> +      case AT_DCACHEBSIZE:      \
> + GLRO(dl_cache_line_size) = av->a_un.a_val;      \
> + break;

Ok.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-support.c b/sysdeps/unix/sysv/linux/powerpc/dl-support.c
> new file mode 100644
> index 0000000000..2a792d7092
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-support.c
> @@ -0,0 +1,23 @@
> +/* Support for dynamic linking code in static libc.  Linux/PPC version.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include "dl-auxv.h"
> +#include <ldsodefs.h>
> +int GLRO(dl_cache_line_size);
> +
> +#include <elf/dl-support.c>

Why is it required? It sould be already covered by inclusion of
sysdeps/powerpc/dl-procinfo.c and it does not required the dl-auxv.h
definitions here.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
> index fa19cc66c0..6d51d6061e 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
> @@ -18,16 +18,6 @@
>  
>  #include <config.h>
>  #include <ldsodefs.h>
> -
> -int __cache_line_size attribute_hidden;
> -
> -/* Scan the Aux Vector for the "Data Cache Block Size" entry.  If found
> -   verify that the static extern __cache_line_size is defined by checking
> -   for not NULL.  If it is defined then assign the cache block size
> -   value to __cache_line_size.  */
> -#define DL_PLATFORM_AUXV      \
> -      case AT_DCACHEBSIZE:      \
> - __cache_line_size = av->a_un.a_val;      \
> - break;
> +#include <dl-auxv.h>
>  
>  #include <sysdeps/unix/sysv/linux/dl-sysdep.c>

This is essentially an empty file that just include dl-auxv.h. I think
it would be better to just create a generic empty dl-auxv.h, include it
on default dl-sysdep.c, and remove this file.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/libc-start.c b/sysdeps/unix/sysv/linux/powerpc/libc-start.c
> index a659a9144f..3a5eb0e952 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/libc-start.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/libc-start.c
> @@ -24,7 +24,6 @@
>  #include <hwcapinfo.h>
>  #endif
>  
> -int __cache_line_size attribute_hidden;
>  /* The main work is done in the generic function.  */
>  #define LIBC_START_MAIN generic_start_main
>  #define LIBC_START_DISABLE_INLINE
> @@ -71,15 +70,12 @@ __libc_start_main (int argc, char **argv,
>        rtld_fini = NULL;
>      }
>  
> -  /* Initialize the __cache_line_size variable from the aux vector.  For the
> -     static case, we also need _dl_hwcap, _dl_hwcap2 and _dl_platform, so we
> -     can call __tcb_parse_hwcap_and_convert_at_platform ().  */
>    for (ElfW (auxv_t) * av = auxvec; av->a_type != AT_NULL; ++av)
>      switch (av->a_type)
>        {
> -      case AT_DCACHEBSIZE:
> - __cache_line_size = av->a_un.a_val;
> - break;
> +      /* For the static case, we also need _dl_hwcap, _dl_hwcap2 and
> +         _dl_platform, so we can call
> +         __tcb_parse_hwcap_and_convert_at_platform ().  */
>  #ifndef SHARED
>        case AT_HWCAP:
>   _dl_hwcap = (unsigned long int) av->a_un.a_val;
>

Ok.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Tulio Magno Quites Machado Filho-3
Adhemerval Zanella <[hidden email]> writes:

> On 23/12/2019 15:45, Tulio Magno Quites Machado Filho wrote:
>> GCC 10.0 enabled -fno-common by default and this started to point that
>> __cache_line_size had been implemented in 2 different places: loader and
>> libc.
>>
>> In order to avoid this duplication, the libc variable has been removed
>> and the loader variable is moved to rtld_global_ro.
>>
>> File sysdeps/unix/sysv/linux/powerpc/dl-auxv.h has been added in order
>> to reuse code for both static and dynamic linking scenarios.
>>
>> Signed-off-by: Tulio Magno Quites Machado Filho <[hidden email]>
>
> We do not use signed-off.  

Ack.

> Although not strickly necessary since the memset.c implementation already
> check for non set _dl_cache_line_size, you may also add the cache-line
> initialization for the static dlopen on dl-static.c (as powerpc
> already does for dl_pagesize).

Good point.  That would allow a dlopened DSO from a statically linked
executable access dl_cache_line_size too.

> I assume you have also checked on powerpc-linux-gnu and powerpc64-linux-gnu,
> correct?

Correct.

>> diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S
>> index 9bc91a8df1..ecfea5c832 100644
>> --- a/sysdeps/powerpc/powerpc32/a2/memcpy.S
>> +++ b/sysdeps/powerpc/powerpc32/a2/memcpy.S
>> @@ -18,6 +18,7 @@
>>     <https://www.gnu.org/licenses/>.  */
>>  
>>  #include <sysdep.h>
>> +#include <rtld-global-offsets.h>
>>  
>>  #define PREFETCH_AHEAD 4        /* no cache lines SRC prefetching ahead  */
>>  #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
>> @@ -106,25 +107,23 @@ EALIGN (memcpy, 5, 0)
>>  L(dst_aligned):
>>  
>>  
>> -#ifdef SHARED
>> +#ifdef PIC
>>   mflr    r0
>> -/* Establishes GOT addressability so we can load __cache_line_size
>> -   from static. This value was set from the aux vector during startup.  */
>> +/* Establishes GOT addressability so we can load the cache line size
>> +   from rtld_global_ro. This value was set from the aux vector during
>> +   startup.  */
>
> My understanding is code guidelines states two spaces after the end of a
> sentence in comments.

Indeed.

>> diff --git a/sysdeps/powerpc/powerpc64/a2/memcpy.S b/sysdeps/powerpc/powerpc64/a2/memcpy.S
>> index 6d5c5afddb..e515255126 100644
>> --- a/sysdeps/powerpc/powerpc64/a2/memcpy.S
>> +++ b/sysdeps/powerpc/powerpc64/a2/memcpy.S
>> @@ -18,6 +18,7 @@
>>     <https://www.gnu.org/licenses/>.  */
>>  
>>  #include <sysdep.h>
>> +#include <rtld-global-offsets.h>
>>  
>>  #ifndef MEMCPY
>>  # define MEMCPY memcpy
>> @@ -27,8 +28,19 @@
>>  #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
>>  
>>   .section        ".toc","aw"
>> -.LC0:
>> - .tc __cache_line_size[TC],__cache_line_size
>> +.LC__dl_cache_line_size:
>> +#ifdef SHARED
>> +# if IS_IN (rtld)
>> + /* Inside ld.so we use the local alias to avoid runtime GOT
>> +   relocations.  */
>> + .tc _rtld_local_ro[TC],_rtld_local_ro
>> +# else
>> + .tc _rtld_global_ro[TC],_rtld_global_ro
>> +# endif
>> +#else
>> + .tc _dl_cache_line_size[TC],_dl_cache_line_size
>> +#endif
>> +
>>   .section        ".text"
>>   .align 2
>>  
>
> Ok. Maybe add a similar macro for powerpc64 as you did for powerpc32 with
> __GLRO ?

Are you suggesting to create a __GLRO macro that would define this label?

>> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-support.c b/sysdeps/unix/sysv/linux/powerpc/dl-support.c
>> new file mode 100644
>> index 0000000000..2a792d7092
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-support.c
>> @@ -0,0 +1,23 @@
>> +/* Support for dynamic linking code in static libc.  Linux/PPC version.
>> +   Copyright (C) 2019 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <https://www.gnu.org/licenses/>.  */
>> +
>> +#include "dl-auxv.h"
>> +#include <ldsodefs.h>
>> +int GLRO(dl_cache_line_size);
>> +
>> +#include <elf/dl-support.c>
>
> Why is it required? It sould be already covered by inclusion of
> sysdeps/powerpc/dl-procinfo.c and it does not required the dl-auxv.h
> definitions here.

This is the code that initializes it statically.  It avoids duplicating code.
dl-procinfo.c does not copy AT_DCACHEBSIZE to GLRO(dl_cache_line_size).

>> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
>> index fa19cc66c0..6d51d6061e 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
>> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
>> @@ -18,16 +18,6 @@
>>  
>>  #include <config.h>
>>  #include <ldsodefs.h>
>> -
>> -int __cache_line_size attribute_hidden;
>> -
>> -/* Scan the Aux Vector for the "Data Cache Block Size" entry.  If found
>> -   verify that the static extern __cache_line_size is defined by checking
>> -   for not NULL.  If it is defined then assign the cache block size
>> -   value to __cache_line_size.  */
>> -#define DL_PLATFORM_AUXV      \
>> -      case AT_DCACHEBSIZE:      \
>> - __cache_line_size = av->a_un.a_val;      \
>> - break;
>> +#include <dl-auxv.h>
>>  
>>  #include <sysdeps/unix/sysv/linux/dl-sysdep.c>
>
> This is essentially an empty file that just include dl-auxv.h. I think
> it would be better to just create a generic empty dl-auxv.h, include it
> on default dl-sysdep.c, and remove this file.

I'm fine with this proposal, but I don't think it's going to help neither alpha
or x86 which have slightly different implementations.

--
Tulio Magno
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Adhemerval Zanella-2


On 27/12/2019 12:42, Tulio Magno Quites Machado Filho wrote:

> Adhemerval Zanella <[hidden email]> writes:
>
>> On 23/12/2019 15:45, Tulio Magno Quites Machado Filho wrote:
>>> GCC 10.0 enabled -fno-common by default and this started to point that
>>> __cache_line_size had been implemented in 2 different places: loader and
>>> libc.
>>>
>>> In order to avoid this duplication, the libc variable has been removed
>>> and the loader variable is moved to rtld_global_ro.
>>>
>>> File sysdeps/unix/sysv/linux/powerpc/dl-auxv.h has been added in order
>>> to reuse code for both static and dynamic linking scenarios.
>>>
>>> Signed-off-by: Tulio Magno Quites Machado Filho <[hidden email]>
>>
>> We do not use signed-off.  
>
> Ack.
>
>> Although not strickly necessary since the memset.c implementation already
>> check for non set _dl_cache_line_size, you may also add the cache-line
>> initialization for the static dlopen on dl-static.c (as powerpc
>> already does for dl_pagesize).
>
> Good point.  That would allow a dlopened DSO from a statically linked
> executable access dl_cache_line_size too.
>
>> I assume you have also checked on powerpc-linux-gnu and powerpc64-linux-gnu,
>> correct?
>
> Correct.
>
>>> diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S
>>> index 9bc91a8df1..ecfea5c832 100644
>>> --- a/sysdeps/powerpc/powerpc32/a2/memcpy.S
>>> +++ b/sysdeps/powerpc/powerpc32/a2/memcpy.S
>>> @@ -18,6 +18,7 @@
>>>     <https://www.gnu.org/licenses/>.  */
>>>  
>>>  #include <sysdep.h>
>>> +#include <rtld-global-offsets.h>
>>>  
>>>  #define PREFETCH_AHEAD 4        /* no cache lines SRC prefetching ahead  */
>>>  #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
>>> @@ -106,25 +107,23 @@ EALIGN (memcpy, 5, 0)
>>>  L(dst_aligned):
>>>  
>>>  
>>> -#ifdef SHARED
>>> +#ifdef PIC
>>>   mflr    r0
>>> -/* Establishes GOT addressability so we can load __cache_line_size
>>> -   from static. This value was set from the aux vector during startup.  */
>>> +/* Establishes GOT addressability so we can load the cache line size
>>> +   from rtld_global_ro. This value was set from the aux vector during
>>> +   startup.  */
>>
>> My understanding is code guidelines states two spaces after the end of a
>> sentence in comments.
>
> Indeed.
>
>>> diff --git a/sysdeps/powerpc/powerpc64/a2/memcpy.S b/sysdeps/powerpc/powerpc64/a2/memcpy.S
>>> index 6d5c5afddb..e515255126 100644
>>> --- a/sysdeps/powerpc/powerpc64/a2/memcpy.S
>>> +++ b/sysdeps/powerpc/powerpc64/a2/memcpy.S
>>> @@ -18,6 +18,7 @@
>>>     <https://www.gnu.org/licenses/>.  */
>>>  
>>>  #include <sysdep.h>
>>> +#include <rtld-global-offsets.h>
>>>  
>>>  #ifndef MEMCPY
>>>  # define MEMCPY memcpy
>>> @@ -27,8 +28,19 @@
>>>  #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
>>>  
>>>   .section        ".toc","aw"
>>> -.LC0:
>>> - .tc __cache_line_size[TC],__cache_line_size
>>> +.LC__dl_cache_line_size:
>>> +#ifdef SHARED
>>> +# if IS_IN (rtld)
>>> + /* Inside ld.so we use the local alias to avoid runtime GOT
>>> +   relocations.  */
>>> + .tc _rtld_local_ro[TC],_rtld_local_ro
>>> +# else
>>> + .tc _rtld_global_ro[TC],_rtld_global_ro
>>> +# endif
>>> +#else
>>> + .tc _dl_cache_line_size[TC],_dl_cache_line_size
>>> +#endif
>>> +
>>>   .section        ".text"
>>>   .align 2
>>>  
>>
>> Ok. Maybe add a similar macro for powerpc64 as you did for powerpc32 with
>> __GLRO ?
>
> Are you suggesting to create a __GLRO macro that would define this label?

Or something similar to avoid the duplicated defition here and at
sysdeps/powerpc/powerpc64/memset.S.

>
>>> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-support.c b/sysdeps/unix/sysv/linux/powerpc/dl-support.c
>>> new file mode 100644
>>> index 0000000000..2a792d7092
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-support.c
>>> @@ -0,0 +1,23 @@
>>> +/* Support for dynamic linking code in static libc.  Linux/PPC version.
>>> +   Copyright (C) 2019 Free Software Foundation, Inc.
>>> +   This file is part of the GNU C Library.
>>> +
>>> +   The GNU C Library is free software; you can redistribute it and/or
>>> +   modify it under the terms of the GNU Lesser General Public
>>> +   License as published by the Free Software Foundation; either
>>> +   version 2.1 of the License, or (at your option) any later version.
>>> +
>>> +   The GNU C Library is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> +   Lesser General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU Lesser General Public
>>> +   License along with the GNU C Library; if not, see
>>> +   <https://www.gnu.org/licenses/>.  */
>>> +
>>> +#include "dl-auxv.h"
>>> +#include <ldsodefs.h>
>>> +int GLRO(dl_cache_line_size);
>>> +
>>> +#include <elf/dl-support.c>
>>
>> Why is it required? It sould be already covered by inclusion of
>> sysdeps/powerpc/dl-procinfo.c and it does not required the dl-auxv.h
>> definitions here.
>
> This is the code that initializes it statically.  It avoids duplicating code.
> dl-procinfo.c does not copy AT_DCACHEBSIZE to GLRO(dl_cache_line_size).

But with a generic dl-auxv.h that defines DL_PLATFORM_AUXV, this
files is not really required.

>
>>> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
>>> index fa19cc66c0..6d51d6061e 100644
>>> --- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
>>> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
>>> @@ -18,16 +18,6 @@
>>>  
>>>  #include <config.h>
>>>  #include <ldsodefs.h>
>>> -
>>> -int __cache_line_size attribute_hidden;
>>> -
>>> -/* Scan the Aux Vector for the "Data Cache Block Size" entry.  If found
>>> -   verify that the static extern __cache_line_size is defined by checking
>>> -   for not NULL.  If it is defined then assign the cache block size
>>> -   value to __cache_line_size.  */
>>> -#define DL_PLATFORM_AUXV      \
>>> -      case AT_DCACHEBSIZE:      \
>>> - __cache_line_size = av->a_un.a_val;      \
>>> - break;
>>> +#include <dl-auxv.h>
>>>  
>>>  #include <sysdeps/unix/sysv/linux/dl-sysdep.c>
>>
>> This is essentially an empty file that just include dl-auxv.h. I think
>> it would be better to just create a generic empty dl-auxv.h, include it
>> on default dl-sysdep.c, and remove this file.
>
> I'm fine with this proposal, but I don't think it's going to help neither alpha
> or x86 which have slightly different implementations.

Indeed, but I think we can refactor alpha or x86 later.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Florian Weimer-5
What's the status here?

I think it's desirable to fix this before the release.

Thanks,
Florian

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Jeff Law
On Thu, 2020-01-09 at 11:29 +0100, Florian Weimer wrote:
> What's the status here?
>
> I think it's desirable to fix this before the release.
Presumably this fixes the problem with it being used as a common symbol
and thus not working with gcc-10 on ppc64?  If so, yea, seems like it'd
be desirable before the release.

jeff

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Tulio Magno Quites Machado Filho-3
Jeff Law <[hidden email]> writes:

> On Thu, 2020-01-09 at 11:29 +0100, Florian Weimer wrote:
>> What's the status here?
>>
>> I think it's desirable to fix this before the release.

While working on the changes requested by Adhemerval, I hit 2 other bugs:

1. getauxval() does not work well from a DSO dlopen'ed by a statically linked
   executable.
2. errno from a DSO is lost in a statically linked executable (BZ #25335).

I've fixed the first issue, but I'm stuck with the second one.

> Presumably this fixes the problem with it being used as a common symbol
> and thus not working with gcc-10 on ppc64?  If so, yea, seems like it'd
> be desirable before the release.

The tests I implemented are failing because of these issues.

Would it be fine if I disabled the failing tests while we do not have a fix
for BZ #25335?

--
Tulio Magno
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Adhemerval Zanella-2


On 09/01/2020 14:20, Tulio Magno Quites Machado Filho wrote:

> Jeff Law <[hidden email]> writes:
>
>> On Thu, 2020-01-09 at 11:29 +0100, Florian Weimer wrote:
>>> What's the status here?
>>>
>>> I think it's desirable to fix this before the release.
>
> While working on the changes requested by Adhemerval, I hit 2 other bugs:
>
> 1. getauxval() does not work well from a DSO dlopen'ed by a statically linked
>    executable.

This is BZ#20802 and I although we might fix it by adding the initialization
on dl-static.c, I think Florian suggestion [1] to proper initialize shared ld.so
variables is a better overall solution. However it is also a much more extensive
change and not feasible on current release phase.

> 2. errno from a DSO is lost in a statically linked executable (BZ #25335).
>
> I've fixed the first issue, but I'm stuck with the second one.

I don't see how easily we would fix it, it probability require to change
how errno is definde in static manner and most likely more hacks to use
the one from loaded libc.so. This would also require some discussion if it
is really an expected scenario we should support.

>
>> Presumably this fixes the problem with it being used as a common symbol
>> and thus not working with gcc-10 on ppc64?  If so, yea, seems like it'd
>> be desirable before the release.
>
> The tests I implemented are failing because of these issues.
>
> Would it be fine if I disabled the failing tests while we do not have a fix
> for BZ #25335?
>

You can change the test to save/restore a passed errno in the wrapper
if it is required to check for errno value. I don't see provide a
fix for BZ#25535 as a blocker here.


[1] https://sourceware.org/ml/libc-alpha/2019-12/msg00483.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Tulio Magno Quites Machado Filho-3
Adhemerval Zanella <[hidden email]> writes:

> This is BZ#20802 and I although we might fix it by adding the initialization
> on dl-static.c, I think Florian suggestion [1] to proper initialize shared ld.so
> variables is a better overall solution. However it is also a much more extensive
> change and not feasible on current release phase.

Ack.

> I don't see how easily we would fix it, it probability require to change
> how errno is definde in static manner and most likely more hacks to use
> the one from loaded libc.so. This would also require some discussion if it
> is really an expected scenario we should support.

Makes sense.

>> Would it be fine if I disabled the failing tests while we do not have a fix
>> for BZ #25335?
>
> You can change the test to save/restore a passed errno in the wrapper
> if it is required to check for errno value. I don't see provide a
> fix for BZ#25535 as a blocker here.

What are you referring as a wrapper in the getauxval() case?

--
Tulio Magno
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Adhemerval Zanella-2


On 09/01/2020 15:49, Tulio Magno Quites Machado Filho wrote:

> Adhemerval Zanella <[hidden email]> writes:
>
>> This is BZ#20802 and I although we might fix it by adding the initialization
>> on dl-static.c, I think Florian suggestion [1] to proper initialize shared ld.so
>> variables is a better overall solution. However it is also a much more extensive
>> change and not feasible on current release phase.
>
> Ack.
>
>> I don't see how easily we would fix it, it probability require to change
>> how errno is definde in static manner and most likely more hacks to use
>> the one from loaded libc.so. This would also require some discussion if it
>> is really an expected scenario we should support.
>
> Makes sense.
>
>>> Would it be fine if I disabled the failing tests while we do not have a fix
>>> for BZ #25335?
>>
>> You can change the test to save/restore a passed errno in the wrapper
>> if it is required to check for errno value. I don't see provide a
>> fix for BZ#25535 as a blocker here.
>
> What are you referring as a wrapper in the getauxval() case?
>

Yes, on the test. Something like:

  unsigned long int
  getauxval_wrapper (unsigned long type, int *errnop)
  {
    errno = *errnop;
    unsigned long int r = getauxval (type);
    *errnop = errno;
    return r;
  }

And then you use on static build which will dlopen it:

  {
    unsigned long int (*wrapper)(unsigned long, int *)
      = xdlsym (handler, "getauxval_wrapper");
    int ierrno = 0;
    TEST_COMPARE (wrapper (...), ...);
    TEST_COMPARE (ierrno, ...);
  }
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Tulio Magno Quites Machado Filho-3
In reply to this post by Tulio Magno Quites Machado Filho-3
Tulio Magno Quites Machado Filho <[hidden email]> writes:

> Adhemerval Zanella <[hidden email]> writes:
>
>> You can change the test to save/restore a passed errno in the wrapper
>> if it is required to check for errno value. I don't see provide a
>> fix for BZ#25535 as a blocker here.
>
> What are you referring as a wrapper in the getauxval() case?

Oh!  Forget it.  You're referring to Florian's patch.
I'll use that.

Thanks!

--
Tulio Magno
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Florian Weimer-5
In reply to this post by Tulio Magno Quites Machado Filho-3
* Tulio Magno Quites Machado Filho:

> Jeff Law <[hidden email]> writes:
>
>> On Thu, 2020-01-09 at 11:29 +0100, Florian Weimer wrote:
>>> What's the status here?
>>>
>>> I think it's desirable to fix this before the release.
>
> While working on the changes requested by Adhemerval, I hit 2 other bugs:
>
> 1. getauxval() does not work well from a DSO dlopen'ed by a statically linked
>    executable.
> 2. errno from a DSO is lost in a statically linked executable (BZ #25335).
>
> I've fixed the first issue, but I'm stuck with the second one.

Based on the subsequent discussion, they are no longer blockers, right?

Thanks,
Florian

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Shawn Landden-4
Not trying to say anything stupid, but why can't we just detect the cache line size using the "dcbz" instruction, as documented in the PowerISA document? It is because the ancient G5 also has a "dcbzl" instruction that clears a wider size (the actually cache line size)?

--
Shawn Landden

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] powerpc: Move cache line size to rtld_global_ro

Adhemerval Zanella-2


On 10/01/2020 13:09, Shawn Landden wrote:
> Not trying to say anything stupid, but why can't we just detect the cache line size using the "dcbz" instruction, as documented in the PowerISA document? It is because the ancient G5 also has a "dcbzl" instruction that clears a wider size (the actually cache line size)?
>

AFAIK you can't detect the data cache block size using the dcbz instruction,
in fact PowerISA 3.0B states that this information should be provided by the
operation system (Book II, Chapter 4.1 Parameters Useful to Application
Programs).

Linux provided it by AT_DCACHEBSIZE field in auxv and it is implemented in the
kernel by a pre-defined table (arch/powerpc/kernel/cputable.c). This is how
glibc obtain such information (sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c).

The issue I pointed out is for static binaries that dlopen a shared library,
mem* calls done by the library (which will call the loaded libc.so one) won't
see the field properly initialized. Some architectures fix rtld initialization
by reimplementing the _dl_static_init.  It looks up for the '_dl_var_init' and
calls it, taking care of relro segments by unmap/mmap the segment.

And the dcbzl seems to be a hack pushed by Apple for some reason, which should
not be required on Linux:

"dcbz" only operates on 32 bytes when the special HID5 compatiblity bit that
apple added to the 970 is set. This is _NOT_ the normal case and this bit isn't
set in linux unless you explicitely set it by modifying the kernel [1]."

[1] https://lists.ozlabs.org/pipermail/linuxppc64-dev/2004-March/001383.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802]

Tulio Magno Quites Machado Filho-2
In reply to this post by Adhemerval Zanella-2
Notice this patch is required in order to let the static dlopen test
from the next patch to work.

----8<----

Initialize dl_auxv, dl_hwcap and dl_hwcap2 in rtld_global_ro for DSOs
that have been statically dlopen'ed.
---
 sysdeps/unix/sysv/linux/powerpc/dl-static.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-static.c b/sysdeps/unix/sysv/linux/powerpc/dl-static.c
index 48fec16dca..59ce4e8972 100644
--- a/sysdeps/unix/sysv/linux/powerpc/dl-static.c
+++ b/sysdeps/unix/sysv/linux/powerpc/dl-static.c
@@ -26,17 +26,26 @@ _dl_var_init (void *array[])
   /* It has to match "variables" below. */
   enum
     {
-      DL_PAGESIZE = 0
+      DL_PAGESIZE = 0,
+      DL_AUXV = 1,
+      DL_HWCAP = 2,
+      DL_HWCAP2 = 3,
     };
 
   GLRO(dl_pagesize) = *((size_t *) array[DL_PAGESIZE]);
+  GLRO(dl_auxv) = (ElfW(auxv_t) *) *((size_t *) array[DL_AUXV]);
+  GLRO(dl_hwcap)  = *((unsigned long int *) array[DL_HWCAP]);
+  GLRO(dl_hwcap2) = *((unsigned long int *) array[DL_HWCAP2]);
 }
 
 #else
 
 static void *variables[] =
 {
-  &GLRO(dl_pagesize)
+  &GLRO(dl_pagesize),
+  &GLRO(dl_auxv),
+  &GLRO(dl_hwcap),
+  &GLRO(dl_hwcap2),
 };
 
 static void
--
2.24.1

12