unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / code / Atom feed
* [PATCH v2] Place ENTRY_POINT in .text.unlikely section [BZ #28153]
@ 2021-07-31 15:13 H.J. Lu via Libc-alpha
  2021-07-31 16:36 ` Florian Weimer via Libc-alpha
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu via Libc-alpha @ 2021-07-31 15:13 UTC (permalink / raw)
  To: libc-alpha

Glibc assumes that ENTRY_POINT is the lowest address for which we need
to keep profiling records and BFD linker uses a linker script to place
the input sections.

Since GCC 4.6, the main function is placed in .text.startup section and
since binutils 2.22, BFD linker with

commit add44f8d5c5c05e08b11e033127a744d61c26aee
Author: Alan Modra <amodra@gmail.com>
Date:   Thu Nov 25 03:03:02 2010 +0000

            * scripttempl/elf.sc: Group .text.exit, text.startup and .text.hot
            sections.

places .text.startup section before .text section.

Because ENTRY_POINT in gcrt1.o, which is passed to the linker first, is
placed in .text section, linker may place the main function below
ENTRY_POINT, which leaves the main function out of profiling records.

Place ENTRY_POINT in .text.unlikely section so that when GNU binutils
2.20 or newer with

commit 4c4fb5dac57a7cc4704fffb1f2fc11634dccc833
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Sep 4 06:35:29 2009 +0000

            * scripttempl/elf.sc (.text): Add cold text sections.

is used, BFD linker places ENTRY_POINT at the lowest address.  This
fixes [BZ #28153].

Tested on Linux/x86-64, Linux/x32 and Linux/i686 as well as with
build-many-glibcs.py.
---
 gmon/tst-gmon-gprof.sh            | 2 ++
 gmon/tst-gmon-static-gprof.sh     | 2 ++
 sysdeps/aarch64/start.S           | 2 +-
 sysdeps/alpha/start.S             | 2 +-
 sysdeps/arc/start.S               | 1 +
 sysdeps/arm/start.S               | 2 +-
 sysdeps/csky/abiv2/start.S        | 2 +-
 sysdeps/hppa/start.S              | 2 +-
 sysdeps/i386/start.S              | 1 +
 sysdeps/ia64/start.S              | 1 +
 sysdeps/m68k/start.S              | 2 +-
 sysdeps/microblaze/start.S        | 2 +-
 sysdeps/mips/start.S              | 2 +-
 sysdeps/nios2/start.S             | 2 +-
 sysdeps/powerpc/powerpc32/start.S | 2 +-
 sysdeps/powerpc/powerpc64/start.S | 2 +-
 sysdeps/riscv/start.S             | 1 +
 sysdeps/s390/s390-32/start.S      | 2 +-
 sysdeps/s390/s390-64/start.S      | 2 +-
 sysdeps/sh/start.S                | 2 +-
 sysdeps/sparc/sparc32/start.S     | 2 +-
 sysdeps/sparc/sparc64/start.S     | 2 +-
 sysdeps/x86_64/start.S            | 1 +
 23 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/gmon/tst-gmon-gprof.sh b/gmon/tst-gmon-gprof.sh
index 9d371582b9..dc0be02110 100644
--- a/gmon/tst-gmon-gprof.sh
+++ b/gmon/tst-gmon-gprof.sh
@@ -39,12 +39,14 @@ trap cleanup 0
 cat > "$expected" <<EOF
 f1 2000
 f2 1000
+f3 1
 EOF
 
 # Special version for powerpc with function descriptors.
 cat > "$expected_dot" <<EOF
 .f1 2000
 .f2 1000
+.f3 1
 EOF
 
 "$GPROF" -C "$program" "$data" \
diff --git a/gmon/tst-gmon-static-gprof.sh b/gmon/tst-gmon-static-gprof.sh
index 79218df967..4cc99c80d0 100644
--- a/gmon/tst-gmon-static-gprof.sh
+++ b/gmon/tst-gmon-static-gprof.sh
@@ -39,6 +39,7 @@ trap cleanup 0
 cat > "$expected" <<EOF
 f1 2000
 f2 1000
+f3 1
 main 1
 EOF
 
@@ -46,6 +47,7 @@ EOF
 cat > "$expected_dot" <<EOF
 .f1 2000
 .f2 1000
+.f3 1
 .main 1
 EOF
 
diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
index 417da8802b..e46e01ed0b 100644
--- a/sysdeps/aarch64/start.S
+++ b/sysdeps/aarch64/start.S
@@ -42,7 +42,7 @@
 					NULL
  */
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 ENTRY(_start)
 	/* Create an initial frame with 0 LR and FP */
 	cfi_undefined (x30)
diff --git a/sysdeps/alpha/start.S b/sysdeps/alpha/start.S
index 65dcd4d392..6658a39813 100644
--- a/sysdeps/alpha/start.S
+++ b/sysdeps/alpha/start.S
@@ -36,7 +36,7 @@
 
 #include <sysdep.h>
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.align 3
 	.globl _start
 	.ent _start, 0
diff --git a/sysdeps/arc/start.S b/sysdeps/arc/start.S
index 5302a57cab..aca293faf9 100644
--- a/sysdeps/arc/start.S
+++ b/sysdeps/arc/start.S
@@ -33,6 +33,7 @@
         env[0...N]      environment variables (pointers)
         NULL.  */
 
+	.section .text.unlikely,"ax",%progbits
 ENTRY (ENTRY_POINT)
 
 	/* Needed to make gdb backtraces stop here.  */
diff --git a/sysdeps/arm/start.S b/sysdeps/arm/start.S
index 9b56bc0cca..4b0b098bb6 100644
--- a/sysdeps/arm/start.S
+++ b/sysdeps/arm/start.S
@@ -69,7 +69,7 @@
 	.syntax unified
 #endif
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.globl _start
 	.type _start,#function
 _start:
diff --git a/sysdeps/csky/abiv2/start.S b/sysdeps/csky/abiv2/start.S
index a565cfa87b..1061da75da 100644
--- a/sysdeps/csky/abiv2/start.S
+++ b/sysdeps/csky/abiv2/start.S
@@ -41,7 +41,7 @@
 
 #include <sysdep.h>
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.globl _start;
 	.type _start,@function;
 	.align 4;
diff --git a/sysdeps/hppa/start.S b/sysdeps/hppa/start.S
index 4a1877f8e8..f1164aa030 100644
--- a/sysdeps/hppa/start.S
+++ b/sysdeps/hppa/start.S
@@ -51,7 +51,7 @@
 .Lp__libc_start_main:
 	.word P%__libc_start_main
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.align 4
 	.globl _start
 	.export _start, ENTRY
diff --git a/sysdeps/i386/start.S b/sysdeps/i386/start.S
index 5296b27e65..ee97f22a27 100644
--- a/sysdeps/i386/start.S
+++ b/sysdeps/i386/start.S
@@ -54,6 +54,7 @@
 
 #include <sysdep.h>
 
+	.section .text.unlikely,"ax",%progbits
 ENTRY (_start)
 	/* Clearing frame pointer is insufficient, use CFI.  */
 	cfi_undefined (eip)
diff --git a/sysdeps/ia64/start.S b/sysdeps/ia64/start.S
index b28f8cb429..6079e67345 100644
--- a/sysdeps/ia64/start.S
+++ b/sysdeps/ia64/start.S
@@ -48,6 +48,7 @@
  *	out6:	stack_end
  */
 
+	.section .text.unlikely,"ax",%progbits
 	.align 32
 	.global _start
 
diff --git a/sysdeps/m68k/start.S b/sysdeps/m68k/start.S
index 98da4db9f3..a2bdac5c44 100644
--- a/sysdeps/m68k/start.S
+++ b/sysdeps/m68k/start.S
@@ -54,7 +54,7 @@
 
 #include <sysdep.h>
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.globl _start
 	.type _start,@function
 _start:
diff --git a/sysdeps/microblaze/start.S b/sysdeps/microblaze/start.S
index 6589bd4dc7..9db90bf4bd 100644
--- a/sysdeps/microblaze/start.S
+++ b/sysdeps/microblaze/start.S
@@ -33,7 +33,7 @@
    License along with the GNU C Library.  If not, see
    <https://www.gnu.org/licenses/>.  */
 
-    .text
+	.section .text.unlikely,"ax",%progbits
     .globl _start
     .type _start,@function
 _start:
diff --git a/sysdeps/mips/start.S b/sysdeps/mips/start.S
index 4ec42a2a7f..c29b87d32f 100644
--- a/sysdeps/mips/start.S
+++ b/sysdeps/mips/start.S
@@ -71,7 +71,7 @@
 		      void (*rtld_fini) (void), void *stack_end)
 */
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.globl ENTRY_POINT
 	.type ENTRY_POINT,@function
 #ifndef __mips16
diff --git a/sysdeps/nios2/start.S b/sysdeps/nios2/start.S
index 7c9696977f..c963c37476 100644
--- a/sysdeps/nios2/start.S
+++ b/sysdeps/nios2/start.S
@@ -65,7 +65,7 @@
 	value, terminated by an AT_NULL tag.
 */
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.globl _start
 	.type _start,%function
 _start:
diff --git a/sysdeps/powerpc/powerpc32/start.S b/sysdeps/powerpc/powerpc32/start.S
index 39ce1a18ff..1ed3669c07 100644
--- a/sysdeps/powerpc/powerpc32/start.S
+++ b/sysdeps/powerpc/powerpc32/start.S
@@ -56,7 +56,7 @@ L(start_addresses):
 	.long 	0 /* Used to be fini.  */
 	ASM_SIZE_DIRECTIVE(L(start_addresses))
 
-	.section ".text"
+	.section .text.unlikely,"ax",%progbits
 ENTRY(_start)
  /* Save the stack pointer, in case we're statically linked under Linux.  */
 	mr	r9,r1
diff --git a/sysdeps/powerpc/powerpc64/start.S b/sysdeps/powerpc/powerpc64/start.S
index 71c0c67926..f1597164c4 100644
--- a/sysdeps/powerpc/powerpc64/start.S
+++ b/sysdeps/powerpc/powerpc64/start.S
@@ -61,7 +61,7 @@ L(start_addresses):
 	.section	".toc","aw"
 .L01:
 	.tc	L(start_addresses)[TC],L(start_addresses)
-	.section ".text"
+	.section .text.unlikely,"ax",%progbits
 ENTRY (_start)
  /* Save the stack pointer, in case we're statically linked under Linux.  */
 	mr	r9,r1
diff --git a/sysdeps/riscv/start.S b/sysdeps/riscv/start.S
index 806f6aacd6..64b8c2699c 100644
--- a/sysdeps/riscv/start.S
+++ b/sysdeps/riscv/start.S
@@ -42,6 +42,7 @@
    a0 contains the address of a function to be passed to atexit.
    __libc_start_main wants this in a5.  */
 
+	.section .text.unlikely,"ax",%progbits
 ENTRY (ENTRY_POINT)
 	/* Terminate call stack by noting ra is undefined.  Use a dummy
 	   .cfi_label to force starting the FDE.  */
diff --git a/sysdeps/s390/s390-32/start.S b/sysdeps/s390/s390-32/start.S
index b6cfa4caf3..734c32505f 100644
--- a/sysdeps/s390/s390-32/start.S
+++ b/sysdeps/s390/s390-32/start.S
@@ -55,7 +55,7 @@
 					NULL
 */
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.globl _start
 	.type _start,@function
 _start:
diff --git a/sysdeps/s390/s390-64/start.S b/sysdeps/s390/s390-64/start.S
index 4e6526308a..473576a456 100644
--- a/sysdeps/s390/s390-64/start.S
+++ b/sysdeps/s390/s390-64/start.S
@@ -55,7 +55,7 @@
 					NULL
 */
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.globl _start
 	.type _start,@function
 _start:
diff --git a/sysdeps/sh/start.S b/sysdeps/sh/start.S
index 606ee59222..f929b5d51b 100644
--- a/sysdeps/sh/start.S
+++ b/sysdeps/sh/start.S
@@ -57,7 +57,7 @@
 					NULL
 */
 
-	.text
+	.section .text.unlikely,"ax",%progbits
 	.globl _start
 	.type _start,@function
 _start:
diff --git a/sysdeps/sparc/sparc32/start.S b/sysdeps/sparc/sparc32/start.S
index 00bf898fb9..2f6467832f 100644
--- a/sysdeps/sparc/sparc32/start.S
+++ b/sysdeps/sparc/sparc32/start.S
@@ -37,7 +37,7 @@
 #include <sysdep.h>
 
 
-	.section ".text"
+	.section .text.unlikely,"ax",%progbits
 	.align 4
 	.global _start
 	.type _start,#function
diff --git a/sysdeps/sparc/sparc64/start.S b/sysdeps/sparc/sparc64/start.S
index 8520717eba..9f7e94d719 100644
--- a/sysdeps/sparc/sparc64/start.S
+++ b/sysdeps/sparc/sparc64/start.S
@@ -37,7 +37,7 @@
 #include <sysdep.h>
 
 
-	.section ".text"
+	.section .text.unlikely,"ax",%progbits
 	.align 4
 	.global _start
 	.type _start,#function
diff --git a/sysdeps/x86_64/start.S b/sysdeps/x86_64/start.S
index 1b3e36826b..5f61764093 100644
--- a/sysdeps/x86_64/start.S
+++ b/sysdeps/x86_64/start.S
@@ -55,6 +55,7 @@
 
 #include <sysdep.h>
 
+	.section .text.unlikely,"ax",%progbits
 ENTRY (_start)
 	/* Clearing frame pointer is insufficient, use CFI.  */
 	cfi_undefined (rip)
-- 
2.31.1


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

* Re: [PATCH v2] Place ENTRY_POINT in .text.unlikely section [BZ #28153]
  2021-07-31 15:13 [PATCH v2] Place ENTRY_POINT in .text.unlikely section [BZ #28153] H.J. Lu via Libc-alpha
@ 2021-07-31 16:36 ` Florian Weimer via Libc-alpha
  2021-07-31 17:06   ` H.J. Lu via Libc-alpha
  0 siblings, 1 reply; 10+ messages in thread
From: Florian Weimer via Libc-alpha @ 2021-07-31 16:36 UTC (permalink / raw)
  To: H.J. Lu via Libc-alpha

* H. J. Lu via Libc-alpha:

> diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
> index 417da8802b..e46e01ed0b 100644
> --- a/sysdeps/aarch64/start.S
> +++ b/sysdeps/aarch64/start.S
> @@ -42,7 +42,7 @@
>  					NULL
>   */
>  
> -	.text
> +	.section .text.unlikely,"ax",%progbits
>  ENTRY(_start)
>  	/* Create an initial frame with 0 LR and FP */
>  	cfi_undefined (x30)

I don't think it's correct to place code that runs during every process
start into .text.unlikely.  Surely we can avoid that page fault.

Can we fix the ENTRY_POINT assumption in profiling instead?

Thanks,
Florian


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

* Re: [PATCH v2] Place ENTRY_POINT in .text.unlikely section [BZ #28153]
  2021-07-31 16:36 ` Florian Weimer via Libc-alpha
@ 2021-07-31 17:06   ` H.J. Lu via Libc-alpha
  2021-07-31 17:09     ` H.J. Lu via Libc-alpha
  2021-07-31 17:11     ` Florian Weimer via Libc-alpha
  0 siblings, 2 replies; 10+ messages in thread
From: H.J. Lu via Libc-alpha @ 2021-07-31 17:06 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha

On Sat, Jul 31, 2021 at 9:36 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
> > index 417da8802b..e46e01ed0b 100644
> > --- a/sysdeps/aarch64/start.S
> > +++ b/sysdeps/aarch64/start.S
> > @@ -42,7 +42,7 @@
> >                                       NULL
> >   */
> >
> > -     .text
> > +     .section .text.unlikely,"ax",%progbits
> >  ENTRY(_start)
> >       /* Create an initial frame with 0 LR and FP */
> >       cfi_undefined (x30)
>
> I don't think it's correct to place code that runs during every process
> start into .text.unlikely.  Surely we can avoid that page fault.
>
> Can we fix the ENTRY_POINT assumption in profiling instead?

We can do

diff --git a/csu/gmon-start.c b/csu/gmon-start.c
index b3432885b3..83322fd586 100644
--- a/csu/gmon-start.c
+++ b/csu/gmon-start.c
@@ -48,7 +48,7 @@
 #ifdef ENTRY_POINT_DECL
 ENTRY_POINT_DECL(extern)
 #else
-extern char ENTRY_POINT[];
+extern char entry_point[] asm (__SYMBOL_PREFIX "main");
 #endif
 extern char etext[];

@@ -56,7 +56,7 @@ extern char etext[];
 # ifdef ENTRY_POINT_DECL
 #  define TEXT_START ENTRY_POINT
 # else
-#  define TEXT_START &ENTRY_POINT
+#  define TEXT_START &entry_point
 # endif
 #endif

But this may only work with BFD linker which places .text.startup
section before .text section.

-- 
H.J.

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

* Re: [PATCH v2] Place ENTRY_POINT in .text.unlikely section [BZ #28153]
  2021-07-31 17:06   ` H.J. Lu via Libc-alpha
@ 2021-07-31 17:09     ` H.J. Lu via Libc-alpha
  2021-07-31 17:11     ` Florian Weimer via Libc-alpha
  1 sibling, 0 replies; 10+ messages in thread
From: H.J. Lu via Libc-alpha @ 2021-07-31 17:09 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha

On Sat, Jul 31, 2021 at 10:06 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Jul 31, 2021 at 9:36 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu via Libc-alpha:
> >
> > > diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
> > > index 417da8802b..e46e01ed0b 100644
> > > --- a/sysdeps/aarch64/start.S
> > > +++ b/sysdeps/aarch64/start.S
> > > @@ -42,7 +42,7 @@
> > >                                       NULL
> > >   */
> > >
> > > -     .text
> > > +     .section .text.unlikely,"ax",%progbits
> > >  ENTRY(_start)
> > >       /* Create an initial frame with 0 LR and FP */
> > >       cfi_undefined (x30)
> >
> > I don't think it's correct to place code that runs during every process
> > start into .text.unlikely.  Surely we can avoid that page fault.
> >
> > Can we fix the ENTRY_POINT assumption in profiling instead?
>
> We can do
>
> diff --git a/csu/gmon-start.c b/csu/gmon-start.c
> index b3432885b3..83322fd586 100644
> --- a/csu/gmon-start.c
> +++ b/csu/gmon-start.c
> @@ -48,7 +48,7 @@
>  #ifdef ENTRY_POINT_DECL
>  ENTRY_POINT_DECL(extern)
>  #else
> -extern char ENTRY_POINT[];
> +extern char entry_point[] asm (__SYMBOL_PREFIX "main");
>  #endif
>  extern char etext[];
>
> @@ -56,7 +56,7 @@ extern char etext[];
>  # ifdef ENTRY_POINT_DECL
>  #  define TEXT_START ENTRY_POINT
>  # else
> -#  define TEXT_START &ENTRY_POINT
> +#  define TEXT_START &entry_point
>  # endif
>  #endif
>
> But this may only work with BFD linker which places .text.startup
> section before .text section.

Another option is to place _start in .text.startup which leaves

    *(.text.unlikely .text.*_unlikely .text.unlikely.*)
    *(.text.exit .text.exit.*)

sections out of profiling records.

-- 
H.J.

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

* Re: [PATCH v2] Place ENTRY_POINT in .text.unlikely section [BZ #28153]
  2021-07-31 17:06   ` H.J. Lu via Libc-alpha
  2021-07-31 17:09     ` H.J. Lu via Libc-alpha
@ 2021-07-31 17:11     ` Florian Weimer via Libc-alpha
  2021-07-31 17:34       ` Andreas Schwab
  2021-07-31 17:43       ` [PATCH v3] Use __executable_start as the lowest address for profiling " H.J. Lu via Libc-alpha
  1 sibling, 2 replies; 10+ messages in thread
From: Florian Weimer via Libc-alpha @ 2021-07-31 17:11 UTC (permalink / raw)
  To: H.J. Lu; +Cc: H.J. Lu via Libc-alpha

* H. J. Lu:

> On Sat, Jul 31, 2021 at 9:36 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu via Libc-alpha:
>>
>> > diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
>> > index 417da8802b..e46e01ed0b 100644
>> > --- a/sysdeps/aarch64/start.S
>> > +++ b/sysdeps/aarch64/start.S
>> > @@ -42,7 +42,7 @@
>> >                                       NULL
>> >   */
>> >
>> > -     .text
>> > +     .section .text.unlikely,"ax",%progbits
>> >  ENTRY(_start)
>> >       /* Create an initial frame with 0 LR and FP */
>> >       cfi_undefined (x30)
>>
>> I don't think it's correct to place code that runs during every process
>> start into .text.unlikely.  Surely we can avoid that page fault.
>>
>> Can we fix the ENTRY_POINT assumption in profiling instead?
>
> We can do
>
> diff --git a/csu/gmon-start.c b/csu/gmon-start.c
> index b3432885b3..83322fd586 100644
> --- a/csu/gmon-start.c
> +++ b/csu/gmon-start.c
> @@ -48,7 +48,7 @@
>  #ifdef ENTRY_POINT_DECL
>  ENTRY_POINT_DECL(extern)
>  #else
> -extern char ENTRY_POINT[];
> +extern char entry_point[] asm (__SYMBOL_PREFIX "main");
>  #endif
>  extern char etext[];
>
> @@ -56,7 +56,7 @@ extern char etext[];
>  # ifdef ENTRY_POINT_DECL
>  #  define TEXT_START ENTRY_POINT
>  # else
> -#  define TEXT_START &ENTRY_POINT
> +#  define TEXT_START &entry_point
>  # endif
>  #endif
>
> But this may only work with BFD linker which places .text.startup
> section before .text section.

Can we get the linker to emit a symbol at the start of the text section?
Like it does for orphan sections?

Then we can use a weak symbol reference in gmon-start.c and use the new
symbol if it is available.

Thanks,
Florian


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

* Re: [PATCH v2] Place ENTRY_POINT in .text.unlikely section [BZ #28153]
  2021-07-31 17:11     ` Florian Weimer via Libc-alpha
@ 2021-07-31 17:34       ` Andreas Schwab
  2021-07-31 17:43       ` [PATCH v3] Use __executable_start as the lowest address for profiling " H.J. Lu via Libc-alpha
  1 sibling, 0 replies; 10+ messages in thread
From: Andreas Schwab @ 2021-07-31 17:34 UTC (permalink / raw)
  To: Florian Weimer via Libc-alpha; +Cc: Florian Weimer

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

> Can we get the linker to emit a symbol at the start of the text section?

There is __executable_start at the start of the text segment.

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] 10+ messages in thread

* [PATCH v3] Use __executable_start as the lowest address for profiling [BZ #28153]
  2021-07-31 17:11     ` Florian Weimer via Libc-alpha
  2021-07-31 17:34       ` Andreas Schwab
@ 2021-07-31 17:43       ` H.J. Lu via Libc-alpha
  1 sibling, 0 replies; 10+ messages in thread
From: H.J. Lu via Libc-alpha @ 2021-07-31 17:43 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha

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

On Sat, Jul 31, 2021 at 10:11 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Sat, Jul 31, 2021 at 9:36 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu via Libc-alpha:
> >>
> >> > diff --git a/sysdeps/aarch64/start.S b/sysdeps/aarch64/start.S
> >> > index 417da8802b..e46e01ed0b 100644
> >> > --- a/sysdeps/aarch64/start.S
> >> > +++ b/sysdeps/aarch64/start.S
> >> > @@ -42,7 +42,7 @@
> >> >                                       NULL
> >> >   */
> >> >
> >> > -     .text
> >> > +     .section .text.unlikely,"ax",%progbits
> >> >  ENTRY(_start)
> >> >       /* Create an initial frame with 0 LR and FP */
> >> >       cfi_undefined (x30)
> >>
> >> I don't think it's correct to place code that runs during every process
> >> start into .text.unlikely.  Surely we can avoid that page fault.
> >>
> >> Can we fix the ENTRY_POINT assumption in profiling instead?
> >
> > We can do
> >
> > diff --git a/csu/gmon-start.c b/csu/gmon-start.c
> > index b3432885b3..83322fd586 100644
> > --- a/csu/gmon-start.c
> > +++ b/csu/gmon-start.c
> > @@ -48,7 +48,7 @@
> >  #ifdef ENTRY_POINT_DECL
> >  ENTRY_POINT_DECL(extern)
> >  #else
> > -extern char ENTRY_POINT[];
> > +extern char entry_point[] asm (__SYMBOL_PREFIX "main");
> >  #endif
> >  extern char etext[];
> >
> > @@ -56,7 +56,7 @@ extern char etext[];
> >  # ifdef ENTRY_POINT_DECL
> >  #  define TEXT_START ENTRY_POINT
> >  # else
> > -#  define TEXT_START &ENTRY_POINT
> > +#  define TEXT_START &entry_point
> >  # endif
> >  #endif
> >
> > But this may only work with BFD linker which places .text.startup
> > section before .text section.
>
> Can we get the linker to emit a symbol at the start of the text section?

We already have __executable_start which is pretty close to what we
need.   Like this.

> Like it does for orphan sections?
>
> Then we can use a weak symbol reference in gmon-start.c and use the new
> symbol if it is available.
>
> Thanks,
> Florian
>


-- 
H.J.

[-- Attachment #2: v3-0001-Use-__executable_start-as-the-lowest-address-for-.patch --]
[-- Type: application/x-patch, Size: 3003 bytes --]

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

* Re: [PATCH v3] Use __executable_start as the lowest address for profiling [BZ #28153]
  2021-08-06  0:16 ` Fangrui Song via Libc-alpha
@ 2021-08-06  7:21   ` Fangrui Song via Libc-alpha
  0 siblings, 0 replies; 10+ messages in thread
From: Fangrui Song via Libc-alpha @ 2021-08-06  7:21 UTC (permalink / raw)
  To: H.J. Lu; +Cc: libc-alpha

On 2021-08-05, Fangrui Song wrote:
>
>On 2021-08-05, H.J. Lu via Libc-alpha wrote:
>>Glibc assumes that ENTRY_POINT is the lowest address for which we need
>>to keep profiling records and BFD linker uses a linker script to place
>>the input sections.
>>
>>Starting from GCC 4.6, the main function is placed in .text.startup
>>section and starting from binutils 2.22, BFD linker with
>>
>>commit add44f8d5c5c05e08b11e033127a744d61c26aee
>>Author: Alan Modra <amodra@gmail.com>
>>Date:   Thu Nov 25 03:03:02 2010 +0000
>>
>>           * scripttempl/elf.sc: Group .text.exit, text.startup and .text.hot
>>           sections.
>>
>>places .text.startup section before .text section, which leave the main
>>function out of profiling records.
>>
>>Starting from binutils 2.15, linker provides __executable_start to mark
>>the lowest address of the executable.  Use __executable_start as the
>>lowest address to keep the main function in profiling records. This fixes
>>[BZ #28153].
>>
>>Tested on Linux/x86-64, Linux/x32 and Linux/i686 as well as with
>>build-many-glibcs.py.
>>---
>>csu/gmon-start.c              | 10 +++++++++-
>>gmon/tst-gmon-gprof.sh        |  2 ++
>>gmon/tst-gmon-static-gprof.sh |  2 ++
>>3 files changed, 13 insertions(+), 1 deletion(-)
>
>LGTM

[I had not subscribed the list until few days ago]

Comment to v2: gold/ld.lld don't necessarily place .text.unlikely
before other text sections.
GNU ld's fixed section ordering may make certain optimization difficult.

RISC architectures typically create range extension thunks to overcome the
limitation of short range branches. Hot code can usually be moved to the
middle to increase the chance that one range extension thunk can be
shared by more code.

8MiB cold
2MiB hot
8MiB cold

(
.text.sorted.* is a recent addition emulation link-time layout for
improving instruction cache and TLB. An optimized case may require the
sections to be interleaved with others.)

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

* Re: [PATCH v3] Use __executable_start as the lowest address for profiling [BZ #28153]
  2021-08-05 12:09 H.J. Lu via Libc-alpha
@ 2021-08-06  0:16 ` Fangrui Song via Libc-alpha
  2021-08-06  7:21   ` Fangrui Song via Libc-alpha
  0 siblings, 1 reply; 10+ messages in thread
From: Fangrui Song via Libc-alpha @ 2021-08-06  0:16 UTC (permalink / raw)
  To: H.J. Lu; +Cc: libc-alpha


On 2021-08-05, H.J. Lu via Libc-alpha wrote:
>Glibc assumes that ENTRY_POINT is the lowest address for which we need
>to keep profiling records and BFD linker uses a linker script to place
>the input sections.
>
>Starting from GCC 4.6, the main function is placed in .text.startup
>section and starting from binutils 2.22, BFD linker with
>
>commit add44f8d5c5c05e08b11e033127a744d61c26aee
>Author: Alan Modra <amodra@gmail.com>
>Date:   Thu Nov 25 03:03:02 2010 +0000
>
>            * scripttempl/elf.sc: Group .text.exit, text.startup and .text.hot
>            sections.
>
>places .text.startup section before .text section, which leave the main
>function out of profiling records.
>
>Starting from binutils 2.15, linker provides __executable_start to mark
>the lowest address of the executable.  Use __executable_start as the
>lowest address to keep the main function in profiling records. This fixes
>[BZ #28153].
>
>Tested on Linux/x86-64, Linux/x32 and Linux/i686 as well as with
>build-many-glibcs.py.
>---
> csu/gmon-start.c              | 10 +++++++++-
> gmon/tst-gmon-gprof.sh        |  2 ++
> gmon/tst-gmon-static-gprof.sh |  2 ++
> 3 files changed, 13 insertions(+), 1 deletion(-)

LGTM

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

* [PATCH v3] Use __executable_start as the lowest address for profiling [BZ #28153]
@ 2021-08-05 12:09 H.J. Lu via Libc-alpha
  2021-08-06  0:16 ` Fangrui Song via Libc-alpha
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu via Libc-alpha @ 2021-08-05 12:09 UTC (permalink / raw)
  To: libc-alpha

Glibc assumes that ENTRY_POINT is the lowest address for which we need
to keep profiling records and BFD linker uses a linker script to place
the input sections.

Starting from GCC 4.6, the main function is placed in .text.startup
section and starting from binutils 2.22, BFD linker with

commit add44f8d5c5c05e08b11e033127a744d61c26aee
Author: Alan Modra <amodra@gmail.com>
Date:   Thu Nov 25 03:03:02 2010 +0000

            * scripttempl/elf.sc: Group .text.exit, text.startup and .text.hot
            sections.

places .text.startup section before .text section, which leave the main
function out of profiling records.

Starting from binutils 2.15, linker provides __executable_start to mark
the lowest address of the executable.  Use __executable_start as the
lowest address to keep the main function in profiling records. This fixes
[BZ #28153].

Tested on Linux/x86-64, Linux/x32 and Linux/i686 as well as with
build-many-glibcs.py.
---
 csu/gmon-start.c              | 10 +++++++++-
 gmon/tst-gmon-gprof.sh        |  2 ++
 gmon/tst-gmon-static-gprof.sh |  2 ++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/csu/gmon-start.c b/csu/gmon-start.c
index b3432885b3..344606a676 100644
--- a/csu/gmon-start.c
+++ b/csu/gmon-start.c
@@ -52,6 +52,11 @@ extern char ENTRY_POINT[];
 #endif
 extern char etext[];
 
+/* Use __executable_start as the lowest address to keep profiling records
+   if it provided by the linker.  */
+extern const char executable_start[] asm ("__executable_start")
+  __attribute__ ((weak, visibility ("hidden")));
+
 #ifndef TEXT_START
 # ifdef ENTRY_POINT_DECL
 #  define TEXT_START ENTRY_POINT
@@ -92,7 +97,10 @@ __gmon_start__ (void)
   called = 1;
 
   /* Start keeping profiling records.  */
-  __monstartup ((u_long) TEXT_START, (u_long) &etext);
+  if (&executable_start != NULL)
+    __monstartup ((u_long) &executable_start, (u_long) &etext);
+  else
+    __monstartup ((u_long) TEXT_START, (u_long) &etext);
 
   /* Call _mcleanup before exiting; it will write out gmon.out from the
      collected data.  */
diff --git a/gmon/tst-gmon-gprof.sh b/gmon/tst-gmon-gprof.sh
index 9d371582b9..dc0be02110 100644
--- a/gmon/tst-gmon-gprof.sh
+++ b/gmon/tst-gmon-gprof.sh
@@ -39,12 +39,14 @@ trap cleanup 0
 cat > "$expected" <<EOF
 f1 2000
 f2 1000
+f3 1
 EOF
 
 # Special version for powerpc with function descriptors.
 cat > "$expected_dot" <<EOF
 .f1 2000
 .f2 1000
+.f3 1
 EOF
 
 "$GPROF" -C "$program" "$data" \
diff --git a/gmon/tst-gmon-static-gprof.sh b/gmon/tst-gmon-static-gprof.sh
index 79218df967..4cc99c80d0 100644
--- a/gmon/tst-gmon-static-gprof.sh
+++ b/gmon/tst-gmon-static-gprof.sh
@@ -39,6 +39,7 @@ trap cleanup 0
 cat > "$expected" <<EOF
 f1 2000
 f2 1000
+f3 1
 main 1
 EOF
 
@@ -46,6 +47,7 @@ EOF
 cat > "$expected_dot" <<EOF
 .f1 2000
 .f2 1000
+.f3 1
 .main 1
 EOF
 
-- 
2.31.1


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

end of thread, other threads:[~2021-08-06  7:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-31 15:13 [PATCH v2] Place ENTRY_POINT in .text.unlikely section [BZ #28153] H.J. Lu via Libc-alpha
2021-07-31 16:36 ` Florian Weimer via Libc-alpha
2021-07-31 17:06   ` H.J. Lu via Libc-alpha
2021-07-31 17:09     ` H.J. Lu via Libc-alpha
2021-07-31 17:11     ` Florian Weimer via Libc-alpha
2021-07-31 17:34       ` Andreas Schwab
2021-07-31 17:43       ` [PATCH v3] Use __executable_start as the lowest address for profiling " H.J. Lu via Libc-alpha
2021-08-05 12:09 H.J. Lu via Libc-alpha
2021-08-06  0:16 ` Fangrui Song via Libc-alpha
2021-08-06  7:21   ` Fangrui Song via Libc-alpha

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/glibc.git

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