[PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use.

linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use.
@ 2012-08-10 22:22 Cody P Schafer
  2012-08-10 22:22 ` [PATCH 01/16] perf symbol: correct comment wrt kallsyms loading Cody P Schafer
                   ` (15 more replies)
  0 siblings, 16 replies; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

1-4,6,7 are small cleanups.

5 fixes a potential segfault.

8 fixes a use after free for dso->long_name

9 avoids a segfault in elfutils when a truncated elf is loaded.

10 properly tracks that a dso had symbols loaded from a vmlinux image

11-16 fix handling of the '.opd' section in the presence of debuginfo. They
      should also fix plt symbol synthesis (haven't seen the plt issues in
      practice).

--

Changes from v1:
 - rebased on top of
   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git  perf/core
 - In #2, make the symbols have size 0 instead of 1.

--

 tools/perf/util/event.c          |   2 +-
 tools/perf/util/map.c            |   8 --
 tools/perf/util/map.h            |   1 -
 tools/perf/util/symbol-elf.c     | 182 ++++++++++++++++++++++++++-------------
 tools/perf/util/symbol-minimal.c |  48 +++++++++--
 tools/perf/util/symbol.c         | 136 +++++++++++++++++------------
 tools/perf/util/symbol.h         |  49 +++++++++--
 7 files changed, 290 insertions(+), 136 deletions(-)


^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH 01/16] perf symbol: correct comment wrt kallsyms loading
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-11 13:14   ` Namhyung Kim
  2012-08-21 16:00   ` [tip:perf/core] perf symbols: Correct " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb Cody P Schafer
                   ` (14 subsequent siblings)
  15 siblings, 2 replies; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

In kallsyms_parse() when calling process_symbol() (a callback argument
to kallsyms_parse()), we pass start as both start & end (ie:
start=start, end=start).

In map__process_kallsym_symbol(), the length is calculated as 'end - start + 1',
making the length 1, not 0.

Essentially, start & end define an inclusive range.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index f02de8a..891f83c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -605,7 +605,7 @@ int kallsyms__parse(const char *filename, void *arg,
 
 		/*
 		 * module symbols are not sorted so we add all
-		 * symbols with zero length and rely on
+		 * symbols, setting length to 1, and rely on
 		 * symbols__fixup_end() to fix it up.
 		 */
 		err = process_symbol(arg, symbol_name,
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
  2012-08-10 22:22 ` [PATCH 01/16] perf symbol: correct comment wrt kallsyms loading Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 16:01   ` [tip:perf/core] perf symbols: Remove " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 03/16] perf symbol: only un-prelink non-zero symbols Cody P Schafer
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

kallsyms__parse() takes a callback that is called on every discovered
symbol. As /proc/kallsyms does not supply symbol sizes, the callback was
simply called with end=start, faking the symbol size to 1.

All of the callbacks (there are 2) used in calls to kallsyms__parse()
are _only_ used as callbacks for kallsyms__parse().

Given that kallsyms__parse() lacks real information about what
end/length should be, don't make up a length in kallsyms__parse().
Instead have the callbacks handle guessing the length.

Also relocate a comment regarding symbol creation to the callback which
does symbol creation (kallsyms__parse() is not in general used to create
symbols).

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/event.c  |  2 +-
 tools/perf/util/symbol.c | 21 ++++++++++-----------
 tools/perf/util/symbol.h |  2 +-
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 2a6f33c..3a0f1a5 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -412,7 +412,7 @@ struct process_symbol_args {
 };
 
 static int find_symbol_cb(void *arg, const char *name, char type,
-			  u64 start, u64 end __used)
+			  u64 start)
 {
 	struct process_symbol_args *args = arg;
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 891f83c..9afe6b1 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -563,7 +563,7 @@ size_t dso__fprintf(struct dso *dso, enum map_type type, FILE *fp)
 
 int kallsyms__parse(const char *filename, void *arg,
 		    int (*process_symbol)(void *arg, const char *name,
-					  char type, u64 start, u64 end))
+					  char type, u64 start))
 {
 	char *line = NULL;
 	size_t n;
@@ -603,13 +603,8 @@ int kallsyms__parse(const char *filename, void *arg,
 			break;
 		}
 
-		/*
-		 * module symbols are not sorted so we add all
-		 * symbols, setting length to 1, and rely on
-		 * symbols__fixup_end() to fix it up.
-		 */
 		err = process_symbol(arg, symbol_name,
-				     symbol_type, start, start);
+				     symbol_type, start);
 		if (err)
 			break;
 	}
@@ -636,7 +631,7 @@ static u8 kallsyms2elf_type(char type)
 }
 
 static int map__process_kallsym_symbol(void *arg, const char *name,
-				       char type, u64 start, u64 end)
+				       char type, u64 start)
 {
 	struct symbol *sym;
 	struct process_kallsyms_args *a = arg;
@@ -645,8 +640,12 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
 	if (!symbol_type__is_a(type, a->map->type))
 		return 0;
 
-	sym = symbol__new(start, end - start + 1,
-			  kallsyms2elf_type(type), name);
+	/*
+	 * module symbols are not sorted so we add all
+	 * symbols, setting length to 0, and rely on
+	 * symbols__fixup_end() to fix it up.
+	 */
+	sym = symbol__new(start, 0, kallsyms2elf_type(type), name);
 	if (sym == NULL)
 		return -ENOMEM;
 	/*
@@ -1731,7 +1730,7 @@ struct process_args {
 };
 
 static int symbol__in_kernel(void *arg, const char *name,
-			     char type __used, u64 start, u64 end __used)
+			     char type __used, u64 start)
 {
 	struct process_args *args = arg;
 
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 38ccbbb..c9534fe 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -299,7 +299,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits);
 int build_id__sprintf(const u8 *build_id, int len, char *bf);
 int kallsyms__parse(const char *filename, void *arg,
 		    int (*process_symbol)(void *arg, const char *name,
-					  char type, u64 start, u64 end));
+					  char type, u64 start));
 int filename__read_debuglink(const char *filename, char *debuglink,
 			     size_t size);
 
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 03/16] perf symbol: only un-prelink non-zero symbols
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
  2012-08-10 22:22 ` [PATCH 01/16] perf symbol: correct comment wrt kallsyms loading Cody P Schafer
  2012-08-10 22:22 ` [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 15:56   ` [tip:perf/core] perf symbols: Only " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 04/16] perf utils: remove unused function map__objdump_2ip Cody P Schafer
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

Prelink only adjusts the addresses of non-zero symbols. Do the same when
we reverse the adjustments.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 9ca89f8..e037609 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -717,7 +717,7 @@ int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
 			goto new_symbol;
 		}
 
-		if (curr_dso->adjust_symbols) {
+		if (curr_dso->adjust_symbols && sym.st_value) {
 			pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
 				  "sh_addr: %#" PRIx64 " sh_offset: %#" PRIx64 "\n", __func__,
 				  (u64)sym.st_value, (u64)shdr.sh_addr,
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 04/16] perf utils: remove unused function map__objdump_2ip
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (2 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 03/16] perf symbol: only un-prelink non-zero symbols Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 15:57   ` [tip:perf/core] perf symbols: Remove " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 05/16] perf symbol: don't try to synthesize plt without dynstr Cody P Schafer
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

map__objdump_2ip was introduced in:
ee11b90b12 perf top: Fix annotate for userspace

And it's last user removed in:
36532461a0 perf top: Ditch private annotation code, share perf annotate's

Remove it.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/map.c | 8 --------
 tools/perf/util/map.h | 1 -
 2 files changed, 9 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 287cb34..7d37159 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -262,14 +262,6 @@ u64 map__rip_2objdump(struct map *map, u64 rip)
 	return addr;
 }
 
-u64 map__objdump_2ip(struct map *map, u64 addr)
-{
-	u64 ip = map->dso->adjust_symbols ?
-			addr :
-			map->unmap_ip(map, addr);	/* RIP -> IP */
-	return ip;
-}
-
 void map_groups__init(struct map_groups *mg)
 {
 	int i;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 1e183d1..6c38d9e 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -104,7 +104,6 @@ static inline u64 identity__map_ip(struct map *map __used, u64 ip)
 
 /* rip/ip <-> addr suitable for passing to `objdump --start-address=` */
 u64 map__rip_2objdump(struct map *map, u64 rip);
-u64 map__objdump_2ip(struct map *map, u64 addr);
 
 struct symbol;
 
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 05/16] perf symbol: don't try to synthesize plt without dynstr.
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (3 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 04/16] perf utils: remove unused function map__objdump_2ip Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 15:58   ` [tip:perf/core] perf symbols: Don' t " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 06/16] perf symbol: remove unneeded call to dso__set_long_name() Cody P Schafer
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

If .dynsym exists but .dynstr is empty (NO_BITS or size==0), a segfault
occurs. Avoid this by checking that .dynstr is not empty.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index e037609..a2e994e 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -232,6 +232,9 @@ int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
 	if (symstrs == NULL)
 		goto out_elf_end;
 
+	if (symstrs->d_size == 0)
+		goto out_elf_end;
+
 	nr_rel_entries = shdr_rel_plt.sh_size / shdr_rel_plt.sh_entsize;
 	plt_offset = shdr_plt.sh_offset;
 
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 06/16] perf symbol: remove unneeded call to dso__set_long_name()
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (4 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 05/16] perf symbol: don't try to synthesize plt without dynstr Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 15:59   ` [tip:perf/core] perf symbols: Remove " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 07/16] perf symbol: simplify out_fixup in kernel syms loading Cody P Schafer
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

dso__set_long_name() is already called by dso__load_vmlinux(), avoid
calling it a second time unnecessarily.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 9afe6b1..2127002 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1387,10 +1387,8 @@ int dso__load_vmlinux_path(struct dso *dso, struct map *map,
 	filename = dso__build_id_filename(dso, NULL, 0);
 	if (filename != NULL) {
 		err = dso__load_vmlinux(dso, map, filename, filter);
-		if (err > 0) {
-			dso__set_long_name(dso, filename);
+		if (err > 0)
 			goto out;
-		}
 		free(filename);
 	}
 
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 07/16] perf symbol: simplify out_fixup in kernel syms loading
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (5 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 06/16] perf symbol: remove unneeded call to dso__set_long_name() Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 16:02   ` [tip:perf/core] perf symbols: Simplify " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 08/16] perf symbol: only set vmlinux longname & mark loaded if really loaded Cody P Schafer
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

The only site that jumps to out_fixup has (kallsyms_filename == NULL).
And all paths that reach 'if (err > 0)' without 'goto out_fixup' have
kallsyms_filename != NULL.

So skip over both the check & dso__set_long_name(), and remove the
check.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2127002..e5c3817 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1503,9 +1503,8 @@ do_kallsyms:
 	free(kallsyms_allocated_filename);
 
 	if (err > 0) {
+		dso__set_long_name(dso, strdup("[kernel.kallsyms]"));
 out_fixup:
-		if (kallsyms_filename != NULL)
-			dso__set_long_name(dso, strdup("[kernel.kallsyms]"));
 		map__fixup_start(map);
 		map__fixup_end(map);
 	}
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 08/16] perf symbol: only set vmlinux longname & mark loaded if really loaded
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (6 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 07/16] perf symbol: simplify out_fixup in kernel syms loading Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 16:03   ` [tip:perf/core] perf symbols: " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 09/16] perf symbol: avoid segfault in elf_strptr Cody P Schafer
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

dso__load_vmlinux() uses the filename passed to it to directly set the
dso long_name, which resulted in a use after free due to
dso__load_vmlinux_path() treating 0 symbols as a load failure and
subsequently freeing the contents of dso->long_name.

Change dso__load_vmlinux() so that finding 0 symbols does not cause it
to consider itself loaded, and do not set long_name in such a case.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index e5c3817..96dbf28 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1364,13 +1364,14 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (fd < 0)
 		return -1;
 
-	dso__set_long_name(dso, (char *)vmlinux);
-	dso__set_loaded(dso, map->type);
 	err = dso__load_sym(dso, map, symfs_vmlinux, fd, filter, 0, 0);
 	close(fd);
 
-	if (err > 0)
+	if (err > 0) {
+		dso__set_long_name(dso, (char *)vmlinux);
+		dso__set_loaded(dso, map->type);
 		pr_debug("Using %s for symbols\n", symfs_vmlinux);
+	}
 
 	return err;
 }
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 09/16] perf symbol: avoid segfault in elf_strptr
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (7 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 08/16] perf symbol: only set vmlinux longname & mark loaded if really loaded Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 16:04   ` [tip:perf/core] perf symbols: Avoid " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 10/16] perf symbol: track symtab_type of vmlinux Cody P Schafer
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

If we call elf_section_by_name() with a truncated elf image (ie: the file
header indicates that the section headers are placed past the end of the
file), elf_strptr() causes a segfault within libelf.

Avoid this by checking that we can access the section string table
properly.

Should really be fixed in libelf/elfutils.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a2e994e..a9a194d 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -129,6 +129,10 @@ static Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
 	Elf_Scn *sec = NULL;
 	size_t cnt = 1;
 
+	/* Elf is corrupted/truncated, avoid calling elf_strptr. */
+	if (!elf_rawdata(elf_getscn(elf, ep->e_shstrndx), NULL))
+		return NULL;
+
 	while ((sec = elf_nextscn(elf, sec)) != NULL) {
 		char *str;
 
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 10/16] perf symbol: track symtab_type of vmlinux
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (8 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 09/16] perf symbol: avoid segfault in elf_strptr Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 16:05   ` [tip:perf/core] perf symbols: Track " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 11/16] perf symbol: introduce symsrc structure Cody P Schafer
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

Previously, symtab_type would have been left at 0, or KALLSYMS, which is not
quite accurate.

Introduce DSO_SYMTAB_TYPE__VMLINUX[_GUEST].

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol.c | 9 +++++++++
 tools/perf/util/symbol.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 96dbf28..8f5cabbf 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -923,6 +923,7 @@ char dso__symtab_origin(const struct dso *dso)
 {
 	static const char origin[] = {
 		[DSO_BINARY_TYPE__KALLSYMS]		= 'k',
+		[DSO_BINARY_TYPE__VMLINUX]		= 'v',
 		[DSO_BINARY_TYPE__JAVA_JIT]		= 'j',
 		[DSO_BINARY_TYPE__DEBUGLINK]		= 'l',
 		[DSO_BINARY_TYPE__BUILD_ID_CACHE]	= 'B',
@@ -933,6 +934,7 @@ char dso__symtab_origin(const struct dso *dso)
 		[DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE]	= 'K',
 		[DSO_BINARY_TYPE__GUEST_KALLSYMS]	= 'g',
 		[DSO_BINARY_TYPE__GUEST_KMODULE]	= 'G',
+		[DSO_BINARY_TYPE__GUEST_VMLINUX]	= 'V',
 	};
 
 	if (dso == NULL || dso->symtab_type == DSO_BINARY_TYPE__NOT_FOUND)
@@ -1008,7 +1010,9 @@ int dso__binary_type_file(struct dso *dso, enum dso_binary_type type,
 
 	default:
 	case DSO_BINARY_TYPE__KALLSYMS:
+	case DSO_BINARY_TYPE__VMLINUX:
 	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
+	case DSO_BINARY_TYPE__GUEST_VMLINUX:
 	case DSO_BINARY_TYPE__JAVA_JIT:
 	case DSO_BINARY_TYPE__NOT_FOUND:
 		ret = -1;
@@ -1364,6 +1368,11 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (fd < 0)
 		return -1;
 
+	if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
+		dso->symtab_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
+	else
+		dso->symtab_type = DSO_BINARY_TYPE__VMLINUX;
+
 	err = dso__load_sym(dso, map, symfs_vmlinux, fd, filter, 0, 0);
 	close(fd);
 
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index c9534fe..37f1ea1 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -158,6 +158,8 @@ struct addr_location {
 enum dso_binary_type {
 	DSO_BINARY_TYPE__KALLSYMS = 0,
 	DSO_BINARY_TYPE__GUEST_KALLSYMS,
+	DSO_BINARY_TYPE__VMLINUX,
+	DSO_BINARY_TYPE__GUEST_VMLINUX,
 	DSO_BINARY_TYPE__JAVA_JIT,
 	DSO_BINARY_TYPE__DEBUGLINK,
 	DSO_BINARY_TYPE__BUILD_ID_CACHE,
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 11/16] perf symbol: introduce symsrc structure.
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (9 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 10/16] perf symbol: track symtab_type of vmlinux Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-11 13:28   ` Namhyung Kim
  2012-08-21 16:06   ` [tip:perf/core] perf symbols: Introduce " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 12/16] perf symbol: set symtab_type in dso__load_sym Cody P Schafer
                   ` (4 subsequent siblings)
  15 siblings, 2 replies; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

Factors opening of certain sections & tracking certain elf info into an
external structure.

The goal here is to keep multiple elfs (and their looked up
sections/indexes) around during the symbol generation process (in
dso__load()).

We need this to properly resolve symbols on PPC due to the
use of function descriptors & the .opd section (ie: symbols which are
functions don't point to their actual location, they point to their
function descriptor in .opd which contains their actual location.

It would be possible to just keep the (Elf *) around, but then we'd end
up with duplicate code for looking up the same sections and checking for
the existence of an important section wouldn't be as clean (and we need
to keep the Elf stuff confined to symtab-elf.c).

Utilized by the later patch
"perf symbol: use both runtime and debug images"

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c     | 119 +++++++++++++++++++++++++++++----------
 tools/perf/util/symbol-minimal.c |  30 +++++++++-
 tools/perf/util/symbol.c         |  22 ++++----
 tools/perf/util/symbol.h         |  36 +++++++++++-
 4 files changed, 163 insertions(+), 44 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a9a194d..6974b2a 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -536,24 +536,25 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
 	return 0;
 }
 
-int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
-		  symbol_filter_t filter, int kmodule, int want_symtab)
+
+void symsrc__destroy(struct symsrc *ss)
+{
+	free(ss->name);
+	elf_end(ss->elf);
+	close(ss->fd);
+}
+
+int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
+		 enum dso_binary_type type)
 {
-	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
-	struct map *curr_map = map;
-	struct dso *curr_dso = dso;
-	Elf_Data *symstrs, *secstrs;
-	uint32_t nr_syms;
 	int err = -1;
-	uint32_t idx;
 	GElf_Ehdr ehdr;
-	GElf_Shdr shdr, opdshdr;
-	Elf_Data *syms, *opddata = NULL;
-	GElf_Sym sym;
-	Elf_Scn *sec, *sec_strndx, *opdsec;
 	Elf *elf;
-	int nr = 0;
-	size_t opdidx = 0;
+	int fd;
+
+	fd = open(name, O_RDONLY);
+	if (fd < 0)
+		return -1;
 
 	elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
 	if (elf == NULL) {
@@ -580,19 +581,88 @@ int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
 			goto out_elf_end;
 	}
 
-	sec = elf_section_by_name(elf, &ehdr, &shdr, ".symtab", NULL);
+	ss->symtab = elf_section_by_name(elf, &ehdr, &ss->symshdr, ".symtab",
+			NULL);
+	if (ss->symshdr.sh_type != SHT_SYMTAB)
+		ss->symtab = NULL;
+
+	ss->dynsym_idx = 0;
+	ss->dynsym = elf_section_by_name(elf, &ehdr, &ss->dynshdr, ".dynsym",
+			&ss->dynsym_idx);
+	if (ss->dynshdr.sh_type != SHT_DYNSYM)
+		ss->dynsym = NULL;
+
+	ss->opdidx = 0;
+	ss->opdsec = elf_section_by_name(elf, &ehdr, &ss->opdshdr, ".opd",
+			&ss->opdidx);
+	if (ss->opdshdr.sh_type != SHT_PROGBITS)
+		ss->opdsec = NULL;
+
+	if (dso->kernel == DSO_TYPE_USER) {
+		GElf_Shdr shdr;
+		ss->adjust_symbols = (ehdr.e_type == ET_EXEC ||
+				elf_section_by_name(elf, &ehdr, &shdr,
+						     ".gnu.prelink_undo",
+						     NULL) != NULL);
+	} else {
+		ss->adjust_symbols = 0;
+	}
+
+	ss->name   = strdup(name);
+	if (!ss->name)
+		goto out_elf_end;
+
+	ss->elf    = elf;
+	ss->fd     = fd;
+	ss->ehdr   = ehdr;
+	ss->type   = type;
+
+	return 0;
+
+out_elf_end:
+	elf_end(elf);
+out_close:
+	close(fd);
+	return err;
+}
+
+int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
+		  symbol_filter_t filter, int kmodule, int want_symtab)
+{
+	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
+	struct map *curr_map = map;
+	struct dso *curr_dso = dso;
+	Elf_Data *symstrs, *secstrs;
+	uint32_t nr_syms;
+	int err = -1;
+	uint32_t idx;
+	GElf_Ehdr ehdr;
+	GElf_Shdr shdr, opdshdr;
+	Elf_Data *syms, *opddata = NULL;
+	GElf_Sym sym;
+	Elf_Scn *sec, *sec_strndx, *opdsec;
+	Elf *elf;
+	int nr = 0;
+	size_t opdidx = 0;
+
+	elf = ss->elf;
+	ehdr = ss->ehdr;
+	sec = ss->symtab;
+	shdr = ss->symshdr;
+
 	if (sec == NULL) {
 		if (want_symtab)
 			goto out_elf_end;
 
-		sec = elf_section_by_name(elf, &ehdr, &shdr, ".dynsym", NULL);
+		sec  = ss->dynsym;
+		shdr = ss->dynshdr;
 		if (sec == NULL)
 			goto out_elf_end;
 	}
 
-	opdsec = elf_section_by_name(elf, &ehdr, &opdshdr, ".opd", &opdidx);
-	if (opdshdr.sh_type != SHT_PROGBITS)
-		opdsec = NULL;
+	opdsec = ss->opdsec;
+	opdshdr = ss->opdshdr;
+	opdidx  = ss->opdidx;
 	if (opdsec)
 		opddata = elf_rawdata(opdsec, NULL);
 
@@ -619,14 +689,7 @@ int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
 	nr_syms = shdr.sh_size / shdr.sh_entsize;
 
 	memset(&sym, 0, sizeof(sym));
-	if (dso->kernel == DSO_TYPE_USER) {
-		dso->adjust_symbols = (ehdr.e_type == ET_EXEC ||
-				elf_section_by_name(elf, &ehdr, &shdr,
-						     ".gnu.prelink_undo",
-						     NULL) != NULL);
-	} else {
-		dso->adjust_symbols = 0;
-	}
+	dso->adjust_symbols = ss->adjust_symbols;
 	elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
 		struct symbol *f;
 		const char *elf_name = elf_sym__name(&sym, symstrs);
@@ -770,8 +833,6 @@ new_symbol:
 	}
 	err = nr;
 out_elf_end:
-	elf_end(elf);
-out_close:
 	return err;
 }
 
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index bd8720b..f8b5764 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -241,6 +241,31 @@ out:
 	return ret;
 }
 
+int symsrc__init(struct symsrc *ss, struct dso *dso __used, const char *name,
+	         enum dso_binary_type type)
+{
+	int fd = open(name, O_RDONLY);
+	if (fd < 0)
+		return -1;
+
+	ss->name   = strdup(name);
+	if (!ss->name)
+		goto out_close;
+
+	ss->type = type;
+
+	return 0;
+out_close:
+	close(fd);
+	return -1;
+}
+
+void symsrc__destroy(struct symsrc *ss)
+{
+	free(ss->name);
+	close(ss->fd);
+}
+
 int dso__synthesize_plt_symbols(struct dso *dso __used, char *name __used,
 				struct map *map __used,
 				symbol_filter_t filter __used)
@@ -248,14 +273,13 @@ int dso__synthesize_plt_symbols(struct dso *dso __used, char *name __used,
 	return 0;
 }
 
-int dso__load_sym(struct dso *dso, struct map *map __used,
-		  const char *name, int fd __used,
+int dso__load_sym(struct dso *dso, struct map *map __used, struct symsrc *ss,
 		  symbol_filter_t filter __used, int kmodule __used,
 		  int want_symtab __used)
 {
 	unsigned char *build_id[BUILD_ID_SIZE];
 
-	if (filename__read_build_id(name, build_id, BUILD_ID_SIZE) > 0) {
+	if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) > 0) {
 		dso__set_build_id(dso, build_id);
 		return 1;
 	}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 8f5cabbf..afec3f0 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1026,7 +1026,7 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 {
 	char *name;
 	int ret = -1;
-	int fd;
+	struct symsrc ss;
 	u_int i;
 	struct machine *machine;
 	char *root_dir = (char *) "";
@@ -1086,13 +1086,12 @@ restart:
 			continue;
 
 		/* Name is now the name of the next image to try */
-		fd = open(name, O_RDONLY);
-		if (fd < 0)
+		if (symsrc__init(&ss, dso, name, dso->symtab_type) < 0)
 			continue;
 
-		ret = dso__load_sym(dso, map, name, fd, filter, 0,
+		ret = dso__load_sym(dso, map, &ss, filter, 0,
 				    want_symtab);
-		close(fd);
+		symsrc__destroy(&ss);
 
 		/*
 		 * Some people seem to have debuginfo files _WITHOUT_ debug
@@ -1359,22 +1358,23 @@ out_failure:
 int dso__load_vmlinux(struct dso *dso, struct map *map,
 		      const char *vmlinux, symbol_filter_t filter)
 {
-	int err = -1, fd;
+	int err = -1;
+	struct symsrc ss;
 	char symfs_vmlinux[PATH_MAX];
 
 	snprintf(symfs_vmlinux, sizeof(symfs_vmlinux), "%s%s",
 		 symbol_conf.symfs, vmlinux);
-	fd = open(symfs_vmlinux, O_RDONLY);
-	if (fd < 0)
-		return -1;
 
 	if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
 		dso->symtab_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
 	else
 		dso->symtab_type = DSO_BINARY_TYPE__VMLINUX;
 
-	err = dso__load_sym(dso, map, symfs_vmlinux, fd, filter, 0, 0);
-	close(fd);
+	if (symsrc__init(&ss, dso, symfs_vmlinux, dso->symtab_type))
+		return -1;
+
+	err = dso__load_sym(dso, map, &ss, filter, 0, 0);
+	symsrc__destroy(&ss);
 
 	if (err > 0) {
 		dso__set_long_name(dso, (char *)vmlinux);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 37f1ea1..5e55f98 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -11,6 +11,12 @@
 #include <stdio.h>
 #include <byteswap.h>
 
+#ifndef NO_LIBELF
+#include <libelf.h>
+#include <gelf.h>
+#include <elf.h>
+#endif
+
 #ifdef HAVE_CPLUS_DEMANGLE
 extern char *cplus_demangle(const char *, int);
 
@@ -219,6 +225,34 @@ struct dso {
 	char		 name[0];
 };
 
+struct symsrc {
+	char *name;
+	int fd;
+	enum dso_binary_type type;
+
+#ifndef NO_LIBELF
+	Elf *elf;
+	GElf_Ehdr ehdr;
+
+	Elf_Scn *opdsec;
+	size_t opdidx;
+	GElf_Shdr opdshdr;
+
+	Elf_Scn *symtab;
+	GElf_Shdr symshdr;
+
+	Elf_Scn *dynsym;
+	size_t dynsym_idx;
+	GElf_Shdr dynshdr;
+
+	bool adjust_symbols;
+#endif
+};
+
+void symsrc__destroy(struct symsrc *ss);
+int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
+		 enum dso_binary_type type);
+
 #define DSO__SWAP(dso, type, val)			\
 ({							\
 	type ____r = val;				\
@@ -334,7 +368,7 @@ ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 			    struct machine *machine, u64 addr,
 			    u8 *data, ssize_t size);
 int dso__test_data(void);
-int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
+int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 		  symbol_filter_t filter, int kmodule, int want_symtab);
 int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
 				symbol_filter_t filter);
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 12/16] perf symbol: set symtab_type in dso__load_sym
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (10 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 11/16] perf symbol: introduce symsrc structure Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 16:07   ` [tip:perf/core] perf symbols: Set " tip-bot for Cody P Schafer
  2012-08-10 22:22 ` [PATCH 13/16] perf symbol: switch dso__synthesize_plt_symbols() to use symsrc Cody P Schafer
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

In certain cases, dso__load requires dso->symbol_type to be set prior to
calling it. With the introduction of symsrc*, the symtab_type is now
stored in a symsrc which is then passed to dso__load_sym().

Change dso__load_sym() to use the symtab_type from them symsrc (setting
dso->symtab_type as well).

Setup for later patch
"perf symbol: use both runtime and debug images"

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c |  2 ++
 tools/perf/util/symbol.c     | 13 +++++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 6974b2a..3a9c38a 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -645,6 +645,8 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 	int nr = 0;
 	size_t opdidx = 0;
 
+	dso->symtab_type = ss->type;
+
 	elf = ss->elf;
 	ehdr = ss->ehdr;
 	sec = ss->symtab;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index afec3f0..2b3495a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1079,14 +1079,14 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 restart:
 	for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
 
-		dso->symtab_type = binary_type_symtab[i];
+		enum dso_binary_type symtab_type = binary_type_symtab[i];
 
-		if (dso__binary_type_file(dso, dso->symtab_type,
+		if (dso__binary_type_file(dso, symtab_type,
 					  root_dir, name, PATH_MAX))
 			continue;
 
 		/* Name is now the name of the next image to try */
-		if (symsrc__init(&ss, dso, name, dso->symtab_type) < 0)
+		if (symsrc__init(&ss, dso, name, symtab_type) < 0)
 			continue;
 
 		ret = dso__load_sym(dso, map, &ss, filter, 0,
@@ -1361,16 +1361,17 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	int err = -1;
 	struct symsrc ss;
 	char symfs_vmlinux[PATH_MAX];
+	enum dso_binary_type symtab_type;
 
 	snprintf(symfs_vmlinux, sizeof(symfs_vmlinux), "%s%s",
 		 symbol_conf.symfs, vmlinux);
 
 	if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
-		dso->symtab_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
+		symtab_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
 	else
-		dso->symtab_type = DSO_BINARY_TYPE__VMLINUX;
+		symtab_type = DSO_BINARY_TYPE__VMLINUX;
 
-	if (symsrc__init(&ss, dso, symfs_vmlinux, dso->symtab_type))
+	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
 		return -1;
 
 	err = dso__load_sym(dso, map, &ss, filter, 0, 0);
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 13/16] perf symbol: switch dso__synthesize_plt_symbols() to use symsrc
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (11 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 12/16] perf symbol: set symtab_type in dso__load_sym Cody P Schafer
@ 2012-08-10 22:22 ` Cody P Schafer
  2012-08-21 16:07   ` [tip:perf/core] perf symbols: Switch " tip-bot for Cody P Schafer
  2012-08-10 22:23 ` [PATCH 14/16] perf symbol: factor want_symtab out of dso__load_sym() Cody P Schafer
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

Previously dso__synthesize_plt_symbols() was reopening the elf file to
obtain dynsyms from it. Rather than reopen the file, use the already
opened reference within the symsrc to access it.

Setup for the later patch
"perf symbol: use both runtime and debug images"

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c     | 25 +++++++------------------
 tools/perf/util/symbol-minimal.c |  3 ++-
 tools/perf/util/symbol.c         |  8 +++++---
 tools/perf/util/symbol.h         |  4 ++--
 4 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 3a9c38a..5915947 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -166,7 +166,7 @@ static Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
  * And always look at the original dso, not at debuginfo packages, that
  * have the PLT data stripped out (shdr_rel_plt.sh_type == SHT_NOBITS).
  */
-int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
+int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss, struct map *map,
 				symbol_filter_t filter)
 {
 	uint32_t nr_rel_entries, idx;
@@ -181,21 +181,15 @@ int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
 	GElf_Ehdr ehdr;
 	char sympltname[1024];
 	Elf *elf;
-	int nr = 0, symidx, fd, err = 0;
+	int nr = 0, symidx, err = 0;
 
-	fd = open(name, O_RDONLY);
-	if (fd < 0)
-		goto out;
-
-	elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
-	if (elf == NULL)
-		goto out_close;
+	elf = ss->elf;
+	ehdr = ss->ehdr;
 
-	if (gelf_getehdr(elf, &ehdr) == NULL)
-		goto out_elf_end;
+	scn_dynsym = ss->dynsym;
+	shdr_dynsym = ss->dynshdr;
+	dynsym_idx = ss->dynsym_idx;
 
-	scn_dynsym = elf_section_by_name(elf, &ehdr, &shdr_dynsym,
-					 ".dynsym", &dynsym_idx);
 	if (scn_dynsym == NULL)
 		goto out_elf_end;
 
@@ -291,13 +285,8 @@ int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
 
 	err = 0;
 out_elf_end:
-	elf_end(elf);
-out_close:
-	close(fd);
-
 	if (err == 0)
 		return nr;
-out:
 	pr_debug("%s: problems reading %s PLT info.\n",
 		 __func__, dso->long_name);
 	return 0;
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index f8b5764..2f1584b 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -266,7 +266,8 @@ void symsrc__destroy(struct symsrc *ss)
 	close(ss->fd);
 }
 
-int dso__synthesize_plt_symbols(struct dso *dso __used, char *name __used,
+int dso__synthesize_plt_symbols(struct dso *dso __used,
+				struct symsrc *ss __used,
 				struct map *map __used,
 				symbol_filter_t filter __used)
 {
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2b3495a..f8a3068 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1091,21 +1091,23 @@ restart:
 
 		ret = dso__load_sym(dso, map, &ss, filter, 0,
 				    want_symtab);
-		symsrc__destroy(&ss);
 
 		/*
 		 * Some people seem to have debuginfo files _WITHOUT_ debug
 		 * info!?!?
 		 */
-		if (!ret)
+		if (!ret) {
+			symsrc__destroy(&ss);
 			continue;
+		}
 
 		if (ret > 0) {
 			int nr_plt;
 
-			nr_plt = dso__synthesize_plt_symbols(dso, name, map, filter);
+			nr_plt = dso__synthesize_plt_symbols(dso, &ss, map, filter);
 			if (nr_plt > 0)
 				ret += nr_plt;
+			symsrc__destroy(&ss);
 			break;
 		}
 	}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 5e55f98..95b3996 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -370,8 +370,8 @@ ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 int dso__test_data(void);
 int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 		  symbol_filter_t filter, int kmodule, int want_symtab);
-int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
-				symbol_filter_t filter);
+int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss,
+				struct map *map, symbol_filter_t filter);
 
 void symbols__insert(struct rb_root *symbols, struct symbol *sym);
 void symbols__fixup_duplicate(struct rb_root *symbols);
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 14/16] perf symbol: factor want_symtab out of dso__load_sym()
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (12 preceding siblings ...)
  2012-08-10 22:22 ` [PATCH 13/16] perf symbol: switch dso__synthesize_plt_symbols() to use symsrc Cody P Schafer
@ 2012-08-10 22:23 ` Cody P Schafer
  2012-08-21 16:08   ` [tip:perf/core] perf symbols: Factor " tip-bot for Cody P Schafer
  2012-08-10 22:23 ` [PATCH 15/16] perf symbol: convert dso__load_syms to take 2 symsrc's Cody P Schafer
  2012-08-10 22:23 ` [PATCH 16/16] perf symbol: use both runtime and debug images Cody P Schafer
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

Only one callsite of dso__load_sym() uses the want_symtab functionality,
so place the logic at the callsite instead of within dso__load_sym().

This sets us up for removal of want_symtab completely once we keep
multiple elf handles (within symsrc's) around.

Setup for the later patch
"perf symbol: use both runtime and debug images"

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c     | 21 ++++++++++-----------
 tools/perf/util/symbol-minimal.c |  8 ++++++--
 tools/perf/util/symbol.c         | 10 +++++++---
 tools/perf/util/symbol.h         |  3 ++-
 4 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 5915947..492ebec 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -525,6 +525,10 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
 	return 0;
 }
 
+bool symsrc__has_symtab(struct symsrc *ss)
+{
+	return ss->symtab != NULL;
+}
 
 void symsrc__destroy(struct symsrc *ss)
 {
@@ -616,7 +620,7 @@ out_close:
 }
 
 int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
-		  symbol_filter_t filter, int kmodule, int want_symtab)
+		  symbol_filter_t filter, int kmodule)
 {
 	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
 	struct map *curr_map = map;
@@ -636,21 +640,16 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 
 	dso->symtab_type = ss->type;
 
+	if (!ss->symtab) {
+		ss->symtab  = ss->dynsym;
+		ss->symshdr = ss->dynshdr;
+	}
+
 	elf = ss->elf;
 	ehdr = ss->ehdr;
 	sec = ss->symtab;
 	shdr = ss->symshdr;
 
-	if (sec == NULL) {
-		if (want_symtab)
-			goto out_elf_end;
-
-		sec  = ss->dynsym;
-		shdr = ss->dynshdr;
-		if (sec == NULL)
-			goto out_elf_end;
-	}
-
 	opdsec = ss->opdsec;
 	opdshdr = ss->opdshdr;
 	opdidx  = ss->opdidx;
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 2f1584b..3290f04 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -260,6 +260,11 @@ out_close:
 	return -1;
 }
 
+bool symsrc__has_symtab(struct symsrc *ss __used)
+{
+	return false;
+}
+
 void symsrc__destroy(struct symsrc *ss)
 {
 	free(ss->name);
@@ -275,8 +280,7 @@ int dso__synthesize_plt_symbols(struct dso *dso __used,
 }
 
 int dso__load_sym(struct dso *dso, struct map *map __used, struct symsrc *ss,
-		  symbol_filter_t filter __used, int kmodule __used,
-		  int want_symtab __used)
+		  symbol_filter_t filter __used, int kmodule __used)
 {
 	unsigned char *build_id[BUILD_ID_SIZE];
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index f8a3068..8e7d74f 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1089,8 +1089,12 @@ restart:
 		if (symsrc__init(&ss, dso, name, symtab_type) < 0)
 			continue;
 
-		ret = dso__load_sym(dso, map, &ss, filter, 0,
-				    want_symtab);
+		if (want_symtab && !symsrc__has_symtab(&ss)) {
+			symsrc__destroy(&ss);
+			continue;
+		}
+
+		ret = dso__load_sym(dso, map, &ss, filter, 0);
 
 		/*
 		 * Some people seem to have debuginfo files _WITHOUT_ debug
@@ -1376,7 +1380,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
 		return -1;
 
-	err = dso__load_sym(dso, map, &ss, filter, 0, 0);
+	err = dso__load_sym(dso, map, &ss, filter, 0);
 	symsrc__destroy(&ss);
 
 	if (err > 0) {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 95b3996..c73f4f2 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -252,6 +252,7 @@ struct symsrc {
 void symsrc__destroy(struct symsrc *ss);
 int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 		 enum dso_binary_type type);
+bool symsrc__has_symtab(struct symsrc *ss);
 
 #define DSO__SWAP(dso, type, val)			\
 ({							\
@@ -369,7 +370,7 @@ ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 			    u8 *data, ssize_t size);
 int dso__test_data(void);
 int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
-		  symbol_filter_t filter, int kmodule, int want_symtab);
+		  symbol_filter_t filter, int kmodule);
 int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss,
 				struct map *map, symbol_filter_t filter);
 
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 15/16] perf symbol: convert dso__load_syms to take 2 symsrc's
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (13 preceding siblings ...)
  2012-08-10 22:23 ` [PATCH 14/16] perf symbol: factor want_symtab out of dso__load_sym() Cody P Schafer
@ 2012-08-10 22:23 ` Cody P Schafer
  2012-08-21 16:09   ` [tip:perf/core] perf symbols: Convert " tip-bot for Cody P Schafer
  2012-08-10 22:23 ` [PATCH 16/16] perf symbol: use both runtime and debug images Cody P Schafer
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

To properly handle platforms with an opd section, both a runtime image
(which contains the opd section but possibly lacks symbols) and a symbol
image (which probably lacks an opd section but has symbols).

The next patch ("perf symbol: use both runtime and debug images")
adjusts the callsite in dso__load() to take advantage of being able to
pass both runtime & debug images.

Assumptions made here:

 - The opd section, if it exists in the runtime image, has headers in
   both the runtime image and the debug/syms image.
 - The index of the opd section (again, only if it exists in the
   runtime image) is the same in both the runtime and debug/symbols
   image.

Both of these are true on RHEL, but it is unclear how accurate they are
in general (on platforms with function descriptors in opd sections).

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c     | 47 ++++++++++++++++++++--------------------
 tools/perf/util/symbol-minimal.c |  1 +
 tools/perf/util/symbol.c         |  4 ++--
 tools/perf/util/symbol.h         |  5 +++--
 4 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 492ebec..36e4a45 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -619,7 +619,8 @@ out_close:
 	return err;
 }
 
-int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
+int dso__load_sym(struct dso *dso, struct map *map,
+		  struct symsrc *syms_ss, struct symsrc *runtime_ss,
 		  symbol_filter_t filter, int kmodule)
 {
 	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
@@ -630,31 +631,27 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 	int err = -1;
 	uint32_t idx;
 	GElf_Ehdr ehdr;
-	GElf_Shdr shdr, opdshdr;
+	GElf_Shdr shdr;
 	Elf_Data *syms, *opddata = NULL;
 	GElf_Sym sym;
-	Elf_Scn *sec, *sec_strndx, *opdsec;
+	Elf_Scn *sec, *sec_strndx;
 	Elf *elf;
 	int nr = 0;
-	size_t opdidx = 0;
 
-	dso->symtab_type = ss->type;
+	dso->symtab_type = syms_ss->type;
 
-	if (!ss->symtab) {
-		ss->symtab  = ss->dynsym;
-		ss->symshdr = ss->dynshdr;
+	if (!syms_ss->symtab) {
+		syms_ss->symtab  = syms_ss->dynsym;
+		syms_ss->symshdr = syms_ss->dynshdr;
 	}
 
-	elf = ss->elf;
-	ehdr = ss->ehdr;
-	sec = ss->symtab;
-	shdr = ss->symshdr;
+	elf = syms_ss->elf;
+	ehdr = syms_ss->ehdr;
+	sec = syms_ss->symtab;
+	shdr = syms_ss->symshdr;
 
-	opdsec = ss->opdsec;
-	opdshdr = ss->opdshdr;
-	opdidx  = ss->opdidx;
-	if (opdsec)
-		opddata = elf_rawdata(opdsec, NULL);
+	if (runtime_ss->opdsec)
+		opddata = elf_rawdata(runtime_ss->opdsec, NULL);
 
 	syms = elf_getdata(sec, NULL);
 	if (syms == NULL)
@@ -679,13 +676,14 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 	nr_syms = shdr.sh_size / shdr.sh_entsize;
 
 	memset(&sym, 0, sizeof(sym));
-	dso->adjust_symbols = ss->adjust_symbols;
+	dso->adjust_symbols = runtime_ss->adjust_symbols;
 	elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
 		struct symbol *f;
 		const char *elf_name = elf_sym__name(&sym, symstrs);
 		char *demangled = NULL;
 		int is_label = elf_sym__is_label(&sym);
 		const char *section_name;
+		bool used_opd = false;
 
 		if (kmap && kmap->ref_reloc_sym && kmap->ref_reloc_sym->name &&
 		    strcmp(elf_name, kmap->ref_reloc_sym->name) == 0)
@@ -704,14 +702,16 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 				continue;
 		}
 
-		if (opdsec && sym.st_shndx == opdidx) {
-			u32 offset = sym.st_value - opdshdr.sh_addr;
+		if (runtime_ss->opdsec && sym.st_shndx == runtime_ss->opdidx) {
+			u32 offset = sym.st_value - syms_ss->opdshdr.sh_addr;
 			u64 *opd = opddata->d_buf + offset;
 			sym.st_value = DSO__SWAP(dso, u64, *opd);
-			sym.st_shndx = elf_addr_to_index(elf, sym.st_value);
+			sym.st_shndx = elf_addr_to_index(runtime_ss->elf,
+					sym.st_value);
+			used_opd = true;
 		}
 
-		sec = elf_getscn(elf, sym.st_shndx);
+		sec = elf_getscn(runtime_ss->elf, sym.st_shndx);
 		if (!sec)
 			goto out_elf_end;
 
@@ -777,7 +777,8 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 			goto new_symbol;
 		}
 
-		if (curr_dso->adjust_symbols && sym.st_value) {
+		if ((used_opd && runtime_ss->adjust_symbols)
+				|| (!used_opd && syms_ss->adjust_symbols)) {
 			pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
 				  "sh_addr: %#" PRIx64 " sh_offset: %#" PRIx64 "\n", __func__,
 				  (u64)sym.st_value, (u64)shdr.sh_addr,
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 3290f04..c0377a9 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -280,6 +280,7 @@ int dso__synthesize_plt_symbols(struct dso *dso __used,
 }
 
 int dso__load_sym(struct dso *dso, struct map *map __used, struct symsrc *ss,
+		  struct symsrc *runtime_ss __used,
 		  symbol_filter_t filter __used, int kmodule __used)
 {
 	unsigned char *build_id[BUILD_ID_SIZE];
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 8e7d74f..739e5a3 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1094,7 +1094,7 @@ restart:
 			continue;
 		}
 
-		ret = dso__load_sym(dso, map, &ss, filter, 0);
+		ret = dso__load_sym(dso, map, &ss, &ss, filter, 0);
 
 		/*
 		 * Some people seem to have debuginfo files _WITHOUT_ debug
@@ -1380,7 +1380,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
 		return -1;
 
-	err = dso__load_sym(dso, map, &ss, filter, 0);
+	err = dso__load_sym(dso, map, &ss, &ss, filter, 0);
 	symsrc__destroy(&ss);
 
 	if (err > 0) {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index c73f4f2..d55635d 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -369,8 +369,9 @@ ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 			    struct machine *machine, u64 addr,
 			    u8 *data, ssize_t size);
 int dso__test_data(void);
-int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
-		  symbol_filter_t filter, int kmodule);
+int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
+		  struct symsrc *runtime_ss, symbol_filter_t filter,
+		  int kmodule);
 int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss,
 				struct map *map, symbol_filter_t filter);
 
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 16/16] perf symbol: use both runtime and debug images
  2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
                   ` (14 preceding siblings ...)
  2012-08-10 22:23 ` [PATCH 15/16] perf symbol: convert dso__load_syms to take 2 symsrc's Cody P Schafer
@ 2012-08-10 22:23 ` Cody P Schafer
  2012-08-21 16:10   ` [tip:perf/core] perf symbols: Use " tip-bot for Cody P Schafer
  15 siblings, 1 reply; 37+ messages in thread
From: Cody P Schafer @ 2012-08-10 22:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen, Namhyung Kim

We keep both a 'runtime' elf image as well as a 'debug' elf image around
and generate symbols by looking at both of these.

This eliminates the need for the want_symtab/goto restart mechanism
combined with iterating over and reopening the elf images a second time.

Also give dso__synthsize_plt_symbols() the runtime image (which has
dynsyms) instead of the symbol image (which may only have a symtab and
no dynsyms).

Previously if a debug image was found all runtime images were ignored.

This fixes 2 issues:

 - Symbol resolution to failure on PowerPC systems with debug symbols
   installed, as the debug images lack a '.opd' section which contains
   function descriptors.

 - On all archs, plt synthesis failed when a debug image was loaded and
   that debug image lacks a dynsym section while a runtime image has a
   dynsym section.

Assumptions:

 - If a .opd section exists, it is contained in the highest priority
   image with a dynsym section.

 - This generally implies that the debug image lacks a dynsym section
   (ie: it is marked as NO_BITS).

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c     |  5 +++
 tools/perf/util/symbol-minimal.c |  6 ++++
 tools/perf/util/symbol.c         | 77 +++++++++++++++++++++++-----------------
 tools/perf/util/symbol.h         |  1 +
 4 files changed, 56 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 36e4a45..5b37e13 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -525,6 +525,11 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
 	return 0;
 }
 
+bool symsrc__possibly_runtime(struct symsrc *ss)
+{
+	return ss->dynsym || ss->opdsec;
+}
+
 bool symsrc__has_symtab(struct symsrc *ss)
 {
 	return ss->symtab != NULL;
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index c0377a9..133a591 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -260,6 +260,12 @@ out_close:
 	return -1;
 }
 
+bool symsrc__possibly_runtime(struct symsrc *ss __used)
+{
+	/* Assume all sym sources could be a runtime image. */
+	return true;
+}
+
 bool symsrc__has_symtab(struct symsrc *ss __used)
 {
 	return false;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 739e5a3..2aa871a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1026,11 +1026,12 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 {
 	char *name;
 	int ret = -1;
-	struct symsrc ss;
 	u_int i;
 	struct machine *machine;
 	char *root_dir = (char *) "";
-	int want_symtab;
+	int ss_pos = 0;
+	struct symsrc ss_[2];
+	struct symsrc *syms_ss = NULL, *runtime_ss = NULL;
 
 	dso__set_loaded(dso, map->type);
 
@@ -1072,12 +1073,12 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 		root_dir = machine->root_dir;
 
 	/* Iterate over candidate debug images.
-	 * On the first pass, only load images if they have a full symtab.
-	 * Failing that, do a second pass where we accept .dynsym also
+	 * Keep track of "interesting" ones (those which have a symtab, dynsym,
+	 * and/or opd section) for processing.
 	 */
-	want_symtab = 1;
-restart:
 	for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
+		struct symsrc *ss = &ss_[ss_pos];
+		bool next_slot = false;
 
 		enum dso_binary_type symtab_type = binary_type_symtab[i];
 
@@ -1086,45 +1087,55 @@ restart:
 			continue;
 
 		/* Name is now the name of the next image to try */
-		if (symsrc__init(&ss, dso, name, symtab_type) < 0)
+		if (symsrc__init(ss, dso, name, symtab_type) < 0)
 			continue;
 
-		if (want_symtab && !symsrc__has_symtab(&ss)) {
-			symsrc__destroy(&ss);
-			continue;
+		if (!syms_ss && symsrc__has_symtab(ss)) {
+			syms_ss = ss;
+			next_slot = true;
 		}
 
-		ret = dso__load_sym(dso, map, &ss, &ss, filter, 0);
-
-		/*
-		 * Some people seem to have debuginfo files _WITHOUT_ debug
-		 * info!?!?
-		 */
-		if (!ret) {
-			symsrc__destroy(&ss);
-			continue;
+		if (!runtime_ss && symsrc__possibly_runtime(ss)) {
+			runtime_ss = ss;
+			next_slot = true;
 		}
 
-		if (ret > 0) {
-			int nr_plt;
+		if (next_slot) {
+			ss_pos++;
 
-			nr_plt = dso__synthesize_plt_symbols(dso, &ss, map, filter);
-			if (nr_plt > 0)
-				ret += nr_plt;
-			symsrc__destroy(&ss);
-			break;
+			if (syms_ss && runtime_ss)
+				break;
 		}
+
 	}
 
-	/*
-	 * If we wanted a full symtab but no image had one,
-	 * relax our requirements and repeat the search.
-	 */
-	if (ret <= 0 && want_symtab) {
-		want_symtab = 0;
-		goto restart;
+	if (!runtime_ss && !syms_ss)
+		goto out_free;
+
+	if (runtime_ss && !syms_ss) {
+		syms_ss = runtime_ss;
+	}
+
+	/* We'll have to hope for the best */
+	if (!runtime_ss && syms_ss)
+		runtime_ss = syms_ss;
+
+	if (syms_ss)
+		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, 0);
+	else
+		ret = -1;
+
+	if (ret > 0 && runtime_ss->dynsym) {
+		int nr_plt;
+
+		nr_plt = dso__synthesize_plt_symbols(dso, runtime_ss, map, filter);
+		if (nr_plt > 0)
+			ret += nr_plt;
 	}
 
+	for (; ss_pos > 0; ss_pos--)
+		symsrc__destroy(&ss_[ss_pos-1]);
+out_free:
 	free(name);
 	if (ret < 0 && strstr(dso->name, " (deleted)") != NULL)
 		return 0;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index d55635d..cf011a8 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -253,6 +253,7 @@ void symsrc__destroy(struct symsrc *ss);
 int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 		 enum dso_binary_type type);
 bool symsrc__has_symtab(struct symsrc *ss);
+bool symsrc__possibly_runtime(struct symsrc *ss);
 
 #define DSO__SWAP(dso, type, val)			\
 ({							\
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH 01/16] perf symbol: correct comment wrt kallsyms loading
  2012-08-10 22:22 ` [PATCH 01/16] perf symbol: correct comment wrt kallsyms loading Cody P Schafer
@ 2012-08-11 13:14   ` Namhyung Kim
  2012-08-21 16:00   ` [tip:perf/core] perf symbols: Correct " tip-bot for Cody P Schafer
  1 sibling, 0 replies; 37+ messages in thread
From: Namhyung Kim @ 2012-08-11 13:14 UTC (permalink / raw)
  To: Cody P Schafer
  Cc: Arnaldo Carvalho de Melo, LKML, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra, Sukadev Bhattiprolu, Matt Hellsley, David Hansen

2012-08-10 (금), 15:22 -0700, Cody P Schafer:
> In kallsyms_parse() when calling process_symbol() (a callback argument
> to kallsyms_parse()), we pass start as both start & end (ie:
> start=start, end=start).
> 
> In map__process_kallsym_symbol(), the length is calculated as 'end - start + 1',
> making the length 1, not 0.
> 
> Essentially, start & end define an inclusive range.
> 

This seems not needed any more due to the patch #2, right?

Thanks,
Namhyung


> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> ---
>  tools/perf/util/symbol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index f02de8a..891f83c 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -605,7 +605,7 @@ int kallsyms__parse(const char *filename, void *arg,
>  
>  		/*
>  		 * module symbols are not sorted so we add all
> -		 * symbols with zero length and rely on
> +		 * symbols, setting length to 1, and rely on
>  		 * symbols__fixup_end() to fix it up.
>  		 */
>  		err = process_symbol(arg, symbol_name,




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 11/16] perf symbol: introduce symsrc structure.
  2012-08-10 22:22 ` [PATCH 11/16] perf symbol: introduce symsrc structure Cody P Schafer
@ 2012-08-11 13:28   ` Namhyung Kim
  2012-08-13 17:36     ` Arnaldo Carvalho de Melo
  2012-08-21 16:06   ` [tip:perf/core] perf symbols: Introduce " tip-bot for Cody P Schafer
  1 sibling, 1 reply; 37+ messages in thread
From: Namhyung Kim @ 2012-08-11 13:28 UTC (permalink / raw)
  To: Cody P Schafer
  Cc: Arnaldo Carvalho de Melo, LKML, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra, Sukadev Bhattiprolu, Matt Hellsley, David Hansen

2012-08-10 (금), 15:22 -0700, Cody P Schafer:
> Factors opening of certain sections & tracking certain elf info into an
> external structure.
> 
> The goal here is to keep multiple elfs (and their looked up
> sections/indexes) around during the symbol generation process (in
> dso__load()).
> 
> We need this to properly resolve symbols on PPC due to the
> use of function descriptors & the .opd section (ie: symbols which are
> functions don't point to their actual location, they point to their
> function descriptor in .opd which contains their actual location.
> 
> It would be possible to just keep the (Elf *) around, but then we'd end
> up with duplicate code for looking up the same sections and checking for
> the existence of an important section wouldn't be as clean (and we need
> to keep the Elf stuff confined to symtab-elf.c).
> 
> Utilized by the later patch
> "perf symbol: use both runtime and debug images"
> 
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> ---
>  tools/perf/util/symbol-elf.c     | 119 +++++++++++++++++++++++++++++----------
>  tools/perf/util/symbol-minimal.c |  30 +++++++++-
>  tools/perf/util/symbol.c         |  22 ++++----
>  tools/perf/util/symbol.h         |  36 +++++++++++-
>  4 files changed, 163 insertions(+), 44 deletions(-)
> 
[SNIP]
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 37f1ea1..5e55f98 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -11,6 +11,12 @@
>  #include <stdio.h>
>  #include <byteswap.h>
>  
> +#ifndef NO_LIBELF

Should be NO_LIBELF_SUPPORT.


> +#include <libelf.h>
> +#include <gelf.h>
> +#include <elf.h>
> +#endif
> +
>  #ifdef HAVE_CPLUS_DEMANGLE
>  extern char *cplus_demangle(const char *, int);
>  
> @@ -219,6 +225,34 @@ struct dso {
>  	char		 name[0];
>  };
>  
> +struct symsrc {
> +	char *name;
> +	int fd;
> +	enum dso_binary_type type;
> +
> +#ifndef NO_LIBELF

Ditto.

Thanks,
Namhyung


> +	Elf *elf;
> +	GElf_Ehdr ehdr;
> +
> +	Elf_Scn *opdsec;
> +	size_t opdidx;
> +	GElf_Shdr opdshdr;
> +
> +	Elf_Scn *symtab;
> +	GElf_Shdr symshdr;
> +
> +	Elf_Scn *dynsym;
> +	size_t dynsym_idx;
> +	GElf_Shdr dynshdr;
> +
> +	bool adjust_symbols;
> +#endif
> +};
> +
> +void symsrc__destroy(struct symsrc *ss);
> +int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
> +		 enum dso_binary_type type);
> +
>  #define DSO__SWAP(dso, type, val)			\
>  ({							\
>  	type ____r = val;				\
> @@ -334,7 +368,7 @@ ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
>  			    struct machine *machine, u64 addr,
>  			    u8 *data, ssize_t size);
>  int dso__test_data(void);
> -int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
> +int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
>  		  symbol_filter_t filter, int kmodule, int want_symtab);
>  int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
>  				symbol_filter_t filter);




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 11/16] perf symbol: introduce symsrc structure.
  2012-08-11 13:28   ` Namhyung Kim
@ 2012-08-13 17:36     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 37+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-08-13 17:36 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Cody P Schafer, LKML, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra, Sukadev Bhattiprolu, Matt Hellsley, David Hansen

Em Sat, Aug 11, 2012 at 10:28:09PM +0900, Namhyung Kim escreveu:
> 2012-08-10 (금), 15:22 -0700, Cody P Schafer:
> > +++ b/tools/perf/util/symbol.h
> > @@ -11,6 +11,12 @@
> >  #include <stdio.h>
> >  #include <byteswap.h>
> >  
> > +#ifndef NO_LIBELF
> 
> Should be NO_LIBELF_SUPPORT.
> 
> > +struct symsrc {
> > +	char *name;
> > +	int fd;
> > +	enum dso_binary_type type;
> > +
> > +#ifndef NO_LIBELF
> 
> Ditto.

Thanks, I fixed those,

- Arnaldo

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Only un-prelink non-zero symbols
  2012-08-10 22:22 ` [PATCH 03/16] perf symbol: only un-prelink non-zero symbols Cody P Schafer
@ 2012-08-21 15:56   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 15:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  8db24c70ab43a4dd38c39b0b0cb4d4874257de55
Gitweb:     http://git.kernel.org/tip/8db24c70ab43a4dd38c39b0b0cb4d4874257de55
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:49 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 12:54:06 -0300

perf symbols: Only un-prelink non-zero symbols

Prelink only adjusts the addresses of non-zero symbols. Do the same when we
reverse the adjustments.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-4-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 9ca89f8..e037609 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -717,7 +717,7 @@ int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
 			goto new_symbol;
 		}
 
-		if (curr_dso->adjust_symbols) {
+		if (curr_dso->adjust_symbols && sym.st_value) {
 			pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
 				  "sh_addr: %#" PRIx64 " sh_offset: %#" PRIx64 "\n", __func__,
 				  (u64)sym.st_value, (u64)shdr.sh_addr,

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Remove unused function map__objdump_2ip
  2012-08-10 22:22 ` [PATCH 04/16] perf utils: remove unused function map__objdump_2ip Cody P Schafer
@ 2012-08-21 15:57   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 15:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  0f75a710dfd8fd5cd9e558e3f26c474c8fa63e3c
Gitweb:     http://git.kernel.org/tip/0f75a710dfd8fd5cd9e558e3f26c474c8fa63e3c
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:50 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 12:55:09 -0300

perf symbols: Remove unused function map__objdump_2ip

map__objdump_2ip was introduced in:

ee11b90b12 perf top: Fix annotate for userspace

And it's last user removed in:

36532461a0 perf top: Ditch private annotation code, share perf annotate's

Remove it.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-5-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/map.c |    8 --------
 tools/perf/util/map.h |    1 -
 2 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 287cb34..7d37159 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -262,14 +262,6 @@ u64 map__rip_2objdump(struct map *map, u64 rip)
 	return addr;
 }
 
-u64 map__objdump_2ip(struct map *map, u64 addr)
-{
-	u64 ip = map->dso->adjust_symbols ?
-			addr :
-			map->unmap_ip(map, addr);	/* RIP -> IP */
-	return ip;
-}
-
 void map_groups__init(struct map_groups *mg)
 {
 	int i;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index c98ab19..25ab4cd 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -104,7 +104,6 @@ static inline u64 identity__map_ip(struct map *map __used, u64 ip)
 
 /* rip/ip <-> addr suitable for passing to `objdump --start-address=` */
 u64 map__rip_2objdump(struct map *map, u64 rip);
-u64 map__objdump_2ip(struct map *map, u64 addr);
 
 struct symbol;
 

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Don' t try to synthesize plt without dynstr
  2012-08-10 22:22 ` [PATCH 05/16] perf symbol: don't try to synthesize plt without dynstr Cody P Schafer
@ 2012-08-21 15:58   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 15:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  52f9ddba516214a59fdd8eb155b2583ab314dc31
Gitweb:     http://git.kernel.org/tip/52f9ddba516214a59fdd8eb155b2583ab314dc31
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:51 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 12:56:45 -0300

perf symbols: Don't try to synthesize plt without dynstr

If .dynsym exists but .dynstr is empty (NO_BITS or size==0), a segfault
occurs.  Avoid this by checking that .dynstr is not empty.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-6-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index e037609..a2e994e 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -232,6 +232,9 @@ int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
 	if (symstrs == NULL)
 		goto out_elf_end;
 
+	if (symstrs->d_size == 0)
+		goto out_elf_end;
+
 	nr_rel_entries = shdr_rel_plt.sh_size / shdr_rel_plt.sh_entsize;
 	plt_offset = shdr_plt.sh_offset;
 

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Remove unneeded call to dso__set_long_name()
  2012-08-10 22:22 ` [PATCH 06/16] perf symbol: remove unneeded call to dso__set_long_name() Cody P Schafer
@ 2012-08-21 15:59   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 15:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  261ee821c256dffa02e5608c7df51744a03992a2
Gitweb:     http://git.kernel.org/tip/261ee821c256dffa02e5608c7df51744a03992a2
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:52 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:04:32 -0300

perf symbols: Remove unneeded call to dso__set_long_name()

dso__set_long_name() is already called by dso__load_vmlinux(), avoid
calling it a second time unnecessarily.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-7-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index f02de8a..42c0d94 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1388,10 +1388,8 @@ int dso__load_vmlinux_path(struct dso *dso, struct map *map,
 	filename = dso__build_id_filename(dso, NULL, 0);
 	if (filename != NULL) {
 		err = dso__load_vmlinux(dso, map, filename, filter);
-		if (err > 0) {
-			dso__set_long_name(dso, filename);
+		if (err > 0)
 			goto out;
-		}
 		free(filename);
 	}
 

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Correct comment wrt kallsyms loading
  2012-08-10 22:22 ` [PATCH 01/16] perf symbol: correct comment wrt kallsyms loading Cody P Schafer
  2012-08-11 13:14   ` Namhyung Kim
@ 2012-08-21 16:00   ` tip-bot for Cody P Schafer
  1 sibling, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  72f86204419e1b83f18b9bc2c97141a52dc534d2
Gitweb:     http://git.kernel.org/tip/72f86204419e1b83f18b9bc2c97141a52dc534d2
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:47 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:10:10 -0300

perf symbols: Correct comment wrt kallsyms loading

In kallsyms_parse() when calling process_symbol() (a callback argument
to kallsyms_parse()), we pass start as both start & end (ie:
start=start, end=start).

In map__process_kallsym_symbol(), the length is calculated as 'end -
start + 1', making the length 1, not 0.

Essentially, start & end define an inclusive range.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-2-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 42c0d94..9f181a8 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -605,7 +605,7 @@ int kallsyms__parse(const char *filename, void *arg,
 
 		/*
 		 * module symbols are not sorted so we add all
-		 * symbols with zero length and rely on
+		 * symbols, setting length to 1, and rely on
 		 * symbols__fixup_end() to fix it up.
 		 */
 		err = process_symbol(arg, symbol_name,

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Remove unused 'end' arg in kallsyms parse cb
  2012-08-10 22:22 ` [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb Cody P Schafer
@ 2012-08-21 16:01   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  82151520938ec79e5e3adb7e61977f002031c38c
Gitweb:     http://git.kernel.org/tip/82151520938ec79e5e3adb7e61977f002031c38c
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:48 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:10:31 -0300

perf symbols: Remove unused 'end' arg in kallsyms parse cb

kallsyms__parse() takes a callback that is called on every discovered
symbol. As /proc/kallsyms does not supply symbol sizes, the callback was
simply called with end=start, faking the symbol size to 1.

All of the callbacks (there are 2) used in calls to kallsyms__parse()
are _only_ used as callbacks for kallsyms__parse().

Given that kallsyms__parse() lacks real information about what
end/length should be, don't make up a length in kallsyms__parse().
Instead have the callbacks handle guessing the length.

Also relocate a comment regarding symbol creation to the callback which
does symbol creation (kallsyms__parse() is not in general used to create
symbols).

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-3-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c  |    2 +-
 tools/perf/util/symbol.c |   21 ++++++++++-----------
 tools/perf/util/symbol.h |    2 +-
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 2a6f33c..3a0f1a5 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -412,7 +412,7 @@ struct process_symbol_args {
 };
 
 static int find_symbol_cb(void *arg, const char *name, char type,
-			  u64 start, u64 end __used)
+			  u64 start)
 {
 	struct process_symbol_args *args = arg;
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 9f181a8..2127002 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -563,7 +563,7 @@ size_t dso__fprintf(struct dso *dso, enum map_type type, FILE *fp)
 
 int kallsyms__parse(const char *filename, void *arg,
 		    int (*process_symbol)(void *arg, const char *name,
-					  char type, u64 start, u64 end))
+					  char type, u64 start))
 {
 	char *line = NULL;
 	size_t n;
@@ -603,13 +603,8 @@ int kallsyms__parse(const char *filename, void *arg,
 			break;
 		}
 
-		/*
-		 * module symbols are not sorted so we add all
-		 * symbols, setting length to 1, and rely on
-		 * symbols__fixup_end() to fix it up.
-		 */
 		err = process_symbol(arg, symbol_name,
-				     symbol_type, start, start);
+				     symbol_type, start);
 		if (err)
 			break;
 	}
@@ -636,7 +631,7 @@ static u8 kallsyms2elf_type(char type)
 }
 
 static int map__process_kallsym_symbol(void *arg, const char *name,
-				       char type, u64 start, u64 end)
+				       char type, u64 start)
 {
 	struct symbol *sym;
 	struct process_kallsyms_args *a = arg;
@@ -645,8 +640,12 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
 	if (!symbol_type__is_a(type, a->map->type))
 		return 0;
 
-	sym = symbol__new(start, end - start + 1,
-			  kallsyms2elf_type(type), name);
+	/*
+	 * module symbols are not sorted so we add all
+	 * symbols, setting length to 0, and rely on
+	 * symbols__fixup_end() to fix it up.
+	 */
+	sym = symbol__new(start, 0, kallsyms2elf_type(type), name);
 	if (sym == NULL)
 		return -ENOMEM;
 	/*
@@ -1729,7 +1728,7 @@ struct process_args {
 };
 
 static int symbol__in_kernel(void *arg, const char *name,
-			     char type __used, u64 start, u64 end __used)
+			     char type __used, u64 start)
 {
 	struct process_args *args = arg;
 
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 38ccbbb..c9534fe 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -299,7 +299,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits);
 int build_id__sprintf(const u8 *build_id, int len, char *bf);
 int kallsyms__parse(const char *filename, void *arg,
 		    int (*process_symbol)(void *arg, const char *name,
-					  char type, u64 start, u64 end));
+					  char type, u64 start));
 int filename__read_debuglink(const char *filename, char *debuglink,
 			     size_t size);
 

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Simplify out_fixup in kernel syms loading
  2012-08-10 22:22 ` [PATCH 07/16] perf symbol: simplify out_fixup in kernel syms loading Cody P Schafer
@ 2012-08-21 16:02   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:02 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  0a0317b41e20770f81bc61d7b208957385466c3f
Gitweb:     http://git.kernel.org/tip/0a0317b41e20770f81bc61d7b208957385466c3f
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:53 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:22:32 -0300

perf symbols: Simplify out_fixup in kernel syms loading

The only site that jumps to out_fixup has (kallsyms_filename == NULL).
And all paths that reach 'if (err > 0)' without 'goto out_fixup' have
kallsyms_filename != NULL.

So skip over both the check & dso__set_long_name(), and remove the
check.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-8-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2127002..e5c3817 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1503,9 +1503,8 @@ do_kallsyms:
 	free(kallsyms_allocated_filename);
 
 	if (err > 0) {
+		dso__set_long_name(dso, strdup("[kernel.kallsyms]"));
 out_fixup:
-		if (kallsyms_filename != NULL)
-			dso__set_long_name(dso, strdup("[kernel.kallsyms]"));
 		map__fixup_start(map);
 		map__fixup_end(map);
 	}

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: only set vmlinux longname & mark loaded if really loaded
  2012-08-10 22:22 ` [PATCH 08/16] perf symbol: only set vmlinux longname & mark loaded if really loaded Cody P Schafer
@ 2012-08-21 16:03   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:03 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  515850e4fbd87c8f249446faa2e5ad98e672711d
Gitweb:     http://git.kernel.org/tip/515850e4fbd87c8f249446faa2e5ad98e672711d
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:54 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:24:12 -0300

perf symbols: only set vmlinux longname & mark loaded if really loaded

dso__load_vmlinux() uses the filename passed to it to directly set the
dso long_name, which resulted in a use after free due to
dso__load_vmlinux_path() treating 0 symbols as a load failure and
subsequently freeing the contents of dso->long_name.

Change dso__load_vmlinux() so that finding 0 symbols does not cause it
to consider itself loaded, and do not set long_name in such a case.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-9-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index e5c3817..96dbf28 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1364,13 +1364,14 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (fd < 0)
 		return -1;
 
-	dso__set_long_name(dso, (char *)vmlinux);
-	dso__set_loaded(dso, map->type);
 	err = dso__load_sym(dso, map, symfs_vmlinux, fd, filter, 0, 0);
 	close(fd);
 
-	if (err > 0)
+	if (err > 0) {
+		dso__set_long_name(dso, (char *)vmlinux);
+		dso__set_loaded(dso, map->type);
 		pr_debug("Using %s for symbols\n", symfs_vmlinux);
+	}
 
 	return err;
 }

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Avoid segfault in elf_strptr
  2012-08-10 22:22 ` [PATCH 09/16] perf symbol: avoid segfault in elf_strptr Cody P Schafer
@ 2012-08-21 16:04   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:04 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  492746546fe380da768c8496213e26aa91b9b3aa
Gitweb:     http://git.kernel.org/tip/492746546fe380da768c8496213e26aa91b9b3aa
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:55 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:25:23 -0300

perf symbols: Avoid segfault in elf_strptr

If we call elf_section_by_name() with a truncated elf image (ie: the
file header indicates that the section headers are placed past the end
of the file), elf_strptr() causes a segfault within libelf.

Avoid this by checking that we can access the section string table
properly.

Should really be fixed in libelf/elfutils.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-10-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a2e994e..a9a194d 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -129,6 +129,10 @@ static Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
 	Elf_Scn *sec = NULL;
 	size_t cnt = 1;
 
+	/* Elf is corrupted/truncated, avoid calling elf_strptr. */
+	if (!elf_rawdata(elf_getscn(elf, ep->e_shstrndx), NULL))
+		return NULL;
+
 	while ((sec = elf_nextscn(elf, sec)) != NULL) {
 		char *str;
 

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Track symtab_type of vmlinux
  2012-08-10 22:22 ` [PATCH 10/16] perf symbol: track symtab_type of vmlinux Cody P Schafer
@ 2012-08-21 16:05   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  21ea4539b4d1b26de7f2eb227b5d1a092b32cc19
Gitweb:     http://git.kernel.org/tip/21ea4539b4d1b26de7f2eb227b5d1a092b32cc19
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:56 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:26:18 -0300

perf symbols: Track symtab_type of vmlinux

Previously, symtab_type would have been left at 0, or KALLSYMS, which is
not quite accurate.

Introduce DSO_BINARY_TYPE__VMLINUX[_GUEST].

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-11-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |    9 +++++++++
 tools/perf/util/symbol.h |    2 ++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 96dbf28..8f5cabbf 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -923,6 +923,7 @@ char dso__symtab_origin(const struct dso *dso)
 {
 	static const char origin[] = {
 		[DSO_BINARY_TYPE__KALLSYMS]		= 'k',
+		[DSO_BINARY_TYPE__VMLINUX]		= 'v',
 		[DSO_BINARY_TYPE__JAVA_JIT]		= 'j',
 		[DSO_BINARY_TYPE__DEBUGLINK]		= 'l',
 		[DSO_BINARY_TYPE__BUILD_ID_CACHE]	= 'B',
@@ -933,6 +934,7 @@ char dso__symtab_origin(const struct dso *dso)
 		[DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE]	= 'K',
 		[DSO_BINARY_TYPE__GUEST_KALLSYMS]	= 'g',
 		[DSO_BINARY_TYPE__GUEST_KMODULE]	= 'G',
+		[DSO_BINARY_TYPE__GUEST_VMLINUX]	= 'V',
 	};
 
 	if (dso == NULL || dso->symtab_type == DSO_BINARY_TYPE__NOT_FOUND)
@@ -1008,7 +1010,9 @@ int dso__binary_type_file(struct dso *dso, enum dso_binary_type type,
 
 	default:
 	case DSO_BINARY_TYPE__KALLSYMS:
+	case DSO_BINARY_TYPE__VMLINUX:
 	case DSO_BINARY_TYPE__GUEST_KALLSYMS:
+	case DSO_BINARY_TYPE__GUEST_VMLINUX:
 	case DSO_BINARY_TYPE__JAVA_JIT:
 	case DSO_BINARY_TYPE__NOT_FOUND:
 		ret = -1;
@@ -1364,6 +1368,11 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (fd < 0)
 		return -1;
 
+	if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
+		dso->symtab_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
+	else
+		dso->symtab_type = DSO_BINARY_TYPE__VMLINUX;
+
 	err = dso__load_sym(dso, map, symfs_vmlinux, fd, filter, 0, 0);
 	close(fd);
 
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index c9534fe..37f1ea1 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -158,6 +158,8 @@ struct addr_location {
 enum dso_binary_type {
 	DSO_BINARY_TYPE__KALLSYMS = 0,
 	DSO_BINARY_TYPE__GUEST_KALLSYMS,
+	DSO_BINARY_TYPE__VMLINUX,
+	DSO_BINARY_TYPE__GUEST_VMLINUX,
 	DSO_BINARY_TYPE__JAVA_JIT,
 	DSO_BINARY_TYPE__DEBUGLINK,
 	DSO_BINARY_TYPE__BUILD_ID_CACHE,

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Introduce symsrc structure.
  2012-08-10 22:22 ` [PATCH 11/16] perf symbol: introduce symsrc structure Cody P Schafer
  2012-08-11 13:28   ` Namhyung Kim
@ 2012-08-21 16:06   ` tip-bot for Cody P Schafer
  1 sibling, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:06 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  b68e2f919c6d3a0422239c98673c35ff503e52fb
Gitweb:     http://git.kernel.org/tip/b68e2f919c6d3a0422239c98673c35ff503e52fb
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:57 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:31:44 -0300

perf symbols: Introduce symsrc structure.

Factors opening of certain sections & tracking certain elf info into an
external structure.

The goal here is to keep multiple elfs (and their looked up
sections/indexes) around during the symbol generation process (in
dso__load()).

We need this to properly resolve symbols on PPC due to the use of
function descriptors & the .opd section (ie: symbols which are functions
don't point to their actual location, they point to their function
descriptor in .opd which contains their actual location.

It would be possible to just keep the (Elf *) around, but then we'd end
up with duplicate code for looking up the same sections and checking for
the existence of an important section wouldn't be as clean (and we need
to keep the Elf stuff confined to symtab-elf.c).

Utilized by the later patch
"perf symbols: Use both runtime and debug images"

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-12-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c     |  119 ++++++++++++++++++++++++++++---------
 tools/perf/util/symbol-minimal.c |   30 +++++++++-
 tools/perf/util/symbol.c         |   22 ++++----
 tools/perf/util/symbol.h         |   36 +++++++++++-
 4 files changed, 163 insertions(+), 44 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a9a194d..6974b2a 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -536,24 +536,25 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
 	return 0;
 }
 
-int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
-		  symbol_filter_t filter, int kmodule, int want_symtab)
+
+void symsrc__destroy(struct symsrc *ss)
+{
+	free(ss->name);
+	elf_end(ss->elf);
+	close(ss->fd);
+}
+
+int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
+		 enum dso_binary_type type)
 {
-	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
-	struct map *curr_map = map;
-	struct dso *curr_dso = dso;
-	Elf_Data *symstrs, *secstrs;
-	uint32_t nr_syms;
 	int err = -1;
-	uint32_t idx;
 	GElf_Ehdr ehdr;
-	GElf_Shdr shdr, opdshdr;
-	Elf_Data *syms, *opddata = NULL;
-	GElf_Sym sym;
-	Elf_Scn *sec, *sec_strndx, *opdsec;
 	Elf *elf;
-	int nr = 0;
-	size_t opdidx = 0;
+	int fd;
+
+	fd = open(name, O_RDONLY);
+	if (fd < 0)
+		return -1;
 
 	elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
 	if (elf == NULL) {
@@ -580,19 +581,88 @@ int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
 			goto out_elf_end;
 	}
 
-	sec = elf_section_by_name(elf, &ehdr, &shdr, ".symtab", NULL);
+	ss->symtab = elf_section_by_name(elf, &ehdr, &ss->symshdr, ".symtab",
+			NULL);
+	if (ss->symshdr.sh_type != SHT_SYMTAB)
+		ss->symtab = NULL;
+
+	ss->dynsym_idx = 0;
+	ss->dynsym = elf_section_by_name(elf, &ehdr, &ss->dynshdr, ".dynsym",
+			&ss->dynsym_idx);
+	if (ss->dynshdr.sh_type != SHT_DYNSYM)
+		ss->dynsym = NULL;
+
+	ss->opdidx = 0;
+	ss->opdsec = elf_section_by_name(elf, &ehdr, &ss->opdshdr, ".opd",
+			&ss->opdidx);
+	if (ss->opdshdr.sh_type != SHT_PROGBITS)
+		ss->opdsec = NULL;
+
+	if (dso->kernel == DSO_TYPE_USER) {
+		GElf_Shdr shdr;
+		ss->adjust_symbols = (ehdr.e_type == ET_EXEC ||
+				elf_section_by_name(elf, &ehdr, &shdr,
+						     ".gnu.prelink_undo",
+						     NULL) != NULL);
+	} else {
+		ss->adjust_symbols = 0;
+	}
+
+	ss->name   = strdup(name);
+	if (!ss->name)
+		goto out_elf_end;
+
+	ss->elf    = elf;
+	ss->fd     = fd;
+	ss->ehdr   = ehdr;
+	ss->type   = type;
+
+	return 0;
+
+out_elf_end:
+	elf_end(elf);
+out_close:
+	close(fd);
+	return err;
+}
+
+int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
+		  symbol_filter_t filter, int kmodule, int want_symtab)
+{
+	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
+	struct map *curr_map = map;
+	struct dso *curr_dso = dso;
+	Elf_Data *symstrs, *secstrs;
+	uint32_t nr_syms;
+	int err = -1;
+	uint32_t idx;
+	GElf_Ehdr ehdr;
+	GElf_Shdr shdr, opdshdr;
+	Elf_Data *syms, *opddata = NULL;
+	GElf_Sym sym;
+	Elf_Scn *sec, *sec_strndx, *opdsec;
+	Elf *elf;
+	int nr = 0;
+	size_t opdidx = 0;
+
+	elf = ss->elf;
+	ehdr = ss->ehdr;
+	sec = ss->symtab;
+	shdr = ss->symshdr;
+
 	if (sec == NULL) {
 		if (want_symtab)
 			goto out_elf_end;
 
-		sec = elf_section_by_name(elf, &ehdr, &shdr, ".dynsym", NULL);
+		sec  = ss->dynsym;
+		shdr = ss->dynshdr;
 		if (sec == NULL)
 			goto out_elf_end;
 	}
 
-	opdsec = elf_section_by_name(elf, &ehdr, &opdshdr, ".opd", &opdidx);
-	if (opdshdr.sh_type != SHT_PROGBITS)
-		opdsec = NULL;
+	opdsec = ss->opdsec;
+	opdshdr = ss->opdshdr;
+	opdidx  = ss->opdidx;
 	if (opdsec)
 		opddata = elf_rawdata(opdsec, NULL);
 
@@ -619,14 +689,7 @@ int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
 	nr_syms = shdr.sh_size / shdr.sh_entsize;
 
 	memset(&sym, 0, sizeof(sym));
-	if (dso->kernel == DSO_TYPE_USER) {
-		dso->adjust_symbols = (ehdr.e_type == ET_EXEC ||
-				elf_section_by_name(elf, &ehdr, &shdr,
-						     ".gnu.prelink_undo",
-						     NULL) != NULL);
-	} else {
-		dso->adjust_symbols = 0;
-	}
+	dso->adjust_symbols = ss->adjust_symbols;
 	elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
 		struct symbol *f;
 		const char *elf_name = elf_sym__name(&sym, symstrs);
@@ -770,8 +833,6 @@ new_symbol:
 	}
 	err = nr;
 out_elf_end:
-	elf_end(elf);
-out_close:
 	return err;
 }
 
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index bd8720b..1b16c27 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -241,6 +241,31 @@ out:
 	return ret;
 }
 
+int symsrc__init(struct symsrc *ss, struct dso *dso __used, const char *name,
+	         enum dso_binary_type type)
+{
+	int fd = open(name, O_RDONLY);
+	if (fd < 0)
+		return -1;
+
+	ss->name = strdup(name);
+	if (!ss->name)
+		goto out_close;
+
+	ss->type = type;
+
+	return 0;
+out_close:
+	close(fd);
+	return -1;
+}
+
+void symsrc__destroy(struct symsrc *ss)
+{
+	free(ss->name);
+	close(ss->fd);
+}
+
 int dso__synthesize_plt_symbols(struct dso *dso __used, char *name __used,
 				struct map *map __used,
 				symbol_filter_t filter __used)
@@ -248,14 +273,13 @@ int dso__synthesize_plt_symbols(struct dso *dso __used, char *name __used,
 	return 0;
 }
 
-int dso__load_sym(struct dso *dso, struct map *map __used,
-		  const char *name, int fd __used,
+int dso__load_sym(struct dso *dso, struct map *map __used, struct symsrc *ss,
 		  symbol_filter_t filter __used, int kmodule __used,
 		  int want_symtab __used)
 {
 	unsigned char *build_id[BUILD_ID_SIZE];
 
-	if (filename__read_build_id(name, build_id, BUILD_ID_SIZE) > 0) {
+	if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) > 0) {
 		dso__set_build_id(dso, build_id);
 		return 1;
 	}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 8f5cabbf..afec3f0 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1026,7 +1026,7 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 {
 	char *name;
 	int ret = -1;
-	int fd;
+	struct symsrc ss;
 	u_int i;
 	struct machine *machine;
 	char *root_dir = (char *) "";
@@ -1086,13 +1086,12 @@ restart:
 			continue;
 
 		/* Name is now the name of the next image to try */
-		fd = open(name, O_RDONLY);
-		if (fd < 0)
+		if (symsrc__init(&ss, dso, name, dso->symtab_type) < 0)
 			continue;
 
-		ret = dso__load_sym(dso, map, name, fd, filter, 0,
+		ret = dso__load_sym(dso, map, &ss, filter, 0,
 				    want_symtab);
-		close(fd);
+		symsrc__destroy(&ss);
 
 		/*
 		 * Some people seem to have debuginfo files _WITHOUT_ debug
@@ -1359,22 +1358,23 @@ out_failure:
 int dso__load_vmlinux(struct dso *dso, struct map *map,
 		      const char *vmlinux, symbol_filter_t filter)
 {
-	int err = -1, fd;
+	int err = -1;
+	struct symsrc ss;
 	char symfs_vmlinux[PATH_MAX];
 
 	snprintf(symfs_vmlinux, sizeof(symfs_vmlinux), "%s%s",
 		 symbol_conf.symfs, vmlinux);
-	fd = open(symfs_vmlinux, O_RDONLY);
-	if (fd < 0)
-		return -1;
 
 	if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
 		dso->symtab_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
 	else
 		dso->symtab_type = DSO_BINARY_TYPE__VMLINUX;
 
-	err = dso__load_sym(dso, map, symfs_vmlinux, fd, filter, 0, 0);
-	close(fd);
+	if (symsrc__init(&ss, dso, symfs_vmlinux, dso->symtab_type))
+		return -1;
+
+	err = dso__load_sym(dso, map, &ss, filter, 0, 0);
+	symsrc__destroy(&ss);
 
 	if (err > 0) {
 		dso__set_long_name(dso, (char *)vmlinux);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 37f1ea1..dd9e867 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -11,6 +11,12 @@
 #include <stdio.h>
 #include <byteswap.h>
 
+#ifndef NO_LIBELF_SUPPORT
+#include <libelf.h>
+#include <gelf.h>
+#include <elf.h>
+#endif
+
 #ifdef HAVE_CPLUS_DEMANGLE
 extern char *cplus_demangle(const char *, int);
 
@@ -219,6 +225,34 @@ struct dso {
 	char		 name[0];
 };
 
+struct symsrc {
+	char *name;
+	int fd;
+	enum dso_binary_type type;
+
+#ifndef NO_LIBELF_SUPPORT
+	Elf *elf;
+	GElf_Ehdr ehdr;
+
+	Elf_Scn *opdsec;
+	size_t opdidx;
+	GElf_Shdr opdshdr;
+
+	Elf_Scn *symtab;
+	GElf_Shdr symshdr;
+
+	Elf_Scn *dynsym;
+	size_t dynsym_idx;
+	GElf_Shdr dynshdr;
+
+	bool adjust_symbols;
+#endif
+};
+
+void symsrc__destroy(struct symsrc *ss);
+int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
+		 enum dso_binary_type type);
+
 #define DSO__SWAP(dso, type, val)			\
 ({							\
 	type ____r = val;				\
@@ -334,7 +368,7 @@ ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 			    struct machine *machine, u64 addr,
 			    u8 *data, ssize_t size);
 int dso__test_data(void);
-int dso__load_sym(struct dso *dso, struct map *map, const char *name, int fd,
+int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 		  symbol_filter_t filter, int kmodule, int want_symtab);
 int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
 				symbol_filter_t filter);

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Set symtab_type in dso__load_sym
  2012-08-10 22:22 ` [PATCH 12/16] perf symbol: set symtab_type in dso__load_sym Cody P Schafer
@ 2012-08-21 16:07   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:07 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  005f92947a0da7eb47b0f1ff611f8fc3d7ab4751
Gitweb:     http://git.kernel.org/tip/005f92947a0da7eb47b0f1ff611f8fc3d7ab4751
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:58 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:33:01 -0300

perf symbols: Set symtab_type in dso__load_sym

In certain cases, dso__load requires dso->symbol_type to be set prior to
calling it. With the introduction of symsrc*, the symtab_type is now
stored in a symsrc which is then passed to dso__load_sym().

Change dso__load_sym() to use the symtab_type from them symsrc (setting
dso->symtab_type as well).

Setup for later patch

"perf symbols: Use both runtime and debug images"

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-13-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c |    2 ++
 tools/perf/util/symbol.c     |   13 +++++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 6974b2a..3a9c38a 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -645,6 +645,8 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 	int nr = 0;
 	size_t opdidx = 0;
 
+	dso->symtab_type = ss->type;
+
 	elf = ss->elf;
 	ehdr = ss->ehdr;
 	sec = ss->symtab;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index afec3f0..2b3495a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1079,14 +1079,14 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 restart:
 	for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
 
-		dso->symtab_type = binary_type_symtab[i];
+		enum dso_binary_type symtab_type = binary_type_symtab[i];
 
-		if (dso__binary_type_file(dso, dso->symtab_type,
+		if (dso__binary_type_file(dso, symtab_type,
 					  root_dir, name, PATH_MAX))
 			continue;
 
 		/* Name is now the name of the next image to try */
-		if (symsrc__init(&ss, dso, name, dso->symtab_type) < 0)
+		if (symsrc__init(&ss, dso, name, symtab_type) < 0)
 			continue;
 
 		ret = dso__load_sym(dso, map, &ss, filter, 0,
@@ -1361,16 +1361,17 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	int err = -1;
 	struct symsrc ss;
 	char symfs_vmlinux[PATH_MAX];
+	enum dso_binary_type symtab_type;
 
 	snprintf(symfs_vmlinux, sizeof(symfs_vmlinux), "%s%s",
 		 symbol_conf.symfs, vmlinux);
 
 	if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
-		dso->symtab_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
+		symtab_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
 	else
-		dso->symtab_type = DSO_BINARY_TYPE__VMLINUX;
+		symtab_type = DSO_BINARY_TYPE__VMLINUX;
 
-	if (symsrc__init(&ss, dso, symfs_vmlinux, dso->symtab_type))
+	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
 		return -1;
 
 	err = dso__load_sym(dso, map, &ss, filter, 0, 0);

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Switch dso__synthesize_plt_symbols() to use symsrc
  2012-08-10 22:22 ` [PATCH 13/16] perf symbol: switch dso__synthesize_plt_symbols() to use symsrc Cody P Schafer
@ 2012-08-21 16:07   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:07 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  a44f605b2f6eadb771a052aa3a5eefb342b38a39
Gitweb:     http://git.kernel.org/tip/a44f605b2f6eadb771a052aa3a5eefb342b38a39
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:22:59 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:34:36 -0300

perf symbols: Switch dso__synthesize_plt_symbols() to use symsrc

Previously dso__synthesize_plt_symbols() was reopening the elf file to
obtain dynsyms from it. Rather than reopen the file, use the already
opened reference within the symsrc to access it.

Setup for the later patch

"perf symbols: Use both runtime and debug images"

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-14-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c     |   25 +++++++------------------
 tools/perf/util/symbol-minimal.c |    3 ++-
 tools/perf/util/symbol.c         |    8 +++++---
 tools/perf/util/symbol.h         |    4 ++--
 4 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 3a9c38a..5915947 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -166,7 +166,7 @@ static Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
  * And always look at the original dso, not at debuginfo packages, that
  * have the PLT data stripped out (shdr_rel_plt.sh_type == SHT_NOBITS).
  */
-int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
+int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss, struct map *map,
 				symbol_filter_t filter)
 {
 	uint32_t nr_rel_entries, idx;
@@ -181,21 +181,15 @@ int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
 	GElf_Ehdr ehdr;
 	char sympltname[1024];
 	Elf *elf;
-	int nr = 0, symidx, fd, err = 0;
+	int nr = 0, symidx, err = 0;
 
-	fd = open(name, O_RDONLY);
-	if (fd < 0)
-		goto out;
-
-	elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
-	if (elf == NULL)
-		goto out_close;
+	elf = ss->elf;
+	ehdr = ss->ehdr;
 
-	if (gelf_getehdr(elf, &ehdr) == NULL)
-		goto out_elf_end;
+	scn_dynsym = ss->dynsym;
+	shdr_dynsym = ss->dynshdr;
+	dynsym_idx = ss->dynsym_idx;
 
-	scn_dynsym = elf_section_by_name(elf, &ehdr, &shdr_dynsym,
-					 ".dynsym", &dynsym_idx);
 	if (scn_dynsym == NULL)
 		goto out_elf_end;
 
@@ -291,13 +285,8 @@ int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
 
 	err = 0;
 out_elf_end:
-	elf_end(elf);
-out_close:
-	close(fd);
-
 	if (err == 0)
 		return nr;
-out:
 	pr_debug("%s: problems reading %s PLT info.\n",
 		 __func__, dso->long_name);
 	return 0;
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 1b16c27..cc580c0 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -266,7 +266,8 @@ void symsrc__destroy(struct symsrc *ss)
 	close(ss->fd);
 }
 
-int dso__synthesize_plt_symbols(struct dso *dso __used, char *name __used,
+int dso__synthesize_plt_symbols(struct dso *dso __used,
+				struct symsrc *ss __used,
 				struct map *map __used,
 				symbol_filter_t filter __used)
 {
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2b3495a..f8a3068 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1091,21 +1091,23 @@ restart:
 
 		ret = dso__load_sym(dso, map, &ss, filter, 0,
 				    want_symtab);
-		symsrc__destroy(&ss);
 
 		/*
 		 * Some people seem to have debuginfo files _WITHOUT_ debug
 		 * info!?!?
 		 */
-		if (!ret)
+		if (!ret) {
+			symsrc__destroy(&ss);
 			continue;
+		}
 
 		if (ret > 0) {
 			int nr_plt;
 
-			nr_plt = dso__synthesize_plt_symbols(dso, name, map, filter);
+			nr_plt = dso__synthesize_plt_symbols(dso, &ss, map, filter);
 			if (nr_plt > 0)
 				ret += nr_plt;
+			symsrc__destroy(&ss);
 			break;
 		}
 	}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index dd9e867..2981513 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -370,8 +370,8 @@ ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 int dso__test_data(void);
 int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 		  symbol_filter_t filter, int kmodule, int want_symtab);
-int dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
-				symbol_filter_t filter);
+int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss,
+				struct map *map, symbol_filter_t filter);
 
 void symbols__insert(struct rb_root *symbols, struct symbol *sym);
 void symbols__fixup_duplicate(struct rb_root *symbols);

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Factor want_symtab out of dso__load_sym()
  2012-08-10 22:23 ` [PATCH 14/16] perf symbol: factor want_symtab out of dso__load_sym() Cody P Schafer
@ 2012-08-21 16:08   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  d26cd12b46cb6b5595143804b43ba5aa7968551e
Gitweb:     http://git.kernel.org/tip/d26cd12b46cb6b5595143804b43ba5aa7968551e
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:23:00 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:37:37 -0300

perf symbols: Factor want_symtab out of dso__load_sym()

Only one callsite of dso__load_sym() uses the want_symtab functionality,
so place the logic at the callsite instead of within dso__load_sym().

This sets us up for removal of want_symtab completely once we keep
multiple elf handles (within symsrc's) around.

Setup for the later patch

"perf symbols: Use both runtime and debug images"

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-15-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c     |   21 ++++++++++-----------
 tools/perf/util/symbol-minimal.c |    8 ++++++--
 tools/perf/util/symbol.c         |   10 +++++++---
 tools/perf/util/symbol.h         |    3 ++-
 4 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 5915947..492ebec 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -525,6 +525,10 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
 	return 0;
 }
 
+bool symsrc__has_symtab(struct symsrc *ss)
+{
+	return ss->symtab != NULL;
+}
 
 void symsrc__destroy(struct symsrc *ss)
 {
@@ -616,7 +620,7 @@ out_close:
 }
 
 int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
-		  symbol_filter_t filter, int kmodule, int want_symtab)
+		  symbol_filter_t filter, int kmodule)
 {
 	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
 	struct map *curr_map = map;
@@ -636,21 +640,16 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 
 	dso->symtab_type = ss->type;
 
+	if (!ss->symtab) {
+		ss->symtab  = ss->dynsym;
+		ss->symshdr = ss->dynshdr;
+	}
+
 	elf = ss->elf;
 	ehdr = ss->ehdr;
 	sec = ss->symtab;
 	shdr = ss->symshdr;
 
-	if (sec == NULL) {
-		if (want_symtab)
-			goto out_elf_end;
-
-		sec  = ss->dynsym;
-		shdr = ss->dynshdr;
-		if (sec == NULL)
-			goto out_elf_end;
-	}
-
 	opdsec = ss->opdsec;
 	opdshdr = ss->opdshdr;
 	opdidx  = ss->opdidx;
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index cc580c0..11c2c60 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -260,6 +260,11 @@ out_close:
 	return -1;
 }
 
+bool symsrc__has_symtab(struct symsrc *ss __used)
+{
+	return false;
+}
+
 void symsrc__destroy(struct symsrc *ss)
 {
 	free(ss->name);
@@ -275,8 +280,7 @@ int dso__synthesize_plt_symbols(struct dso *dso __used,
 }
 
 int dso__load_sym(struct dso *dso, struct map *map __used, struct symsrc *ss,
-		  symbol_filter_t filter __used, int kmodule __used,
-		  int want_symtab __used)
+		  symbol_filter_t filter __used, int kmodule __used)
 {
 	unsigned char *build_id[BUILD_ID_SIZE];
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index f8a3068..8e7d74f 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1089,8 +1089,12 @@ restart:
 		if (symsrc__init(&ss, dso, name, symtab_type) < 0)
 			continue;
 
-		ret = dso__load_sym(dso, map, &ss, filter, 0,
-				    want_symtab);
+		if (want_symtab && !symsrc__has_symtab(&ss)) {
+			symsrc__destroy(&ss);
+			continue;
+		}
+
+		ret = dso__load_sym(dso, map, &ss, filter, 0);
 
 		/*
 		 * Some people seem to have debuginfo files _WITHOUT_ debug
@@ -1376,7 +1380,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
 		return -1;
 
-	err = dso__load_sym(dso, map, &ss, filter, 0, 0);
+	err = dso__load_sym(dso, map, &ss, filter, 0);
 	symsrc__destroy(&ss);
 
 	if (err > 0) {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 2981513..fa9f6b1 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -252,6 +252,7 @@ struct symsrc {
 void symsrc__destroy(struct symsrc *ss);
 int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 		 enum dso_binary_type type);
+bool symsrc__has_symtab(struct symsrc *ss);
 
 #define DSO__SWAP(dso, type, val)			\
 ({							\
@@ -369,7 +370,7 @@ ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 			    u8 *data, ssize_t size);
 int dso__test_data(void);
 int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
-		  symbol_filter_t filter, int kmodule, int want_symtab);
+		  symbol_filter_t filter, int kmodule);
 int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss,
 				struct map *map, symbol_filter_t filter);
 

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Convert dso__load_syms to take 2 symsrc's
  2012-08-10 22:23 ` [PATCH 15/16] perf symbol: convert dso__load_syms to take 2 symsrc's Cody P Schafer
@ 2012-08-21 16:09   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:09 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  261360b6e90a782f0a63d8f61a67683c376c88cf
Gitweb:     http://git.kernel.org/tip/261360b6e90a782f0a63d8f61a67683c376c88cf
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:23:01 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:41:33 -0300

perf symbols: Convert dso__load_syms to take 2 symsrc's

To properly handle platforms with an opd section, both a runtime image
(which contains the opd section but possibly lacks symbols) and a symbol
image (which probably lacks an opd section but has symbols).

The next patch ("perf symbol: use both runtime and debug images")
adjusts the callsite in dso__load() to take advantage of being able to
pass both runtime & debug images.

Assumptions made here:

 - The opd section, if it exists in the runtime image, has headers in
   both the runtime image and the debug/syms image.

 - The index of the opd section (again, only if it exists in the runtime
   image) is the same in both the runtime and debug/symbols image.

Both of these are true on RHEL, but it is unclear how accurate they are
in general (on platforms with function descriptors in opd sections).

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-16-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c     |   47 +++++++++++++++++++------------------
 tools/perf/util/symbol-minimal.c |    1 +
 tools/perf/util/symbol.c         |    4 +-
 tools/perf/util/symbol.h         |    5 ++-
 4 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 492ebec..36e4a45 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -619,7 +619,8 @@ out_close:
 	return err;
 }
 
-int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
+int dso__load_sym(struct dso *dso, struct map *map,
+		  struct symsrc *syms_ss, struct symsrc *runtime_ss,
 		  symbol_filter_t filter, int kmodule)
 {
 	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
@@ -630,31 +631,27 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 	int err = -1;
 	uint32_t idx;
 	GElf_Ehdr ehdr;
-	GElf_Shdr shdr, opdshdr;
+	GElf_Shdr shdr;
 	Elf_Data *syms, *opddata = NULL;
 	GElf_Sym sym;
-	Elf_Scn *sec, *sec_strndx, *opdsec;
+	Elf_Scn *sec, *sec_strndx;
 	Elf *elf;
 	int nr = 0;
-	size_t opdidx = 0;
 
-	dso->symtab_type = ss->type;
+	dso->symtab_type = syms_ss->type;
 
-	if (!ss->symtab) {
-		ss->symtab  = ss->dynsym;
-		ss->symshdr = ss->dynshdr;
+	if (!syms_ss->symtab) {
+		syms_ss->symtab  = syms_ss->dynsym;
+		syms_ss->symshdr = syms_ss->dynshdr;
 	}
 
-	elf = ss->elf;
-	ehdr = ss->ehdr;
-	sec = ss->symtab;
-	shdr = ss->symshdr;
+	elf = syms_ss->elf;
+	ehdr = syms_ss->ehdr;
+	sec = syms_ss->symtab;
+	shdr = syms_ss->symshdr;
 
-	opdsec = ss->opdsec;
-	opdshdr = ss->opdshdr;
-	opdidx  = ss->opdidx;
-	if (opdsec)
-		opddata = elf_rawdata(opdsec, NULL);
+	if (runtime_ss->opdsec)
+		opddata = elf_rawdata(runtime_ss->opdsec, NULL);
 
 	syms = elf_getdata(sec, NULL);
 	if (syms == NULL)
@@ -679,13 +676,14 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 	nr_syms = shdr.sh_size / shdr.sh_entsize;
 
 	memset(&sym, 0, sizeof(sym));
-	dso->adjust_symbols = ss->adjust_symbols;
+	dso->adjust_symbols = runtime_ss->adjust_symbols;
 	elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
 		struct symbol *f;
 		const char *elf_name = elf_sym__name(&sym, symstrs);
 		char *demangled = NULL;
 		int is_label = elf_sym__is_label(&sym);
 		const char *section_name;
+		bool used_opd = false;
 
 		if (kmap && kmap->ref_reloc_sym && kmap->ref_reloc_sym->name &&
 		    strcmp(elf_name, kmap->ref_reloc_sym->name) == 0)
@@ -704,14 +702,16 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 				continue;
 		}
 
-		if (opdsec && sym.st_shndx == opdidx) {
-			u32 offset = sym.st_value - opdshdr.sh_addr;
+		if (runtime_ss->opdsec && sym.st_shndx == runtime_ss->opdidx) {
+			u32 offset = sym.st_value - syms_ss->opdshdr.sh_addr;
 			u64 *opd = opddata->d_buf + offset;
 			sym.st_value = DSO__SWAP(dso, u64, *opd);
-			sym.st_shndx = elf_addr_to_index(elf, sym.st_value);
+			sym.st_shndx = elf_addr_to_index(runtime_ss->elf,
+					sym.st_value);
+			used_opd = true;
 		}
 
-		sec = elf_getscn(elf, sym.st_shndx);
+		sec = elf_getscn(runtime_ss->elf, sym.st_shndx);
 		if (!sec)
 			goto out_elf_end;
 
@@ -777,7 +777,8 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 			goto new_symbol;
 		}
 
-		if (curr_dso->adjust_symbols && sym.st_value) {
+		if ((used_opd && runtime_ss->adjust_symbols)
+				|| (!used_opd && syms_ss->adjust_symbols)) {
 			pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
 				  "sh_addr: %#" PRIx64 " sh_offset: %#" PRIx64 "\n", __func__,
 				  (u64)sym.st_value, (u64)shdr.sh_addr,
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 11c2c60..7747ea6 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -280,6 +280,7 @@ int dso__synthesize_plt_symbols(struct dso *dso __used,
 }
 
 int dso__load_sym(struct dso *dso, struct map *map __used, struct symsrc *ss,
+		  struct symsrc *runtime_ss __used,
 		  symbol_filter_t filter __used, int kmodule __used)
 {
 	unsigned char *build_id[BUILD_ID_SIZE];
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 8e7d74f..739e5a3 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1094,7 +1094,7 @@ restart:
 			continue;
 		}
 
-		ret = dso__load_sym(dso, map, &ss, filter, 0);
+		ret = dso__load_sym(dso, map, &ss, &ss, filter, 0);
 
 		/*
 		 * Some people seem to have debuginfo files _WITHOUT_ debug
@@ -1380,7 +1380,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
 		return -1;
 
-	err = dso__load_sym(dso, map, &ss, filter, 0);
+	err = dso__load_sym(dso, map, &ss, &ss, filter, 0);
 	symsrc__destroy(&ss);
 
 	if (err > 0) {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index fa9f6b1..1f3eed2 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -369,8 +369,9 @@ ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 			    struct machine *machine, u64 addr,
 			    u8 *data, ssize_t size);
 int dso__test_data(void);
-int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
-		  symbol_filter_t filter, int kmodule);
+int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
+		  struct symsrc *runtime_ss, symbol_filter_t filter,
+		  int kmodule);
 int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss,
 				struct map *map, symbol_filter_t filter);
 

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [tip:perf/core] perf symbols: Use both runtime and debug images
  2012-08-10 22:23 ` [PATCH 16/16] perf symbol: use both runtime and debug images Cody P Schafer
@ 2012-08-21 16:10   ` tip-bot for Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: tip-bot for Cody P Schafer @ 2012-08-21 16:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, cody,
	a.p.zijlstra, matthltc, dave, namhyung, sukadev, tglx

Commit-ID:  3aafe5ae08f2aed378e06d78b207883879d25cbe
Gitweb:     http://git.kernel.org/tip/3aafe5ae08f2aed378e06d78b207883879d25cbe
Author:     Cody P Schafer <cody@linux.vnet.ibm.com>
AuthorDate: Fri, 10 Aug 2012 15:23:02 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2012 14:46:55 -0300

perf symbols: Use both runtime and debug images

We keep both a 'runtime' elf image as well as a 'debug' elf image around
and generate symbols by looking at both of these.

This eliminates the need for the want_symtab/goto restart mechanism
combined with iterating over and reopening the elf images a second time.

Also give dso__synthsize_plt_symbols() the runtime image (which has
dynsyms) instead of the symbol image (which may only have a symtab and
no dynsyms).

Previously if a debug image was found all runtime images were ignored.

This fixes 2 issues:

 - Symbol resolution to failure on PowerPC systems with debug symbols
   installed, as the debug images lack a '.opd' section which contains
   function descriptors.

 - On all archs, plt synthesis failed when a debug image was loaded and
   that debug image lacks a dynsym section while a runtime image has a
   dynsym section.

Assumptions:

 - If a .opd section exists, it is contained in the highest priority
   image with a dynsym section.

 - This generally implies that the debug image lacks a dynsym section
   (ie: it is marked as NO_BITS).

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: David Hansen <dave@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Hellsley <matthltc@us.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/1344637382-22789-17-git-send-email-cody@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c     |    5 ++
 tools/perf/util/symbol-minimal.c |    6 +++
 tools/perf/util/symbol.c         |   77 +++++++++++++++++++++----------------
 tools/perf/util/symbol.h         |    1 +
 4 files changed, 56 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 36e4a45..5b37e13 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -525,6 +525,11 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
 	return 0;
 }
 
+bool symsrc__possibly_runtime(struct symsrc *ss)
+{
+	return ss->dynsym || ss->opdsec;
+}
+
 bool symsrc__has_symtab(struct symsrc *ss)
 {
 	return ss->symtab != NULL;
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 7747ea6..6738ea1 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -260,6 +260,12 @@ out_close:
 	return -1;
 }
 
+bool symsrc__possibly_runtime(struct symsrc *ss __used)
+{
+	/* Assume all sym sources could be a runtime image. */
+	return true;
+}
+
 bool symsrc__has_symtab(struct symsrc *ss __used)
 {
 	return false;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 739e5a3..2293a4a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1026,11 +1026,12 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 {
 	char *name;
 	int ret = -1;
-	struct symsrc ss;
 	u_int i;
 	struct machine *machine;
 	char *root_dir = (char *) "";
-	int want_symtab;
+	int ss_pos = 0;
+	struct symsrc ss_[2];
+	struct symsrc *syms_ss = NULL, *runtime_ss = NULL;
 
 	dso__set_loaded(dso, map->type);
 
@@ -1072,12 +1073,12 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 		root_dir = machine->root_dir;
 
 	/* Iterate over candidate debug images.
-	 * On the first pass, only load images if they have a full symtab.
-	 * Failing that, do a second pass where we accept .dynsym also
+	 * Keep track of "interesting" ones (those which have a symtab, dynsym,
+	 * and/or opd section) for processing.
 	 */
-	want_symtab = 1;
-restart:
 	for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
+		struct symsrc *ss = &ss_[ss_pos];
+		bool next_slot = false;
 
 		enum dso_binary_type symtab_type = binary_type_symtab[i];
 
@@ -1086,45 +1087,55 @@ restart:
 			continue;
 
 		/* Name is now the name of the next image to try */
-		if (symsrc__init(&ss, dso, name, symtab_type) < 0)
+		if (symsrc__init(ss, dso, name, symtab_type) < 0)
 			continue;
 
-		if (want_symtab && !symsrc__has_symtab(&ss)) {
-			symsrc__destroy(&ss);
-			continue;
+		if (!syms_ss && symsrc__has_symtab(ss)) {
+			syms_ss = ss;
+			next_slot = true;
 		}
 
-		ret = dso__load_sym(dso, map, &ss, &ss, filter, 0);
-
-		/*
-		 * Some people seem to have debuginfo files _WITHOUT_ debug
-		 * info!?!?
-		 */
-		if (!ret) {
-			symsrc__destroy(&ss);
-			continue;
+		if (!runtime_ss && symsrc__possibly_runtime(ss)) {
+			runtime_ss = ss;
+			next_slot = true;
 		}
 
-		if (ret > 0) {
-			int nr_plt;
+		if (next_slot) {
+			ss_pos++;
 
-			nr_plt = dso__synthesize_plt_symbols(dso, &ss, map, filter);
-			if (nr_plt > 0)
-				ret += nr_plt;
-			symsrc__destroy(&ss);
-			break;
+			if (syms_ss && runtime_ss)
+				break;
 		}
+
 	}
 
-	/*
-	 * If we wanted a full symtab but no image had one,
-	 * relax our requirements and repeat the search.
-	 */
-	if (ret <= 0 && want_symtab) {
-		want_symtab = 0;
-		goto restart;
+	if (!runtime_ss && !syms_ss)
+		goto out_free;
+
+	if (runtime_ss && !syms_ss) {
+		syms_ss = runtime_ss;
+	}
+
+	/* We'll have to hope for the best */
+	if (!runtime_ss && syms_ss)
+		runtime_ss = syms_ss;
+
+	if (syms_ss)
+		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, 0);
+	else
+		ret = -1;
+
+	if (ret > 0 && runtime_ss->dynsym) {
+		int nr_plt;
+
+		nr_plt = dso__synthesize_plt_symbols(dso, runtime_ss, map, filter);
+		if (nr_plt > 0)
+			ret += nr_plt;
 	}
 
+	for (; ss_pos > 0; ss_pos--)
+		symsrc__destroy(&ss_[ss_pos - 1]);
+out_free:
 	free(name);
 	if (ret < 0 && strstr(dso->name, " (deleted)") != NULL)
 		return 0;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 1f3eed2..fc4b1e6 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -253,6 +253,7 @@ void symsrc__destroy(struct symsrc *ss);
 int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 		 enum dso_binary_type type);
 bool symsrc__has_symtab(struct symsrc *ss);
+bool symsrc__possibly_runtime(struct symsrc *ss);
 
 #define DSO__SWAP(dso, type, val)			\
 ({							\

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 15/16] perf symbol: convert dso__load_syms to take 2 symsrc's
  2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
  0 siblings, 0 replies; 37+ messages in thread
From: Cody P Schafer @ 2012-08-09 22:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Sukadev Bhattiprolu, Matt Hellsley, David Hansen

To properly handle platforms with an opd section, both a runtime image
(which contains the opd section but possibly lacks symbols) and a symbol
image (which probably lacks an opd section but has symbols).

The next patch ("perf symbol: use both runtime and debug images")
adjusts the callsite in dso__load() to take advantage of being able to
pass both runtime & debug images.

Assumptions made here:

 - The opd section, if it exists in the runtime image, has headers in
   both the runtime image and the debug/syms image.
 - The index of the opd section (again, only if it exists in the
   runtime image) is the same in both the runtime and debug/symbols
   image.

Both of these are true on RHEL, but it is unclear how accurate they are
in general (on platforms with function descriptors in opd sections).

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 tools/perf/util/symbol.c | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 83d2276..e740dc1 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1319,7 +1319,8 @@ static size_t elf_addr_to_index(Elf *elf, GElf_Addr addr)
 	return -1;
 }
 
-static int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
+static int dso__load_sym(struct dso *dso, struct map *map,
+			struct symsrc *syms_ss, struct symsrc *runtime_ss,
 		         symbol_filter_t filter, int kmodule)
 {
 	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
@@ -1330,26 +1331,22 @@ static int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 	int err = -1;
 	uint32_t idx;
 	GElf_Ehdr ehdr;
-	GElf_Shdr shdr, opdshdr;
+	GElf_Shdr shdr;
 	Elf_Data *syms, *opddata = NULL;
 	GElf_Sym sym;
-	Elf_Scn *sec, *sec_strndx, *opdsec;
+	Elf_Scn *sec, *sec_strndx;
 	Elf *elf;
 	int nr = 0;
-	size_t opdidx = 0;
 
-	dso->symtab_type = ss->type;
+	dso->symtab_type = syms_ss->type;
 
-	elf = ss->elf;
-	ehdr = ss->ehdr;
-	sec = ss->symtab;
-	shdr = ss->symshdr;
+	elf = syms_ss->elf;
+	ehdr = syms_ss->ehdr;
+	sec = syms_ss->symtab;
+	shdr = syms_ss->symshdr;
 
-	opdsec = ss->opdsec;
-	opdshdr = ss->opdshdr;
-	opdidx  = ss->opdidx;
-	if (opdsec)
-		opddata = elf_rawdata(opdsec, NULL);
+	if (runtime_ss->opdsec)
+		opddata = elf_rawdata(runtime_ss->opdsec, NULL);
 
 	syms = elf_getdata(sec, NULL);
 	if (syms == NULL)
@@ -1374,13 +1371,14 @@ static int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 	nr_syms = shdr.sh_size / shdr.sh_entsize;
 
 	memset(&sym, 0, sizeof(sym));
-	dso->adjust_symbols = ss->adjust_symbols;
+	dso->adjust_symbols = runtime_ss->adjust_symbols;
 	elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
 		struct symbol *f;
 		const char *elf_name = elf_sym__name(&sym, symstrs);
 		char *demangled = NULL;
 		int is_label = elf_sym__is_label(&sym);
 		const char *section_name;
+		bool used_opd = false;
 
 		if (kmap && kmap->ref_reloc_sym && kmap->ref_reloc_sym->name &&
 		    strcmp(elf_name, kmap->ref_reloc_sym->name) == 0)
@@ -1399,14 +1397,16 @@ static int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 				continue;
 		}
 
-		if (opdsec && sym.st_shndx == opdidx) {
-			u32 offset = sym.st_value - opdshdr.sh_addr;
+		if (runtime_ss->opdsec && sym.st_shndx == runtime_ss->opdidx) {
+			u32 offset = sym.st_value - syms_ss->opdshdr.sh_addr;
 			u64 *opd = opddata->d_buf + offset;
 			sym.st_value = DSO__SWAP(dso, u64, *opd);
-			sym.st_shndx = elf_addr_to_index(elf, sym.st_value);
+			sym.st_shndx = elf_addr_to_index(runtime_ss->elf,
+					sym.st_value);
+			used_opd = true;
 		}
 
-		sec = elf_getscn(elf, sym.st_shndx);
+		sec = elf_getscn(runtime_ss->elf, sym.st_shndx);
 		if (!sec)
 			goto out_elf_end;
 
@@ -1472,7 +1472,8 @@ static int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
 			goto new_symbol;
 		}
 
-		if (curr_dso->adjust_symbols && sym.st_value) {
+		if ((used_opd && runtime_ss->adjust_symbols)
+				|| (!used_opd && syms_ss->adjust_symbols)) {
 			pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
 				  "sh_addr: %#" PRIx64 " sh_offset: %#" PRIx64 "\n", __func__,
 				  (u64)sym.st_value, (u64)shdr.sh_addr,
@@ -1944,7 +1945,7 @@ restart:
 			ss.symshdr = ss.dynshdr;
 		}
 
-		ret = dso__load_sym(dso, map, &ss, filter, 0);
+		ret = dso__load_sym(dso, map, &ss, &ss, filter, 0);
 
 		/*
 		 * Some people seem to have debuginfo files _WITHOUT_ debug
@@ -2249,7 +2250,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
 		return -1;
 
-	err = dso__load_sym(dso, map, &ss, filter, 0);
+	err = dso__load_sym(dso, map, &ss, &ss, filter, 0);
 	symsrc__destroy(&ss);
 
 	if (err > 0) {
-- 
1.7.11.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2012-08-21 16:11 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-10 22:22 [PATCH v2 00/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
2012-08-10 22:22 ` [PATCH 01/16] perf symbol: correct comment wrt kallsyms loading Cody P Schafer
2012-08-11 13:14   ` Namhyung Kim
2012-08-21 16:00   ` [tip:perf/core] perf symbols: Correct " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb Cody P Schafer
2012-08-21 16:01   ` [tip:perf/core] perf symbols: Remove " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 03/16] perf symbol: only un-prelink non-zero symbols Cody P Schafer
2012-08-21 15:56   ` [tip:perf/core] perf symbols: Only " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 04/16] perf utils: remove unused function map__objdump_2ip Cody P Schafer
2012-08-21 15:57   ` [tip:perf/core] perf symbols: Remove " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 05/16] perf symbol: don't try to synthesize plt without dynstr Cody P Schafer
2012-08-21 15:58   ` [tip:perf/core] perf symbols: Don' t " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 06/16] perf symbol: remove unneeded call to dso__set_long_name() Cody P Schafer
2012-08-21 15:59   ` [tip:perf/core] perf symbols: Remove " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 07/16] perf symbol: simplify out_fixup in kernel syms loading Cody P Schafer
2012-08-21 16:02   ` [tip:perf/core] perf symbols: Simplify " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 08/16] perf symbol: only set vmlinux longname & mark loaded if really loaded Cody P Schafer
2012-08-21 16:03   ` [tip:perf/core] perf symbols: " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 09/16] perf symbol: avoid segfault in elf_strptr Cody P Schafer
2012-08-21 16:04   ` [tip:perf/core] perf symbols: Avoid " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 10/16] perf symbol: track symtab_type of vmlinux Cody P Schafer
2012-08-21 16:05   ` [tip:perf/core] perf symbols: Track " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 11/16] perf symbol: introduce symsrc structure Cody P Schafer
2012-08-11 13:28   ` Namhyung Kim
2012-08-13 17:36     ` Arnaldo Carvalho de Melo
2012-08-21 16:06   ` [tip:perf/core] perf symbols: Introduce " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 12/16] perf symbol: set symtab_type in dso__load_sym Cody P Schafer
2012-08-21 16:07   ` [tip:perf/core] perf symbols: Set " tip-bot for Cody P Schafer
2012-08-10 22:22 ` [PATCH 13/16] perf symbol: switch dso__synthesize_plt_symbols() to use symsrc Cody P Schafer
2012-08-21 16:07   ` [tip:perf/core] perf symbols: Switch " tip-bot for Cody P Schafer
2012-08-10 22:23 ` [PATCH 14/16] perf symbol: factor want_symtab out of dso__load_sym() Cody P Schafer
2012-08-21 16:08   ` [tip:perf/core] perf symbols: Factor " tip-bot for Cody P Schafer
2012-08-10 22:23 ` [PATCH 15/16] perf symbol: convert dso__load_syms to take 2 symsrc's Cody P Schafer
2012-08-21 16:09   ` [tip:perf/core] perf symbols: Convert " tip-bot for Cody P Schafer
2012-08-10 22:23 ` [PATCH 16/16] perf symbol: use both runtime and debug images Cody P Schafer
2012-08-21 16:10   ` [tip:perf/core] perf symbols: Use " tip-bot for Cody P Schafer
  -- strict thread matches above, loose matches on Subject: below --
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
2012-08-09 22:18 ` [PATCH 15/16] perf symbol: convert dso__load_syms to take 2 symsrc's Cody P Schafer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).