* [PATCH 01/16] perf symbol: correct comment wrt kallsyms loading
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
2012-08-09 22:18 ` [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb Cody P Schafer
` (14 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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 07e7bd6..df4736d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -616,7 +616,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] 20+ messages in thread* [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb
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 01/16] perf symbol: correct comment wrt kallsyms loading Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-10 2:39 ` Namhyung Kim
2012-08-09 22:18 ` [PATCH 03/16] perf symbol: only un-prelink non-zero symbols Cody P Schafer
` (13 subsequent siblings)
15 siblings, 1 reply; 20+ 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
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 df4736d..b2f7597 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -574,7 +574,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;
@@ -614,13 +614,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;
}
@@ -647,7 +642,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;
@@ -656,8 +651,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 1, and rely on
+ * symbols__fixup_end() to fix it up.
+ */
+ sym = symbol__new(start, 1, kallsyms2elf_type(type), name);
if (sym == NULL)
return -ENOMEM;
/*
@@ -2528,7 +2527,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 1fe733a..c8ec1d7 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -297,7 +297,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));
void machine__destroy_kernel_maps(struct machine *machine);
int __machine__create_kernel_maps(struct machine *machine, struct dso *kernel);
--
1.7.11.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb
2012-08-09 22:18 ` [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb Cody P Schafer
@ 2012-08-10 2:39 ` Namhyung Kim
2012-08-10 17:11 ` Cody P Schafer
0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2012-08-10 2:39 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
Hi, Cody
On Thu, 9 Aug 2012 15:18:27 -0700, Cody P. Schafer wrote:
> 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 df4736d..b2f7597 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -574,7 +574,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;
> @@ -614,13 +614,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;
> }
> @@ -647,7 +642,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;
> @@ -656,8 +651,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 1, and rely on
> + * symbols__fixup_end() to fix it up.
> + */
> + sym = symbol__new(start, 1, kallsyms2elf_type(type), name);
I guess that length of 1 effectively same as zero length in this case
since we end up calling symbols__fixup_end. The 'end - start + 1' part
looks like a leftover from previous change and not needed anymore -
KSYM_NAME_LEN check too, IMHO - so I suggest using 0 length to make it
clear.
And it seems you need to rebase the series onto Arnaldo's current
perf/core branch which separates out ELF bits to symbol-elf.c.
Thanks,
Namhyung
> if (sym == NULL)
> return -ENOMEM;
> /*
> @@ -2528,7 +2527,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 1fe733a..c8ec1d7 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -297,7 +297,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));
>
> void machine__destroy_kernel_maps(struct machine *machine);
> int __machine__create_kernel_maps(struct machine *machine, struct dso *kernel);
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb
2012-08-10 2:39 ` Namhyung Kim
@ 2012-08-10 17:11 ` Cody P Schafer
0 siblings, 0 replies; 20+ messages in thread
From: Cody P Schafer @ 2012-08-10 17:11 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, LKML, Ingo Molnar, Paul Mackerras,
Peter Zijlstra, Sukadev Bhattiprolu, Matt Hellsley, David Hansen
> I guess that length of 1 effectively same as zero length in this case
> since we end up calling symbols__fixup_end. The 'end - start + 1' part
> looks like a leftover from previous change and not needed anymore -
> KSYM_NAME_LEN check too, IMHO - so I suggest using 0 length to make it
> clear.
Got it.
> And it seems you need to rebase the series onto Arnaldo's current
> perf/core branch which separates out ELF bits to symbol-elf.c.
Will do. It apparently wasn't pushed out when I sent these patches, look
for v2 shortly.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 03/16] perf symbol: only un-prelink non-zero symbols
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 01/16] perf symbol: correct comment wrt kallsyms loading Cody P Schafer
2012-08-09 22:18 ` [PATCH 02/16] perf symbol: remove unused 'end' arg in kallsyms parse cb Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 04/16] perf utils: remove unused function map__objdump_2ip Cody P Schafer
` (12 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b2f7597..254d4d8 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1401,7 +1401,7 @@ static int dso__load_sym(struct dso *dso, struct map *map, const char *name,
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] 20+ messages in thread* [PATCH 04/16] perf utils: remove unused function map__objdump_2ip
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (2 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 03/16] perf symbol: only un-prelink non-zero symbols Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 05/16] perf symbol: don't try to synthesize plt without dynstr Cody P Schafer
` (11 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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 cc33486..23d9130 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -242,14 +242,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 03a1e9b..cb45f14 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] 20+ messages in thread* [PATCH 05/16] perf symbol: don't try to synthesize plt without dynstr.
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (3 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 04/16] perf utils: remove unused function map__objdump_2ip Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 06/16] perf symbol: remove unneeded call to dso__set_long_name() Cody P Schafer
` (10 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 254d4d8..8ae159a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1077,6 +1077,9 @@ 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] 20+ messages in thread* [PATCH 06/16] perf symbol: remove unneeded call to dso__set_long_name()
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (4 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 05/16] perf symbol: don't try to synthesize plt without dynstr Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 07/16] perf symbol: symplify out_fixup in kernel syms loading Cody P Schafer
` (9 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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 8ae159a..caab61a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2187,10 +2187,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] 20+ messages in thread* [PATCH 07/16] perf symbol: symplify out_fixup in kernel syms loading
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (5 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 06/16] perf symbol: remove unneeded call to dso__set_long_name() Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 08/16] perf symbol: only set vmlinux longname & mark loaded if really loaded Cody P Schafer
` (8 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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 caab61a..90d2760 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2303,9 +2303,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] 20+ messages in thread* [PATCH 08/16] perf symbol: only set vmlinux longname & mark loaded if really loaded
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (6 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 07/16] perf symbol: symplify out_fixup in kernel syms loading Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 09/16] perf symbol: avoid segfault in elf_strptr Cody P Schafer
` (7 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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 90d2760..9e31cbb 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2164,13 +2164,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] 20+ messages in thread* [PATCH 09/16] perf symbol: avoid segfault in elf_strptr
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (7 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 08/16] perf symbol: only set vmlinux longname & mark loaded if really loaded Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 10/16] perf symbol: track symtab_type of vmlinux Cody P Schafer
` (6 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 9e31cbb..6afc92d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -973,6 +973,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] 20+ messages in thread* [PATCH 10/16] perf symbol: track symtab_type of vmlinux
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (8 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 09/16] perf symbol: avoid segfault in elf_strptr Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 11/16] perf symbol: introduce symsrc structure Cody P Schafer
` (5 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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 6afc92d..662d1a2 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1708,6 +1708,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',
@@ -1718,6 +1719,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)
@@ -1793,7 +1795,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;
@@ -2168,6 +2172,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 c8ec1d7..e92bd07 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] 20+ messages in thread* [PATCH 11/16] perf symbol: introduce symsrc structure.
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (9 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 10/16] perf symbol: track symtab_type of vmlinux Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 12/16] perf symbol: set symtab_type in dso__load_sym Cody P Schafer
` (4 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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.
Additionally, move dso__swap_init() so we can use it.
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.c | 240 +++++++++++++++++++++++++++++++----------------
1 file changed, 161 insertions(+), 79 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 662d1a2..00a8226 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -993,6 +993,144 @@ static Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
return sec;
}
+static int dso__swap_init(struct dso *dso, unsigned char eidata)
+{
+ static unsigned int const endian = 1;
+
+ dso->needs_swap = DSO_SWAP__NO;
+
+ switch (eidata) {
+ case ELFDATA2LSB:
+ /* We are big endian, DSO is little endian. */
+ if (*(unsigned char const *)&endian != 1)
+ dso->needs_swap = DSO_SWAP__YES;
+ break;
+
+ case ELFDATA2MSB:
+ /* We are little endian, DSO is big endian. */
+ if (*(unsigned char const *)&endian != 0)
+ dso->needs_swap = DSO_SWAP__YES;
+ break;
+
+ default:
+ pr_err("unrecognized DSO data encoding %d\n", eidata);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+struct symsrc {
+ 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;
+
+ char *name;
+ int fd;
+ enum dso_binary_type type;
+
+ bool adjust_symbols;
+};
+
+static void symsrc__destroy(struct symsrc *ss)
+{
+ free(ss->name);
+ elf_end(ss->elf);
+ close(ss->fd);
+}
+
+static int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
+ enum dso_binary_type type)
+{
+ int err = -1;
+ GElf_Ehdr ehdr;
+ Elf *elf;
+ 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) {
+ pr_debug("%s: cannot read %s ELF file.\n", __func__, name);
+ goto out_close;
+ }
+
+ if (gelf_getehdr(elf, &ehdr) == NULL) {
+ pr_debug("%s: cannot get elf header.\n", __func__);
+ goto out_elf_end;
+ }
+
+ if (dso__swap_init(dso, ehdr.e_ident[EI_DATA]))
+ goto out_elf_end;
+
+ /* Always reject images with a mismatched build-id: */
+ if (dso->has_build_id) {
+ u8 build_id[BUILD_ID_SIZE];
+
+ if (elf_read_build_id(elf, build_id, BUILD_ID_SIZE) < 0)
+ goto out_elf_end;
+
+ if (!dso__build_id_equal(dso, build_id))
+ goto out_elf_end;
+ }
+
+ 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;
+}
+
#define elf_section__for_each_rel(reldata, pos, pos_mem, idx, nr_entries) \
for (idx = 0, pos = gelf_getrel(reldata, 0, &pos_mem); \
idx < nr_entries; \
@@ -1192,35 +1330,8 @@ static size_t elf_addr_to_index(Elf *elf, GElf_Addr addr)
return -1;
}
-static int dso__swap_init(struct dso *dso, unsigned char eidata)
-{
- static unsigned int const endian = 1;
-
- dso->needs_swap = DSO_SWAP__NO;
-
- switch (eidata) {
- case ELFDATA2LSB:
- /* We are big endian, DSO is little endian. */
- if (*(unsigned char const *)&endian != 1)
- dso->needs_swap = DSO_SWAP__YES;
- break;
-
- case ELFDATA2MSB:
- /* We are little endian, DSO is big endian. */
- if (*(unsigned char const *)&endian != 0)
- dso->needs_swap = DSO_SWAP__YES;
- break;
-
- default:
- pr_err("unrecognized DSO data encoding %d\n", eidata);
- return -EINVAL;
- }
-
- return 0;
-}
-
-static int dso__load_sym(struct dso *dso, struct map *map, const char *name,
- int fd, symbol_filter_t filter, int kmodule,
+static 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;
@@ -1239,44 +1350,24 @@ static int dso__load_sym(struct dso *dso, struct map *map, const char *name,
int nr = 0;
size_t opdidx = 0;
- elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
- if (elf == NULL) {
- pr_debug("%s: cannot read %s ELF file.\n", __func__, name);
- goto out_close;
- }
+ elf = ss->elf;
+ ehdr = ss->ehdr;
+ sec = ss->symtab;
+ shdr = ss->symshdr;
- if (gelf_getehdr(elf, &ehdr) == NULL) {
- pr_debug("%s: cannot get elf header.\n", __func__);
- goto out_elf_end;
- }
-
- if (dso__swap_init(dso, ehdr.e_ident[EI_DATA]))
- goto out_elf_end;
-
- /* Always reject images with a mismatched build-id: */
- if (dso->has_build_id) {
- u8 build_id[BUILD_ID_SIZE];
-
- if (elf_read_build_id(elf, build_id, BUILD_ID_SIZE) < 0)
- goto out_elf_end;
-
- if (!dso__build_id_equal(dso, build_id))
- goto out_elf_end;
- }
-
- sec = elf_section_by_name(elf, &ehdr, &shdr, ".symtab", NULL);
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);
@@ -1303,14 +1394,7 @@ static int dso__load_sym(struct dso *dso, struct map *map, const char *name,
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);
@@ -1466,8 +1550,6 @@ new_symbol:
}
err = nr;
out_elf_end:
- elf_end(elf);
-out_close:
return err;
}
@@ -1811,7 +1893,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 *) "";
@@ -1871,13 +1953,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
@@ -2163,22 +2244,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);
--
1.7.11.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 12/16] perf symbol: set symtab_type in dso__load_sym
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (10 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 11/16] perf symbol: introduce symsrc structure Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 13/16] perf symbol: switch dso__synthesize_plt_symbols() to use symsrc Cody P Schafer
` (3 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 00a8226..61b9cdb 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1350,6 +1350,8 @@ static 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;
@@ -1946,14 +1948,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,
@@ -2247,16 +2249,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] 20+ messages in thread* [PATCH 13/16] perf symbol: switch dso__synthesize_plt_symbols() to use symsrc
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (11 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 12/16] perf symbol: set symtab_type in dso__load_sym Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 14/16] perf symbol: factor want_symtab out of dso__load_sym() Cody P Schafer
` (2 subsequent siblings)
15 siblings, 0 replies; 20+ 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
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.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 61b9cdb..f8fbde2 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1149,7 +1149,7 @@ out_close:
* have the PLT data stripped out (shdr_rel_plt.sh_type == SHT_NOBITS).
*/
static int
-dso__synthesize_plt_symbols(struct dso *dso, char *name, struct map *map,
+dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss, struct map *map,
symbol_filter_t filter)
{
uint32_t nr_rel_entries, idx;
@@ -1164,21 +1164,15 @@ 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;
@@ -1274,13 +1268,8 @@ 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;
@@ -1960,21 +1949,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;
}
}
--
1.7.11.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 14/16] perf symbol: factor want_symtab out of dso__load_sym()
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (12 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 13/16] perf symbol: switch dso__synthesize_plt_symbols() to use symsrc Cody P Schafer
@ 2012-08-09 22:18 ` 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
2012-08-09 22:18 ` [PATCH 16/16] perf symbol: use both runtime and debug images Cody P Schafer
15 siblings, 0 replies; 20+ 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
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.c | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index f8fbde2..83d2276 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1320,8 +1320,7 @@ static size_t elf_addr_to_index(Elf *elf, GElf_Addr addr)
}
static 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;
@@ -1346,16 +1345,6 @@ static int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *ss,
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;
@@ -1947,8 +1936,15 @@ 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 && !ss.symtab) {
+ symsrc__destroy(&ss);
+ continue;
+ } else if (!want_symtab) {
+ ss.symtab = ss.dynsym;
+ ss.symshdr = ss.dynshdr;
+ }
+
+ ret = dso__load_sym(dso, map, &ss, filter, 0);
/*
* Some people seem to have debuginfo files _WITHOUT_ debug
@@ -2253,7 +2249,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) {
--
1.7.11.3
^ permalink raw reply related [flat|nested] 20+ 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
` (13 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 14/16] perf symbol: factor want_symtab out of dso__load_sym() Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
2012-08-09 22:18 ` [PATCH 16/16] perf symbol: use both runtime and debug images Cody P Schafer
15 siblings, 0 replies; 20+ 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] 20+ messages in thread* [PATCH 16/16] perf symbol: use both runtime and debug images
2012-08-09 22:18 [PATCH 0/16] perf: various symbol resolution fixes, including .opd section use Cody P Schafer
` (14 preceding siblings ...)
2012-08-09 22:18 ` [PATCH 15/16] perf symbol: convert dso__load_syms to take 2 symsrc's Cody P Schafer
@ 2012-08-09 22:18 ` Cody P Schafer
15 siblings, 0 replies; 20+ 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
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 exsists, 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.c | 82 +++++++++++++++++++++++++++---------------------
1 file changed, 46 insertions(+), 36 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index e740dc1..cdb723a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1874,11 +1874,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);
@@ -1920,12 +1921,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];
@@ -1934,48 +1935,57 @@ 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 && !ss.symtab) {
- symsrc__destroy(&ss);
- continue;
- } else if (!want_symtab) {
- ss.symtab = ss.dynsym;
- ss.symshdr = ss.dynshdr;
+ if (!syms_ss && ss->symtab) {
+ 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 && (ss->dynsym || ss->opdsec)) {
+ 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;
+ syms_ss->symtab = syms_ss->dynsym;
+ syms_ss->symshdr = syms_ss->dynshdr;
+ }
+
+ /* 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;
--
1.7.11.3
^ permalink raw reply related [flat|nested] 20+ messages in thread