[PATCH v2 0/4] perf: android: configure hardcoded paths

[PATCH v2 0/4] perf: android: configure hardcoded paths

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

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(-)

-- 
1.7.9.5

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

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

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 <irina.tirdea@intel.com>
---
 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(-)

diff --git a/tools/perf/Documentation/jit-interface.txt b/tools/perf/Documentation/jit-interface.txt
index a8656f5..10cb6ec 100644
--- a/tools/perf/Documentation/jit-interface.txt
+++ b/tools/perf/Documentation/jit-interface.txt
@@ -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.
 
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 5077f8e..eab4a36 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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 $<
 
diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
index e919306..058c3b7 100644
--- a/tools/perf/perf-archive.sh
+++ b/tools/perf/perf-archive.sh
@@ -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 | \
diff --git a/tools/perf/util/dso-test-data.c b/tools/perf/util/dso-test-data.c
index c6caede..ca81e65 100644
--- a/tools/perf/util/dso-test-data.c
+++ b/tools/perf/util/dso-test-data.c
@@ -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;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index b442ee4..c5b4632 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -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;
 		}
 
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 8a2229d..5ebde18 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -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;
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index e2e8c69..d8aca82 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -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)
diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index a8d81c3..e50ea61 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -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");
 
-- 
1.7.9.5

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

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

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 <irina.tirdea@intel.com>
---
 tools/perf/Makefile         |    6 +++++-
 tools/perf/builtin-help.c   |    2 +-
 tools/perf/builtin-script.c |   12 ++++++------
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index eab4a36..fae52c8 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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) \
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 25c8b94..e7d640c 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -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));
 }
 
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 1be843a..b626cf1 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -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);
 	}
-- 
1.7.9.5

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

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

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 <irina.tirdea@intel.com>
---
 tools/perf/Makefile        |    4 ++++
 tools/perf/util/annotate.c |    2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index fae52c8..7a3f572 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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)"' $<
 
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index f0a9103..d7036b3 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -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),
-- 
1.7.9.5

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

[Namhyung Kim]

Hi Irina,

2012-09-23 (일), 22:27 +0300, Irina Tirdea:
> From: Irina Tirdea <irina.tirdea@intel.com>
> 
> 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

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

[Irina Tirdea]

On Tue, Sep 25, 2012 at 4:08 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> Hi Irina,

Hi Namhyung,

> 2012-09-23 (일), 22:27 +0300, Irina Tirdea:
>> From: Irina Tirdea <irina.tirdea@intel.com>
>>
>> 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?
>

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

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

[Namhyung Kim]

On Thu, 27 Sep 2012 03:51:07 +0300, Irina Tirdea wrote:
> On Tue, Sep 25, 2012 at 4:08 PM, Namhyung Kim <namhyung@kernel.org> wrote:
>> 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.

Ok.

>
> 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?

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

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

[Irina Tirdea]

>> 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?
>
> 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

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

[Namhyung Kim]

On Thu, 27 Sep 2012 14:25:10 +0300, Irina Tirdea wrote:
>>> 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?
>>
>> 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.

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


>From 7068a43a4b450c60fdcc8a3916ce624c1ef2c9b2 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung.kim@lge.com>
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 <irina.tirdea@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 5973f383eb8e..b189229eb576 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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
diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
new file mode 100644
index 000000000000..c3872427d135
--- /dev/null
+++ b/tools/perf/arch/common.c
@@ -0,0 +1,42 @@
+#include <stdio.h>
+
+const char * const arm_triplets[] = {
+	"arm-eabi-",
+	"arm-unknown-linux-",
+	"arm-unknown-linux-gnu-",
+	"arm-unknown-linux-gnueabi-",
+	NULL
+};
+
+const char * const powerpc_triplets[] = {
+	"powerpc-unknown-linux-gnu-",
+	"powerpc64-unknown-linux-gnu-",
+	NULL
+};
+
+const char * const s390_triplets[] = {
+	"s390-ibm-linux-",
+	NULL
+};
+
+const char * const sh_triplets[] = {
+	"sh-unknown-linux-gnu-",
+	"sh64-unknown-linux-gnu-",
+	NULL
+};
+
+const char * const sparc_triplets[] = {
+	"sparc-unknown-linux-gnu-",
+	"sparc64-unknown-linux-gnu-",
+	NULL
+};
+
+const char * const x86_triplets[] = {
+	"x86_64-pc-linux-gnu-",
+	"x86_64-unknown-linux-gnu-",
+	"i686-pc-linux-gnu-",
+	"i586-pc-linux-gnu-",
+	"i486-pc-linux-gnu-",
+	"i386-pc-linux-gnu-",
+	NULL
+};
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 9ea38540b873..20fe9bb6505b 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -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;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index f0a910371377..afe181af56d6 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -15,6 +15,7 @@
 #include "debug.h"
 #include "annotate.h"
 #include <pthread.h>
+#include <sys/utsname.h>
 
 const char 	*disassembler_style;
 const char	*objdump_path;
@@ -1140,3 +1141,98 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 
 	return 0;
 }
+
+static int lookup_path(char *name)
+{
+	char *path, *tmp;
+	char buf[PATH_MAX];
+	char *env = getenv("PATH");
+
+	if (!env)
+		return 0;
+
+	path = strtok_r(env, ":", &tmp);
+	while (path) {
+		scnprintf(buf, sizeof(buf), "%s%s", path, name);
+		if (access(buf, F_OK) == 0)
+			return 1;
+
+		strtok_r(NULL, ":", &tmp);
+	}
+	return 0;
+}
+
+static int lookup_triplets(const char **triplets, const char *name)
+{
+	int i;
+	char buf[PATH_MAX];
+
+	for (i = 0; triplets[i] != NULL; i++) {
+		scnprintf(buf, sizeof(buf), "%s%s", triplets[i], name);
+		if (lookup_path(buf))
+			return i;
+	}
+	return -1;
+}
+
+static char *try_binutils_path(struct perf_session *session, const char *name)
+{
+	int idx;
+	char *arch, *env;
+	struct utsname uts;
+	const char **path_list;
+	char buf[PATH_MAX];
+
+	if (uname(&uts) < 0)
+		return NULL;
+
+	/*
+	 * We don't need to try to find objdump path for native system.
+	 * Just use default "objdump".
+	 */
+	if (!strcmp(uts.machine, session->header.env.arch))
+		return NULL;
+
+	env = getenv("CROSS_COMPILE");
+	if (env) {
+		scnprintf(buf, sizeof(buf), "%s%s", env, name);
+		if (buf[0] == '/') {
+			if (access(buf, F_OK) == 0)
+				return strdup(buf);
+
+			return NULL;
+		}
+
+		if (lookup_path(buf))
+			return strdup(buf);
+	}
+
+	arch = session->header.env.arch;
+
+	if (!strcmp(arch, "arm"))
+		path_list = arm_triplets;
+	else if (!strcmp(arch, "powerpc"))
+		path_list = powerpc_triplets;
+	else if (!strcmp(arch, "sh"))
+		path_list = sh_triplets;
+	else if (!strcmp(arch, "s390"))
+		path_list = s390_triplets;
+	else if (!strcmp(arch, "sparc"))
+		path_list = sparc_triplets;
+	else if (!strcmp(arch, "x86"))
+		path_list = x86_triplets;
+	else
+		BUG_ON(1);
+
+	idx = lookup_triplets(path_list, name);
+	if (idx < 0)
+		return NULL;
+
+	scnprintf(buf, sizeof(buf), path_list[idx], name);
+	return strdup(buf);
+}
+
+void try_objdump_path(struct perf_session *session)
+{
+	objdump_path = try_binutils_path(session, "objdump");
+}
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 9b5b21e7b032..cbf2d9537320 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -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 */
-- 
1.7.11.4

[RFC v2] perf tools: Try to find cross-built objdump path

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

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 <irina.tirdea@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
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

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 5973f383eb8e..b189229eb576 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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
diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
new file mode 100644
index 000000000000..c3872427d135
--- /dev/null
+++ b/tools/perf/arch/common.c
@@ -0,0 +1,42 @@
+#include <stdio.h>
+
+const char * const arm_triplets[] = {
+	"arm-eabi-",
+	"arm-unknown-linux-",
+	"arm-unknown-linux-gnu-",
+	"arm-unknown-linux-gnueabi-",
+	NULL
+};
+
+const char * const powerpc_triplets[] = {
+	"powerpc-unknown-linux-gnu-",
+	"powerpc64-unknown-linux-gnu-",
+	NULL
+};
+
+const char * const s390_triplets[] = {
+	"s390-ibm-linux-",
+	NULL
+};
+
+const char * const sh_triplets[] = {
+	"sh-unknown-linux-gnu-",
+	"sh64-unknown-linux-gnu-",
+	NULL
+};
+
+const char * const sparc_triplets[] = {
+	"sparc-unknown-linux-gnu-",
+	"sparc64-unknown-linux-gnu-",
+	NULL
+};
+
+const char * const x86_triplets[] = {
+	"x86_64-pc-linux-gnu-",
+	"x86_64-unknown-linux-gnu-",
+	"i686-pc-linux-gnu-",
+	"i586-pc-linux-gnu-",
+	"i486-pc-linux-gnu-",
+	"i386-pc-linux-gnu-",
+	NULL
+};
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 9ea38540b873..20fe9bb6505b 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -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;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index f0a910371377..df17c443e3b3 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -15,6 +15,7 @@
 #include "debug.h"
 #include "annotate.h"
 #include <pthread.h>
+#include <sys/utsname.h>
 
 const char 	*disassembler_style;
 const char	*objdump_path;
@@ -1140,3 +1141,105 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 
 	return 0;
 }
+
+static bool lookup_path(char *name)
+{
+	bool found = false;
+	char *path, *tmp;
+	char buf[PATH_MAX];
+	char *env = getenv("PATH");
+
+	if (!env)
+		return false;
+
+	env = strdup(env);
+	if (!env)
+		return false;
+
+	path = strtok_r(env, ":", &tmp);
+	while (path) {
+		scnprintf(buf, sizeof(buf), "%s%s", path, name);
+		if (access(buf, F_OK) == 0) {
+			found = true;
+			break;
+		}
+		strtok_r(NULL, ":", &tmp);
+	}
+	free(env);
+	return found;
+}
+
+static int lookup_triplets(const char **triplets, const char *name)
+{
+	int i;
+	char buf[PATH_MAX];
+
+	for (i = 0; triplets[i] != NULL; i++) {
+		scnprintf(buf, sizeof(buf), "%s%s", triplets[i], name);
+		if (lookup_path(buf))
+			return i;
+	}
+	return -1;
+}
+
+static char *try_binutils_path(struct perf_session *session, const char *name)
+{
+	int idx;
+	char *arch, *env;
+	struct utsname uts;
+	const char **path_list;
+	char buf[PATH_MAX];
+
+	if (uname(&uts) < 0)
+		return NULL;
+
+	/*
+	 * We don't need to try to find objdump path for native system.
+	 * Just use default "objdump".
+	 */
+	if (!strcmp(uts.machine, session->header.env.arch))
+		return NULL;
+
+	env = getenv("CROSS_COMPILE");
+	if (env) {
+		scnprintf(buf, sizeof(buf), "%s%s", env, name);
+		if (buf[0] == '/') {
+			if (access(buf, F_OK) == 0)
+				return strdup(buf);
+
+			return NULL;
+		}
+
+		if (lookup_path(buf))
+			return strdup(buf);
+	}
+
+	arch = session->header.env.arch;
+
+	if (!strcmp(arch, "arm"))
+		path_list = arm_triplets;
+	else if (!strcmp(arch, "powerpc"))
+		path_list = powerpc_triplets;
+	else if (!strcmp(arch, "sh"))
+		path_list = sh_triplets;
+	else if (!strcmp(arch, "s390"))
+		path_list = s390_triplets;
+	else if (!strcmp(arch, "sparc"))
+		path_list = sparc_triplets;
+	else if (!strcmp(arch, "x86"))
+		path_list = x86_triplets;
+	else
+		BUG_ON(1);
+
+	idx = lookup_triplets(path_list, name);
+	if (idx < 0)
+		return NULL;
+
+	scnprintf(buf, sizeof(buf), path_list[idx], name);
+	return strdup(buf);
+}
+
+void try_objdump_path(struct perf_session *session)
+{
+	objdump_path = try_binutils_path(session, "objdump");
+}
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 9b5b21e7b032..cbf2d9537320 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -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 */
-- 
1.7.11.4

[RFC v3] perf tools: Try to find cross-built objdump path

[Irina Tirdea]

From: Namhyung Kim <namhyung.kim@lge.com>

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 <namhyung@kernel.org>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---

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

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index e5e71e7..816fc04 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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
diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
new file mode 100644
index 0000000..6904777
--- /dev/null
+++ b/tools/perf/arch/common.c
@@ -0,0 +1,51 @@
+#include <stdio.h>
+
+const char *const arm_triplets[] = {
+	"arm-eabi-",
+	"arm-linux-androideabi-",
+	"arm-unknown-linux-",
+	"arm-unknown-linux-gnu-",
+	"arm-unknown-linux-gnueabi-",
+	NULL
+};
+
+const char *const powerpc_triplets[] = {
+	"powerpc-unknown-linux-gnu-",
+	"powerpc64-unknown-linux-gnu-",
+	NULL
+};
+
+const char *const s390_triplets[] = {
+	"s390-ibm-linux-",
+	NULL
+};
+
+const char *const sh_triplets[] = {
+	"sh-unknown-linux-gnu-",
+	"sh64-unknown-linux-gnu-",
+	NULL
+};
+
+const char *const sparc_triplets[] = {
+	"sparc-unknown-linux-gnu-",
+	"sparc64-unknown-linux-gnu-",
+	NULL
+};
+
+const char *const x86_triplets[] = {
+	"x86_64-pc-linux-gnu-",
+	"x86_64-unknown-linux-gnu-",
+	"i686-pc-linux-gnu-",
+	"i586-pc-linux-gnu-",
+	"i486-pc-linux-gnu-",
+	"i386-pc-linux-gnu-",
+	"i686-linux-android-",
+	"i686-android-linux-",
+	NULL
+};
+
+const char *const mips_triplets[] = {
+	"mips-unknown-linux-gnu-",
+	"mipsel-linux-android-",
+	NULL
+};
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 9ea3854..20fe9bb 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -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;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index f0a9103..36f3af0 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -15,6 +15,7 @@
 #include "debug.h"
 #include "annotate.h"
 #include <pthread.h>
+#include <sys/utsname.h>
 
 const char 	*disassembler_style;
 const char	*objdump_path;
@@ -1140,3 +1141,109 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 
 	return 0;
 }
+
+static bool lookup_path(char *name)
+{
+	bool found = false;
+	char *path, *tmp;
+	char buf[PATH_MAX];
+	char *env = getenv("PATH");
+
+	if (!env)
+		return false;
+
+	env = strdup(env);
+	if (!env)
+		return false;
+
+	path = strtok_r(env, ":", &tmp);
+	while (path) {
+		scnprintf(buf, sizeof(buf), "%s/%s", path, name);
+		if (access(buf, F_OK) == 0) {
+			found = true;
+			break;
+		}
+		path = strtok_r(NULL, ":", &tmp);
+	}
+	free(env);
+	return found;
+}
+
+static int lookup_triplets(const char **triplets, const char *name)
+{
+	int i;
+	char buf[PATH_MAX];
+
+	for (i = 0; triplets[i] != NULL; i++) {
+		scnprintf(buf, sizeof(buf), "%s%s", triplets[i], name);
+		if (lookup_path(buf))
+			return i;
+	}
+	return -1;
+}
+
+static char *try_binutils_path(struct perf_session *session, const char *name)
+{
+	int idx;
+	char *arch, *env;
+	struct utsname uts;
+	const char **path_list;
+	char buf[PATH_MAX];
+
+	if (uname(&uts) < 0)
+		return NULL;
+
+	/*
+	 * We don't need to try to find objdump path for native system.
+	 * Just use default "objdump".
+	 */
+	if (!strcmp(uts.machine, session->header.env.arch))
+		return NULL;
+
+	env = getenv("CROSS_COMPILE");
+	if (env) {
+		scnprintf(buf, sizeof(buf), "%s%s", env, name);
+		if (buf[0] == '/') {
+			if (access(buf, F_OK) == 0)
+				return strdup(buf);
+
+			return NULL;
+		}
+
+		if (lookup_path(buf))
+			return strdup(buf);
+	}
+
+	arch = session->header.env.arch;
+
+	if (!strcmp(arch, "arm"))
+		path_list = arm_triplets;
+	else if (!strcmp(arch, "powerpc"))
+		path_list = powerpc_triplets;
+	else if (!strcmp(arch, "sh"))
+		path_list = sh_triplets;
+	else if (!strcmp(arch, "s390"))
+		path_list = s390_triplets;
+	else if (!strcmp(arch, "sparc"))
+		path_list = sparc_triplets;
+	else if (!strcmp(arch, "x86") || !strcmp(arch, "i386") ||
+		 !strcmp(arch, "i486") || !strcmp(arch, "i586") ||
+		 !strcmp(arch, "i686"))
+		path_list = x86_triplets;
+	else if (!strcmp(arch, "mips"))
+		path_list = mips_triplets;
+	else
+		BUG_ON(1);
+
+	idx = lookup_triplets(path_list, name);
+	if (idx < 0)
+		return NULL;
+
+	scnprintf(buf, sizeof(buf), "%s%s", path_list[idx], name);
+	return strdup(buf);
+}
+
+void try_objdump_path(struct perf_session *session)
+{
+	objdump_path = try_binutils_path(session, "objdump");
+}
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 9b5b21e..4918d1f 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -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 */
-- 
1.7.9.5

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

[Irina Tirdea]

On Thu, Sep 27, 2012 at 4:16 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> On Thu, 27 Sep 2012 14:25:10 +0300, Irina Tirdea wrote:
>>>> 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?
>>>
>>> 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.
>
> 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

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

[Ingo Molnar]

* Namhyung Kim <namhyung@kernel.org> wrote:

> On Thu, 27 Sep 2012 14:25:10 +0300, Irina Tirdea wrote:
> >>> 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?
> >>
> >> 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.
> 
> 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? :)

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

Thanks,

	Ingo

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

[Arnaldo Carvalho de Melo]

Em Mon, Oct 01, 2012 at 09:21:54AM +0200, Ingo Molnar escreveu:
> * Namhyung Kim <namhyung@kernel.org> wrote:
> > 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? :)

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

Indeed, great idea!

- Arnaldo

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

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

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 <irina.tirdea@intel.com>
---
 tools/perf/Makefile        |    6 +++++-
 tools/perf/util/annotate.c |    3 ++-
 tools/perf/util/sort.c     |    2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 7a3f572..745d12b 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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)"' $<
 
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index d7036b3..7734a03 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -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;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index b5b1b92..357dfc8 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -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)
-- 
1.7.9.5

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

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

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 <irina.tirdea@intel.com>
---

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(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 7a3f572..745d12b 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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)"' $<
 
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index d7036b3..7734a03 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -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;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index b5b1b92..357dfc8 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -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)
-- 
1.7.9.5