perf: android: configure hardcoded paths

PATCH Superseded Irina Tirdea 4 participants 16 messages Sep 23, 2012 – Oct 1, 2012 perf events

lore

Share & tools

lore

Irina Tirdea

author

2012-09-23 19:27 UTC #0 lore raw

v2 cover perf: android: configure hardcoded paths

From: Irina Tirdea <[email protected]>

This is version 2 of the set of patches that replace hardcoded paths used in perf with configurable options in the Makefile.

First version can be found at https://lkml.org/lkml/2012/9/20/537.

Thanks everybody for the reviews! This version fixes all issues mentioned in reviews.

Changes for v2:
() updated tools/perf/Documentation/jit-interface.txt regarding PERF_TMP_DIR (patch 1)
() fixed hardcoded size for PERF_TMP_DIR"/perf-" (patch 1)
() use PERF_SHELL_PATH instead of PERF_SHELL_DIR (patch 2)
() use DEFAULT_OBJDUMP_PATH and DEFAULT_ADDR2LINE_PATH (patch 3,4) () add CROSS_COMPILE prefix to addr2line and objdump in Makefile (patch 3,4)

Thanks,
Irina

Irina Tirdea (4):
perf tools: configure tmp path at build time perf tools: configure shell path at compile time perf annotate: configure objdump path at compile time perf tools: configure addr2line path at compile time

 tools/perf/Documentation/jit-interface.txt |    4 +++-
 tools/perf/Makefile                        |   30 ++++++++++++++++++++++++++--
 tools/perf/builtin-help.c                  |    2 +-
 tools/perf/builtin-script.c                |   12 +++++------
 tools/perf/perf-archive.sh                 |   13 ++++++++++--
 tools/perf/util/annotate.c                 |    5 +++--
 tools/perf/util/dso-test-data.c            |    2 +-
 tools/perf/util/map.c                      |    3 ++-
 tools/perf/util/pmu.c                      |    2 +-
 tools/perf/util/sort.c                     |    2 +-
 tools/perf/util/symbol.c                   |    4 +++-
 tools/perf/util/trace-event-info.c         |    2 +-
 12 files changed, 61 insertions(+), 20 deletions(-)

Signature · 1 line

1.7.9.5

Irina Tirdea

author

2012-09-23 19:29 UTC #1 lore raw

PATCH v2 1/4 perf tools: configure tmp path at build time

From: Irina Tirdea <[email protected]>

Temporary perf files are hardcoded to point to /tmp. Android does not have a /tmp directory so it needs to set this path at compile time.

Add a compile-time definition (PERF_TMP_DIR) in the Makefile that sets the path to temp directory. By default it points to /tmp.

Signed-off-by Irina Tirdea <[email protected]>

---
 tools/perf/Documentation/jit-interface.txt |    4 +++-
 tools/perf/Makefile                        |   16 +++++++++++++++-
 tools/perf/perf-archive.sh                 |   13 +++++++++++--
 tools/perf/util/dso-test-data.c            |    2 +-
 tools/perf/util/map.c                      |    3 ++-
 tools/perf/util/pmu.c                      |    2 +-
 tools/perf/util/symbol.c                   |    4 +++-
 tools/perf/util/trace-event-info.c         |    2 +-
 8 files changed, 37 insertions(+), 9 deletions(-)

tools/perf/Documentation/jit-interface.txt +3 -1

index a8656f5..10cb6ec 100644
@@ -1,7 +1,9 @@
 perf supports a simple JIT interface to resolve symbols for dynamic code generated
 by a JIT.
 
-The JIT has to write a /tmp/perf-%d.map  (%d = pid of process) file
+The JIT has to write a $PERF_TMP_DIR/perf-%d.map  (%d = pid of process) file.
+You can set $PERF_TMP_DIR at compile time in the Makefile.
+By default it is /tmp.
 
 This is a text file.

tools/perf/Makefile +15 -1

index 5077f8e..eab4a36 100644
@@ -139,6 +139,7 @@ sysconfdir = $(prefix)/etc
 ETC_PERFCONFIG = etc/perfconfig
 endif
 lib = lib
+PERF_TMP_DIR = /tmp
 
 export prefix bindir sharedir sysconfdir

@@ -246,7 +247,8 @@ $(OUTPUT)util/pmu-bison.c: util/pmu.y
 	$(QUIET_BISON)$(BISON) -v util/pmu.y -d -o $(OUTPUT)util/pmu-bison.c
 
 $(OUTPUT)util/parse-events.o: $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-bison.c
-$(OUTPUT)util/pmu.o: $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c
+$(OUTPUT)util/pmu.o: $(OUTPUT)util/pmu.c $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
 
 LIB_FILE=$(OUTPUT)libperf.a

@@ -880,6 +882,18 @@ $(OUTPUT)util/exec_cmd.o: util/exec_cmd.c $(OUTPUT)PERF-CFLAGS
 $(OUTPUT)util/config.o: util/config.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
 
+$(OUTPUT)util/dso-test-data.o: util/dso-test-data.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
+
+$(OUTPUT)util/map.o: util/map.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
+
+$(OUTPUT)util/symbol.o: util/symbol.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
+
+$(OUTPUT)util/trace-event-info.o: util/trace-event-info.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
+
 $(OUTPUT)ui/browser.o: ui/browser.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<

tools/perf/perf-archive.sh +11 -2

index e919306..058c3b7 100644
@@ -18,7 +18,16 @@ else
         PERF_BUILDID_DIR=$PERF_BUILDID_DIR/
 fi
 
-BUILDIDS=$(mktemp /tmp/perf-archive-buildids.XXXXXX)
+#
+# PERF_TMP_DIR environment variable set by perf
+# path to temp directory, default to /tmp
+#
+if [ -z $PERF_TMP_DIR ]; then
+	PERF_TMP_DIR=/tmp
+fi
+
+
+BUILDIDS=$(mktemp $PERF_TMP_DIR/perf-archive-buildids.XXXXXX)
 NOBUILDID=0000000000000000000000000000000000000000
 
 perf buildid-list -i $PERF_DATA --with-hits | grep -v "^$NOBUILDID " > $BUILDIDS

@@ -28,7 +37,7 @@ if [ ! -s $BUILDIDS ] ; then
 	exit 1
 fi
 
-MANIFEST=$(mktemp /tmp/perf-archive-manifest.XXXXXX)
+MANIFEST=$(mktemp $PERF_TMP_DIR/perf-archive-manifest.XXXXXX)
 PERF_BUILDID_LINKDIR=$(readlink -f $PERF_BUILDID_DIR)/
 
 cut -d ' ' -f 1 $BUILDIDS | \

tools/perf/util/dso-test-data.c +1 -1

index c6caede..ca81e65 100644
@@ -18,7 +18,7 @@ do { \
 
 static char *test_file(int size)
 {
-	static char buf_templ[] = "/tmp/test-XXXXXX";
+	static char buf_templ[] = PERF_TMP_DIR "/test-XXXXXX";
 	char *templ = buf_templ;
 	int fd, i;
 	unsigned char *buf;

tools/perf/util/map.c +2 -1

index b442ee4..c5b4632 100644
@@ -59,7 +59,8 @@ struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
 		no_dso = is_no_dso_memory(filename);
 
 		if (anon) {
-			snprintf(newfilename, sizeof(newfilename), "/tmp/perf-%d.map", pid);
+			snprintf(newfilename, sizeof(newfilename),
+				 PERF_TMP_DIR "/perf-%d.map", pid);
 			filename = newfilename;
 		}

tools/perf/util/pmu.c +1 -1

index 8a2229d..5ebde18 100644
@@ -637,7 +637,7 @@ static char *test_format_dir_get(void)
 	static char dir[PATH_MAX];
 	unsigned int i;
 
-	snprintf(dir, PATH_MAX, "/tmp/perf-pmu-test-format-XXXXXX");
+	snprintf(dir, PATH_MAX, PERF_TMP_DIR "/perf-pmu-test-format-XXXXXX");
 	if (!mkdtemp(dir))
 		return NULL;

tools/perf/util/symbol.c +3 -1

index e2e8c69..d8aca82 100644
@@ -1051,7 +1051,9 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 
 	dso->adjust_symbols = 0;
 
-	if (strncmp(dso->name, "/tmp/perf-", 10) == 0) {
+	if (strncmp
+	    (dso->name, PERF_TMP_DIR "/perf-",
+	     strlen(PERF_TMP_DIR "/perf-")) == 0) {
 		struct stat st;
 
 		if (lstat(dso->name, &st) < 0)

tools/perf/util/trace-event-info.c +1 -1

index a8d81c3..e50ea61 100644
@@ -481,7 +481,7 @@ struct tracing_data *tracing_data_get(struct list_head *pattrs,
 		int temp_fd;
 
 		snprintf(tdata->temp_file, sizeof(tdata->temp_file),
-			 "/tmp/perf-XXXXXX");
+			 PERF_TMP_DIR "/perf-XXXXXX");
 		if (!mkstemp(tdata->temp_file))
 			die("Can't make temp file");

Signature · 1 line

1.7.9.5

Irina Tirdea

author

2012-09-23 19:29 UTC #2 lore raw

PATCH v2 2/4 perf tools: configure shell path at compile time

In reply to Irina Tirdea

From: Irina Tirdea <[email protected]>

Shell path /bin/sh is hardcoded in various places in perf. Android has a different folder structure and does not have /bin/sh.

Set the shell path at compile time in the Makefile by setting PERF_SHELL_PATH. By default it is set to /bin/sh.

Signed-off-by Irina Tirdea <[email protected]>

---
 tools/perf/Makefile         |    6 +++++-
 tools/perf/builtin-help.c   |    2 +-
 tools/perf/builtin-script.c |   12 ++++++------
 3 files changed, 12 insertions(+), 8 deletions(-)

tools/perf/Makefile +5 -1

index eab4a36..fae52c8 100644
@@ -828,7 +828,11 @@ $(OUTPUT)builtin-help.o: builtin-help.c $(OUTPUT)common-cmds.h $(OUTPUT)PERF-CFL
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) \
 		'-DPERF_HTML_PATH="$(htmldir_SQ)"' \
 		'-DPERF_MAN_PATH="$(mandir_SQ)"' \
-		'-DPERF_INFO_PATH="$(infodir_SQ)"' $<
+		'-DPERF_INFO_PATH="$(infodir_SQ)"' \
+		'-DPERF_SHELL_PATH="/bin/sh"' $<
+
+$(OUTPUT)builtin-script.o: builtin-script.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_SHELL_PATH='"/bin/sh"' $<
 
 $(OUTPUT)builtin-timechart.o: builtin-timechart.c $(OUTPUT)common-cmds.h $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) \

tools/perf/builtin-help.c +1 -1

index 25c8b94..e7d640c 100644
@@ -171,7 +171,7 @@ static void exec_man_cmd(const char *cmd, const char *page)
 {
 	struct strbuf shell_cmd = STRBUF_INIT;
 	strbuf_addf(&shell_cmd, "%s %s", cmd, page);
-	execl("/bin/sh", "sh", "-c", shell_cmd.buf, NULL);
+	execl(PERF_SHELL_PATH, "sh", "-c", shell_cmd.buf, NULL);
 	warning("failed to exec '%s': %s", cmd, strerror(errno));
 }

tools/perf/builtin-script.c +6 -6

index 1be843a..b626cf1 100644
@@ -1326,7 +1326,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 				goto out;
 			}
 
-			__argv[j++] = "/bin/sh";
+			__argv[j++] = PERF_SHELL_PATH;
 			__argv[j++] = rec_script_path;
 			if (system_wide)
 				__argv[j++] = "-a";

@@ -1337,7 +1337,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 				__argv[j++] = argv[i];
 			__argv[j++] = NULL;
 
-			execvp("/bin/sh", (char **)__argv);
+			execvp(PERF_SHELL_PATH, (char **)__argv);
 			free(__argv);
 			exit(-1);
 		}

@@ -1353,7 +1353,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 
 		j = 0;
-		__argv[j++] = "/bin/sh";
+		__argv[j++] = PERF_SHELL_PATH;
 		__argv[j++] = rep_script_path;
 		for (i = 1; i < rep_args + 1; i++)
 			__argv[j++] = argv[i];

@@ -1361,7 +1361,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 		__argv[j++] = "-";
 		__argv[j++] = NULL;
 
-		execvp("/bin/sh", (char **)__argv);
+		execvp(PERF_SHELL_PATH, (char **)__argv);
 		free(__argv);
 		exit(-1);
 	}

@@ -1390,7 +1390,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 			goto out;
 		}
 
-		__argv[j++] = "/bin/sh";
+		__argv[j++] = PERF_SHELL_PATH;
 		__argv[j++] = script_path;
 		if (system_wide)
 			__argv[j++] = "-a";

@@ -1398,7 +1398,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 			__argv[j++] = argv[i];
 		__argv[j++] = NULL;
 
-		execvp("/bin/sh", (char **)__argv);
+		execvp(PERF_SHELL_PATH, (char **)__argv);
 		free(__argv);
 		exit(-1);
 	}

Signature · 1 line

1.7.9.5

Irina Tirdea

author

2012-09-23 19:29 UTC #3 lore raw

PATCH v2 3/4 perf annotate: configure objdump path at compile time

In reply to Irina Tirdea

From: Irina Tirdea <[email protected]>

The default name for objdump is "objdump". For cross-compiling the name of objdump will be different (e.g. arm-eabi-objdump in Android).

Set the default objdump name in the Makefile with DEFAULT_OBJDUMP_PATH.

Signed-off-by Irina Tirdea <[email protected]>

---
 tools/perf/Makefile        |    4 ++++
 tools/perf/util/annotate.c |    2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

tools/perf/Makefile +4 -0

index fae52c8..7a3f572 100644
@@ -140,6 +140,7 @@ ETC_PERFCONFIG = etc/perfconfig
 endif
 lib = lib
 PERF_TMP_DIR = /tmp
+OBJDUMP = $(CROSS_COMPILE)objdump
 
 export prefix bindir sharedir sysconfdir

@@ -883,6 +884,9 @@ $(OUTPUT)util/exec_cmd.o: util/exec_cmd.c $(OUTPUT)PERF-CFLAGS
 		'-DPREFIX="$(prefix_SQ)"' \
 		$<
 
+$(OUTPUT)util/annotate.o: util/annotate.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DDEFAULT_OBJDUMP_PATH='"$(OBJDUMP)"' $<
+
 $(OUTPUT)util/config.o: util/config.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<

tools/perf/util/annotate.c +1 -1

index f0a9103..d7036b3 100644
@@ -824,7 +824,7 @@ fallback:
 		 "%s %s%s --start-address=0x%016" PRIx64
 		 " --stop-address=0x%016" PRIx64
 		 " -d %s %s -C %s|grep -v %s|expand",
-		 objdump_path ? objdump_path : "objdump",
+		 objdump_path ? objdump_path : DEFAULT_OBJDUMP_PATH,
 		 disassembler_style ? "-M " : "",
 		 disassembler_style ? disassembler_style : "",
 		 map__rip_2objdump(map, sym->start),

Signature · 1 line

1.7.9.5

Namhyung Kim

maintainer

2012-09-25 13:09 UTC #4 lore raw

Re: [PATCH v2 3/4] perf annotate: configure objdump path at compile time

Hi Irina,

2012-09-23 (일), 22:27 +0300, Irina Tirdea:

Quoted 6 lines

From: Irina Tirdea <[email protected]>
The default name for objdump is "objdump". For cross-compiling the name of objdump will be different (e.g. arm-eabi-objdump in Android).
Set the default objdump name in the Makefile with DEFAULT_OBJDUMP_PATH.

I thought about it twice and confused.

For cross-compiling, the resulting perf binary will run on target - say Android - but the toolchain runs on host, right? So with this change the cross-built perf will try to find the arm-eabi-objdump on Android. Is it an intended behavior? Is there an arm-eabi-objdump on Android?

Thanks,
Namhyung

Irina Tirdea

author

2012-09-27 00:51 UTC #5 lore raw

Re: [PATCH v2 3/4] perf annotate: configure objdump path at compile time

Quoted 1 line · Namhyung Kim

Hi Irina,

Hi Namhyung,

Quoted 6 lines

2012-09-23 (일), 22:27 +0300, Irina Tirdea:
Quoted 5 lines
From: Irina Tirdea <[email protected]>
The default name for objdump is "objdump". For cross-compiling the name of objdump will be different (e.g. arm-eabi-objdump in Android).

Quoted 9 lines

Quoted 1 line
Set the default objdump name in the Makefile with DEFAULT_OBJDUMP_PATH.
I thought about it twice and confused.
For cross-compiling, the resulting perf binary will run on target - say Android - but the toolchain runs on host, right? So with this change the cross-built perf will try to find the arm-eabi-objdump on Android. Is it an intended behavior? Is there an arm-eabi-objdump on Android?

Apparently I got confused about this as well...

There are two perf binaries built for Android: one for the target (that will run on Android) and the other one for the host (that can be used to analyse data recorded on the target).

As you mentioned, the perf built to run on Android needs to use objdump as "objdump" (actually Android does not yet have objdump, but this is the naming convention). In this case, objdump should not have the CROSS_COMPILE prefix.

The perf built to run on the host needs to use arm-eabi-objdump from the toolchain so that it can analyse data recorded on Android. This patch is targeting this scenario, not the previous one. In this case, the CROSS_COMPILE option would be different than arm-eabi- so using $(CROSS_COMPILE)objdump would be wrong. objdump should be overridden when running make since there is no connection between the toolchain used here and the path for objdump. I am always overriding objdump when calling make, so I did not catch this.

I think that I should change DEFAULT_OBJDUMP_PATH=objdump in the Makefile to handle the first scenario. I'll also explain this in the commit message so that it is more clear and make the same change for the addr2line patch.

What do you think?

Thanks,
Irina

Namhyung Kim

maintainer

2012-09-27 02:00 UTC #6 lore raw

Re: [PATCH v2 3/4] perf annotate: configure objdump path at compile time

Quoted 10 lines · Irina Tirdea

Quoted 7 lines · Namhyung Kim
I thought about it twice and confused.
For cross-compiling, the resulting perf binary will run on target - say Android - but the toolchain runs on host, right? So with this change the cross-built perf will try to find the arm-eabi-objdump on Android. Is it an intended behavior? Is there an arm-eabi-objdump on Android?
Apparently I got confused about this as well...

Quoted 9 lines

There are two perf binaries built for Android: one for the target (that will run on Android) and the other one for the host (that can be used to analyse data recorded on the target).
As you mentioned, the perf built to run on Android needs to use objdump as "objdump" (actually Android does not yet have objdump, but this is the naming convention). In this case, objdump should not have the CROSS_COMPILE prefix.

Ok.

Quoted 10 lines

The perf built to run on the host needs to use arm-eabi-objdump from the toolchain so that it can analyse data recorded on Android. This patch is targeting this scenario, not the previous one. In this case, the CROSS_COMPILE option would be different than arm-eabi- so using $(CROSS_COMPILE)objdump would be wrong. objdump should be overridden when running make since there is no connection between the toolchain used here and the path for objdump. I am always overriding objdump when calling make, so I did not catch this.

Quoted 6 lines

I think that I should change DEFAULT_OBJDUMP_PATH=objdump in the Makefile to handle the first scenario. I'll also explain this in the commit message so that it is more clear and make the same change for the addr2line patch.
What do you think?

I think the right thing to do is finding a correct objdump at runtime in some way. Why do you want to make it compile-time configurable?

Thanks,
Namhyung

Irina Tirdea

author

2012-09-27 11:25 UTC #7 lore raw

Re: [PATCH v2 3/4] perf annotate: configure objdump path at compile time

Quoted 14 lines

Quoted 14 lines
The perf built to run on the host needs to use arm-eabi-objdump from the toolchain so that it can analyse data recorded on Android. This patch is targeting this scenario, not the previous one. In this case, the CROSS_COMPILE option would be different than arm-eabi- so using $(CROSS_COMPILE)objdump would be wrong. objdump should be overridden when running make since there is no connection between the toolchain used here and the path for objdump. I am always overriding objdump when calling make, so I did not catch this.
I think that I should change DEFAULT_OBJDUMP_PATH=objdump in the Makefile to handle the first scenario. I'll also explain this in the commit message so that it is more clear and make the same change for the addr2line patch.

Quoted 5 lines

Quoted 1 line
What do you think?
I think the right thing to do is finding a correct objdump at runtime in some way. Why do you want to make it compile-time configurable?

The correct objdump path can be detected at runtime by setting the toolchain path. But since the name is arm-eabi-objdump and not objdump, it does not know to use it instead.

The only way (I can think of) to change objdump at runtime would be to use the --objdump option for perf annotate (and provide a similar option for addr2line). The problem with this approach is that the user has to be aware that perf annotate uses the objdump tool and that he has to use the cross-compiler version instead. Since the user will have perf compiled for host as part of his Android tree, he will expect it to work without these further changes from his part. The path for objdump can be set in the Android Makefile at compile time so that the user doesn't need to be aware of it.

If compiling directly from the kernel tree, the advantage is not that obvious. The user would still have to specify the name for objdump in the command line when running make. But I think this is still a better user experience compared to having to set this every time when using perf annotate.

Thanks,
Irina

Namhyung Kim

maintainer

2012-09-27 13:24 UTC #8 lore raw

Re: [PATCH v2 3/4] perf annotate: configure objdump path at compile time

Quoted 24 lines · Irina Tirdea

Quoted 14 lines
Quoted 14 lines
The perf built to run on the host needs to use arm-eabi-objdump from the toolchain so that it can analyse data recorded on Android. This patch is targeting this scenario, not the previous one. In this case, the CROSS_COMPILE option would be different than arm-eabi- so using $(CROSS_COMPILE)objdump would be wrong. objdump should be overridden when running make since there is no connection between the toolchain used here and the path for objdump. I am always overriding objdump when calling make, so I did not catch this.
I think that I should change DEFAULT_OBJDUMP_PATH=objdump in the Makefile to handle the first scenario. I'll also explain this in the commit message so that it is more clear and make the same change for the addr2line patch.
Quoted 5 lines
Quoted 1 line
What do you think?
I think the right thing to do is finding a correct objdump at runtime in some way. Why do you want to make it compile-time configurable?
The correct objdump path can be detected at runtime by setting the toolchain path. But since the name is arm-eabi-objdump and not objdump, it does not know to use it instead.

Quoted 9 lines

The only way (I can think of) to change objdump at runtime would be to use the --objdump option for perf annotate (and provide a similar option for addr2line). The problem with this approach is that the user has to be aware that perf annotate uses the objdump tool and that he has to use the cross-compiler version instead. Since the user will have perf compiled for host as part of his Android tree, he will expect it to work without these further changes from his part. The path for objdump can be set in the Android Makefile at compile time so that the user doesn't need to be aware of it.

What I'm thinking is that perf can try to find cross-built binutils when it detects perf.data file is came from other machine/architecture. Fortunately perf_session_env was added recently and it has the arch information from the file so we can use it to find the path.

Following patch is a proof-of-concept patch and only build tested. What do you think? Could you play with it for some time? :)

Thanks,
Namhyung

Quoted 1 line

From 7068a43a4b450c60fdcc8a3916ce624c1ef2c9b2 Mon Sep 17 00:00:00 2001

From: Namhyung Kim <[email protected]>
Date: Thu, 27 Sep 2012 21:54:33 +0900
Subject: [RFC] perf tools: Try to find cross-built objdump path

As we have architecture information of saved perf.data file, we can try to find cross-built objdump path.

The triplets are incomplete and maybe need some regexp works.

Cc Irina Tirdea <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/Makefile           |  2 +
 tools/perf/arch/common.c      | 42 +++++++++++++++++++
 tools/perf/builtin-annotate.c |  3 ++
 tools/perf/util/annotate.c    | 96 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/annotate.h    | 10 +++++
 5 files changed, 153 insertions(+)
 create mode 100644 tools/perf/arch/common.c

tools/perf/Makefile +2 -0

index 5973f383eb8e..b189229eb576 100644
@@ -412,6 +412,8 @@ LIB_OBJS += $(OUTPUT)ui/helpline.o
 LIB_OBJS += $(OUTPUT)ui/hist.o
 LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
 
+LIB_OBJS += $(OUTPUT)arch/common.o
+
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
 # Benchmark modules

tools/perf/arch/common.c new file +42 -0

42 lines not on this page · raw message

tools/perf/builtin-annotate.c +3 -0

index 9ea38540b873..20fe9bb6505b 100644
@@ -186,6 +186,9 @@ static int __cmd_annotate(struct perf_annotate *ann)
 			goto out_delete;
 	}
 
+	if (!objdump_path)
+		try_objdump_path(session);
+
 	ret = perf_session__process_events(session, &ann->tool);
 	if (ret)
 		goto out_delete;

tools/perf/util/annotate.c +96 -0

105 lines not on this page · raw message

tools/perf/util/annotate.h +10 -0

index 9b5b21e7b032..cbf2d9537320 100644
@@ -5,6 +5,7 @@
 #include <stdint.h>
 #include "types.h"
 #include "symbol.h"
+#include "session.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include <pthread.h>

@@ -156,4 +157,13 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 extern const char	*disassembler_style;
 extern const char	*objdump_path;
 
+extern const char *arm_triplets[];
+extern const char *powerpc_triplets[];
+extern const char *sh_triplets[];
+extern const char *s390_triplets[];
+extern const char *sparc_triplets[];
+extern const char *x86_triplets[];
+
+void try_objdump_path(struct perf_session *session);
+
 #endif	/* __PERF_ANNOTATE_H */

Signature · 1 line

1.7.11.4

Namhyung Kim

maintainer

2012-09-27 13:38 UTC #9 lore raw

RFC v2 1/1 perf tools: Try to find cross-built objdump path

From: Namhyung Kim <[email protected]>

As we have architecture information of saved perf.data file, we can try to find cross-built objdump path.

The triplets are incomplete and maybe need some regexp works.

Cc Irina Tirdea <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
v2: don't modify env string

 tools/perf/Makefile           |   2 +
 tools/perf/arch/common.c      |  42 +++++++++++++++++
 tools/perf/builtin-annotate.c |   3 ++
 tools/perf/util/annotate.c    | 103 ++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/annotate.h    |  10 ++++
 5 files changed, 160 insertions(+)
 create mode 100644 tools/perf/arch/common.c

tools/perf/Makefile +2 -0

index 5973f383eb8e..b189229eb576 100644
@@ -412,6 +412,8 @@ LIB_OBJS += $(OUTPUT)ui/helpline.o
 LIB_OBJS += $(OUTPUT)ui/hist.o
 LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
 
+LIB_OBJS += $(OUTPUT)arch/common.o
+
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
 # Benchmark modules

tools/perf/arch/common.c new file +42 -0

42 lines not on this page · raw message

tools/perf/builtin-annotate.c +3 -0

index 9ea38540b873..20fe9bb6505b 100644
@@ -186,6 +186,9 @@ static int __cmd_annotate(struct perf_annotate *ann)
 			goto out_delete;
 	}
 
+	if (!objdump_path)
+		try_objdump_path(session);
+
 	ret = perf_session__process_events(session, &ann->tool);
 	if (ret)
 		goto out_delete;

tools/perf/util/annotate.c +103 -0

112 lines not on this page · raw message

tools/perf/util/annotate.h +10 -0

index 9b5b21e7b032..cbf2d9537320 100644
@@ -5,6 +5,7 @@
 #include <stdint.h>
 #include "types.h"
 #include "symbol.h"
+#include "session.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include <pthread.h>

@@ -156,4 +157,13 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 extern const char	*disassembler_style;
 extern const char	*objdump_path;
 
+extern const char *arm_triplets[];
+extern const char *powerpc_triplets[];
+extern const char *sh_triplets[];
+extern const char *s390_triplets[];
+extern const char *sparc_triplets[];
+extern const char *x86_triplets[];
+
+void try_objdump_path(struct perf_session *session);
+
 #endif	/* __PERF_ANNOTATE_H */

Signature · 1 line

1.7.11.4

Irina Tirdea

author

2012-10-01 00:42 UTC #10 lore raw

RFC v3 1/1 perf tools: Try to find cross-built objdump path

From: Namhyung Kim <[email protected]>

As we have architecture information of saved perf.data file, we can try to find cross-built objdump path.

The triplets include support for Android (arm, x86 and mips architectures).

Signed-off-by Namhyung Kim <[email protected]>

Signed-off-by Irina Tirdea <[email protected]>

---

v3:
() update triplets to include Android toolchains for arm, x86 and mips () fixed a few small bugs
() tested on Android

v2: don't modify env string

 tools/perf/Makefile           |    2 +
 tools/perf/arch/common.c      |   51 ++++++++++++++++++++
 tools/perf/builtin-annotate.c |    3 ++
 tools/perf/util/annotate.c    |  107 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/annotate.h    |   11 +++++
 5 files changed, 174 insertions(+)
 create mode 100644 tools/perf/arch/common.c

tools/perf/Makefile +2 -0

index e5e71e7..816fc04 100644
@@ -412,6 +412,8 @@ LIB_OBJS += $(OUTPUT)ui/helpline.o
 LIB_OBJS += $(OUTPUT)ui/hist.o
 LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
 
+LIB_OBJS += $(OUTPUT)arch/common.o
+
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
 # Benchmark modules

tools/perf/arch/common.c new file +51 -0

51 lines not on this page · raw message

tools/perf/builtin-annotate.c +3 -0

index 9ea3854..20fe9bb 100644
@@ -186,6 +186,9 @@ static int __cmd_annotate(struct perf_annotate *ann)
 			goto out_delete;
 	}
 
+	if (!objdump_path)
+		try_objdump_path(session);
+
 	ret = perf_session__process_events(session, &ann->tool);
 	if (ret)
 		goto out_delete;

tools/perf/util/annotate.c +107 -0

116 lines not on this page · raw message

tools/perf/util/annotate.h +11 -0

index 9b5b21e..4918d1f 100644
@@ -5,6 +5,7 @@
 #include <stdint.h>
 #include "types.h"
 #include "symbol.h"
+#include "session.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include <pthread.h>

@@ -156,4 +157,14 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 extern const char	*disassembler_style;
 extern const char	*objdump_path;
 
+extern const char *arm_triplets[];
+extern const char *powerpc_triplets[];
+extern const char *sh_triplets[];
+extern const char *s390_triplets[];
+extern const char *sparc_triplets[];
+extern const char *x86_triplets[];
+extern const char *mips_triplets[];
+
+void try_objdump_path(struct perf_session *session);
+
 #endif	/* __PERF_ANNOTATE_H */

Signature · 1 line

1.7.9.5

Irina Tirdea

author

2012-09-30 23:37 UTC #11 lore raw

Re: [PATCH v2 3/4] perf annotate: configure objdump path at compile time

In reply to Namhyung Kim

Quoted 35 lines · Namhyung Kim

Quoted 24 lines · Irina Tirdea
Quoted 14 lines
Quoted 14 lines
The perf built to run on the host needs to use arm-eabi-objdump from the toolchain so that it can analyse data recorded on Android. This patch is targeting this scenario, not the previous one. In this case, the CROSS_COMPILE option would be different than arm-eabi- so using $(CROSS_COMPILE)objdump would be wrong. objdump should be overridden when running make since there is no connection between the toolchain used here and the path for objdump. I am always overriding objdump when calling make, so I did not catch this.
I think that I should change DEFAULT_OBJDUMP_PATH=objdump in the Makefile to handle the first scenario. I'll also explain this in the commit message so that it is more clear and make the same change for the addr2line patch.
Quoted 5 lines
Quoted 1 line
What do you think?
I think the right thing to do is finding a correct objdump at runtime in some way. Why do you want to make it compile-time configurable?
The correct objdump path can be detected at runtime by setting the toolchain path. But since the name is arm-eabi-objdump and not objdump, it does not know to use it instead.
Quoted 9 lines
The only way (I can think of) to change objdump at runtime would be to use the --objdump option for perf annotate (and provide a similar option for addr2line). The problem with this approach is that the user has to be aware that perf annotate uses the objdump tool and that he has to use the cross-compiler version instead. Since the user will have perf compiled for host as part of his Android tree, he will expect it to work without these further changes from his part. The path for objdump can be set in the Android Makefile at compile time so that the user doesn't need to be aware of it.

Quoted 8 lines

What I'm thinking is that perf can try to find cross-built binutils when it detects perf.data file is came from other machine/architecture. Fortunately perf_session_env was added recently and it has the arch information from the file so we can use it to find the path.
Following patch is a proof-of-concept patch and only build tested. What do you think? Could you play with it for some time? :)

Thanks, this looks like the right approach. :)
I'm going to test it for Android cross-compiling and submit an update if needed.

Thanks,
Irina

Ingo Molnar

2012-10-01 07:22 UTC #12 lore raw

Re: [PATCH v2 3/4] perf annotate: configure objdump path at compile time

In reply to Namhyung Kim

Quoted 41 lines · Namhyung Kim

Quoted 24 lines · Irina Tirdea
Quoted 14 lines
Quoted 14 lines
The perf built to run on the host needs to use arm-eabi-objdump from the toolchain so that it can analyse data recorded on Android. This patch is targeting this scenario, not the previous one. In this case, the CROSS_COMPILE option would be different than arm-eabi- so using $(CROSS_COMPILE)objdump would be wrong. objdump should be overridden when running make since there is no connection between the toolchain used here and the path for objdump. I am always overriding objdump when calling make, so I did not catch this.
I think that I should change DEFAULT_OBJDUMP_PATH=objdump in the Makefile to handle the first scenario. I'll also explain this in the commit message so that it is more clear and make the same change for the addr2line patch.
Quoted 5 lines
Quoted 1 line
What do you think?
I think the right thing to do is finding a correct objdump at runtime in some way. Why do you want to make it compile-time configurable?
The correct objdump path can be detected at runtime by setting the toolchain path. But since the name is arm-eabi-objdump and not objdump, it does not know to use it instead.
Quoted 9 lines
The only way (I can think of) to change objdump at runtime would be to use the --objdump option for perf annotate (and provide a similar option for addr2line). The problem with this approach is that the user has to be aware that perf annotate uses the objdump tool and that he has to use the cross-compiler version instead. Since the user will have perf compiled for host as part of his Android tree, he will expect it to work without these further changes from his part. The path for objdump can be set in the Android Makefile at compile time so that the user doesn't need to be aware of it.
What I'm thinking is that perf can try to find cross-built binutils when it detects perf.data file is came from other machine/architecture. Fortunately perf_session_env was added recently and it has the arch information from the file so we can use it to find the path.

Quoted 3 lines

Following patch is a proof-of-concept patch and only build tested. What do you think? Could you play with it for some time? :)

This is a pretty clever idea - much better than hard-coding architecture details at build time.

Thanks,

Ingo

Arnaldo Carvalho de Melo

2012-10-01 14:27 UTC #13 lore raw

Re: [PATCH v2 3/4] perf annotate: configure objdump path at compile time

Em Mon, Oct 01, 2012 at 09:21:54AM +0200, Ingo Molnar escreveu:

Quoted 7 lines · Namhyung Kim

Quoted 5 lines
What I'm thinking is that perf can try to find cross-built binutils when it detects perf.data file is came from other machine/architecture. Fortunately perf_session_env was added recently and it has the arch information from the file so we can use it to find the path.

Quoted 5 lines

Quoted 2 lines
Following patch is a proof-of-concept patch and only build tested. What do you think? Could you play with it for some time? :)
This is a pretty clever idea - much better than hard-coding architecture details at build time.

Indeed, great idea!

- Arnaldo

Irina Tirdea

author

2012-09-23 19:29 UTC #14 lore raw

PATCH v2 4/4 perf tools: configure addr2line path at compile time

In reply to Irina Tirdea

From: Irina Tirdea <[email protected]>

The default name for addr2line is hardcoded to "addr2line". When cross-compiling the name of addr2line will be different (e.g. arm-eabi-addr2line in Android).

Sett the default addr2line name in the Makefile with DEFAULT_ADDR2LINE_PATH.

Signed-off-by Irina Tirdea <[email protected]>

---
 tools/perf/Makefile        |    6 +++++-
 tools/perf/util/annotate.c |    3 ++-
 tools/perf/util/sort.c     |    2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

tools/perf/Makefile +5 -1

index 7a3f572..745d12b 100644
@@ -141,6 +141,7 @@ endif
 lib = lib
 PERF_TMP_DIR = /tmp
 OBJDUMP = $(CROSS_COMPILE)objdump
+ADDR2LINE = $(CROSS_COMPILE)addr2line
 
 export prefix bindir sharedir sysconfdir

@@ -885,7 +886,7 @@ $(OUTPUT)util/exec_cmd.o: util/exec_cmd.c $(OUTPUT)PERF-CFLAGS
 		$<
 
 $(OUTPUT)util/annotate.o: util/annotate.c $(OUTPUT)PERF-CFLAGS
-	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DDEFAULT_OBJDUMP_PATH='"$(OBJDUMP)"' $<
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DDEFAULT_OBJDUMP_PATH='"$(OBJDUMP)"' -DDEFAULT_ADDR2LINE_PATH='"$(ADDR2LIN)"' $<
 
 $(OUTPUT)util/config.o: util/config.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<

@@ -896,6 +897,9 @@ $(OUTPUT)util/dso-test-data.o: util/dso-test-data.c $(OUTPUT)PERF-CFLAGS
 $(OUTPUT)util/map.o: util/map.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
 
+$(OUTPUT)util/sort.o: util/sort.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DDEFAULT_ADDR2LINE_PATH='"$(addr2line)"' $<
+
 $(OUTPUT)util/symbol.o: util/symbol.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<

tools/perf/util/annotate.c +2 -1

index d7036b3..7734a03 100644
@@ -915,7 +915,8 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
 			continue;
 
 		offset = start + i;
-		sprintf(cmd, "addr2line -e %s %016" PRIx64, filename, offset);
+		sprintf(cmd, DEFAULT_ADDR2LINE_PATH " -e %s %016" PRIx64,
+			filename, offset);
 		fp = popen(cmd, "r");
 		if (!fp)
 			continue;

tools/perf/util/sort.c +1 -1

index b5b1b92..357dfc8 100644
@@ -260,7 +260,7 @@ static int hist_entry__srcline_snprintf(struct hist_entry *self, char *bf,
 	if (path != NULL)
 		goto out_path;
 
-	snprintf(cmd, sizeof(cmd), "addr2line -e %s %016" PRIx64,
+	snprintf(cmd, sizeof(cmd), DEFAULT_ADDR2LINE_PATH " -e %s %016" PRIx64,
 		 self->ms.map->dso->long_name, self->ip);
 	fp = popen(cmd, "r");
 	if (!fp)

Signature · 1 line

1.7.9.5

Irina Tirdea

author

2012-09-23 19:50 UTC #15 lore raw

PATCH v3 4/4 perf tools: configure addr2line path at compile time

From: Irina Tirdea <[email protected]>

The default name for addr2line is hardcoded to "addr2line". When cross-compiling the name of addr2line will be different (e.g. arm-eabi-addr2line in Android).

Set the default addr2line name in the Makefile with DEFAULT_ADDR2LINE_PATH.

Signed-off-by Irina Tirdea <[email protected]>

---

Changes in v3:
() Fixed typo in commit message.

 tools/perf/Makefile        |    6 +++++-
 tools/perf/util/annotate.c |    3 ++-
 tools/perf/util/sort.c     |    2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

tools/perf/Makefile +5 -1

index 7a3f572..745d12b 100644
@@ -141,6 +141,7 @@ endif
 lib = lib
 PERF_TMP_DIR = /tmp
 OBJDUMP = $(CROSS_COMPILE)objdump
+ADDR2LINE = $(CROSS_COMPILE)addr2line
 
 export prefix bindir sharedir sysconfdir

@@ -885,7 +886,7 @@ $(OUTPUT)util/exec_cmd.o: util/exec_cmd.c $(OUTPUT)PERF-CFLAGS
 		$<
 
 $(OUTPUT)util/annotate.o: util/annotate.c $(OUTPUT)PERF-CFLAGS
-	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DDEFAULT_OBJDUMP_PATH='"$(OBJDUMP)"' $<
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DDEFAULT_OBJDUMP_PATH='"$(OBJDUMP)"' -DDEFAULT_ADDR2LINE_PATH='"$(ADDR2LIN)"' $<
 
 $(OUTPUT)util/config.o: util/config.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<

@@ -896,6 +897,9 @@ $(OUTPUT)util/dso-test-data.o: util/dso-test-data.c $(OUTPUT)PERF-CFLAGS
 $(OUTPUT)util/map.o: util/map.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<
 
+$(OUTPUT)util/sort.o: util/sort.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DDEFAULT_ADDR2LINE_PATH='"$(addr2line)"' $<
+
 $(OUTPUT)util/symbol.o: util/symbol.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DPERF_TMP_DIR='"$(PERF_TMP_DIR)"' $<

tools/perf/util/annotate.c +2 -1

index d7036b3..7734a03 100644
@@ -915,7 +915,8 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
 			continue;
 
 		offset = start + i;
-		sprintf(cmd, "addr2line -e %s %016" PRIx64, filename, offset);
+		sprintf(cmd, DEFAULT_ADDR2LINE_PATH " -e %s %016" PRIx64,
+			filename, offset);
 		fp = popen(cmd, "r");
 		if (!fp)
 			continue;

tools/perf/util/sort.c +1 -1

index b5b1b92..357dfc8 100644
@@ -260,7 +260,7 @@ static int hist_entry__srcline_snprintf(struct hist_entry *self, char *bf,
 	if (path != NULL)
 		goto out_path;
 
-	snprintf(cmd, sizeof(cmd), "addr2line -e %s %016" PRIx64,
+	snprintf(cmd, sizeof(cmd), DEFAULT_ADDR2LINE_PATH " -e %s %016" PRIx64,
 		 self->ms.map->dso->long_name, self->ip);
 	fp = popen(cmd, "r");
 	if (!fp)

Signature · 1 line

1.7.9.5