Patchwork Support profiling PIE [BZ #22284]

login
register
mail settings
Submitter H.J. Lu
Date Oct. 12, 2017, 2:19 a.m.
Message ID <20171012021950.GA21344@gmail.com>
Download mbox | patch
Permalink /patch/23486/
State New
Headers show

Comments

H.J. Lu - Oct. 12, 2017, 2:19 a.m.
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?

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
Carlos O'Donell - Oct. 12, 2017, 2:47 a.m.
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

Patch

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