[PATCH] Test for profiling support (_mcount/gprof)

[PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

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

The attached patch adds an integration test for gcc -pg, _mcount/gmon,
and gprof.

It passes on aarch64, ppc, ppc64, ppc64le, s390, s390x, x86_64, and on
i386 with the _mcount internal_function replacement applied.

The test is compiled with -pg and is linked against gcrt1.o, via the new
CRT-* makefile hook.  As far as I can tell, it is properly isolated from
system glibc (only GCC objects are picked up).

Thanks,
Florian

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gmon.patch --]
[-- Type: text/x-patch; name="gmon.patch", Size: 7104 bytes --]

gmon: Add test for basic mcount/gprof functionality

2017-08-15  Florian Weimer  <fweimer@redhat.com>

	* gmon/Makefile (tests): Add tst-gmon.
	(CFLAGS-tst-gmon.c, LDFLAGS-tst-gmon, CRT-tst-gmon, tst-gmon-ENV):
	Set.
	(tests-special): Add tst-gmon-prof.out.
	(tst-gmon.out): Depend on clean-tst-gmon-data.
	(clean-tst-gmon-data, tst-gmon-gprof.out): New targets.
	* gmon/tst-gmon.c, gmon/tst-gmon-gprof.sh: New files.
	* Makeconfig (+link-before-libc): Add CRT-* hook to override the
	startup object.
	* aclocal.m4 (GPROF): Set and substitute.
	* config.amke.in (GPROF): Set.
	* configure: Regenerate.

diff --git a/Makeconfig b/Makeconfig
index 80aed2a987..19b559701a 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -465,7 +465,8 @@ else  # not build-pie-default
 +link-before-libc = $(CC) -nostdlib -nostartfiles -o $@ \
 	      $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
 	      $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
-	      $(addprefix $(csu-objpfx),$(start-installed-name)) \
+	      $(or $(CRT-$(@F)), $ \
+		$(addprefix $(csu-objpfx),$(start-installed-name))) \
 	      $(+preinit) $(+prector) \
 	      $(filter-out $(addprefix $(csu-objpfx),start.o \
 						     $(start-installed-name))\
diff --git a/aclocal.m4 b/aclocal.m4
index 69021558af..fe2a3713cc 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -121,6 +121,8 @@ OBJDUMP=`$CC -print-prog-name=objdump`
 AC_SUBST(OBJDUMP)
 OBJCOPY=`$CC -print-prog-name=objcopy`
 AC_SUBST(OBJCOPY)
+GPROF=`$CC -print-prog-name=gprof`
+AC_SUBST(GPROF)
 
 # Determine whether we are using GNU binutils.
 AC_CACHE_CHECK(whether $AS is GNU as, libc_cv_prog_as_gnu,
diff --git a/config.make.in b/config.make.in
index 7eff1daf6a..ea7a42cc19 100644
--- a/config.make.in
+++ b/config.make.in
@@ -120,6 +120,7 @@ BISON = @BISON@
 AUTOCONF = @AUTOCONF@
 OBJDUMP = @OBJDUMP@
 OBJCOPY = @OBJCOPY@
+GPROF = @GPROF@
 READELF = @READELF@
 
 # Installation tools.
diff --git a/configure b/configure
index e6a54d7841..5cb5210107 100755
--- a/configure
+++ b/configure
@@ -651,6 +651,7 @@ MSGFMT
 MAKE
 LD
 AS
+GPROF
 OBJCOPY
 OBJDUMP
 AR
@@ -4572,6 +4573,8 @@ OBJDUMP=`$CC -print-prog-name=objdump`
 
 OBJCOPY=`$CC -print-prog-name=objcopy`
 
+GPROF=`$CC -print-prog-name=gprof`
+
 
 # Determine whether we are using GNU binutils.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $AS is GNU as" >&5
diff --git a/gmon/Makefile b/gmon/Makefile
index 6ff4cb0dfb..947e6b5905 100644
--- a/gmon/Makefile
+++ b/gmon/Makefile
@@ -27,7 +27,7 @@ routines := gmon mcount profil sprofil bb_init_func bb_exit_func prof-freq
 
 elide-routines.os = bb_init_func bb_exit_func
 
-tests	= tst-sprofil
+tests	= tst-sprofil tst-gmon
 ifeq ($(build-profile),yes)
 tests	+= tst-profile-static
 tests-static	+= tst-profile-static
@@ -38,6 +38,12 @@ endif
 # The mcount code won't work without a frame pointer.
 CFLAGS-mcount.c := -fno-omit-frame-pointer
 
+CFLAGS-tst-gmon.c := -pg
+LDFLAGS-tst-gmon := $(no-pie-ldflag)
+CRT-tst-gmon := $(csu-objpfx)gcrt1.o
+tst-gmon-ENV := GMON_OUT_PREFIX=$(objpfx)tst-gmon.data
+tests-special += $(objpfx)tst-gmon-gprof.out
+
 include ../Rules
 
 # We cannot compile mcount.c with -pg because that would
@@ -53,3 +59,13 @@ endif
 $(noprof:%=$(objpfx)%.op): %.op: %.o
 	rm -f $@
 	ln $< $@
+
+# GMON_OUTPUT_PREFIX only sets the output prefix.  The actual file
+# name contains the PID as well.
+$(objpfx)tst-gmon.out: clean-tst-gmon-data
+clean-tst-gmon-data:
+	rm -f $(objpfx)tst-gmon.data.*
+
+$(objpfx)tst-gmon-gprof.out: tst-gmon-gprof.sh $(objpfx)tst-gmon.out
+	$(SHELL) $< $(GPROF) $(objpfx)tst-gmon $(objpfx)tst-gmon.data.* > $@; \
+	$(evaluate-test)
diff --git a/gmon/tst-gmon-gprof.sh b/gmon/tst-gmon-gprof.sh
new file mode 100644
index 0000000000..b10a6e0763
--- /dev/null
+++ b/gmon/tst-gmon-gprof.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+# Check the output of gprof against a carfully crafted binary.
+# Copyright (C) 2017 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
+# <http://www.gnu.org/licenses/>.
+
+LC_ALL=C
+export LC_ALL
+set -e
+exec 2>&1
+
+GPROF="$1"
+program="$2"
+data="$3"
+
+actual=$(mktemp)
+expected=$(mktemp)
+expected_dot=$(mktemp)
+cleanup () {
+    rm -f "$actual"
+    rm -f "$expected_dot"
+}
+trap cleanup 0
+
+cat > "$expected" <<EOF
+f1 2000
+f2 1000
+EOF
+
+# Special version for powerpc with function descriptors.
+cat > "$expected_dot" <<EOF
+.f1 2000
+.f2 1000
+EOF
+
+"$GPROF" -C "$program" "$data" \
+    | awk -F  '[(): ]' '/executions/{print $5, $8}' \
+    | sort > "$actual"
+
+if cmp -s "$actual" "$expected_dot" \
+   || diff -u --label expected "$expected" --label actual "$actual" ; then
+    echo "PASS"
+else
+    echo "FAIL"
+    exit 1
+fi
diff --git a/gmon/tst-gmon.c b/gmon/tst-gmon.c
new file mode 100644
index 0000000000..ce81aa41b8
--- /dev/null
+++ b/gmon/tst-gmon.c
@@ -0,0 +1,50 @@
+/* Test program for profiling information collection (_mcount/gprof).
+   Copyright (C) 2017 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
+   <http://www.gnu.org/licenses/>.  */
+
+/* This program does not use the test harness because we want tight
+   control over the call graph.  */
+
+__attribute__ ((noinline, noclone, weak)) void
+f1 (void)
+{
+}
+
+__attribute__ ((noinline, noclone, weak)) void
+f2 (void)
+{
+  f1 ();
+  /* Prevent tail call.  */
+  asm volatile ("");
+}
+
+__attribute__ ((noinline, noclone, weak)) void
+f3 (int count)
+{
+  for (int i = 0; i < count; ++i)
+    {
+      f1 ();
+      f2 ();
+    }
+}
+
+int
+main (void)
+{
+  f3 (1000);
+  return 0;
+}

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:

> diff --git a/Makeconfig b/Makeconfig
> index 80aed2a987..19b559701a 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -465,7 +465,8 @@ else  # not build-pie-default
>  +link-before-libc = $(CC) -nostdlib -nostartfiles -o $@ \
>  	      $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
>  	      $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
> -	      $(addprefix $(csu-objpfx),$(start-installed-name)) \
> +	      $(or $(CRT-$(@F)), $ \
> +		$(addprefix $(csu-objpfx),$(start-installed-name))) \

$(or) is a 3.81 feature, we only require 3.79.  You can use $(firstword)
instead.  The lone $ looks like a typo.

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 08/15/2017 02:28 PM, Andreas Schwab wrote:
> On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> diff --git a/Makeconfig b/Makeconfig
>> index 80aed2a987..19b559701a 100644
>> --- a/Makeconfig
>> +++ b/Makeconfig
>> @@ -465,7 +465,8 @@ else  # not build-pie-default
>>  +link-before-libc = $(CC) -nostdlib -nostartfiles -o $@ \
>>  	      $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
>>  	      $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
>> -	      $(addprefix $(csu-objpfx),$(start-installed-name)) \
>> +	      $(or $(CRT-$(@F)), $ \
>> +		$(addprefix $(csu-objpfx),$(start-installed-name))) \
> 
> $(or) is a 3.81 feature, we only require 3.79.  You can use $(firstword)
> instead.

Oh, good point.  $(firstword …) does not really align with the
subsequent $(addprefix …) usage because that has the implication that
$(start-installed-name) contains multiple words.  We can turn

  $(addprefix $(csu-objpfx),$(start-installed-name))

into

  $(csu-objpfx)$(start-installed-name)

though; it should work on all in-tree architectures.  Then $(firstword
…) should have the right semantics.

But can we require make 3.81 instead?  It was released in April 2006,
after all.  That does not mean we'll start using controversial 3.81
features such as $(eval …).

> The lone $ looks like a typo.

Right.  Fixed.

Thanks,
Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:

> On 08/15/2017 02:28 PM, Andreas Schwab wrote:
>> On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:
>> 
>>> diff --git a/Makeconfig b/Makeconfig
>>> index 80aed2a987..19b559701a 100644
>>> --- a/Makeconfig
>>> +++ b/Makeconfig
>>> @@ -465,7 +465,8 @@ else  # not build-pie-default
>>>  +link-before-libc = $(CC) -nostdlib -nostartfiles -o $@ \
>>>  	      $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
>>>  	      $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
>>> -	      $(addprefix $(csu-objpfx),$(start-installed-name)) \
>>> +	      $(or $(CRT-$(@F)), $ \
>>> +		$(addprefix $(csu-objpfx),$(start-installed-name))) \
>> 
>> $(or) is a 3.81 feature, we only require 3.79.  You can use $(firstword)
>> instead.
>
> Oh, good point.  $(firstword …) does not really align with the
> subsequent $(addprefix …) usage because that has the implication that
> $(start-installed-name) contains multiple words.

Other uses already require $(start-installed-name) to be a single word.
A lot of places use addprefix when not really needed.

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

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

On 08/15/2017 02:56 PM, Andreas Schwab wrote:
> On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 08/15/2017 02:28 PM, Andreas Schwab wrote:
>>> On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>>> diff --git a/Makeconfig b/Makeconfig
>>>> index 80aed2a987..19b559701a 100644
>>>> --- a/Makeconfig
>>>> +++ b/Makeconfig
>>>> @@ -465,7 +465,8 @@ else  # not build-pie-default
>>>>  +link-before-libc = $(CC) -nostdlib -nostartfiles -o $@ \
>>>>  	      $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
>>>>  	      $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
>>>> -	      $(addprefix $(csu-objpfx),$(start-installed-name)) \
>>>> +	      $(or $(CRT-$(@F)), $ \
>>>> +		$(addprefix $(csu-objpfx),$(start-installed-name))) \
>>>
>>> $(or) is a 3.81 feature, we only require 3.79.  You can use $(firstword)
>>> instead.
>>
>> Oh, good point.  $(firstword …) does not really align with the
>> subsequent $(addprefix …) usage because that has the implication that
>> $(start-installed-name) contains multiple words.
> 
> Other uses already require $(start-installed-name) to be a single word.
> A lot of places use addprefix when not really needed.

Okay.  What about the attached patch?  It uses $(firstword …) without
the comma.

Thanks,
Florian

[-- Attachment #2: gmon-2.patch --]
[-- Type: text/x-patch, Size: 6851 bytes --]

gmon: Add test for basic mcount/gprof functionality

2017-08-15  Florian Weimer  <fweimer@redhat.com>

	* gmon/Makefile (tests): Add tst-gmon.
	(CFLAGS-tst-gmon.c, LDFLAGS-tst-gmon, CRT-tst-gmon, tst-gmon-ENV):
	Set.
	(tests-special): Add tst-gmon-prof.out.
	(tst-gmon.out): Depend on clean-tst-gmon-data.
	(clean-tst-gmon-data, tst-gmon-gprof.out): New targets.
	* gmon/tst-gmon.c, gmon/tst-gmon-gprof.sh: New files.
	* Makeconfig (+link-before-libc): Add CRT-* hook to override the
	startup object.
	* aclocal.m4 (GPROF): Set and substitute.
	* config.amke.in (GPROF): Set.
	* configure: Regenerate.

diff --git a/Makeconfig b/Makeconfig
index 80aed2a987..5f2469116c 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -465,7 +465,7 @@ else  # not build-pie-default
 +link-before-libc = $(CC) -nostdlib -nostartfiles -o $@ \
 	      $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
 	      $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
-	      $(addprefix $(csu-objpfx),$(start-installed-name)) \
+	      $(firstword $(CRT-$(@F)) $(csu-objpfx)$(start-installed-name)) \
 	      $(+preinit) $(+prector) \
 	      $(filter-out $(addprefix $(csu-objpfx),start.o \
 						     $(start-installed-name))\
diff --git a/aclocal.m4 b/aclocal.m4
index 69021558af..fe2a3713cc 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -121,6 +121,8 @@ OBJDUMP=`$CC -print-prog-name=objdump`
 AC_SUBST(OBJDUMP)
 OBJCOPY=`$CC -print-prog-name=objcopy`
 AC_SUBST(OBJCOPY)
+GPROF=`$CC -print-prog-name=gprof`
+AC_SUBST(GPROF)
 
 # Determine whether we are using GNU binutils.
 AC_CACHE_CHECK(whether $AS is GNU as, libc_cv_prog_as_gnu,
diff --git a/config.make.in b/config.make.in
index 7eff1daf6a..ea7a42cc19 100644
--- a/config.make.in
+++ b/config.make.in
@@ -120,6 +120,7 @@ BISON = @BISON@
 AUTOCONF = @AUTOCONF@
 OBJDUMP = @OBJDUMP@
 OBJCOPY = @OBJCOPY@
+GPROF = @GPROF@
 READELF = @READELF@
 
 # Installation tools.
diff --git a/configure b/configure
index e6a54d7841..5cb5210107 100755
--- a/configure
+++ b/configure
@@ -651,6 +651,7 @@ MSGFMT
 MAKE
 LD
 AS
+GPROF
 OBJCOPY
 OBJDUMP
 AR
@@ -4572,6 +4573,8 @@ OBJDUMP=`$CC -print-prog-name=objdump`
 
 OBJCOPY=`$CC -print-prog-name=objcopy`
 
+GPROF=`$CC -print-prog-name=gprof`
+
 
 # Determine whether we are using GNU binutils.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $AS is GNU as" >&5
diff --git a/gmon/Makefile b/gmon/Makefile
index 6ff4cb0dfb..947e6b5905 100644
--- a/gmon/Makefile
+++ b/gmon/Makefile
@@ -27,7 +27,7 @@ routines := gmon mcount profil sprofil bb_init_func bb_exit_func prof-freq
 
 elide-routines.os = bb_init_func bb_exit_func
 
-tests	= tst-sprofil
+tests	= tst-sprofil tst-gmon
 ifeq ($(build-profile),yes)
 tests	+= tst-profile-static
 tests-static	+= tst-profile-static
@@ -38,6 +38,12 @@ endif
 # The mcount code won't work without a frame pointer.
 CFLAGS-mcount.c := -fno-omit-frame-pointer
 
+CFLAGS-tst-gmon.c := -pg
+LDFLAGS-tst-gmon := $(no-pie-ldflag)
+CRT-tst-gmon := $(csu-objpfx)gcrt1.o
+tst-gmon-ENV := GMON_OUT_PREFIX=$(objpfx)tst-gmon.data
+tests-special += $(objpfx)tst-gmon-gprof.out
+
 include ../Rules
 
 # We cannot compile mcount.c with -pg because that would
@@ -53,3 +59,13 @@ endif
 $(noprof:%=$(objpfx)%.op): %.op: %.o
 	rm -f $@
 	ln $< $@
+
+# GMON_OUTPUT_PREFIX only sets the output prefix.  The actual file
+# name contains the PID as well.
+$(objpfx)tst-gmon.out: clean-tst-gmon-data
+clean-tst-gmon-data:
+	rm -f $(objpfx)tst-gmon.data.*
+
+$(objpfx)tst-gmon-gprof.out: tst-gmon-gprof.sh $(objpfx)tst-gmon.out
+	$(SHELL) $< $(GPROF) $(objpfx)tst-gmon $(objpfx)tst-gmon.data.* > $@; \
+	$(evaluate-test)
diff --git a/gmon/tst-gmon-gprof.sh b/gmon/tst-gmon-gprof.sh
new file mode 100644
index 0000000000..b10a6e0763
--- /dev/null
+++ b/gmon/tst-gmon-gprof.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+# Check the output of gprof against a carfully crafted binary.
+# Copyright (C) 2017 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
+# <http://www.gnu.org/licenses/>.
+
+LC_ALL=C
+export LC_ALL
+set -e
+exec 2>&1
+
+GPROF="$1"
+program="$2"
+data="$3"
+
+actual=$(mktemp)
+expected=$(mktemp)
+expected_dot=$(mktemp)
+cleanup () {
+    rm -f "$actual"
+    rm -f "$expected_dot"
+}
+trap cleanup 0
+
+cat > "$expected" <<EOF
+f1 2000
+f2 1000
+EOF
+
+# Special version for powerpc with function descriptors.
+cat > "$expected_dot" <<EOF
+.f1 2000
+.f2 1000
+EOF
+
+"$GPROF" -C "$program" "$data" \
+    | awk -F  '[(): ]' '/executions/{print $5, $8}' \
+    | sort > "$actual"
+
+if cmp -s "$actual" "$expected_dot" \
+   || diff -u --label expected "$expected" --label actual "$actual" ; then
+    echo "PASS"
+else
+    echo "FAIL"
+    exit 1
+fi
diff --git a/gmon/tst-gmon.c b/gmon/tst-gmon.c
new file mode 100644
index 0000000000..ce81aa41b8
--- /dev/null
+++ b/gmon/tst-gmon.c
@@ -0,0 +1,50 @@
+/* Test program for profiling information collection (_mcount/gprof).
+   Copyright (C) 2017 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
+   <http://www.gnu.org/licenses/>.  */
+
+/* This program does not use the test harness because we want tight
+   control over the call graph.  */
+
+__attribute__ ((noinline, noclone, weak)) void
+f1 (void)
+{
+}
+
+__attribute__ ((noinline, noclone, weak)) void
+f2 (void)
+{
+  f1 ();
+  /* Prevent tail call.  */
+  asm volatile ("");
+}
+
+__attribute__ ((noinline, noclone, weak)) void
+f3 (int count)
+{
+  for (int i = 0; i < count; ++i)
+    {
+      f1 ();
+      f2 ();
+    }
+}
+
+int
+main (void)
+{
+  f3 (1000);
+  return 0;
+}

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:

> diff --git a/gmon/tst-gmon-gprof.sh b/gmon/tst-gmon-gprof.sh
> new file mode 100644
> index 0000000000..b10a6e0763
> --- /dev/null
> +++ b/gmon/tst-gmon-gprof.sh
> @@ -0,0 +1,59 @@
> +#!/bin/sh
> +# Check the output of gprof against a carfully crafted binary.
> +# Copyright (C) 2017 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
> +# <http://www.gnu.org/licenses/>.
> +
> +LC_ALL=C
> +export LC_ALL
> +set -e
> +exec 2>&1
> +
> +GPROF="$1"
> +program="$2"
> +data="$3"
> +
> +actual=$(mktemp)
> +expected=$(mktemp)
> +expected_dot=$(mktemp)
> +cleanup () {
> +    rm -f "$actual"
> +    rm -f "$expected_dot"
> +}
> +trap cleanup 0

That doesn't clean $expected.

Ok with that change.

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 08/15/2017 03:34 PM, Andreas Schwab wrote:
> On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> diff --git a/gmon/tst-gmon-gprof.sh b/gmon/tst-gmon-gprof.sh
>> new file mode 100644
>> index 0000000000..b10a6e0763
>> --- /dev/null
>> +++ b/gmon/tst-gmon-gprof.sh
>> @@ -0,0 +1,59 @@
>> +#!/bin/sh
>> +# Check the output of gprof against a carfully crafted binary.
>> +# Copyright (C) 2017 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
>> +# <http://www.gnu.org/licenses/>.
>> +
>> +LC_ALL=C
>> +export LC_ALL
>> +set -e
>> +exec 2>&1
>> +
>> +GPROF="$1"
>> +program="$2"
>> +data="$3"
>> +
>> +actual=$(mktemp)
>> +expected=$(mktemp)
>> +expected_dot=$(mktemp)
>> +cleanup () {
>> +    rm -f "$actual"
>> +    rm -f "$expected_dot"
>> +}
>> +trap cleanup 0
> 
> That doesn't clean $expected.
> 
> Ok with that change.

Thanks, pushed with a fix.

Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Joseph Myers]

The new test in tests-special needs to be conditional on 
run-built-tests=yes; right now it makes run-built-tests=no cross tests, as 
with build-many-glibcs.py, fail as it tries to run tst-gmon natively.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

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

On 08/15/2017 06:32 PM, Joseph Myers wrote:
> The new test in tests-special needs to be conditional on 
> run-built-tests=yes; right now it makes run-built-tests=no cross tests, as 
> with build-many-glibcs.py, fail as it tries to run tst-gmon natively.

Oops.  Like this?

Thanks,
Florian

[-- Attachment #2: gmon-3.patch --]
[-- Type: text/x-patch, Size: 583 bytes --]

gmon: Run tst-gmon-prof only for run-built-tests=yes

2017-08-15  Florian Weimer  <fweimer@redhat.com>

	* gmon/Makefile (tests-special): Add tst-gmon-prof only if
	run-built-tests.

diff --git a/gmon/Makefile b/gmon/Makefile
index 947e6b5905..39f11acec2 100644
--- a/gmon/Makefile
+++ b/gmon/Makefile
@@ -42,7 +42,9 @@ CFLAGS-tst-gmon.c := -pg
 LDFLAGS-tst-gmon := $(no-pie-ldflag)
 CRT-tst-gmon := $(csu-objpfx)gcrt1.o
 tst-gmon-ENV := GMON_OUT_PREFIX=$(objpfx)tst-gmon.data
+ifeq ($(run-built-tests),yes)
 tests-special += $(objpfx)tst-gmon-gprof.out
+endif
 
 include ../Rules
 

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Joseph Myers]

On Tue, 15 Aug 2017, Florian Weimer wrote:

> On 08/15/2017 06:32 PM, Joseph Myers wrote:
> > The new test in tests-special needs to be conditional on 
> > run-built-tests=yes; right now it makes run-built-tests=no cross tests, as 
> > with build-many-glibcs.py, fail as it tries to run tst-gmon natively.
> 
> Oops.  Like this?

This patch is OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:

> @@ -38,6 +38,12 @@ endif
>  # The mcount code won't work without a frame pointer.
>  CFLAGS-mcount.c := -fno-omit-frame-pointer
>  
> +CFLAGS-tst-gmon.c := -pg

This fails if CFLAGS contains -fomit-frame-pointer.

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 08/16/2017 08:45 AM, Andreas Schwab wrote:
> On Aug 15 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> @@ -38,6 +38,12 @@ endif
>>  # The mcount code won't work without a frame pointer.
>>  CFLAGS-mcount.c := -fno-omit-frame-pointer
>>  
>> +CFLAGS-tst-gmon.c := -pg
> 
> This fails if CFLAGS contains -fomit-frame-pointer.

Is the problem that -pg and -fomit-frame-pointer is incompatible?

Do you compile with -fomit-frame-pointer because your GCC was built with
--enable-frame-pointer?

Thanks,
Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:glibc/glibc-testsuite/f/i586

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

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

On 08/16/2017 09:43 AM, Andreas Schwab wrote:
> https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:glibc/glibc-testsuite/f/i586

One has to download the whole log file to find this:

[ 2374s] gcc: error: -pg and -fomit-frame-pointer are incompatible

This looks like a GCC deficiency.  Anyway, please try the attached patch.

Thanks,
Florian

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gmon-4.patch --]
[-- Type: text/x-patch; name="gmon-4.patch", Size: 860 bytes --]

tst-gmon: Build with -fno-omit-frame-pointer

If glibc is built with -fomit-frame-pointer to undo the effect of
configuring GCC with --enable-frame-pointer, using -pg by itself results
in a build failure:

gcc: error: -pg and -fomit-frame-pointer are incompatible

2017-08-16  Florian Weimer  <fweimer@redhat.com>

	* gmon/Makefile (CFLAGS-tst-gmon.c): Add -fno-omit-frame-pointer.

diff --git a/gmon/Makefile b/gmon/Makefile
index 39f11acec2..62d96543ff 100644
--- a/gmon/Makefile
+++ b/gmon/Makefile
@@ -38,7 +38,7 @@ endif
 # The mcount code won't work without a frame pointer.
 CFLAGS-mcount.c := -fno-omit-frame-pointer
 
-CFLAGS-tst-gmon.c := -pg
+CFLAGS-tst-gmon.c := -fno-omit-frame-pointer -pg
 LDFLAGS-tst-gmon := $(no-pie-ldflag)
 CRT-tst-gmon := $(csu-objpfx)gcrt1.o
 tst-gmon-ENV := GMON_OUT_PREFIX=$(objpfx)tst-gmon.data

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Szabolcs Nagy]

On 16/08/17 09:09, Florian Weimer wrote:
> [ 2374s] gcc: error: -pg and -fomit-frame-pointer are incompatible
> 
> This looks like a GCC deficiency.  Anyway, please try the attached patch.

for the record:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69690

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:

> tst-gmon: Build with -fno-omit-frame-pointer
>
> If glibc is built with -fomit-frame-pointer to undo the effect of
> configuring GCC with --enable-frame-pointer, using -pg by itself results
> in a build failure:

This works, but what does this have to do with --enable-frame-pointer?

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 08/16/2017 05:05 PM, Andreas Schwab wrote:
> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> tst-gmon: Build with -fno-omit-frame-pointer
>>
>> If glibc is built with -fomit-frame-pointer to undo the effect of
>> configuring GCC with --enable-frame-pointer, using -pg by itself results
>> in a build failure:
> 
> This works, but what does this have to do with --enable-frame-pointer?

If you don't use --enable-frame-pointer for GCC, there's no apparent
reason to me to build glibc with --fomit-frame-pointer because it's the
default (albeit in a fashion that is compatible with -pg).

Thanks,
Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:

> If you don't use --enable-frame-pointer for GCC, there's no apparent
> reason to me to build glibc with --fomit-frame-pointer because it's the
> default (albeit in a fashion that is compatible with -pg).

That is not true.

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 08/16/2017 05:20 PM, Andreas Schwab wrote:
> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> If you don't use --enable-frame-pointer for GCC, there's no apparent
>> reason to me to build glibc with --fomit-frame-pointer because it's the
>> default (albeit in a fashion that is compatible with -pg).
> 
> That is not true.

It is currently true—I don't see why anyone would want to build glibc
this way.  Could you enlighten me?

Thanks,
Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:

> On 08/16/2017 05:20 PM, Andreas Schwab wrote:
>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>> 
>>> If you don't use --enable-frame-pointer for GCC, there's no apparent
>>> reason to me to build glibc with --fomit-frame-pointer because it's the
>>> default (albeit in a fashion that is compatible with -pg).
>> 
>> That is not true.
>
> It is currently true—I don't see why anyone would want to build glibc
> this way.  Could you enlighten me?

To omit the frame pointer.

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 08/16/2017 05:30 PM, Andreas Schwab wrote:
> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 08/16/2017 05:20 PM, Andreas Schwab wrote:
>>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>>> If you don't use --enable-frame-pointer for GCC, there's no apparent
>>>> reason to me to build glibc with --fomit-frame-pointer because it's the
>>>> default (albeit in a fashion that is compatible with -pg).
>>>
>>> That is not true.
>>
>> It is currently true—I don't see why anyone would want to build glibc
>> this way.  Could you enlighten me?
> 
> To omit the frame pointer.

But GCC does it by default if you don't build with
--enable-frame-pointer, at least on i386, which is the architecture for
which you reported the failure.

Thanks,
Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[H.J. Lu]

On Wed, Aug 16, 2017 at 8:32 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/16/2017 05:30 PM, Andreas Schwab wrote:
>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>
>>> On 08/16/2017 05:20 PM, Andreas Schwab wrote:
>>>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>>
>>>>> If you don't use --enable-frame-pointer for GCC, there's no apparent
>>>>> reason to me to build glibc with --fomit-frame-pointer because it's the
>>>>> default (albeit in a fashion that is compatible with -pg).
>>>>
>>>> That is not true.
>>>
>>> It is currently true—I don't see why anyone would want to build glibc
>>> this way.  Could you enlighten me?
>>
>> To omit the frame pointer.
>
> But GCC does it by default if you don't build with
> --enable-frame-pointer, at least on i386, which is the architecture for
> which you reported the failure.

gmon/tst-gmon-gprof also fails with GCC defaulting to PIE.


-- 
H.J.

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 08/16/2017 10:03 PM, H.J. Lu wrote:
> On Wed, Aug 16, 2017 at 8:32 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 08/16/2017 05:30 PM, Andreas Schwab wrote:
>>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>>> On 08/16/2017 05:20 PM, Andreas Schwab wrote:
>>>>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>
>>>>>> If you don't use --enable-frame-pointer for GCC, there's no apparent
>>>>>> reason to me to build glibc with --fomit-frame-pointer because it's the
>>>>>> default (albeit in a fashion that is compatible with -pg).
>>>>>
>>>>> That is not true.
>>>>
>>>> It is currently true—I don't see why anyone would want to build glibc
>>>> this way.  Could you enlighten me?
>>>
>>> To omit the frame pointer.
>>
>> But GCC does it by default if you don't build with
>> --enable-frame-pointer, at least on i386, which is the architecture for
>> which you reported the failure.
> 
> gmon/tst-gmon-gprof also fails with GCC defaulting to PIE.

That would be a real failure (due to ASLR).  Does linking with -pg
ordinarily disable PIE?  I suppose we could add -pg to LDFLAGS for this
test because as Andreas explained earlier, it won't link in the system
profiling libraries.

Thanks,
Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[H.J. Lu]

On Wed, Aug 16, 2017 at 2:04 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/16/2017 10:03 PM, H.J. Lu wrote:
>> On Wed, Aug 16, 2017 at 8:32 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 08/16/2017 05:30 PM, Andreas Schwab wrote:
>>>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>>
>>>>> On 08/16/2017 05:20 PM, Andreas Schwab wrote:
>>>>>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>>
>>>>>>> If you don't use --enable-frame-pointer for GCC, there's no apparent
>>>>>>> reason to me to build glibc with --fomit-frame-pointer because it's the
>>>>>>> default (albeit in a fashion that is compatible with -pg).
>>>>>>
>>>>>> That is not true.
>>>>>
>>>>> It is currently true—I don't see why anyone would want to build glibc
>>>>> this way.  Could you enlighten me?
>>>>
>>>> To omit the frame pointer.
>>>
>>> But GCC does it by default if you don't build with
>>> --enable-frame-pointer, at least on i386, which is the architecture for
>>> which you reported the failure.
>>
>> gmon/tst-gmon-gprof also fails with GCC defaulting to PIE.
>
> That would be a real failure (due to ASLR).  Does linking with -pg
> ordinarily disable PIE?  I suppose we could add -pg to LDFLAGS for this
> test because as Andreas explained earlier, it won't link in the system
> profiling libraries.
>

-pg and -pie are mutually exclusive since they need different crt1.os,
gcrrt1.o vs Scrt1.o.   This patch works for me:

diff --git a/Makeconfig b/Makeconfig
index 5f2469116c..b51904b797 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -409,7 +409,7 @@ ifndef +link-pie
 +link-pie-before-libc = $(CC) -pie -Wl,-O1 -nostdlib -nostartfiles -o $@ \
        $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
        $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
-       $(addprefix $(csu-objpfx),S$(start-installed-name)) \
+       $(firstword $(CRT-$(@F)) $(csu-objpfx)S$(start-installed-name)) \
        $(+preinit) $(+prectorS) \
        $(filter-out $(addprefix $(csu-objpfx),start.o \
                      S$(start-installed-name))\


-- 
H.J.

Re: [PATCH] Test for profiling support (_mcount/gprof)

[H.J. Lu]

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

On Wed, Aug 16, 2017 at 2:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Aug 16, 2017 at 2:04 PM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 08/16/2017 10:03 PM, H.J. Lu wrote:
>>> On Wed, Aug 16, 2017 at 8:32 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>> On 08/16/2017 05:30 PM, Andreas Schwab wrote:
>>>>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>
>>>>>> On 08/16/2017 05:20 PM, Andreas Schwab wrote:
>>>>>>> On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>>>>>>
>>>>>>>> If you don't use --enable-frame-pointer for GCC, there's no apparent
>>>>>>>> reason to me to build glibc with --fomit-frame-pointer because it's the
>>>>>>>> default (albeit in a fashion that is compatible with -pg).
>>>>>>>
>>>>>>> That is not true.
>>>>>>
>>>>>> It is currently true—I don't see why anyone would want to build glibc
>>>>>> this way.  Could you enlighten me?
>>>>>
>>>>> To omit the frame pointer.
>>>>
>>>> But GCC does it by default if you don't build with
>>>> --enable-frame-pointer, at least on i386, which is the architecture for
>>>> which you reported the failure.
>>>
>>> gmon/tst-gmon-gprof also fails with GCC defaulting to PIE.
>>
>> That would be a real failure (due to ASLR).  Does linking with -pg
>> ordinarily disable PIE?  I suppose we could add -pg to LDFLAGS for this
>> test because as Andreas explained earlier, it won't link in the system
>> profiling libraries.
>>
>
> -pg and -pie are mutually exclusive since they need different crt1.os,
> gcrrt1.o vs Scrt1.o.   This patch works for me:
>
> diff --git a/Makeconfig b/Makeconfig
> index 5f2469116c..b51904b797 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -409,7 +409,7 @@ ifndef +link-pie
>  +link-pie-before-libc = $(CC) -pie -Wl,-O1 -nostdlib -nostartfiles -o $@ \
>         $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
>         $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
> -       $(addprefix $(csu-objpfx),S$(start-installed-name)) \
> +       $(firstword $(CRT-$(@F)) $(csu-objpfx)S$(start-installed-name)) \
>         $(+preinit) $(+prectorS) \
>         $(filter-out $(addprefix $(csu-objpfx),start.o \
>                       S$(start-installed-name))\
>
>

Tested  with GCC defaulting to PIE.  OK for master?


-- 
H.J.

[-- Attachment #2: 0001-Support-pg-with-GCC-defaulting-to-PIE.patch --]
[-- Type: text/x-patch, Size: 1097 bytes --]

From f19721b98346b9493f3ecb5cd41527e102c7fedf Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 16 Aug 2017 14:15:43 -0700
Subject: [PATCH] Support -pg with GCC defaulting to PIE

-pg and -pie are mutually exclusive since they need different crt1.o,
gcrrt1.o vs Scrt1.o.  Make -pg overrdies -pie.

	* Makeconfig (+link-pie-before-libc): Add CRT-* hook to override
	the startup object.
---
 Makeconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makeconfig b/Makeconfig
index 5f2469116c..b51904b797 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -409,7 +409,7 @@ ifndef +link-pie
 +link-pie-before-libc = $(CC) -pie -Wl,-O1 -nostdlib -nostartfiles -o $@ \
 	     $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
 	     $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
-	     $(addprefix $(csu-objpfx),S$(start-installed-name)) \
+	     $(firstword $(CRT-$(@F)) $(csu-objpfx)S$(start-installed-name)) \
 	     $(+preinit) $(+prectorS) \
 	     $(filter-out $(addprefix $(csu-objpfx),start.o \
 						    S$(start-installed-name))\
-- 
2.13.5

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Aug 16 2017, Florian Weimer <fweimer@redhat.com> wrote:

> But GCC does it by default if you don't build with
> --enable-frame-pointer,

No, it doesn't.  This has no effect in general.

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 08/16/2017 11:19 PM, H.J. Lu wrote:
> -pg and -pie are mutually exclusive since they need different crt1.o,
> gcrrt1.o vs Scrt1.o.  Make -pg overrdies -pie.
> 
> 	* Makeconfig (+link-pie-before-libc): Add CRT-* hook to override
> 	the startup object.

Typo (gcrrt1.o), and the commit message is confusing.  There is no -pg
involved at the linking stage.

gcrt1.o is deliberately compiled so that it is compatible with PIE, so
the patch itself is okay.

Thanks,
Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[H.J. Lu]

On Thu, Aug 17, 2017 at 12:44 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/16/2017 11:19 PM, H.J. Lu wrote:
>> -pg and -pie are mutually exclusive since they need different crt1.o,
>> gcrrt1.o vs Scrt1.o.  Make -pg overrdies -pie.
>>
>>       * Makeconfig (+link-pie-before-libc): Add CRT-* hook to override
>>       the startup object.
>
> Typo (gcrrt1.o), and the commit message is confusing.  There is no -pg
> involved at the linking stage.
>
> gcrt1.o is deliberately compiled so that it is compatible with PIE, so
> the patch itself is okay.

This is what I checked in.

-- 
H.J.
---
From eb13870b4379abf2f15dac8683276e96b83fff4e Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 16 Aug 2017 14:15:43 -0700
Subject: [PATCH] Support mcount/gprof test with GCC defaulting to PIE

The mcount/gprof test should be linked with gcrt1.o, not Scrt1.o.

* Makeconfig (+link-pie-before-libc): Add CRT-* hook to override
the startup object.
---
 Makeconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makeconfig b/Makeconfig
index 5f2469116c..b51904b797 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -409,7 +409,7 @@ ifndef +link-pie
 +link-pie-before-libc = $(CC) -pie -Wl,-O1 -nostdlib -nostartfiles -o $@ \
      $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
      $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
-     $(addprefix $(csu-objpfx),S$(start-installed-name)) \
+     $(firstword $(CRT-$(@F)) $(csu-objpfx)S$(start-installed-name)) \
      $(+preinit) $(+prectorS) \
      $(filter-out $(addprefix $(csu-objpfx),start.o \
     S$(start-installed-name))\
--

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 08/16/2017 10:09 AM, Florian Weimer wrote:
> tst-gmon: Build with -fno-omit-frame-pointer
> 
> If glibc is built with -fomit-frame-pointer to undo the effect of
> configuring GCC with --enable-frame-pointer, using -pg by itself results
> in a build failure:
> 
> gcc: error: -pg and -fomit-frame-pointer are incompatible
> 
> 2017-08-16  Florian Weimer<fweimer@redhat.com>
> 
> 	* gmon/Makefile (CFLAGS-tst-gmon.c): Add -fno-omit-frame-pointer.
> 
> diff --git a/gmon/Makefile b/gmon/Makefile
> index 39f11acec2..62d96543ff 100644
> --- a/gmon/Makefile
> +++ b/gmon/Makefile
> @@ -38,7 +38,7 @@ endif
>   # The mcount code won't work without a frame pointer.
>   CFLAGS-mcount.c := -fno-omit-frame-pointer
>   
> -CFLAGS-tst-gmon.c := -pg
> +CFLAGS-tst-gmon.c := -fno-omit-frame-pointer -pg
>   LDFLAGS-tst-gmon := $(no-pie-ldflag)
>   CRT-tst-gmon := $(csu-objpfx)gcrt1.o
>   tst-gmon-ENV := GMON_OUT_PREFIX=$(objpfx)tst-gmon.data

Is this patch still needed?

Thanks,
Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Okt 05 2017, Florian Weimer <fweimer@redhat.com> wrote:

> On 08/16/2017 10:09 AM, Florian Weimer wrote:
>> tst-gmon: Build with -fno-omit-frame-pointer
>>
>> If glibc is built with -fomit-frame-pointer to undo the effect of
>> configuring GCC with --enable-frame-pointer, using -pg by itself results
>> in a build failure:
>>
>> gcc: error: -pg and -fomit-frame-pointer are incompatible
>>
>> 2017-08-16  Florian Weimer<fweimer@redhat.com>
>>
>> 	* gmon/Makefile (CFLAGS-tst-gmon.c): Add -fno-omit-frame-pointer.
>>
>> diff --git a/gmon/Makefile b/gmon/Makefile
>> index 39f11acec2..62d96543ff 100644
>> --- a/gmon/Makefile
>> +++ b/gmon/Makefile
>> @@ -38,7 +38,7 @@ endif
>>   # The mcount code won't work without a frame pointer.
>>   CFLAGS-mcount.c := -fno-omit-frame-pointer
>>   -CFLAGS-tst-gmon.c := -pg
>> +CFLAGS-tst-gmon.c := -fno-omit-frame-pointer -pg
>>   LDFLAGS-tst-gmon := $(no-pie-ldflag)
>>   CRT-tst-gmon := $(csu-objpfx)gcrt1.o
>>   tst-gmon-ENV := GMON_OUT_PREFIX=$(objpfx)tst-gmon.data
>
> Is this patch still needed?

Sure.

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 10/05/2017 02:19 PM, Andreas Schwab wrote:
> On Okt 05 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 08/16/2017 10:09 AM, Florian Weimer wrote:
>>> tst-gmon: Build with -fno-omit-frame-pointer
>>>
>>> If glibc is built with -fomit-frame-pointer to undo the effect of
>>> configuring GCC with --enable-frame-pointer, using -pg by itself results
>>> in a build failure:
>>>
>>> gcc: error: -pg and -fomit-frame-pointer are incompatible
>>>
>>> 2017-08-16  Florian Weimer<fweimer@redhat.com>
>>>
>>> 	* gmon/Makefile (CFLAGS-tst-gmon.c): Add -fno-omit-frame-pointer.
>>>
>>> diff --git a/gmon/Makefile b/gmon/Makefile
>>> index 39f11acec2..62d96543ff 100644
>>> --- a/gmon/Makefile
>>> +++ b/gmon/Makefile
>>> @@ -38,7 +38,7 @@ endif
>>>    # The mcount code won't work without a frame pointer.
>>>    CFLAGS-mcount.c := -fno-omit-frame-pointer
>>>    -CFLAGS-tst-gmon.c := -pg
>>> +CFLAGS-tst-gmon.c := -fno-omit-frame-pointer -pg
>>>    LDFLAGS-tst-gmon := $(no-pie-ldflag)
>>>    CRT-tst-gmon := $(csu-objpfx)gcrt1.o
>>>    tst-gmon-ENV := GMON_OUT_PREFIX=$(objpfx)tst-gmon.data
>>
>> Is this patch still needed?
> 
> Sure.

Shall I commit it, then?

Thanks,
Florian

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Andreas Schwab]

On Okt 05 2017, Florian Weimer <fweimer@redhat.com> wrote:

> Shall I commit it, then?

Yes, please.

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

Re: [PATCH] Test for profiling support (_mcount/gprof)

[Florian Weimer]

On 10/05/2017 02:31 PM, Andreas Schwab wrote:
> On Okt 05 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> Shall I commit it, then?
> 
> Yes, please.

Done.

Thanks,
Florian