unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* powerpc build failure with GCC mainline -fno-common change
@ 2019-11-21  1:19 Joseph Myers
  2019-11-21  9:23 ` Andreas Schwab
  2019-11-22 16:58 ` powerpc build failure with GCC mainline -fno-common change Tulio Magno Quites Machado Filho
  0 siblings, 2 replies; 37+ messages in thread
From: Joseph Myers @ 2019-11-21  1:19 UTC (permalink / raw)
  To: libc-alpha; +Cc: tuliom

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
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: powerpc build failure with GCC mainline -fno-common change
  2019-11-21  1:19 powerpc build failure with GCC mainline -fno-common change Joseph Myers
@ 2019-11-21  9:23 ` Andreas Schwab
  2019-12-23 18:45   ` [PATCH] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
  2019-11-22 16:58 ` powerpc build failure with GCC mainline -fno-common change Tulio Magno Quites Machado Filho
  1 sibling, 1 reply; 37+ messages in thread
From: Andreas Schwab @ 2019-11-21  9:23 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha, tuliom

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, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: powerpc build failure with GCC mainline -fno-common change
  2019-11-21  1:19 powerpc build failure with GCC mainline -fno-common change Joseph Myers
  2019-11-21  9:23 ` Andreas Schwab
@ 2019-11-22 16:58 ` Tulio Magno Quites Machado Filho
  2019-11-23 13:23   ` Florian Weimer
  1 sibling, 1 reply; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2019-11-22 16:58 UTC (permalink / raw)
  To: Joseph Myers, libc-alpha, Andreas Schwab

Joseph Myers <joseph@codesourcery.com> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: powerpc build failure with GCC mainline -fno-common change
  2019-11-22 16:58 ` powerpc build failure with GCC mainline -fno-common change Tulio Magno Quites Machado Filho
@ 2019-11-23 13:23   ` Florian Weimer
  2019-11-26 13:26     ` Tulio Magno Quites Machado Filho
  0 siblings, 1 reply; 37+ messages in thread
From: Florian Weimer @ 2019-11-23 13:23 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho; +Cc: Joseph Myers, libc-alpha, Andreas Schwab

* 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.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: powerpc build failure with GCC mainline -fno-common change
  2019-11-23 13:23   ` Florian Weimer
@ 2019-11-26 13:26     ` Tulio Magno Quites Machado Filho
  0 siblings, 0 replies; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2019-11-26 13:26 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Joseph Myers, libc-alpha, Andreas Schwab

Florian Weimer <fw@deneb.enyo.de> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH] powerpc: Move cache line size to rtld_global_ro
  2019-11-21  9:23 ` Andreas Schwab
@ 2019-12-23 18:45   ` Tulio Magno Quites Machado Filho
  2019-12-26 17:04     ` Adhemerval Zanella
  0 siblings, 1 reply; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2019-12-23 18:45 UTC (permalink / raw)
  To: libc-alpha; +Cc: Andreas Schwab, Joseph Myers, fw

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 <tuliom@linux.ibm.com>
---
 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


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2019-12-23 18:45   ` [PATCH] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
@ 2019-12-26 17:04     ` Adhemerval Zanella
  2019-12-27 15:42       ` Tulio Magno Quites Machado Filho
  0 siblings, 1 reply; 37+ messages in thread
From: Adhemerval Zanella @ 2019-12-26 17:04 UTC (permalink / raw)
  To: libc-alpha



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 <tuliom@linux.ibm.com>

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.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2019-12-26 17:04     ` Adhemerval Zanella
@ 2019-12-27 15:42       ` Tulio Magno Quites Machado Filho
  2019-12-27 16:47         ` Adhemerval Zanella
  0 siblings, 1 reply; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2019-12-27 15:42 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

Adhemerval Zanella <adhemerval.zanella@linaro.org> 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 <tuliom@linux.ibm.com>
>
> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2019-12-27 15:42       ` Tulio Magno Quites Machado Filho
@ 2019-12-27 16:47         ` Adhemerval Zanella
  2020-01-09 10:29           ` Florian Weimer
  0 siblings, 1 reply; 37+ messages in thread
From: Adhemerval Zanella @ 2019-12-27 16:47 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho, libc-alpha



On 27/12/2019 12:42, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> 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 <tuliom@linux.ibm.com>
>>
>> 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.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2019-12-27 16:47         ` Adhemerval Zanella
@ 2020-01-09 10:29           ` Florian Weimer
  2020-01-09 16:43             ` Jeff Law
  0 siblings, 1 reply; 37+ messages in thread
From: Florian Weimer @ 2020-01-09 10:29 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Tulio Magno Quites Machado Filho, libc-alpha

What's the status here?

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

Thanks,
Florian


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2020-01-09 10:29           ` Florian Weimer
@ 2020-01-09 16:43             ` Jeff Law
  2020-01-09 17:20               ` Tulio Magno Quites Machado Filho
  0 siblings, 1 reply; 37+ messages in thread
From: Jeff Law @ 2020-01-09 16:43 UTC (permalink / raw)
  To: Florian Weimer, Adhemerval Zanella
  Cc: Tulio Magno Quites Machado Filho, libc-alpha

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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2020-01-09 16:43             ` Jeff Law
@ 2020-01-09 17:20               ` Tulio Magno Quites Machado Filho
  2020-01-09 18:03                 ` Adhemerval Zanella
  2020-01-10  9:38                 ` Florian Weimer
  0 siblings, 2 replies; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2020-01-09 17:20 UTC (permalink / raw)
  To: law, Florian Weimer, Adhemerval Zanella; +Cc: libc-alpha

Jeff Law <law@redhat.com> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2020-01-09 17:20               ` Tulio Magno Quites Machado Filho
@ 2020-01-09 18:03                 ` Adhemerval Zanella
  2020-01-09 18:49                   ` Tulio Magno Quites Machado Filho
  2020-01-10  9:38                 ` Florian Weimer
  1 sibling, 1 reply; 37+ messages in thread
From: Adhemerval Zanella @ 2020-01-09 18:03 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho, law, Florian Weimer; +Cc: libc-alpha



On 09/01/2020 14:20, Tulio Magno Quites Machado Filho wrote:
> Jeff Law <law@redhat.com> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2020-01-09 18:03                 ` Adhemerval Zanella
@ 2020-01-09 18:49                   ` Tulio Magno Quites Machado Filho
  2020-01-09 18:54                     ` Adhemerval Zanella
  2020-01-09 18:56                     ` [PATCH] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
  0 siblings, 2 replies; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2020-01-09 18:49 UTC (permalink / raw)
  To: Adhemerval Zanella, law, Florian Weimer; +Cc: libc-alpha

Adhemerval Zanella <adhemerval.zanella@linaro.org> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2020-01-09 18:49                   ` Tulio Magno Quites Machado Filho
@ 2020-01-09 18:54                     ` Adhemerval Zanella
  2020-01-10 22:27                       ` [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802] Tulio Magno Quites Machado Filho
  2020-01-09 18:56                     ` [PATCH] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
  1 sibling, 1 reply; 37+ messages in thread
From: Adhemerval Zanella @ 2020-01-09 18:54 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho, law, Florian Weimer; +Cc: libc-alpha



On 09/01/2020 15:49, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> 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, ...);
  }

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2020-01-09 18:49                   ` Tulio Magno Quites Machado Filho
  2020-01-09 18:54                     ` Adhemerval Zanella
@ 2020-01-09 18:56                     ` Tulio Magno Quites Machado Filho
  1 sibling, 0 replies; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2020-01-09 18:56 UTC (permalink / raw)
  To: Adhemerval Zanella, law, Florian Weimer; +Cc: libc-alpha

Tulio Magno Quites Machado Filho <tuliom@ascii.art.br> writes:

> Adhemerval Zanella <adhemerval.zanella@linaro.org> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2020-01-09 17:20               ` Tulio Magno Quites Machado Filho
  2020-01-09 18:03                 ` Adhemerval Zanella
@ 2020-01-10  9:38                 ` Florian Weimer
  2020-01-10 16:09                   ` Shawn Landden
  1 sibling, 1 reply; 37+ messages in thread
From: Florian Weimer @ 2020-01-10  9:38 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho; +Cc: law, Adhemerval Zanella, libc-alpha

* Tulio Magno Quites Machado Filho:

> Jeff Law <law@redhat.com> 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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2020-01-10  9:38                 ` Florian Weimer
@ 2020-01-10 16:09                   ` Shawn Landden
  2020-01-10 18:19                     ` Adhemerval Zanella
  0 siblings, 1 reply; 37+ messages in thread
From: Shawn Landden @ 2020-01-10 16:09 UTC (permalink / raw)
  To: Florian Weimer, Tulio Magno Quites Machado Filho
  Cc: law@redhat.com, Adhemerval Zanella, libc-alpha@sourceware.org

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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc: Move cache line size to rtld_global_ro
  2020-01-10 16:09                   ` Shawn Landden
@ 2020-01-10 18:19                     ` Adhemerval Zanella
  0 siblings, 0 replies; 37+ messages in thread
From: Adhemerval Zanella @ 2020-01-10 18:19 UTC (permalink / raw)
  To: Shawn Landden, Florian Weimer, Tulio Magno Quites Machado Filho
  Cc: law@redhat.com, libc-alpha@sourceware.org



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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802]
  2020-01-09 18:54                     ` Adhemerval Zanella
@ 2020-01-10 22:27                       ` Tulio Magno Quites Machado Filho
  2020-01-10 22:27                         ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
  2020-01-16 16:21                         ` [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802] Carlos O'Donell
  0 siblings, 2 replies; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2020-01-10 22:27 UTC (permalink / raw)
  To: Adhemerval Zanella, law, Florian Weimer, libc-alpha

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


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-01-10 22:27                       ` [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802] Tulio Magno Quites Machado Filho
@ 2020-01-10 22:27                         ` Tulio Magno Quites Machado Filho
  2020-01-16 16:37                           ` Carlos O'Donell
                                             ` (2 more replies)
  2020-01-16 16:21                         ` [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802] Carlos O'Donell
  1 sibling, 3 replies; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2020-01-10 22:27 UTC (permalink / raw)
  To: Adhemerval Zanella, law, Florian Weimer, libc-alpha

Changes since v1:
 - Updated copyright dates
 - Added tests
 - Fixed coding style issues
 - Added macros __GLRO_DEF and __GLRO in the 64-bit case.
 - Removed sysdeps/unix/sysv/linux/powerpc/dl-support.c in favor of
   sysdeps/generic/dl-auxv.h which is included by elf/dl-support.c and
   elf/dl-sysdep.c
 - Removed sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c

----8<----

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.
---
 elf/dl-support.c                              |  3 +-
 elf/dl-sysdep.c                               |  3 +-
 sysdeps/generic/dl-auxv.h                     | 21 ++++++++
 sysdeps/powerpc/Makefile                      | 17 ++++++
 sysdeps/powerpc/dl-procinfo.c                 | 17 ++++++
 sysdeps/powerpc/mod-cache-ppc.c               | 45 ++++++++++++++++
 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         | 13 +++--
 sysdeps/powerpc/powerpc64/memset.S            | 11 ++--
 sysdeps/powerpc/powerpc64/sysdep.h            | 24 +++++++++
 sysdeps/powerpc/rtld-global-offsets.sym       |  1 +
 sysdeps/powerpc/tst-cache-ppc-static-dlopen.c | 54 +++++++++++++++++++
 sysdeps/powerpc/tst-cache-ppc-static.c        | 20 +++++++
 sysdeps/powerpc/tst-cache-ppc.c               | 29 ++++++++++
 .../linux/powerpc/{dl-sysdep.c => dl-auxv.h}  | 19 +++----
 sysdeps/unix/sysv/linux/powerpc/dl-static.c   |  3 ++
 sysdeps/unix/sysv/linux/powerpc/libc-start.c  | 10 ++--
 20 files changed, 312 insertions(+), 67 deletions(-)
 create mode 100644 sysdeps/generic/dl-auxv.h
 create mode 100644 sysdeps/powerpc/mod-cache-ppc.c
 create mode 100644 sysdeps/powerpc/tst-cache-ppc-static-dlopen.c
 create mode 100644 sysdeps/powerpc/tst-cache-ppc-static.c
 create mode 100644 sysdeps/powerpc/tst-cache-ppc.c
 rename sysdeps/unix/sysv/linux/powerpc/{dl-sysdep.c => dl-auxv.h} (60%)

diff --git a/elf/dl-support.c b/elf/dl-support.c
index ad791ab6ab..7704c101c5 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -36,6 +36,7 @@
 #include <stackinfo.h>
 #include <dl-vdso.h>
 #include <dl-vdso-setup.h>
+#include <dl-auxv.h>
 
 extern char *__progname;
 char **_dl_argv = &__progname;	/* This is checked for some error messages.  */
@@ -293,9 +294,7 @@ _dl_aux_init (ElfW(auxv_t) *av)
       case AT_RANDOM:
 	_dl_random = (void *) av->a_un.a_val;
 	break;
-# ifdef DL_PLATFORM_AUXV
       DL_PLATFORM_AUXV
-# endif
       }
   if (seen == 0xf)
     {
diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
index 53bbee14f4..854570821c 100644
--- a/elf/dl-sysdep.c
+++ b/elf/dl-sysdep.c
@@ -45,6 +45,7 @@
 #include <tls.h>
 
 #include <dl-tunables.h>
+#include <dl-auxv.h>
 
 extern char **_environ attribute_hidden;
 extern char _end[] attribute_hidden;
@@ -180,9 +181,7 @@ _dl_sysdep_start (void **start_argptr,
       case AT_RANDOM:
 	_dl_random = (void *) av->a_un.a_val;
 	break;
-#ifdef DL_PLATFORM_AUXV
       DL_PLATFORM_AUXV
-#endif
       }
 
 #ifndef HAVE_AUX_SECURE
diff --git a/sysdeps/generic/dl-auxv.h b/sysdeps/generic/dl-auxv.h
new file mode 100644
index 0000000000..bf3c01182e
--- /dev/null
+++ b/sysdeps/generic/dl-auxv.h
@@ -0,0 +1,21 @@
+/* Auxiliary vector processing.  Generic version.
+   Copyright (C) 2020 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/>.  */
+
+/* Define DL_PLATFORM_AUXV in order to process platform-specific AUXV entries
+   during the initialization of the loader or of a static libc.  */
+#define DL_PLATFORM_AUXV
diff --git a/sysdeps/powerpc/Makefile b/sysdeps/powerpc/Makefile
index df45d348d2..d1c71a0ca4 100644
--- a/sysdeps/powerpc/Makefile
+++ b/sysdeps/powerpc/Makefile
@@ -14,6 +14,23 @@ mod-tlsopt-powerpc.so-no-z-defs = yes
 tests += tst-tlsopt-powerpc
 $(objpfx)tst-tlsopt-powerpc: $(objpfx)mod-tlsopt-powerpc.so
 
+tests-static += tst-cache-ppc-static
+tests-internal += tst-cache-ppc-static
+
+ifeq (yes,$(build-shared))
+modules-names += mod-cache-ppc
+tests += tst-cache-ppc tst-cache-ppc-static-dlopen
+tests-static += tst-cache-ppc-static-dlopen
+test-internal-extras += mod-cache-ppc
+
+mod-cache-ppc.so-no-z-defs = yes
+tst-cache-ppc-static-dlopen-ENV = LD_LIBRARY_PATH=$(objpfx):$(common-objpfx):$(common-objpfx)elf
+$(objpfx)tst-cache-ppc-static-dlopen: $(common-objpfx)dlfcn/libdl.a
+$(objpfx)tst-cache-ppc-static-dlopen.out: $(objpfx)mod-cache-ppc.so
+
+$(objpfx)tst-cache-ppc: $(objpfx)mod-cache-ppc.so
+endif
+
 ifneq (no,$(multi-arch))
 tests-static += tst-tlsifunc-static
 tests-internal += tst-tlsifunc-static
diff --git a/sysdeps/powerpc/dl-procinfo.c b/sysdeps/powerpc/dl-procinfo.c
index 2ae68c41f1..7a7d93dd0a 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/mod-cache-ppc.c b/sysdeps/powerpc/mod-cache-ppc.c
new file mode 100644
index 0000000000..81fad52078
--- /dev/null
+++ b/sysdeps/powerpc/mod-cache-ppc.c
@@ -0,0 +1,45 @@
+/* Test if an executable can read from rtld_global_ro._dl_cache_line_size.
+   Copyright (C) 2020 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 <stdio.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <sys/auxv.h>
+#include <ldsodefs.h>
+#include <errno.h>
+
+/* errnop is required in order to work around BZ #20802.  */
+int
+test_cache (int *errnop)
+{
+  int cls1 = GLRO (dl_cache_line_size);
+  errno = *errnop;
+  uint64_t cls2 = getauxval (AT_DCACHEBSIZE);
+  *errnop = errno;
+
+  printf ("AT_DCACHEBSIZE      = %" PRIu64 " B\n", cls2);
+  printf ("_dl_cache_line_size = %d B\n", cls1);
+
+  if (cls1 != cls2)
+    {
+      printf ("error: _dl_cache_line_size != AT_DCACHEBSIZE\n");
+      return 1;
+    }
+
+  return 0;
+}
diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S
index fe5dab847a..6f4d8a7b34 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 d5ea4b97f4..6090e60d3c 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 5f614c07d7..26c37f8a17 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 ceed9ef158..0dee5f2757 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 0e3c435f3c..1162cc2207 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,9 @@
 #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
 
 	.section        ".toc","aw"
-.LC0:
-	.tc __cache_line_size[TC],__cache_line_size
+__GLRO_DEF(dl_cache_line_size)
+
+
 	.section        ".text"
 	.align 2
 
@@ -55,10 +57,11 @@ 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 the cache line size.  */
+	__GLRO (r9, dl_cache_line_size,
+		RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)
 	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) */
 	cmpldi  cr0,r8,0        /* Were we aligned on a 16 byte bdy? */
 	addi    r10,r9,-1       /* Cache line mask */
 	beq+    L(dst_aligned)
@@ -121,7 +124,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 857c023755..2fa98e6e2d 100644
--- a/sysdeps/powerpc/powerpc64/memset.S
+++ b/sysdeps/powerpc/powerpc64/memset.S
@@ -17,10 +17,11 @@
    <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
+__GLRO_DEF(dl_cache_line_size)
+
 	.section	".text"
 	.align 2
 
@@ -146,8 +147,10 @@ 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)
-	lwz	rCLS,0(rCLS)
+	/* Read the cache line size.  */
+	__GLRO (rCLS, dl_cache_line_size,
+		RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)
+
 /* 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/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
index aefd29a14d..d6616ac905 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -342,6 +342,30 @@ LT_LABELSUFFIX(name,_name_end): ; \
 #define	PSEUDO_END_ERRVAL(name) \
   END (name)
 
+#ifdef SHARED
+# if IS_IN (rtld)
+	 /* Inside ld.so we use the local alias to avoid runtime GOT
+	    relocations.  */
+#  define __GLRO_DEF(var)				\
+.LC__ ## var:						\
+	.tc _rtld_local_ro[TC],_rtld_local_ro
+# else
+#  define __GLRO_DEF(var)				\
+.LC__ ## var:						\
+	.tc _rtld_global_ro[TC],_rtld_global_ro
+# endif
+# define __GLRO(rOUT, var, offset)		\
+	ld	rOUT,.LC__ ## var@toc(r2);	\
+	lwz	rOUT,offset(rOUT)
+#else
+# define __GLRO_DEF(var)			\
+.LC__ ## var:					\
+	.tc _ ## var[TC],_ ## var
+# define __GLRO(rOUT, var, offset)		\
+	ld	rOUT,.LC__ ## var@toc(r2);	\
+	lwz	rOUT,0(rOUT)
+#endif
+
 #else /* !__ASSEMBLER__ */
 
 #if _CALL_ELF != 2
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/powerpc/tst-cache-ppc-static-dlopen.c b/sysdeps/powerpc/tst-cache-ppc-static-dlopen.c
new file mode 100644
index 0000000000..296d0f4397
--- /dev/null
+++ b/sysdeps/powerpc/tst-cache-ppc-static-dlopen.c
@@ -0,0 +1,54 @@
+/* Test dl_cache_line_size from a dlopen'ed DSO from a static executable.
+   Copyright (C) 2020 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 <dlfcn.h>
+#include <stdio.h>
+#include <errno.h>
+
+int test_cache(int *);
+
+static int
+do_test (void)
+{
+  int ret;
+  void *handle;
+  int (*test_cache) (int *);
+
+  handle = dlopen ("mod-cache-ppc.so", RTLD_LAZY | RTLD_LOCAL);
+  if (handle == NULL)
+    {
+      printf ("dlopen (mod-cache-ppc.so): %s\n", dlerror ());
+      return 1;
+    }
+
+  test_cache = dlsym (handle, "test_cache");
+  if (test_cache == NULL)
+    {
+      printf ("dlsym (test_cache): %s\n", dlerror ());
+      return 1;
+    }
+
+  ret = test_cache(&errno);
+
+  test_cache = NULL;
+  dlclose (handle);
+
+  return ret;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/powerpc/tst-cache-ppc-static.c b/sysdeps/powerpc/tst-cache-ppc-static.c
new file mode 100644
index 0000000000..b0c417e822
--- /dev/null
+++ b/sysdeps/powerpc/tst-cache-ppc-static.c
@@ -0,0 +1,20 @@
+/* Test if an executable can read from _dl_cache_line_size.
+   Copyright (C) 2020 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 "tst-cache-ppc.c"
+#include "mod-cache-ppc.c"
diff --git a/sysdeps/powerpc/tst-cache-ppc.c b/sysdeps/powerpc/tst-cache-ppc.c
new file mode 100644
index 0000000000..86c7117c43
--- /dev/null
+++ b/sysdeps/powerpc/tst-cache-ppc.c
@@ -0,0 +1,29 @@
+/* Test if an executable can read from rtld_global_ro._dl_cache_line_size.
+   Copyright (C) 2020 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 <errno.h>
+
+int test_cache(int *);
+
+static int
+do_test (void)
+{
+  return test_cache(&errno);
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
similarity index 60%
rename from sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
rename to sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
index 5d65bc6303..be2189732a 100644
--- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
+++ b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
@@ -1,5 +1,5 @@
-/* Operating system support for run-time dynamic linker.  Linux/PPC version.
-   Copyright (C) 1997-2020 Free Software Foundation, Inc.
+/* Auxiliary vector processing.  Linux/PPC version.
+   Copyright (C) 2020 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
@@ -16,18 +16,15 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <config.h>
 #include <ldsodefs.h>
 
-int __cache_line_size attribute_hidden;
+#if IS_IN (libc) && !defined SHARED
+int GLRO(dl_cache_line_size);
+#endif
 
-/* 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.  */
+/* 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:						      \
-	__cache_line_size = av->a_un.a_val;				      \
+	GLRO(dl_cache_line_size) = av->a_un.a_val;			      \
 	break;
-
-#include <sysdeps/unix/sysv/linux/dl-sysdep.c>
diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-static.c b/sysdeps/unix/sysv/linux/powerpc/dl-static.c
index 59ce4e8972..a77e07b503 100644
--- a/sysdeps/unix/sysv/linux/powerpc/dl-static.c
+++ b/sysdeps/unix/sysv/linux/powerpc/dl-static.c
@@ -30,12 +30,14 @@ _dl_var_init (void *array[])
       DL_AUXV = 1,
       DL_HWCAP = 2,
       DL_HWCAP2 = 3,
+      DL_CACHE_LINE_SIZE = 4
     };
 
   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]);
+  GLRO(dl_cache_line_size) = (int) *((int *) array[DL_CACHE_LINE_SIZE]);
 }
 
 #else
@@ -46,6 +48,7 @@ static void *variables[] =
   &GLRO(dl_auxv),
   &GLRO(dl_hwcap),
   &GLRO(dl_hwcap2),
+  &GLRO(dl_cache_line_size)
 };
 
 static void
diff --git a/sysdeps/unix/sysv/linux/powerpc/libc-start.c b/sysdeps/unix/sysv/linux/powerpc/libc-start.c
index 93f8659fa6..fc86d6e234 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


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802]
  2020-01-10 22:27                       ` [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802] Tulio Magno Quites Machado Filho
  2020-01-10 22:27                         ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
@ 2020-01-16 16:21                         ` Carlos O'Donell
  2020-01-16 16:25                           ` Siddhesh Poyarekar
  1 sibling, 1 reply; 37+ messages in thread
From: Carlos O'Donell @ 2020-01-16 16:21 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho, Adhemerval Zanella, law,
	Florian Weimer, libc-alpha

On 1/10/20 5:27 PM, Tulio Magno Quites Machado Filho wrote:
> Notice this patch is required in order to let the static dlopen test
> from the next patch to work.

OK for master. Please check with Siddhesh about committing this.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ----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,

OK. Add auxv, hwcap and hwcap2 in that order.

>      };
>  
>    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]);

OK. Matches order above for auxv, hwcap, and hwcap2.

>  }
>  
>  #else
>  
>  static void *variables[] =
>  {
> -  &GLRO(dl_pagesize)
> +  &GLRO(dl_pagesize),
> +  &GLRO(dl_auxv),
> +  &GLRO(dl_hwcap),
> +  &GLRO(dl_hwcap2),

OK. Matches order above.

>  };
>  
>  static void
> 


-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802]
  2020-01-16 16:21                         ` [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802] Carlos O'Donell
@ 2020-01-16 16:25                           ` Siddhesh Poyarekar
  0 siblings, 0 replies; 37+ messages in thread
From: Siddhesh Poyarekar @ 2020-01-16 16:25 UTC (permalink / raw)
  To: Carlos O'Donell, Tulio Magno Quites Machado Filho,
	Adhemerval Zanella, law, Florian Weimer, libc-alpha

On 16/01/20 9:51 pm, Carlos O'Donell wrote:
> On 1/10/20 5:27 PM, Tulio Magno Quites Machado Filho wrote:
>> Notice this patch is required in order to let the static dlopen test
>> from the next patch to work.
> 
> OK for master. Please check with Siddhesh about committing this.
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 

Fine with me.

Siddhesh

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-01-10 22:27                         ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
@ 2020-01-16 16:37                           ` Carlos O'Donell
  2020-01-17  3:56                             ` Siddhesh Poyarekar
  2020-01-17 23:45                           ` [PATCH] powerpc32: Fix syntax error in __GLRO macro Andreas Schwab
  2020-07-23 14:47                           ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Joseph Myers
  2 siblings, 1 reply; 37+ messages in thread
From: Carlos O'Donell @ 2020-01-16 16:37 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho, Adhemerval Zanella, law,
	Florian Weimer, libc-alpha, Siddhesh Poyarekar

On 1/10/20 5:27 PM, Tulio Magno Quites Machado Filho wrote:
> Changes since v1:
>  - Updated copyright dates
>  - Added tests
>  - Fixed coding style issues
>  - Added macros __GLRO_DEF and __GLRO in the 64-bit case.
>  - Removed sysdeps/unix/sysv/linux/powerpc/dl-support.c in favor of
>    sysdeps/generic/dl-auxv.h which is included by elf/dl-support.c and
>    elf/dl-sysdep.c
>  - Removed sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
> 

OK for master. This is probably the smallest change you can make to fix
all of this up.

This needs approval again by Siddhesh.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ----8<----
> 
> 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.
> ---
>  elf/dl-support.c                              |  3 +-
>  elf/dl-sysdep.c                               |  3 +-
>  sysdeps/generic/dl-auxv.h                     | 21 ++++++++
>  sysdeps/powerpc/Makefile                      | 17 ++++++
>  sysdeps/powerpc/dl-procinfo.c                 | 17 ++++++
>  sysdeps/powerpc/mod-cache-ppc.c               | 45 ++++++++++++++++
>  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         | 13 +++--
>  sysdeps/powerpc/powerpc64/memset.S            | 11 ++--
>  sysdeps/powerpc/powerpc64/sysdep.h            | 24 +++++++++
>  sysdeps/powerpc/rtld-global-offsets.sym       |  1 +
>  sysdeps/powerpc/tst-cache-ppc-static-dlopen.c | 54 +++++++++++++++++++
>  sysdeps/powerpc/tst-cache-ppc-static.c        | 20 +++++++
>  sysdeps/powerpc/tst-cache-ppc.c               | 29 ++++++++++
>  .../linux/powerpc/{dl-sysdep.c => dl-auxv.h}  | 19 +++----
>  sysdeps/unix/sysv/linux/powerpc/dl-static.c   |  3 ++
>  sysdeps/unix/sysv/linux/powerpc/libc-start.c  | 10 ++--
>  20 files changed, 312 insertions(+), 67 deletions(-)
>  create mode 100644 sysdeps/generic/dl-auxv.h
>  create mode 100644 sysdeps/powerpc/mod-cache-ppc.c
>  create mode 100644 sysdeps/powerpc/tst-cache-ppc-static-dlopen.c
>  create mode 100644 sysdeps/powerpc/tst-cache-ppc-static.c
>  create mode 100644 sysdeps/powerpc/tst-cache-ppc.c
>  rename sysdeps/unix/sysv/linux/powerpc/{dl-sysdep.c => dl-auxv.h} (60%)
> 
> diff --git a/elf/dl-support.c b/elf/dl-support.c
> index ad791ab6ab..7704c101c5 100644
> --- a/elf/dl-support.c
> +++ b/elf/dl-support.c
> @@ -36,6 +36,7 @@
>  #include <stackinfo.h>
>  #include <dl-vdso.h>
>  #include <dl-vdso-setup.h>
> +#include <dl-auxv.h>

OK.

>  
>  extern char *__progname;
>  char **_dl_argv = &__progname;	/* This is checked for some error messages.  */
> @@ -293,9 +294,7 @@ _dl_aux_init (ElfW(auxv_t) *av)
>        case AT_RANDOM:
>  	_dl_random = (void *) av->a_un.a_val;
>  	break;
> -# ifdef DL_PLATFORM_AUXV
>        DL_PLATFORM_AUXV
> -# endif

OK. Because DL_PLATFORM_AUXV is always defined now.

>        }
>    if (seen == 0xf)
>      {
> diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
> index 53bbee14f4..854570821c 100644
> --- a/elf/dl-sysdep.c
> +++ b/elf/dl-sysdep.c
> @@ -45,6 +45,7 @@
>  #include <tls.h>
>  
>  #include <dl-tunables.h>
> +#include <dl-auxv.h>

OK.

>  
>  extern char **_environ attribute_hidden;
>  extern char _end[] attribute_hidden;
> @@ -180,9 +181,7 @@ _dl_sysdep_start (void **start_argptr,
>        case AT_RANDOM:
>  	_dl_random = (void *) av->a_un.a_val;
>  	break;
> -#ifdef DL_PLATFORM_AUXV
>        DL_PLATFORM_AUXV
> -#endif

OK. Because DL_PLATFORM_AUXV is always defined now.

>        }
>  
>  #ifndef HAVE_AUX_SECURE
> diff --git a/sysdeps/generic/dl-auxv.h b/sysdeps/generic/dl-auxv.h
> new file mode 100644
> index 0000000000..bf3c01182e
> --- /dev/null
> +++ b/sysdeps/generic/dl-auxv.h
> @@ -0,0 +1,21 @@
> +/* Auxiliary vector processing.  Generic version.
> +   Copyright (C) 2020 Free Software Foundation, Inc.

OK.

> +   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/>.  */
> +
> +/* Define DL_PLATFORM_AUXV in order to process platform-specific AUXV entries
> +   during the initialization of the loader or of a static libc.  */
> +#define DL_PLATFORM_AUXV

OK. Default version does nothing.

> diff --git a/sysdeps/powerpc/Makefile b/sysdeps/powerpc/Makefile
> index df45d348d2..d1c71a0ca4 100644
> --- a/sysdeps/powerpc/Makefile
> +++ b/sysdeps/powerpc/Makefile
> @@ -14,6 +14,23 @@ mod-tlsopt-powerpc.so-no-z-defs = yes
>  tests += tst-tlsopt-powerpc
>  $(objpfx)tst-tlsopt-powerpc: $(objpfx)mod-tlsopt-powerpc.so
>  
> +tests-static += tst-cache-ppc-static
> +tests-internal += tst-cache-ppc-static

OK. Add one test.

> +
> +ifeq (yes,$(build-shared))
> +modules-names += mod-cache-ppc
> +tests += tst-cache-ppc tst-cache-ppc-static-dlopen
> +tests-static += tst-cache-ppc-static-dlopen
> +test-internal-extras += mod-cache-ppc
> +
> +mod-cache-ppc.so-no-z-defs = yes
> +tst-cache-ppc-static-dlopen-ENV = LD_LIBRARY_PATH=$(objpfx):$(common-objpfx):$(common-objpfx)elf
> +$(objpfx)tst-cache-ppc-static-dlopen: $(common-objpfx)dlfcn/libdl.a
> +$(objpfx)tst-cache-ppc-static-dlopen.out: $(objpfx)mod-cache-ppc.so
> +
> +$(objpfx)tst-cache-ppc: $(objpfx)mod-cache-ppc.so
> +endif

OK. Add two tests and modules.

> +
>  ifneq (no,$(multi-arch))
>  tests-static += tst-tlsifunc-static
>  tests-internal += tst-tlsifunc-static
> diff --git a/sysdeps/powerpc/dl-procinfo.c b/sysdeps/powerpc/dl-procinfo.c
> index 2ae68c41f1..7a7d93dd0a 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

OK. Define 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/mod-cache-ppc.c b/sysdeps/powerpc/mod-cache-ppc.c
> new file mode 100644
> index 0000000000..81fad52078
> --- /dev/null
> +++ b/sysdeps/powerpc/mod-cache-ppc.c
> @@ -0,0 +1,45 @@
> +/* Test if an executable can read from rtld_global_ro._dl_cache_line_size.
> +   Copyright (C) 2020 Free Software Foundation, Inc.

OK.

> +   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 <stdio.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +#include <sys/auxv.h>
> +#include <ldsodefs.h>
> +#include <errno.h>
> +
> +/* errnop is required in order to work around BZ #20802.  */
> +int
> +test_cache (int *errnop)
> +{
> +  int cls1 = GLRO (dl_cache_line_size);

OK.

> +  errno = *errnop;
> +  uint64_t cls2 = getauxval (AT_DCACHEBSIZE);

OK.

> +  *errnop = errno;

OK. Copy back errno.

> +
> +  printf ("AT_DCACHEBSIZE      = %" PRIu64 " B\n", cls2);
> +  printf ("_dl_cache_line_size = %d B\n", cls1);
> +
> +  if (cls1 != cls2)
> +    {
> +      printf ("error: _dl_cache_line_size != AT_DCACHEBSIZE\n");
> +      return 1;
> +    }
> +
> +  return 0;
> +}
> diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S
> index fe5dab847a..6f4d8a7b34 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>

OK.

>  
>  #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.  */

OK.

>  	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

OK.

>  #endif
> +	__GLRO(r9, r9, _dl_cache_line_size,
> +	       RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)

OK.

>  
>  	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 */

OK.

>  	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 d5ea4b97f4..6090e60d3c 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;

OK. Remove the use of __cache_line_size.

> -
> -
>  /* 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.  */

OK.

>  
>        /* 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)

OK.

>  	/* Convert bytes to words.  */
> -	line_size_words = __cache_line_size / 4;
> +	line_size_words = GLRO(dl_cache_line_size) / 4;

OK.

>  
>        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 5f614c07d7..26c37f8a17 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>

OK.

>  
>  /* 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).

OK.

>     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

OK.

>     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

OK.

>  	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)

OK.

>  
>  /* 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 ceed9ef158..0dee5f2757 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)

OK. Within rtld use local.

> +#  else
> +#   define __GLRO(rOUT, rGOT, member, offset)				\
> +	lwz     rOUT,_rtld_global_ro@got(rGOT);				\
> +	lwz     rOUT,offset(rOUT)

OK.

> +#  endif
> +# else
> +#  define __GLRO(rOUT, rGOT, member, offset)				\
> +	lwz     rOUT,member@got(rGOT);					\
> +	lwz     rOUT,0(rOUT)

OK.

> +# 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)

OK.

> +#endif	/* PIC */
> +
>  #endif	/* __ASSEMBLER__ */
> diff --git a/sysdeps/powerpc/powerpc64/a2/memcpy.S b/sysdeps/powerpc/powerpc64/a2/memcpy.S
> index 0e3c435f3c..1162cc2207 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>

OK.

>  
>  #ifndef MEMCPY
>  # define MEMCPY memcpy
> @@ -27,8 +28,9 @@
>  #define ZERO_AHEAD 2            /* no cache lines DST zeroing ahead  */
>  
>  	.section        ".toc","aw"
> -.LC0:
> -	.tc __cache_line_size[TC],__cache_line_size
> +__GLRO_DEF(dl_cache_line_size)
> +
> +

OK.

>  	.section        ".text"
>  	.align 2
>  
> @@ -55,10 +57,11 @@ 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 the cache line size.  */
> +	__GLRO (r9, dl_cache_line_size,
> +		RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)

OK.

>  	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) */
>  	cmpldi  cr0,r8,0        /* Were we aligned on a 16 byte bdy? */
>  	addi    r10,r9,-1       /* Cache line mask */
>  	beq+    L(dst_aligned)
> @@ -121,7 +124,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 */

OK.

>  	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 857c023755..2fa98e6e2d 100644
> --- a/sysdeps/powerpc/powerpc64/memset.S
> +++ b/sysdeps/powerpc/powerpc64/memset.S
> @@ -17,10 +17,11 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <sysdep.h>
> +#include <rtld-global-offsets.h>

OK.

>  
>  	.section	".toc","aw"
> -.LC0:
> -	.tc __cache_line_size[TC],__cache_line_size
> +__GLRO_DEF(dl_cache_line_size)
> +

OK.

>  	.section	".text"
>  	.align 2
>  
> @@ -146,8 +147,10 @@ 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)
> -	lwz	rCLS,0(rCLS)
> +	/* Read the cache line size.  */
> +	__GLRO (rCLS, dl_cache_line_size,
> +		RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)
> +

OK.

>  /* 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/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
> index aefd29a14d..d6616ac905 100644
> --- a/sysdeps/powerpc/powerpc64/sysdep.h
> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
> @@ -342,6 +342,30 @@ LT_LABELSUFFIX(name,_name_end): ; \
>  #define	PSEUDO_END_ERRVAL(name) \
>    END (name)
>  
> +#ifdef SHARED
> +# if IS_IN (rtld)
> +	 /* Inside ld.so we use the local alias to avoid runtime GOT
> +	    relocations.  */
> +#  define __GLRO_DEF(var)				\
> +.LC__ ## var:						\
> +	.tc _rtld_local_ro[TC],_rtld_local_ro

OK.

> +# else
> +#  define __GLRO_DEF(var)				\
> +.LC__ ## var:						\
> +	.tc _rtld_global_ro[TC],_rtld_global_ro
> +# endif
> +# define __GLRO(rOUT, var, offset)		\
> +	ld	rOUT,.LC__ ## var@toc(r2);	\
> +	lwz	rOUT,offset(rOUT)
> +#else
> +# define __GLRO_DEF(var)			\
> +.LC__ ## var:					\
> +	.tc _ ## var[TC],_ ## var
> +# define __GLRO(rOUT, var, offset)		\
> +	ld	rOUT,.LC__ ## var@toc(r2);	\
> +	lwz	rOUT,0(rOUT)
> +#endif

OK.

> +
>  #else /* !__ASSEMBLER__ */
>  
>  #if _CALL_ELF != 2
> 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/powerpc/tst-cache-ppc-static-dlopen.c b/sysdeps/powerpc/tst-cache-ppc-static-dlopen.c
> new file mode 100644
> index 0000000000..296d0f4397
> --- /dev/null
> +++ b/sysdeps/powerpc/tst-cache-ppc-static-dlopen.c
> @@ -0,0 +1,54 @@
> +/* Test dl_cache_line_size from a dlopen'ed DSO from a static executable.
> +   Copyright (C) 2020 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 <dlfcn.h>
> +#include <stdio.h>
> +#include <errno.h>
> +
> +int test_cache(int *);
> +
> +static int
> +do_test (void)
> +{
> +  int ret;
> +  void *handle;
> +  int (*test_cache) (int *);
> +
> +  handle = dlopen ("mod-cache-ppc.so", RTLD_LAZY | RTLD_LOCAL);
> +  if (handle == NULL)
> +    {
> +      printf ("dlopen (mod-cache-ppc.so): %s\n", dlerror ());
> +      return 1;
> +    }
> +
> +  test_cache = dlsym (handle, "test_cache");
> +  if (test_cache == NULL)
> +    {
> +      printf ("dlsym (test_cache): %s\n", dlerror ());
> +      return 1;
> +    }
> +
> +  ret = test_cache(&errno);
> +
> +  test_cache = NULL;
> +  dlclose (handle);

OK.

> +
> +  return ret;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/powerpc/tst-cache-ppc-static.c b/sysdeps/powerpc/tst-cache-ppc-static.c
> new file mode 100644
> index 0000000000..b0c417e822
> --- /dev/null
> +++ b/sysdeps/powerpc/tst-cache-ppc-static.c
> @@ -0,0 +1,20 @@
> +/* Test if an executable can read from _dl_cache_line_size.
> +   Copyright (C) 2020 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 "tst-cache-ppc.c"
> +#include "mod-cache-ppc.c"

OK.

> diff --git a/sysdeps/powerpc/tst-cache-ppc.c b/sysdeps/powerpc/tst-cache-ppc.c
> new file mode 100644
> index 0000000000..86c7117c43
> --- /dev/null
> +++ b/sysdeps/powerpc/tst-cache-ppc.c
> @@ -0,0 +1,29 @@
> +/* Test if an executable can read from rtld_global_ro._dl_cache_line_size.
> +   Copyright (C) 2020 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 <errno.h>
> +
> +int test_cache(int *);
> +
> +static int
> +do_test (void)
> +{
> +  return test_cache(&errno);
> +}

OK.

> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
> similarity index 60%
> rename from sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
> rename to sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
> index 5d65bc6303..be2189732a 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h
> @@ -1,5 +1,5 @@
> -/* Operating system support for run-time dynamic linker.  Linux/PPC version.
> -   Copyright (C) 1997-2020 Free Software Foundation, Inc.
> +/* Auxiliary vector processing.  Linux/PPC version.
> +   Copyright (C) 2020 Free Software Foundation, Inc.

OK.

>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -16,18 +16,15 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <config.h>
>  #include <ldsodefs.h>
>  
> -int __cache_line_size attribute_hidden;
> +#if IS_IN (libc) && !defined SHARED
> +int GLRO(dl_cache_line_size);
> +#endif
>  
> -/* 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.  */
> +/* 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:						      \
> -	__cache_line_size = av->a_un.a_val;				      \
> +	GLRO(dl_cache_line_size) = av->a_un.a_val;			      \

OK.

>  	break;
> -
> -#include <sysdeps/unix/sysv/linux/dl-sysdep.c>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-static.c b/sysdeps/unix/sysv/linux/powerpc/dl-static.c
> index 59ce4e8972..a77e07b503 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/dl-static.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-static.c
> @@ -30,12 +30,14 @@ _dl_var_init (void *array[])
>        DL_AUXV = 1,
>        DL_HWCAP = 2,
>        DL_HWCAP2 = 3,
> +      DL_CACHE_LINE_SIZE = 4

OK.

>      };
>  
>    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]);
> +  GLRO(dl_cache_line_size) = (int) *((int *) array[DL_CACHE_LINE_SIZE]);

OK.

>  }
>  
>  #else
> @@ -46,6 +48,7 @@ static void *variables[] =
>    &GLRO(dl_auxv),
>    &GLRO(dl_hwcap),
>    &GLRO(dl_hwcap2),
> +  &GLRO(dl_cache_line_size)

OK.

>  };
>  
>  static void
> diff --git a/sysdeps/unix/sysv/linux/powerpc/libc-start.c b/sysdeps/unix/sysv/linux/powerpc/libc-start.c
> index 93f8659fa6..fc86d6e234 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;

OK.

>  /* 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 ().  */

OK.

>    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 ().  */

OK.

>  #ifndef SHARED
>        case AT_HWCAP:
>  	_dl_hwcap = (unsigned long int) av->a_un.a_val;
> 


-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-01-16 16:37                           ` Carlos O'Donell
@ 2020-01-17  3:56                             ` Siddhesh Poyarekar
  2020-01-17 12:39                               ` Tulio Magno Quites Machado Filho
  0 siblings, 1 reply; 37+ messages in thread
From: Siddhesh Poyarekar @ 2020-01-17  3:56 UTC (permalink / raw)
  To: Carlos O'Donell, Tulio Magno Quites Machado Filho,
	Adhemerval Zanella, law, Florian Weimer, libc-alpha

On 16/01/20 10:07 pm, Carlos O'Donell wrote:
> On 1/10/20 5:27 PM, Tulio Magno Quites Machado Filho wrote:
>> Changes since v1:
>>  - Updated copyright dates
>>  - Added tests
>>  - Fixed coding style issues
>>  - Added macros __GLRO_DEF and __GLRO in the 64-bit case.
>>  - Removed sysdeps/unix/sysv/linux/powerpc/dl-support.c in favor of
>>    sysdeps/generic/dl-auxv.h which is included by elf/dl-support.c and
>>    elf/dl-sysdep.c
>>  - Removed sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
>>
> 
> OK for master. This is probably the smallest change you can make to fix
> all of this up.
> 
> This needs approval again by Siddhesh.
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 

This is fine.

Siddhesh


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-01-17  3:56                             ` Siddhesh Poyarekar
@ 2020-01-17 12:39                               ` Tulio Magno Quites Machado Filho
  2020-01-17 17:14                                 ` Florian Weimer
  2020-01-17 17:15                                 ` Joseph Myers
  0 siblings, 2 replies; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2020-01-17 12:39 UTC (permalink / raw)
  To: Siddhesh Poyarekar, Carlos O'Donell, Adhemerval Zanella, law,
	Florian Weimer, libc-alpha

Siddhesh Poyarekar <siddhesh@gotplt.org> writes:

> On 16/01/20 10:07 pm, Carlos O'Donell wrote:
>> On 1/10/20 5:27 PM, Tulio Magno Quites Machado Filho wrote:
>>> Changes since v1:
>>>  - Updated copyright dates
>>>  - Added tests
>>>  - Fixed coding style issues
>>>  - Added macros __GLRO_DEF and __GLRO in the 64-bit case.
>>>  - Removed sysdeps/unix/sysv/linux/powerpc/dl-support.c in favor of
>>>    sysdeps/generic/dl-auxv.h which is included by elf/dl-support.c and
>>>    elf/dl-sysdep.c
>>>  - Removed sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
>>>
>> 
>> OK for master. This is probably the smallest change you can make to fix
>> all of this up.
>> 
>> This needs approval again by Siddhesh.
>> 
>> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> This is fine.

I've just pushed both patches to master.

Thanks!

-- 
Tulio Magno

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-01-17 12:39                               ` Tulio Magno Quites Machado Filho
@ 2020-01-17 17:14                                 ` Florian Weimer
  2020-01-17 17:15                                 ` Joseph Myers
  1 sibling, 0 replies; 37+ messages in thread
From: Florian Weimer @ 2020-01-17 17:14 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho
  Cc: Siddhesh Poyarekar, Carlos O'Donell, Adhemerval Zanella, law,
	libc-alpha

* Tulio Magno Quites Machado Filho:

> Siddhesh Poyarekar <siddhesh@gotplt.org> writes:
>
>> On 16/01/20 10:07 pm, Carlos O'Donell wrote:
>>> On 1/10/20 5:27 PM, Tulio Magno Quites Machado Filho wrote:
>>>> Changes since v1:
>>>>  - Updated copyright dates
>>>>  - Added tests
>>>>  - Fixed coding style issues
>>>>  - Added macros __GLRO_DEF and __GLRO in the 64-bit case.
>>>>  - Removed sysdeps/unix/sysv/linux/powerpc/dl-support.c in favor of
>>>>    sysdeps/generic/dl-auxv.h which is included by elf/dl-support.c and
>>>>    elf/dl-sysdep.c
>>>>  - Removed sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
>>>>
>>> 
>>> OK for master. This is probably the smallest change you can make to fix
>>> all of this up.
>>> 
>>> This needs approval again by Siddhesh.
>>> 
>>> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>>
>> This is fine.
>
> I've just pushed both patches to master.

This appears to have caused (on powerpc-linux-gnu):

../sysdeps/powerpc/powerpc32/memset.S: Assembler messages:
../sysdeps/powerpc/powerpc32/memset.S:248: Error: syntax error; found ` ', expected `,'
../sysdeps/powerpc/powerpc32/memset.S:248: Error: junk at end of line: `lwz 8,(_dl_cache_line_size+4)@l(8)'

Thanks,
Florian


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-01-17 12:39                               ` Tulio Magno Quites Machado Filho
  2020-01-17 17:14                                 ` Florian Weimer
@ 2020-01-17 17:15                                 ` Joseph Myers
  2020-01-17 22:28                                   ` Tulio Magno Quites Machado Filho
  1 sibling, 1 reply; 37+ messages in thread
From: Joseph Myers @ 2020-01-17 17:15 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho
  Cc: Siddhesh Poyarekar, Carlos O'Donell, Adhemerval Zanella, law,
	Florian Weimer, libc-alpha

One of these changes appears to have broken the build for 32-bit powerpc 
(all the 32-bit configurations in build-many-glibcs.py), at least with 
GCC 8 / binutils 2.33.

https://sourceware.org/ml/libc-testresults/2020-q1/msg00077.html

../sysdeps/powerpc/powerpc32/memset.S: Assembler messages:
../sysdeps/powerpc/powerpc32/memset.S:248: Error: syntax error; found ` ', expected `,'
../sysdeps/powerpc/powerpc32/memset.S:248: Error: junk at end of line: `lwz 8,(_dl_cache_line_size+4)@l(8)'

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-01-17 17:15                                 ` Joseph Myers
@ 2020-01-17 22:28                                   ` Tulio Magno Quites Machado Filho
  0 siblings, 0 replies; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2020-01-17 22:28 UTC (permalink / raw)
  To: Joseph Myers, Florian Weimer, libc-alpha; +Cc: Siddhesh Poyarekar

Joseph Myers <joseph@codesourcery.com> writes:

> One of these changes appears to have broken the build for 32-bit powerpc 
> (all the 32-bit configurations in build-many-glibcs.py), at least with 
> GCC 8 / binutils 2.33.
>
> https://sourceware.org/ml/libc-testresults/2020-q1/msg00077.html
>
> ../sysdeps/powerpc/powerpc32/memset.S: Assembler messages:
> ../sysdeps/powerpc/powerpc32/memset.S:248: Error: syntax error; found ` ', expected `,'
> ../sysdeps/powerpc/powerpc32/memset.S:248: Error: junk at end of line: `lwz 8,(_dl_cache_line_size+4)@l(8)'

I've just reproduced this.
Interestingly, I can't reproduce it from outside a build-many-glibcs.py with
Binutils 2.33.1 (Debian 10). I tried with GCC 7, 8 and 9.

I'll continue to investigate it.

Thanks!

-- 
Tulio Magno

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH] powerpc32: Fix syntax error in __GLRO macro
  2020-01-10 22:27                         ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
  2020-01-16 16:37                           ` Carlos O'Donell
@ 2020-01-17 23:45                           ` Andreas Schwab
  2020-01-17 23:56                             ` Tulio Magno Quites Machado Filho
  2020-07-23 14:47                           ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Joseph Myers
  2 siblings, 1 reply; 37+ messages in thread
From: Andreas Schwab @ 2020-01-17 23:45 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho
  Cc: Adhemerval Zanella, law, Florian Weimer, libc-alpha

---
 sysdeps/powerpc/powerpc32/sysdep.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
index 0dee5f2757..2ba009e919 100644
--- a/sysdeps/powerpc/powerpc32/sysdep.h
+++ b/sysdeps/powerpc/powerpc32/sysdep.h
@@ -179,7 +179,7 @@ GOT_LABEL:			;					      \
 #else
 /* Position-dependent code does not require access to the GOT.  */
 # define __GLRO(rOUT, rGOT, member, offset)				\
-	lis     rOUT,(member+LOWORD)@ha					\
+	lis     rOUT,(member+LOWORD)@ha;					\
 	lwz     rOUT,(member+LOWORD)@l(rOUT)
 #endif	/* PIC */
 
-- 
2.25.0

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH] powerpc32: Fix syntax error in __GLRO macro
  2020-01-17 23:45                           ` [PATCH] powerpc32: Fix syntax error in __GLRO macro Andreas Schwab
@ 2020-01-17 23:56                             ` Tulio Magno Quites Machado Filho
  0 siblings, 0 replies; 37+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2020-01-17 23:56 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Adhemerval Zanella, law, Florian Weimer, libc-alpha

Andreas Schwab <schwab@linux-m68k.org> writes:

> diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
> index 0dee5f2757..2ba009e919 100644
> --- a/sysdeps/powerpc/powerpc32/sysdep.h
> +++ b/sysdeps/powerpc/powerpc32/sysdep.h
> @@ -179,7 +179,7 @@ GOT_LABEL:			;					      \
>  #else
>  /* Position-dependent code does not require access to the GOT.  */
>  # define __GLRO(rOUT, rGOT, member, offset)				\
> -	lis     rOUT,(member+LOWORD)@ha					\
> +	lis     rOUT,(member+LOWORD)@ha;					\
>  	lwz     rOUT,(member+LOWORD)@l(rOUT)
>  #endif	/* PIC */

Thanks!

-- 
Tulio Magno

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-01-10 22:27                         ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
  2020-01-16 16:37                           ` Carlos O'Donell
  2020-01-17 23:45                           ` [PATCH] powerpc32: Fix syntax error in __GLRO macro Andreas Schwab
@ 2020-07-23 14:47                           ` Joseph Myers
  2020-07-23 16:46                             ` Andreas Schwab
  2020-07-31 12:42                             ` Florian Weimer via Libc-alpha
  2 siblings, 2 replies; 37+ messages in thread
From: Joseph Myers @ 2020-07-23 14:47 UTC (permalink / raw)
  To: Tulio Magno Quites Machado Filho; +Cc: Florian Weimer, libc-alpha

I'm seeing all the statically linked glibc tests fail for 32-bit powerpc 
when built with GCC 10:

malloc.c:2394: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.
Aborted

(A statically linked program with empty main produces that error on 
startup.)

If I build glibc with -fcommon, at least a trivial statically linked 
binary no longer fails.  So I think there may have been something missing 
from the fixes to build with -fno-common; they got glibc building again, 
but not working in the statically linked case.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-07-23 14:47                           ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Joseph Myers
@ 2020-07-23 16:46                             ` Andreas Schwab
  2020-07-31 12:42                             ` Florian Weimer via Libc-alpha
  1 sibling, 0 replies; 37+ messages in thread
From: Andreas Schwab @ 2020-07-23 16:46 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Florian Weimer, Tulio Magno Quites Machado Filho, libc-alpha

On Jul 23 2020, Joseph Myers wrote:

> I'm seeing all the statically linked glibc tests fail for 32-bit powerpc 
> when built with GCC 10:
>
> malloc.c:2394: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.
> Aborted

I don't see that here.

https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:glibc/glibc:testsuite/p/ppc

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-07-23 14:47                           ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Joseph Myers
  2020-07-23 16:46                             ` Andreas Schwab
@ 2020-07-31 12:42                             ` Florian Weimer via Libc-alpha
  2020-07-31 18:35                               ` Joseph Myers
  1 sibling, 1 reply; 37+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-07-31 12:42 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Tulio Magno Quites Machado Filho, libc-alpha

* Joseph Myers:

> I'm seeing all the statically linked glibc tests fail for 32-bit powerpc 
> when built with GCC 10:
>
> malloc.c:2394: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.
> Aborted
>
> (A statically linked program with empty main produces that error on 
> startup.)
>
> If I build glibc with -fcommon, at least a trivial statically linked 
> binary no longer fails.  So I think there may have been something missing 
> from the fixes to build with -fno-common; they got glibc building again, 
> but not working in the statically linked case.

What's your binutils version?

Does this affect the statically linked test binaries built by
build-many-glibcs.py?

Thanks,
Florian


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-07-31 12:42                             ` Florian Weimer via Libc-alpha
@ 2020-07-31 18:35                               ` Joseph Myers
  2020-08-03  8:15                                 ` Florian Weimer via Libc-alpha
  0 siblings, 1 reply; 37+ messages in thread
From: Joseph Myers @ 2020-07-31 18:35 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Tulio Magno Quites Machado Filho, libc-alpha

On Fri, 31 Jul 2020, Florian Weimer via Libc-alpha wrote:

> * Joseph Myers:
> 
> > I'm seeing all the statically linked glibc tests fail for 32-bit powerpc 
> > when built with GCC 10:
> >
> > malloc.c:2394: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.
> > Aborted
> >
> > (A statically linked program with empty main produces that error on 
> > startup.)
> >
> > If I build glibc with -fcommon, at least a trivial statically linked 
> > binary no longer fails.  So I think there may have been something missing 
> > from the fixes to build with -fno-common; they got glibc building again, 
> > but not working in the statically linked case.
> 
> What's your binutils version?

This testing was with 2.35.50.20200720.

> Does this affect the statically linked test binaries built by
> build-many-glibcs.py?

Yes.  I tested build-many-glibcs.py for powerpc-linux-gnu with current 
default versions of everything (so binutils 2.35 branch in this case); 
running the math/atest-exp binary left from a --keep=all build produces 
that same assertion failure.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-07-31 18:35                               ` Joseph Myers
@ 2020-08-03  8:15                                 ` Florian Weimer via Libc-alpha
  2020-08-03 16:07                                   ` Carlos O'Donell via Libc-alpha
  0 siblings, 1 reply; 37+ messages in thread
From: Florian Weimer via Libc-alpha @ 2020-08-03  8:15 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Tulio Magno Quites Machado Filho, libc-alpha

* Joseph Myers:

>> Does this affect the statically linked test binaries built by
>> build-many-glibcs.py?
>
> Yes.  I tested build-many-glibcs.py? for powerpc-linux-gnu with current 
> default versions of everything (so binutils 2.35 branch in this case); 
> running the math/atest-exp binary left from a --keep=all build produces 
> that same assertion failure.

I can reproduce it:

(gdb) bt
#0  0x10007d74 in __libc_signal_restore_set (set=0xfffed708)
    at ../sysdeps/unix/sysv/linux/internal-signals.h:104
#1  raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:47
#2  0x10000268 in abort () at abort.c:79
#3  0x1001c7b8 in __malloc_assert (
    assertion=assertion@entry=0x10081ec4 "(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)", 
    file=file@entry=0x10081690 "malloc.c", line=line@entry=2394, 
    function=function@entry=0x100828b8 <__PRETTY_FUNCTION__.3> "sysmalloc") at malloc.c:298
#4  0x1001ee1c in sysmalloc (nb=nb@entry=64, 
    av=av@entry=0x100c03d4 <main_arena>) at malloc.c:2394
#5  0x10020614 in _int_malloc (av=av@entry=0x100c03d4 <main_arena>, 
    bytes=bytes@entry=53) at malloc.c:4169
#6  0x10021804 in __libc_malloc (bytes=53) at malloc.c:3078
#7  0x10060f44 in _dl_get_origin ()
    at ../sysdeps/unix/sysv/linux/dl-origin.c:49
#8  0x100302f4 in _dl_non_dynamic_init () at dl-support.c:311
#9  0x10031908 in __libc_init_first (argc=argc@entry=2, 
    argv=argv@entry=0xfffeecd4, envp=0xfffeece0) at init-first.c:72
#10 0x100024dc in generic_start_main (main=0x100003d4 <main>, 
    argc=argc@entry=2, argv=argv@entry=0xfffeecd4, 
    auxvec=auxvec@entry=0xfffeed9c, init=0x10002be0 <__libc_csu_init>, 
    fini=0x10002d58 <__libc_csu_fini>, rtld_fini=rtld_fini@entry=0x0, 
    stack_end=stack_end@entry=0xfffeecd0) at ../csu/libc-start.c:250
#11 0x10002774 in __libc_start_main (argc=2, argv=0xfffeecd4, 
    ev=<optimized out>, auxvec=0xfffeed9c, rtld_fini=0x0, 
    stinfo=0x1007f3e0, stack_on_entry=0xfffeecd0)
    at ../sysdeps/unix/sysv/linux/powerpc/libc-start.c:98
#12 0x00000000 in ?? ()

main_arena.top->mchunk_size gets overwritten during tcache_init:

#0  memset () at ../sysdeps/powerpc/powerpc32/memset.S:291
#1  0x10021660 in tcache_init () at malloc.c:3021
#2  0x10021af4 in __libc_malloc (bytes=bytes@entry=53) at malloc.c:3064
#3  0x10021d80 in malloc_hook_ini (sz=53, caller=<optimized out>) at hooks.c:32
#4  0x10021aa0 in __libc_malloc (bytes=53) at malloc.c:3053
#5  0x100614c4 in _dl_get_origin () at ../sysdeps/unix/sysv/linux/dl-origin.c:49
#6  0x10030874 in _dl_non_dynamic_init () at dl-support.c:311
#7  0x10031e88 in __libc_init_first (argc=argc@entry=2, 
    argv=argv@entry=0xfffeecd4, envp=0xfffeece0) at init-first.c:72
#8  0x100024dc in generic_start_main (main=0x100003d4 <main>, argc=argc@entry=2, 
    argv=argv@entry=0xfffeecd4, auxvec=auxvec@entry=0xfffeed9c, 
    init=0x10002be0 <__libc_csu_init>, fini=0x10002d58 <__libc_csu_fini>, 
    rtld_fini=rtld_fini@entry=0x0, stack_end=stack_end@entry=0xfffeecd0)
    at ../csu/libc-start.c:250
#9  0x10002774 in __libc_start_main (argc=2, argv=0xfffeecd4, 
    ev=<optimized out>, auxvec=0xfffeed9c, rtld_fini=0x0, stinfo=0x1007f960, 
    stack_on_entry=0xfffeecd0)
    at ../sysdeps/unix/sysv/linux/powerpc/libc-start.c:98
#10 0x00000000 in ?? ()

The memset goes wrong because it loads the cache line size as 1 here:

/* Load rtld_global_ro._dl_cache_line_size.  */
	__GLRO(rCLS, rGOT, _dl_cache_line_size,
	       RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)

0x100259e4 <+576>:   lis     r8,4108
=> 0x100259e8 <+580>:   lwz     r8,3912(r8)

(gdb) print (void*)($r8 + 3912)
$24 = (void *) 0x100c0f48 <__libc_enable_secure_decided>

This is not the address that is seen by the debugger for
_dl_cache_line_size:

(gdb) print &_dl_cache_line_size 
$26 = (int *) 0x100c0f44 <_dl_cache_line_size>

The symbol table looks pretty reasonable:
  1461: 100c0f44     4 OBJECT  GLOBAL DEFAULT   23 _dl_cache_line_size
  1495: 100c0f40     4 OBJECT  GLOBAL DEFAULT   23 _dl_platform
  1512: 100c0f48     4 OBJECT  GLOBAL DEFAULT   23 __libc_enable_secure_decided
  2135: 100c0f4c     4 OBJECT  GLOBAL DEFAULT   23 __libc_argv

For some reason, we have relocations with displacements in
string/memset.o:

 244:   3d 00 00 00     lis     r8,0
                        246: R_PPC_ADDR16_HA    _dl_cache_line_size+0x4
 248:   81 08 00 00     lwz     r8,0(r8)
                        24a: R_PPC_ADDR16_LO    _dl_cache_line_size+0x4

This is due to the definition of __GLRO:

#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 */

And LOWORD is 4 on big-endian PowerPC:

/* The 32-bit words of a 64-bit dword are at these offsets in memory.  */
#if defined __LITTLE_ENDIAN__ || defined _LITTLE_ENDIAN
# define LOWORD 0
# define HIWORD 4
#else
# define LOWORD 4
# define HIWORD 0
#endif

I believe we should remove the “+LOWORD” part here:

diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
index 2ba009e9..829eec26 100644
--- a/sysdeps/powerpc/powerpc32/sysdep.h
+++ b/sysdeps/powerpc/powerpc32/sysdep.h
@@ -179,8 +179,8 @@ GOT_LABEL:                  ;                               
              \
 #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)
+       lis     rOUT,(member)@ha;                                       \
+       lwz     rOUT,(member)@l(rOUT)
 #endif /* PIC */
 
 #endif /* __ASSEMBLER__ */

It fixes math/atest-exp for me.

Tulio, I believe you constructed this macro from
sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S, where it
is needed because we are loading the lower 32 bits of a 64-bit value.
It's not correct for loading a 32-bit quantity.

Technically, this bug is not a release blocker.  It's not a regression,
it's present in 2.31 as well.  I will file a bug and post a proper patch.

Thanks,
Florian


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro
  2020-08-03  8:15                                 ` Florian Weimer via Libc-alpha
@ 2020-08-03 16:07                                   ` Carlos O'Donell via Libc-alpha
  0 siblings, 0 replies; 37+ messages in thread
From: Carlos O'Donell via Libc-alpha @ 2020-08-03 16:07 UTC (permalink / raw)
  To: Florian Weimer, Joseph Myers; +Cc: Tulio Magno Quites Machado Filho, libc-alpha

On 8/3/20 4:15 AM, Florian Weimer wrote:
> Technically, this bug is not a release blocker.  It's not a regression,
> it's present in 2.31 as well.  I will file a bug and post a proper patch.

I just ack'd your patch for inclusion in 2.32.

Please commit for 2.32 so the release goes out without the bug.

Tulio, please review ASAP to double check our results here.

-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2020-08-03 16:07 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-21  1:19 powerpc build failure with GCC mainline -fno-common change Joseph Myers
2019-11-21  9:23 ` Andreas Schwab
2019-12-23 18:45   ` [PATCH] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
2019-12-26 17:04     ` Adhemerval Zanella
2019-12-27 15:42       ` Tulio Magno Quites Machado Filho
2019-12-27 16:47         ` Adhemerval Zanella
2020-01-09 10:29           ` Florian Weimer
2020-01-09 16:43             ` Jeff Law
2020-01-09 17:20               ` Tulio Magno Quites Machado Filho
2020-01-09 18:03                 ` Adhemerval Zanella
2020-01-09 18:49                   ` Tulio Magno Quites Machado Filho
2020-01-09 18:54                     ` Adhemerval Zanella
2020-01-10 22:27                       ` [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802] Tulio Magno Quites Machado Filho
2020-01-10 22:27                         ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
2020-01-16 16:37                           ` Carlos O'Donell
2020-01-17  3:56                             ` Siddhesh Poyarekar
2020-01-17 12:39                               ` Tulio Magno Quites Machado Filho
2020-01-17 17:14                                 ` Florian Weimer
2020-01-17 17:15                                 ` Joseph Myers
2020-01-17 22:28                                   ` Tulio Magno Quites Machado Filho
2020-01-17 23:45                           ` [PATCH] powerpc32: Fix syntax error in __GLRO macro Andreas Schwab
2020-01-17 23:56                             ` Tulio Magno Quites Machado Filho
2020-07-23 14:47                           ` [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro Joseph Myers
2020-07-23 16:46                             ` Andreas Schwab
2020-07-31 12:42                             ` Florian Weimer via Libc-alpha
2020-07-31 18:35                               ` Joseph Myers
2020-08-03  8:15                                 ` Florian Weimer via Libc-alpha
2020-08-03 16:07                                   ` Carlos O'Donell via Libc-alpha
2020-01-16 16:21                         ` [PATCH 1/2] powerpc: Initialize rtld_global_ro for static dlopen [BZ #20802] Carlos O'Donell
2020-01-16 16:25                           ` Siddhesh Poyarekar
2020-01-09 18:56                     ` [PATCH] powerpc: Move cache line size to rtld_global_ro Tulio Magno Quites Machado Filho
2020-01-10  9:38                 ` Florian Weimer
2020-01-10 16:09                   ` Shawn Landden
2020-01-10 18:19                     ` Adhemerval Zanella
2019-11-22 16:58 ` powerpc build failure with GCC mainline -fno-common change Tulio Magno Quites Machado Filho
2019-11-23 13:23   ` Florian Weimer
2019-11-26 13:26     ` Tulio Magno Quites Machado Filho

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).