From b843f62ad942f3879c2d7acd27b44f931c609685 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 Apr 2017 21:21:09 -0300 Subject: [PATCH] perf symbols: Accept symbols starting at address 0 That is the case of _text on s390, and we have some functions that return an address, using address zero to report problems, oops. This would lead the symbol loading routines to not use "_text" as the reference relocation symbol, or the first symbol for the kernel, but use instead "_stext", that is at the same address on x86_64 and others, but not on s390: [acme@localhost perf-4.11.0-rc6]$ head -15 /proc/kallsyms 0000000000000000 T _text 0000000000000418 t iplstart 0000000000000800 T start 000000000000080a t .base 000000000000082e t .sk8x8 0000000000000834 t .gotr 0000000000000842 t .cmd 0000000000000846 t .parm 000000000000084a t .lowcase 0000000000010000 T startup 0000000000010010 T startup_kdump 0000000000010214 t startup_kdump_relocated 0000000000011000 T startup_continue 00000000000112a0 T _ehead 0000000000100000 T _stext [acme@localhost perf-4.11.0-rc6]$ Which in turn would make 'perf test vmlinux' to fail because it wouldn't find the symbols before "_stext" in kallsyms. Fix it by using the return value only for errors and storing the address, when the symbol is successfully found, in a provided pointer arg. Before this patch: After: [acme@localhost perf-4.11.0-rc6]$ tools/perf/perf test -v 1 1: vmlinux symtab matches kallsyms : --- start --- test child forked, pid 40693 Looking at the vmlinux_path (8 entries long) Using /usr/lib/debug/lib/modules/3.10.0-654.el7.s390x/vmlinux for symbols ERR : 0: _text not on kallsyms ERR : 0x418: iplstart not on kallsyms ERR : 0x800: start not on kallsyms ERR : 0x80a: .base not on kallsyms ERR : 0x82e: .sk8x8 not on kallsyms ERR : 0x834: .gotr not on kallsyms ERR : 0x842: .cmd not on kallsyms ERR : 0x846: .parm not on kallsyms ERR : 0x84a: .lowcase not on kallsyms ERR : 0x10000: startup not on kallsyms ERR : 0x10010: startup_kdump not on kallsyms ERR : 0x10214: startup_kdump_relocated not on kallsyms ERR : 0x11000: startup_continue not on kallsyms ERR : 0x112a0: _ehead not on kallsyms test child finished with -1 ---- end ---- vmlinux symtab matches kallsyms: FAILED! [acme@localhost perf-4.11.0-rc6]$ After: [acme@localhost perf-4.11.0-rc6]$ tools/perf/perf test -v 1 1: vmlinux symtab matches kallsyms : --- start --- test child forked, pid 47160 test child finished with 0 ---- end ---- vmlinux symtab matches kallsyms: Ok [acme@localhost perf-4.11.0-rc6]$ Reported-by: Michael Petlan Cc: Adrian Hunter Cc: David Ahern Cc: Jiri Olsa Cc: Namhyung Kim Cc: Wang Nan Link: http://lkml.kernel.org/n/tip-9x9bwgd3btwdk1u51xie93fz@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-buildid-cache.c | 13 ++++++++----- tools/perf/util/event.c | 9 +++++---- tools/perf/util/event.h | 4 ++-- tools/perf/util/machine.c | 28 +++++++++++++++++----------- tools/perf/util/symbol.c | 11 +++++------ 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c index 64b44e81c771..9eba7f1add1f 100644 --- a/tools/perf/builtin-buildid-cache.c +++ b/tools/perf/builtin-buildid-cache.c @@ -49,19 +49,22 @@ static bool same_kallsyms_reloc(const char *from_dir, char *to_dir) char to[PATH_MAX]; const char *name; u64 addr1 = 0, addr2 = 0; - int i; + int i, err = -1; scnprintf(from, sizeof(from), "%s/kallsyms", from_dir); scnprintf(to, sizeof(to), "%s/kallsyms", to_dir); for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) { - addr1 = kallsyms__get_function_start(from, name); - if (addr1) + err = kallsyms__get_function_start(from, name, &addr1); + if (!err) break; } - if (name) - addr2 = kallsyms__get_function_start(to, name); + if (err) + return false; + + if (kallsyms__get_function_start(to, name, &addr2)) + return false; return addr1 == addr2; } diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index 437194fe0434..dc5c3bb69d73 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -768,15 +768,16 @@ static int find_symbol_cb(void *arg, const char *name, char type, return 1; } -u64 kallsyms__get_function_start(const char *kallsyms_filename, - const char *symbol_name) +int kallsyms__get_function_start(const char *kallsyms_filename, + const char *symbol_name, u64 *addr) { struct process_symbol_args args = { .name = symbol_name, }; if (kallsyms__parse(kallsyms_filename, &args, find_symbol_cb) <= 0) - return 0; + return -1; - return args.start; + *addr = args.start; + return 0; } int perf_event__synthesize_kernel_mmap(struct perf_tool *tool, diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h index cfbe32bd7413..27ac047490c3 100644 --- a/tools/perf/util/event.h +++ b/tools/perf/util/event.h @@ -675,8 +675,8 @@ size_t perf_event__fprintf_cpu_map(union perf_event *event, FILE *fp); size_t perf_event__fprintf_namespaces(union perf_event *event, FILE *fp); size_t perf_event__fprintf(union perf_event *event, FILE *fp); -u64 kallsyms__get_function_start(const char *kallsyms_filename, - const char *symbol_name); +int kallsyms__get_function_start(const char *kallsyms_filename, + const char *symbol_name, u64 *addr); void *cpu_map_data__alloc(struct cpu_map *map, size_t *size, u16 *type, int *max); void cpu_map_data__synthesize(struct cpu_map_data *data, struct cpu_map *map, diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 7a47f52ccfcc..d97e014c3df3 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -796,11 +796,11 @@ const char *ref_reloc_sym_names[] = {"_text", "_stext", NULL}; * Returns the name of the start symbol in *symbol_name. Pass in NULL as * symbol_name if it's not that important. */ -static u64 machine__get_running_kernel_start(struct machine *machine, - const char **symbol_name) +static int machine__get_running_kernel_start(struct machine *machine, + const char **symbol_name, u64 *start) { char filename[PATH_MAX]; - int i; + int i, err = -1; const char *name; u64 addr = 0; @@ -810,21 +810,28 @@ static u64 machine__get_running_kernel_start(struct machine *machine, return 0; for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) { - addr = kallsyms__get_function_start(filename, name); - if (addr) + err = kallsyms__get_function_start(filename, name, &addr); + if (!err) break; } + if (err) + return -1; + if (symbol_name) *symbol_name = name; - return addr; + *start = addr; + return 0; } int __machine__create_kernel_maps(struct machine *machine, struct dso *kernel) { int type; - u64 start = machine__get_running_kernel_start(machine, NULL); + u64 start = 0; + + if (machine__get_running_kernel_start(machine, NULL, &start)) + return -1; /* In case of renewal the kernel map, destroy previous one */ machine__destroy_kernel_maps(machine); @@ -1185,8 +1192,8 @@ static int machine__create_modules(struct machine *machine) int machine__create_kernel_maps(struct machine *machine) { struct dso *kernel = machine__get_kernel(machine); - const char *name; - u64 addr; + const char *name = NULL; + u64 addr = 0; int ret; if (kernel == NULL) @@ -1211,8 +1218,7 @@ int machine__create_kernel_maps(struct machine *machine) */ map_groups__fixup_end(&machine->kmaps); - addr = machine__get_running_kernel_start(machine, &name); - if (!addr) { + if (machine__get_running_kernel_start(machine, &name, &addr)) { } else if (maps__set_kallsyms_ref_reloc_sym(machine->vmlinux_maps, name, addr)) { machine__destroy_kernel_maps(machine); return -1; diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 2cb7665e9973..b349e8eda0e2 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -466,7 +466,7 @@ void dso__insert_symbol(struct dso *dso, enum map_type type, struct symbol *sym) struct symbol *dso__find_symbol(struct dso *dso, enum map_type type, u64 addr) { - if (dso->last_find_result[type].addr != addr) { + if (dso->last_find_result[type].addr != addr || dso->last_find_result[type].symbol == NULL) { dso->last_find_result[type].addr = addr; dso->last_find_result[type].symbol = symbols__find(&dso->symbols[type], addr); } @@ -1075,8 +1075,9 @@ static int validate_kcore_addresses(const char *kallsyms_filename, if (kmap->ref_reloc_sym && kmap->ref_reloc_sym->name) { u64 start; - start = kallsyms__get_function_start(kallsyms_filename, - kmap->ref_reloc_sym->name); + if (kallsyms__get_function_start(kallsyms_filename, + kmap->ref_reloc_sym->name, &start)) + return -ENOENT; if (start != kmap->ref_reloc_sym->addr) return -EINVAL; } @@ -1248,9 +1249,7 @@ static int kallsyms__delta(struct map *map, const char *filename, u64 *delta) if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->name) return 0; - addr = kallsyms__get_function_start(filename, - kmap->ref_reloc_sym->name); - if (!addr) + if (kallsyms__get_function_start(filename, kmap->ref_reloc_sym->name, &addr)) return -1; *delta = addr - kmap->ref_reloc_sym->addr;