This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Support profiling PIE [BZ #22284]
- From: Carlos O'Donell <carlos at redhat dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Wed, 11 Oct 2017 19:47:14 -0700
- Subject: Re: [PATCH] Support profiling PIE [BZ #22284]
- Authentication-results: sourceware.org; auth=none
- References: <20171012021950.GA21344@gmail.com>
On 10/11/2017 07:19 PM, H.J. Lu wrote:
> Since PIE can be loaded at any address, we need to subtract load address
> from PCs.
>
> Tested on i686 and x86-64. OK for master?
OK, with one added comment, see below.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> H.J.
> ---
> [BZ #22284]
> * gmon/Makefile [$(have-fpie)$(build-shared) == yesyes] (tests,
> tests-pie): Add tst-gmon-pie.
> (CFLAGS-tst-gmon-pie.c): New.
> (CRT-tst-gmon-pie): Likewise.
> (tst-gmon-pie-ENV): Likewise.
> [$(have-fpie)$(build-shared) == yesyes] (tests-special): Likewise.
> ($(objpfx)tst-gmon-pie.out): Likewise.
> (clean-tst-gmon-pie-data): Likewise.
> ($(objpfx)tst-gmon-pie-gprof.out): Likewise.
> * gmon/gmon.c [PIC]: Include <link.h>.
> [PIC] (callback): New function.
> (write_hist): Add an argument for load address. Subtract load
> address from PCs.
> (write_call_graph): Likewise.
> (write_gmon): Call __dl_iterate_phdr to get load address, pass
> it to write_hist and write_call_graph.
> ---
> gmon/Makefile | 21 +++++++++++++++++++++
> gmon/gmon.c | 44 ++++++++++++++++++++++++++++++++++----------
> gmon/tst-gmon-pie.c | 1 +
> 3 files changed, 56 insertions(+), 10 deletions(-)
> create mode 100644 gmon/tst-gmon-pie.c
>
> diff --git a/gmon/Makefile b/gmon/Makefile
> index 79e29d188f..2cd077dece 100644
> --- a/gmon/Makefile
> +++ b/gmon/Makefile
> @@ -33,6 +33,11 @@ tests-static += tst-profile-static
> LDFLAGS-tst-profile-static = -profile
> endif
>
> +ifeq (yesyes,$(have-fpie)$(build-shared))
> +tests += tst-gmon-pie
> +tests-pie += tst-gmon-pie
> +endif
OK.
> +
> # The mcount code won't work without a frame pointer.
> CFLAGS-mcount.c := -fno-omit-frame-pointer
>
> @@ -44,6 +49,14 @@ ifeq ($(run-built-tests),yes)
> tests-special += $(objpfx)tst-gmon-gprof.out
> endif
>
> +CFLAGS-tst-gmon-pie.c := $(PIE-ccflag) -fno-omit-frame-pointer -pg
> +CRT-tst-gmon-pie := $(csu-objpfx)gcrt1.o
> +tst-gmon-pie-ENV := GMON_OUT_PREFIX=$(objpfx)tst-gmon-pie.data
> +ifeq ($(run-built-tests),yes)
> +tests-special += $(objpfx)tst-gmon-pie-gprof.out
> +endif
OK.
> +
> +
> include ../Rules
>
> # We cannot compile mcount.c with -pg because that would
> @@ -69,3 +82,11 @@ clean-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)
> +
> +$(objpfx)tst-gmon-pie.out: clean-tst-gmon-pie-data
> +clean-tst-gmon-pie-data:
> + rm -f $(objpfx)tst-gmon-pie.data.*
> +
> +$(objpfx)tst-gmon-pie-gprof.out: tst-gmon-gprof.sh $(objpfx)tst-gmon-pie.out
> + $(SHELL) $< $(GPROF) $(objpfx)tst-gmon-pie $(objpfx)tst-gmon-pie.data.* > $@; \
> + $(evaluate-test)
OK.
> diff --git a/gmon/gmon.c b/gmon/gmon.c
> index f1aa3b776c..5f569cc994 100644
> --- a/gmon/gmon.c
> +++ b/gmon/gmon.c
> @@ -46,6 +46,23 @@
> #include <libc-internal.h>
> #include <not-cancel.h>
>
> +#ifdef PIC
> +# include <link.h>
> +
> +static int
> +callback (struct dl_phdr_info *info, size_t size, void *data)
> +{
> + u_long *load_address = data;
> +
Does the dl_iterate_phdr ABI mandate taht the application istelf
is always an empty string?
You should add a comment to that effect here if you believe that
to be true.
> + if (info->dlpi_name[0] == '\0')
> + {
> + *load_address = (u_long) info->dlpi_addr;
> + return 1;
> + }
> +
> + return 0;
> +}
> +#endif
>
> /* Head of basic-block list or NULL. */
> struct __bb *__bb_head attribute_hidden;
> @@ -63,8 +80,8 @@ static int s_scale;
> void moncontrol (int mode);
> void __moncontrol (int mode);
> libc_hidden_proto (__moncontrol)
> -static void write_hist (int fd);
> -static void write_call_graph (int fd);
> +static void write_hist (int fd, u_long load_address);
> +static void write_call_graph (int fd, u_long load_address);
OK.
> static void write_bb_counts (int fd);
>
> /*
> @@ -173,7 +190,7 @@ weak_alias (__monstartup, monstartup)
>
>
> static void
> -write_hist (int fd)
> +write_hist (int fd, u_long load_address)
> {
> u_char tag = GMON_TAG_TIME_HIST;
>
> @@ -210,8 +227,8 @@ write_hist (int fd)
> != offsetof (struct gmon_hist_hdr, dimen_abbrev)))
> abort ();
>
> - thdr.low_pc = (char *) _gmonparam.lowpc;
> - thdr.high_pc = (char *) _gmonparam.highpc;
> + thdr.low_pc = (char *) _gmonparam.lowpc - load_address;
> + thdr.high_pc = (char *) _gmonparam.highpc - load_address;
OK.
> thdr.hist_size = _gmonparam.kcountsize / sizeof (HISTCOUNTER);
> thdr.prof_rate = __profile_frequency ();
> strncpy (thdr.dimen, "seconds", sizeof (thdr.dimen));
> @@ -223,7 +240,7 @@ write_hist (int fd)
>
>
> static void
> -write_call_graph (int fd)
> +write_call_graph (int fd, u_long load_address)
> {
> #define NARCS_PER_WRITEV 32
> u_char tag = GMON_TAG_CG_ARC;
> @@ -266,8 +283,9 @@ write_call_graph (int fd)
> }
> arc;
>
> - arc.frompc = (char *) frompc;
> - arc.selfpc = (char *) _gmonparam.tos[to_index].selfpc;
> + arc.frompc = (char *) frompc - load_address;
> + arc.selfpc = ((char *) _gmonparam.tos[to_index].selfpc
> + - load_address);
OK.
> arc.count = _gmonparam.tos[to_index].count;
> memcpy (raw_arc + nfilled, &arc, sizeof (raw_arc [0]));
>
> @@ -376,11 +394,17 @@ write_gmon (void)
> memset (ghdr.spare, '\0', sizeof (ghdr.spare));
> __write_nocancel (fd, &ghdr, sizeof (struct gmon_hdr));
>
> + /* Get load_address to profile PIE. */
> + u_long load_address = 0;
> +#ifdef PIC
> + __dl_iterate_phdr (callback, &load_address);
> +#endif
OK.
> +
> /* write PC histogram: */
> - write_hist (fd);
> + write_hist (fd, load_address);
OK.
>
> /* write call-graph: */
> - write_call_graph (fd);
> + write_call_graph (fd, load_address);
OK.
>
> /* write basic-block execution counts: */
> write_bb_counts (fd);
> diff --git a/gmon/tst-gmon-pie.c b/gmon/tst-gmon-pie.c
> new file mode 100644
> index 0000000000..1eef2583b6
> --- /dev/null
> +++ b/gmon/tst-gmon-pie.c
> @@ -0,0 +1 @@
> +#include "tst-gmon.c"
> -- 2.13.6
--
Cheers,
Carlos.