Add support for remote unwind

There is no need to check for DWARF unwinding support when using the 'dwarf' callchain record method, as this will only ask the kernel to collect stack dumps for later DWARF CFI processing, which can be done in another machine, where the support for DWARF unwinding need to be present.

Signed-off-by He Kuang <[email protected]>

Acked-by Jiri Olsa <[email protected]>

---
 tools/perf/util/util.c | 2 --
 1 file changed, 2 deletions(-)

tools/perf/util/util.c +0 -2

index b7766c5..e5ebfd4 100644
@@ -471,7 +471,6 @@ int parse_callchain_record(const char *arg, struct callchain_param *param)
 				       "needed for --call-graph fp\n");
 			break;
 
-#ifdef HAVE_DWARF_UNWIND_SUPPORT
 		/* Dwarf style */
 		} else if (!strncmp(name, "dwarf", sizeof("dwarf"))) {
 			const unsigned long default_stack_dump_size = 8192;

@@ -487,7 +486,6 @@ int parse_callchain_record(const char *arg, struct callchain_param *param)
 				ret = get_stack_size(tok, &size);
 				param->dump_size = size;
 			}
-#endif /* HAVE_DWARF_UNWIND_SUPPORT */
 		} else if (!strncmp(name, "lbr", sizeof("lbr"))) {
 			if (!strtok_r(NULL, ",", &saveptr)) {
 				param->record_mode = CALLCHAIN_LBR;

Signature · 1 line

1.8.5.2

When unwinding callchains on a different machine, vdso info should be provided so the unwind process won't be interrupted if address falls into vdso region.

Currently, perf does try to read vdso binary in '.debug' folder, but the filename of the vdso file is generated randomly based on VDSO__TEMP_FILE_NAME template, such a filename is not reliable and users need a way to provide the path of their own vdso binary file.

Signed-off-by He Kuang <[email protected]>

---
 tools/perf/builtin-script.c | 2 ++
 tools/perf/util/dso.c       | 7 +++++++
 tools/perf/util/dso.h       | 1 +
 tools/perf/util/symbol.c    | 1 +
 tools/perf/util/symbol.h    | 1 +
 5 files changed, 12 insertions(+)

tools/perf/builtin-script.c +2 -0

index 8f6ab2a..c88b547 100644
@@ -2001,6 +2001,8 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "file", "vmlinux pathname"),
 	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
 		   "file", "kallsyms pathname"),
+	OPT_STRING(0, "vdso", &symbol_conf.vdso_name,
+		   "file", "vdso pathname"),
 	OPT_BOOLEAN('G', "hide-call-graph", &no_callchain,
 		    "When printing symbols do not display call chain"),
 	OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",

tools/perf/util/dso.c +7 -0

index 8e639543..6ed1cce 100644
@@ -13,6 +13,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__VDSO]				= 'D',
 		[DSO_BINARY_TYPE__JAVA_JIT]			= 'j',
 		[DSO_BINARY_TYPE__DEBUGLINK]			= 'l',
 		[DSO_BINARY_TYPE__BUILD_ID_CACHE]		= 'B',

@@ -113,6 +114,11 @@ int dso__read_binary_type_filename(const struct dso *dso,
 			 build_id_hex, build_id_hex + 2);
 		break;
 
+	case DSO_BINARY_TYPE__VDSO:
+	{
+		snprintf(filename, size, "%s", symbol_conf.vdso_name);
+		break;
+	}
 	case DSO_BINARY_TYPE__VMLINUX:
 	case DSO_BINARY_TYPE__GUEST_VMLINUX:
 	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:

@@ -487,6 +493,7 @@ static void try_to_open_dso(struct dso *dso, struct machine *machine)
 	enum dso_binary_type binary_type_data[] = {
 		DSO_BINARY_TYPE__BUILD_ID_CACHE,
 		DSO_BINARY_TYPE__SYSTEM_PATH_DSO,
+		DSO_BINARY_TYPE__VDSO,
 		DSO_BINARY_TYPE__NOT_FOUND,
 	};
 	int i = 0;

tools/perf/util/dso.h +1 -0

index 0953280..05fac98 100644
@@ -30,6 +30,7 @@ enum dso_binary_type {
 	DSO_BINARY_TYPE__KCORE,
 	DSO_BINARY_TYPE__GUEST_KCORE,
 	DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
+	DSO_BINARY_TYPE__VDSO,
 	DSO_BINARY_TYPE__NOT_FOUND,
 };
 

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

index e7588dc..4630751 100644
@@ -1363,6 +1363,7 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
 	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
 	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
 	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
+	case DSO_BINARY_TYPE__VDSO:
 		return !kmod && dso->kernel == DSO_TYPE_USER;
 
 	case DSO_BINARY_TYPE__KALLSYMS:

tools/perf/util/symbol.h +1 -0

index c8b7544..4e6910e 100644
@@ -114,6 +114,7 @@ struct symbol_conf {
 			report_hierarchy;
 	const char	*vmlinux_name,
 			*kallsyms_name,
+			*vdso_name,
 			*source_prefix,
 			*field_sep;
 	const char	*default_guest_vmlinux_name,

Signature · 1 line

1.8.5.2

Em Tue, May 10, 2016 at 07:40:30AM +0000, He Kuang escreveu:

Quoted 8 lines

When unwinding callchains on a different machine, vdso info should be provided so the unwind process won't be interrupted if address falls into vdso region.

Currently, perf does try to read vdso binary in '.debug' folder, but the filename of the vdso file is generated randomly based on VDSO__TEMP_FILE_NAME template, such a filename is not reliable and users need a way to provide the path of their own vdso binary file.

But this becomes one more burden to the user :-\ Why not calculate a build-id from the VDSO file contents, copy it to ~/.debug/.build-id/ab/cdef01020304 and insert that into the perf.data file so that at analysis time we do all this automatically?

We can of course have a way to manually provide it, but having it as the main way for users doesn't seem a good interface. Am I missing something?

- Arnaldo

Quoted 93 lines

Signed-off-by He Kuang <[email protected]>

---
  tools/perf/builtin-script.c | 2 ++
  tools/perf/util/dso.c       | 7 +++++++
  tools/perf/util/dso.h       | 1 +
  tools/perf/util/symbol.c    | 1 +
  tools/perf/util/symbol.h    | 1 +
  5 files changed, 12 insertions(+)

tools/perf/builtin-script.c +2 -0

index 8f6ab2a..c88b547 100644
@@ -2001,6 +2001,8 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
  		   "file", "vmlinux pathname"),
  	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,
  		   "file", "kallsyms pathname"),
+	OPT_STRING(0, "vdso", &symbol_conf.vdso_name,
+		   "file", "vdso pathname"),
  	OPT_BOOLEAN('G', "hide-call-graph", &no_callchain,
  		    "When printing symbols do not display call chain"),
  	OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",

tools/perf/util/dso.c +7 -0

index 8e639543..6ed1cce 100644
@@ -13,6 +13,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__VDSO]				= 'D',
  		[DSO_BINARY_TYPE__JAVA_JIT]			= 'j',
  		[DSO_BINARY_TYPE__DEBUGLINK]			= 'l',
  		[DSO_BINARY_TYPE__BUILD_ID_CACHE]		= 'B',

@@ -113,6 +114,11 @@ int dso__read_binary_type_filename(const struct dso *dso,
  			 build_id_hex, build_id_hex + 2);
  		break;
 
+	case DSO_BINARY_TYPE__VDSO:
+	{
+		snprintf(filename, size, "%s", symbol_conf.vdso_name);
+		break;
+	}
  	case DSO_BINARY_TYPE__VMLINUX:
  	case DSO_BINARY_TYPE__GUEST_VMLINUX:
  	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:

@@ -487,6 +493,7 @@ static void try_to_open_dso(struct dso *dso, struct machine *machine)
  	enum dso_binary_type binary_type_data[] = {
  		DSO_BINARY_TYPE__BUILD_ID_CACHE,
  		DSO_BINARY_TYPE__SYSTEM_PATH_DSO,
+		DSO_BINARY_TYPE__VDSO,
  		DSO_BINARY_TYPE__NOT_FOUND,
  	};
  	int i = 0;

tools/perf/util/dso.h +1 -0

index 0953280..05fac98 100644
@@ -30,6 +30,7 @@ enum dso_binary_type {
  	DSO_BINARY_TYPE__KCORE,
  	DSO_BINARY_TYPE__GUEST_KCORE,
  	DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
+	DSO_BINARY_TYPE__VDSO,
  	DSO_BINARY_TYPE__NOT_FOUND,
  };
 

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

index e7588dc..4630751 100644
@@ -1363,6 +1363,7 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
  	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
  	case DSO_BINARY_TYPE__BUILDID_DEBUGINFO:
  	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
+	case DSO_BINARY_TYPE__VDSO:
  		return !kmod && dso->kernel == DSO_TYPE_USER;
 
  	case DSO_BINARY_TYPE__KALLSYMS:

tools/perf/util/symbol.h +1 -0

index c8b7544..4e6910e 100644
@@ -114,6 +114,7 @@ struct symbol_conf {
  			report_hierarchy;
  	const char	*vmlinux_name,
  			*kallsyms_name,
+			*vdso_name,
  			*source_prefix,
  			*field_sep;
  	const char	*default_guest_vmlinux_name,

Signature · 1 line

1.8.5.2

Currently only test for local libunwind. We should check all supported platforms so we can use them to parse perf.data with callchain info on different machines.

Signed-off-by He Kuang <[email protected]>

Quoted 5 lines

Currently only test for local libunwind. We should check all supported platforms so we can use them to parse perf.data with callchain info on different machines.

Signed-off-by He Kuang <[email protected]>

Acked-by Jiri Olsa <[email protected]>

Debug-frame for remote platforms is not related to the host platform, so we should test each platform separately.

Signed-off-by He Kuang <[email protected]>

Quoted 4 lines

Debug-frame for remote platforms is not related to the host platform, so we should test each platform separately.

Signed-off-by He Kuang <[email protected]>

Acked-by Jiri Olsa <[email protected]>

32-bit programs can be run on 64-bit machines, so we should choose unwind methods according to 'thread->map' instead of the host architecture.

This patch adds methods to test whether a dso is 64-bit or 32-bit by the class info in elf.

Signed-off-by He Kuang <[email protected]>

---
 tools/perf/util/symbol-elf.c | 16 +++++++++++++++
 tools/perf/util/symbol.c     | 49 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h     |  2 ++
 3 files changed, 67 insertions(+)

tools/perf/util/symbol-elf.c +16 -0

index 3f9d679..9f290b9 100644
@@ -636,6 +636,22 @@ bool __weak elf__needs_adjust_symbols(GElf_Ehdr ehdr)
 	return ehdr.e_type == ET_EXEC || ehdr.e_type == ET_REL;
 }
 
+int elf_is_64_bit(char *name)
+{
+	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)
+		return -1;
+
+	return (gelf_getclass(elf) == ELFCLASS64);
+}
+
 int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 		 enum dso_binary_type type)
 {

tools/perf/util/symbol.c +49 -0

index 4630751..592bf8c 100644
@@ -1395,6 +1395,55 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
 	}
 }
 
+int dso_is_64_bit(struct dso *dso, struct map *map)
+{
+	char *name;
+	u_int i;
+	bool kmod;
+	char *root_dir = (char *) "";
+	struct machine *machine;
+
+	if (map->groups && map->groups->machine)
+		machine = map->groups->machine;
+	else
+		machine = NULL;
+
+	if (machine)
+		root_dir = machine->root_dir;
+
+	name = malloc(PATH_MAX);
+	if (!name)
+		return -1;
+
+	kmod = dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE ||
+		dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP ||
+		dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE ||
+		dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
+
+	/*
+	 * Iterate over candidate debug images.
+	 * Keep track of "interesting" ones (those which have a symtab, dynsym,
+	 * and/or opd section) for processing.
+	 */
+	for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
+		enum dso_binary_type symtab_type = binary_type_symtab[i];
+
+		if (!dso__is_compatible_symtab_type(dso, kmod, symtab_type))
+			continue;
+
+		if (dso__read_binary_type_filename(dso, symtab_type,
+						   root_dir, name, PATH_MAX))
+			continue;
+
+		if (!is_regular_file(name))
+			continue;
+
+		return elf_is_64_bit(name);
+	}
+
+	return -1;
+}
+
 int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 {
 	char *name;

tools/perf/util/symbol.h +2 -0

index 4e6910e..d33fbf4 100644
@@ -308,6 +308,8 @@ int setup_list(struct strlist **list, const char *list_str,
 	       const char *list_name);
 int setup_intlist(struct intlist **list, const char *list_str,
 		  const char *list_name);
+int elf_is_64_bit(char *name);
+int dso_is_64_bit(struct dso *dso, struct map *map);
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);

Quoted 6 lines · He Kuang

32-bit programs can be run on 64-bit machines, so we should choose unwind methods according to 'thread->map' instead of the host architecture.

This patch adds methods to test whether a dso is 64-bit or 32-bit by the class info in elf.

What about using dso->is_64_bit set by dso__load_sym() ?

Quoted 109 lines

Signed-off-by He Kuang <[email protected]>

---
  tools/perf/util/symbol-elf.c | 16 +++++++++++++++
  tools/perf/util/symbol.c     | 49 ++++++++++++++++++++++++++++++++++++++++++++
  tools/perf/util/symbol.h     |  2 ++
  3 files changed, 67 insertions(+)

tools/perf/util/symbol-elf.c +16 -0

index 3f9d679..9f290b9 100644
@@ -636,6 +636,22 @@ bool __weak elf__needs_adjust_symbols(GElf_Ehdr ehdr)
  	return ehdr.e_type == ET_EXEC || ehdr.e_type == ET_REL;
  }
 
+int elf_is_64_bit(char *name)
+{
+	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)
+		return -1;
+
+	return (gelf_getclass(elf) == ELFCLASS64);
+}
+
  int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
  		 enum dso_binary_type type)
  {

tools/perf/util/symbol.c +49 -0

index 4630751..592bf8c 100644
@@ -1395,6 +1395,55 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
  	}
  }
 
+int dso_is_64_bit(struct dso *dso, struct map *map)
+{
+	char *name;
+	u_int i;
+	bool kmod;
+	char *root_dir = (char *) "";
+	struct machine *machine;
+
+	if (map->groups && map->groups->machine)
+		machine = map->groups->machine;
+	else
+		machine = NULL;
+
+	if (machine)
+		root_dir = machine->root_dir;
+
+	name = malloc(PATH_MAX);
+	if (!name)
+		return -1;
+
+	kmod = dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE ||
+		dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP ||
+		dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE ||
+		dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
+
+	/*
+	 * Iterate over candidate debug images.
+	 * Keep track of "interesting" ones (those which have a symtab, dynsym,
+	 * and/or opd section) for processing.
+	 */
+	for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
+		enum dso_binary_type symtab_type = binary_type_symtab[i];
+
+		if (!dso__is_compatible_symtab_type(dso, kmod, symtab_type))
+			continue;
+
+		if (dso__read_binary_type_filename(dso, symtab_type,
+						   root_dir, name, PATH_MAX))
+			continue;
+
+		if (!is_regular_file(name))
+			continue;
+
+		return elf_is_64_bit(name);
+	}
+
+	return -1;
+}
+
  int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
  {
  	char *name;

tools/perf/util/symbol.h +2 -0

index 4e6910e..d33fbf4 100644
@@ -308,6 +308,8 @@ int setup_list(struct strlist **list, const char *list_str,
  	       const char *list_name);
  int setup_intlist(struct intlist **list, const char *list_str,
  		  const char *list_name);
+int elf_is_64_bit(char *name);
+int dso_is_64_bit(struct dso *dso, struct map *map);
 
  #ifdef HAVE_LIBELF_SUPPORT
  bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);

hi

Quoted 5 lines · Adrian Hunter

Quoted 4 lines · He Kuang

32-bit programs can be run on 64-bit machines, so we should choose unwind methods according to 'thread->map' instead of the host architecture.

Quoted 3 lines

Quoted 2 lines

This patch adds methods to test whether a dso is 64-bit or 32-bit by the class info in elf.

What about using dso->is_64_bit set by dso__load_sym() ?

I've noticed this variable, but it's value is not as its name said:

util/dso.c: 1067 dso->is_64_bit = (sizeof(void *) == 8);

This is only related to the host architecture.

A closer one is 'is_64_bit' in 'struct symsrc', but the value is assigned after dso
loaded. So I think we should provide individual methods to get that value.

Thanks.

Quoted 110 lines

Quoted 109 lines

Signed-off-by He Kuang <[email protected]>

---
  tools/perf/util/symbol-elf.c | 16 +++++++++++++++
  tools/perf/util/symbol.c     | 49
++++++++++++++++++++++++++++++++++++++++++++
  tools/perf/util/symbol.h     |  2 ++
  3 files changed, 67 insertions(+)

tools/perf/util/symbol-elf.c +15 -0

index 3f9d679..9f290b9 100644
@@ -636,6 +636,22 @@ bool __weak elf__needs_adjust_symbols(GElf_Ehdr ehdr)
      return ehdr.e_type == ET_EXEC || ehdr.e_type == ET_REL;
  }
  +int elf_is_64_bit(char *name)
+{
+    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)
+        return -1;
+
+    return (gelf_getclass(elf) == ELFCLASS64);
+}
+
  int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
           enum dso_binary_type type)
  {

tools/perf/util/symbol.c +48 -0

index 4630751..592bf8c 100644
@@ -1395,6 +1395,55 @@ static bool dso__is_compatible_symtab_type(struct
 dso *dso, bool kmod,
      }
  }
  +int dso_is_64_bit(struct dso *dso, struct map *map)
+{
+    char *name;
+    u_int i;
+    bool kmod;
+    char *root_dir = (char *) "";
+    struct machine *machine;
+
+    if (map->groups && map->groups->machine)
+        machine = map->groups->machine;
+    else
+        machine = NULL;
+
+    if (machine)
+        root_dir = machine->root_dir;
+
+    name = malloc(PATH_MAX);
+    if (!name)
+        return -1;
+
+    kmod = dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE ||
+        dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP ||
+        dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE ||
+        dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
+
+    /*
+     * Iterate over candidate debug images.
+     * Keep track of "interesting" ones (those which have a symtab, dynsym,
+     * and/or opd section) for processing.
+     */
+    for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
+        enum dso_binary_type symtab_type = binary_type_symtab[i];
+
+        if (!dso__is_compatible_symtab_type(dso, kmod, symtab_type))
+            continue;
+
+        if (dso__read_binary_type_filename(dso, symtab_type,
+                           root_dir, name, PATH_MAX))
+            continue;
+
+        if (!is_regular_file(name))
+            continue;
+
+        return elf_is_64_bit(name);
+    }
+
+    return -1;
+}
+
  int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
  {
      char *name;

tools/perf/util/symbol.h +2 -0

index 4e6910e..d33fbf4 100644
@@ -308,6 +308,8 @@ int setup_list(struct strlist **list, const char
 *list_str,
             const char *list_name);
  int setup_intlist(struct intlist **list, const char *list_str,
            const char *list_name);
+int elf_is_64_bit(char *name);
+int dso_is_64_bit(struct dso *dso, struct map *map);
    #ifdef HAVE_LIBELF_SUPPORT
  bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);

Quoted 8 lines · Hekuang

hi

Quoted 5 lines · Adrian Hunter

Quoted 4 lines · He Kuang

32-bit programs can be run on 64-bit machines, so we should choose unwind methods according to 'thread->map' instead of the host architecture.

Quoted 6 lines

Quoted 3 lines

Quoted 2 lines

This patch adds methods to test whether a dso is 64-bit or 32-bit by the class info in elf.

What about using dso->is_64_bit set by dso__load_sym() ?

I've noticed this variable, but it's value is not as its name said:

util/dso.c: 1067 dso->is_64_bit = (sizeof(void *) == 8);

That is just initialization i.e. before we know what it is we assume it is the same as the host.

Quoted 5 lines

This is only related to the host architecture.

A closer one is 'is_64_bit' in 'struct symsrc', but the value is assigned after dso
loaded. So I think we should provide individual methods to get that value.

Are you saying you don't load dsos? Or that is_64_bit is set incorrectly?

Quoted 112 lines

Thanks.

Quoted 108 lines

Quoted 108 lines

Signed-off-by He Kuang <[email protected]>

---
   tools/perf/util/symbol-elf.c | 16 +++++++++++++++
   tools/perf/util/symbol.c     | 49
++++++++++++++++++++++++++++++++++++++++++++
   tools/perf/util/symbol.h     |  2 ++
   3 files changed, 67 insertions(+)

tools/perf/util/symbol-elf.c +15 -0

index 3f9d679..9f290b9 100644
@@ -636,6 +636,22 @@ bool __weak elf__needs_adjust_symbols(GElf_Ehdr ehdr)
       return ehdr.e_type == ET_EXEC || ehdr.e_type == ET_REL;
   }
   +int elf_is_64_bit(char *name)
+{
+    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)
+        return -1;
+
+    return (gelf_getclass(elf) == ELFCLASS64);
+}
+
   int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
            enum dso_binary_type type)
   {

tools/perf/util/symbol.c +48 -0

index 4630751..592bf8c 100644
@@ -1395,6 +1395,55 @@ static bool dso__is_compatible_symtab_type(struct
 dso *dso, bool kmod,
       }
   }
   +int dso_is_64_bit(struct dso *dso, struct map *map)
+{
+    char *name;
+    u_int i;
+    bool kmod;
+    char *root_dir = (char *) "";
+    struct machine *machine;
+
+    if (map->groups && map->groups->machine)
+        machine = map->groups->machine;
+    else
+        machine = NULL;
+
+    if (machine)
+        root_dir = machine->root_dir;
+
+    name = malloc(PATH_MAX);
+    if (!name)
+        return -1;
+
+    kmod = dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE ||
+        dso->symtab_type == DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP ||
+        dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE ||
+        dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
+
+    /*
+     * Iterate over candidate debug images.
+     * Keep track of "interesting" ones (those which have a symtab, dynsym,
+     * and/or opd section) for processing.
+     */
+    for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) {
+        enum dso_binary_type symtab_type = binary_type_symtab[i];
+
+        if (!dso__is_compatible_symtab_type(dso, kmod, symtab_type))
+            continue;
+
+        if (dso__read_binary_type_filename(dso, symtab_type,
+                           root_dir, name, PATH_MAX))
+            continue;
+
+        if (!is_regular_file(name))
+            continue;
+
+        return elf_is_64_bit(name);
+    }
+
+    return -1;
+}
+
   int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
   {
       char *name;

tools/perf/util/symbol.h +2 -0

index 4e6910e..d33fbf4 100644
@@ -308,6 +308,8 @@ int setup_list(struct strlist **list, const char
 *list_str,
              const char *list_name);
   int setup_intlist(struct intlist **list, const char *list_str,
             const char *list_name);
+int elf_is_64_bit(char *name);
+int dso_is_64_bit(struct dso *dso, struct map *map);
     #ifdef HAVE_LIBELF_SUPPORT
   bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);

Quoted 18 lines · Adrian Hunter

Quoted 8 lines · Hekuang

hi

Quoted 5 lines · Adrian Hunter

Quoted 4 lines · He Kuang

32-bit programs can be run on 64-bit machines, so we should choose unwind methods according to 'thread->map' instead of the host architecture.

Quoted 6 lines

Quoted 3 lines

Quoted 2 lines

This patch adds methods to test whether a dso is 64-bit or 32-bit by the class info in elf.

What about using dso->is_64_bit set by dso__load_sym() ?

I've noticed this variable, but it's value is not as its name said:

util/dso.c: 1067 dso->is_64_bit = (sizeof(void *) == 8);

That is just initialization i.e. before we know what it is we assume it is the same as the host.

Quoted 7 lines

Quoted 5 lines

This is only related to the host architecture.

A closer one is 'is_64_bit' in 'struct symsrc', but the value is assigned after dso
loaded. So I think we should provide individual methods to get that value.

Are you saying you don't load dsos? Or that is_64_bit is set incorrectly?

Yes, I know it's the inital value, but the correct value is assigned in function dso__load_sym(), and have a look at the call stack(gdb):

#0  dso__load_sym
#1  in dso__load
#2  in map__load
#3  in map__find_symbol
#4  in thread__find_addr_location
#5  in entry
#6  in get_entries
#7  in _Ux86__unwind__get_entries
#8  in thread__resolve_callchain

I think we should choose the right unwind method before dso__load_sym(). i.e. line#7, which is called before dso__load_sym().

I'm not very familiar with this, what's your opinion?

Quoted 41 lines · Hekuang

Quoted 18 lines · Adrian Hunter

Quoted 8 lines · Hekuang

hi

Quoted 5 lines · Adrian Hunter

Quoted 4 lines · He Kuang

32-bit programs can be run on 64-bit machines, so we should choose unwind methods according to 'thread->map' instead of the host architecture.

Quoted 6 lines

Quoted 3 lines

Quoted 2 lines

This patch adds methods to test whether a dso is 64-bit or 32-bit by the class info in elf.

What about using dso->is_64_bit set by dso__load_sym() ?

I've noticed this variable, but it's value is not as its name said:

util/dso.c: 1067 dso->is_64_bit = (sizeof(void *) == 8);

That is just initialization i.e. before we know what it is we assume it is the same as the host.

Quoted 7 lines

Quoted 5 lines

This is only related to the host architecture.

A closer one is 'is_64_bit' in 'struct symsrc', but the value is assigned after dso
loaded. So I think we should provide individual methods to get that value.

Are you saying you don't load dsos? Or that is_64_bit is set incorrectly?

Yes, I know it's the inital value, but the correct value is assigned in function dso__load_sym(), and have a look at the call stack(gdb):

#0  dso__load_sym
#1  in dso__load
#2  in map__load
#3  in map__find_symbol
#4  in thread__find_addr_location
#5  in entry
#6  in get_entries
#7  in _Ux86__unwind__get_entries
#8  in thread__resolve_callchain

Quoted 4 lines

I think we should choose the right unwind method before dso__load_sym(). i.e. line#7, which is called before dso__load_sym().

I'm not very familiar with this, what's your opinion?

Have you considered calling map__load() instead of dso_is_64_bit()