From patchwork Thu Oct 12 10:47:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 23497 Received: (qmail 34950 invoked by alias); 12 Oct 2017 10:47:47 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 34886 invoked by uid 89); 12 Oct 2017 10:47:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-oi0-f68.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=KND71xZKpmrxgQ4u2mynrc1MBlJPOdsrpIe7mkvKhto=; b=Xc/1I1KoJ5zHWZfN7Q1GF9MMlaYDQEkOjYrFuD0OW6cT4ZaYz2xrAAPMBaoPe6kVtn oMXVUEcEwRLcPSsYtYxDkx/cAtII9YiGLIkTRBAEs1d4sPnHh01txlKoGzQG0QaRn4EI SWZqAJsLJgCMS9oItpinop7/XpTONEbL6oP3eZLJq2k1NilsAs6k1iMk36qDBMsHHtKB Aq7V8CklR2gwhJJXXtpWDPMiWd3Kh+MxeSRXs5UYAR6Qwk7nhw9ocGhiPdegpwv5VF3K Y7qODPnoBALHh5lyIJEK0LH8mXJFkUrk+JKbc8afeISsrn03UFF3RnN4noCdqL3FIZKB YD2A== X-Gm-Message-State: AMCzsaWMVkfdxDGT8vXsAyNWLGdpc9PptVzM5k+RdOCIuzgaYwoy8RAg KOrhYhuGN3TlvvYNiRImUNrSfIjEq8H/WiLl6gM= X-Google-Smtp-Source: AOwi7QA8P7GqZ94kyRhK6pB/m3DY5TeIG4zR74prKtsVzezMlDr7g1XRr/pXuvAcHidS+4/UzYO6gJpkMApy0w+EdY8= X-Received: by 10.202.215.67 with SMTP id o64mr1048295oig.224.1507805260666; Thu, 12 Oct 2017 03:47:40 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <19c0b00a-b988-d0da-3cb6-57583908e740@redhat.com> References: <20171012021950.GA21344@gmail.com> <19c0b00a-b988-d0da-3cb6-57583908e740@redhat.com> From: "H.J. Lu" Date: Thu, 12 Oct 2017 03:47:39 -0700 Message-ID: Subject: Re: [PATCH] Support profiling PIE [BZ #22284] To: "Carlos O'Donell" Cc: GNU C Library On 10/11/17, Carlos O'Donell wrote: > 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 > >> >> 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 . >> [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 >> #include >> >> +#ifdef PIC >> +# include >> + >> +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. This is what I checked in. Thanks. >> + 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. > From 9278574fac90d2092edbe8229f16104993f60e14 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Thu, 12 Oct 2017 03:45:09 -0700 Subject: [PATCH] Support profiling PIE [BZ #22284] Since PIE can be loaded at any address, we need to subtract load address from PCs. [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 . [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 | 47 +++++++++++++++++++++++++++++++++++++---------- gmon/tst-gmon-pie.c | 1 + 3 files changed, 59 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 + # 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 + + 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) diff --git a/gmon/gmon.c b/gmon/gmon.c index f1aa3b776c..dee64803ad 100644 --- a/gmon/gmon.c +++ b/gmon/gmon.c @@ -46,6 +46,26 @@ #include #include +#ifdef PIC +# include + +static int +callback (struct dl_phdr_info *info, size_t size, void *data) +{ + if (info->dlpi_name[0] == '\0') + { + /* The link map for the executable is created by calling + _dl_new_object with "" as filename. dl_iterate_phdr + calls the callback function with filename from the + link map as dlpi_name. */ + u_long *load_address = data; + *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 +83,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); static void write_bb_counts (int fd); /* @@ -173,7 +193,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 +230,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; thdr.hist_size = _gmonparam.kcountsize / sizeof (HISTCOUNTER); thdr.prof_rate = __profile_frequency (); strncpy (thdr.dimen, "seconds", sizeof (thdr.dimen)); @@ -223,7 +243,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 +286,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); arc.count = _gmonparam.tos[to_index].count; memcpy (raw_arc + nfilled, &arc, sizeof (raw_arc [0])); @@ -376,11 +397,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 + /* write PC histogram: */ - write_hist (fd); + write_hist (fd, load_address); /* write call-graph: */ - write_call_graph (fd); + write_call_graph (fd, load_address); /* 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