[PATCH v3 0/7] Add support for remote unwind

[PATCH v3 0/7] Add support for remote unwind

[He Kuang]

v2 url:
  http://thread.gmane.org/gmane.linux.kernel/2218373

Currently, perf script uses host unwind methods(local unwind) to parse
perf.data callchain info regardless of the target architecture. So we
get wrong result and no promotion when do remote unwind on other
platforms/machines.

This patchset adds build tests for the supported platforms for remote
unwinding, and checks whether a dso is 32-bit or 64-bit according to
elf class info for each thread to let perf use the correct remote
unwind methods instead.

Only x86 and aarch64 is added in this patchset to show the work flow,
other platforms can be added easily.

We can see the right result for unwind info on different machines, for
example: perf.data recorded on i686 qemu with '-g' option and parsed
on x86_64 machine.

before this patchset:

  hello  1219 [001] 72190.667975: probe:sys_close: (c1169d60)
                  c1169d61 sys_close ([kernel.kallsyms])
                  c189c0d7 sysenter_past_esp ([kernel.kallsyms])
                  b777aba9 [unknown] ([vdso32])
  
after:
(Add vdso into buildid-cache first by 'perf buildid-cache -a' and
libraries are provided in symfs dir)

  hello  1219 [001] 72190.667975: probe:sys_close: (c1169d60)
                  c1169d61 sys_close ([kernel.kallsyms])
                  c189c0d7 sysenter_past_esp ([kernel.kallsyms])
                  b777aba9 __kernel_vsyscall ([vdso32])
                  b76971cc close (/lib/libc-2.22.so)
                   804842e fib (/tmp/hello)
                   804849d main (/tmp/hello)
                  b75d746e __libc_start_main (/lib/libc-2.22.so)
                   8048341 _start (/tmp/hello)

v3:

 - Remove --vdso option, store vdso buildid in perf.data and let perf
   fetch it automatically.
 - Use existing dso__type() function to test if dso is 32-bit or
   64-bit.

v2:

 - Explain the reason why we can omit dwarf judgement when recording
   in commit message.
 - Elaborate on why we need to add a custom vdso path option, and
   change the type name to DSO_BINARY_TYPE__VDSO.
 - Hide the build tests status for cross platform unwind.
 - Keep generic version of libunwind-debug-frame test.
 - Put 32/64-bit test functions into separate patch.
 - Extract unwind related functions to unwind-libunwind.c and add new
   file for common parts used by both local and remote unwind.
 - Eliminate most of the ifdefs in .c file.

Thanks.

He Kuang (7):
  perf tools: Set vdso name to vdso[64,32] depending on platform
  perf tools: Store vdso buildid unconditionally
  perf tools: Remove the logical that skip buildid cache if symfs is
    given
  perf tools: Promote proper messages for cross-platform unwind
  perf callchain: Add support for cross-platform unwind
  perf callchain: Support x86 target platform
  perf callchain: Support aarch64 cross-platform

 .../arch/arm64/include/libunwind/libunwind-arch.h  |  18 ++++
 tools/perf/arch/arm64/util/unwind-libunwind.c      |   5 +-
 .../arch/x86/include/libunwind/libunwind-arch.h    |  18 ++++
 tools/perf/arch/x86/util/unwind-libunwind.c        |  42 ++++++++
 tools/perf/config/Makefile                         |  35 ++++++-
 tools/perf/util/Build                              |  13 ++-
 tools/perf/util/build-id.c                         |   2 +-
 tools/perf/util/dso.c                              |   6 +-
 tools/perf/util/thread.c                           |   7 +-
 tools/perf/util/thread.h                           |  14 ++-
 tools/perf/util/unwind-libunwind.c                 |  49 +++++++--
 tools/perf/util/unwind-libunwind_common.c          | 114 +++++++++++++++++++++
 tools/perf/util/unwind.h                           |  48 +++++++--
 tools/perf/util/vdso.h                             |   7 +-
 14 files changed, 348 insertions(+), 30 deletions(-)
 create mode 100644 tools/perf/arch/arm64/include/libunwind/libunwind-arch.h
 create mode 100644 tools/perf/arch/x86/include/libunwind/libunwind-arch.h
 create mode 100644 tools/perf/util/unwind-libunwind_common.c

-- 
1.8.5.2

[PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform

[He Kuang]

This is a preparation for cross-platform vdso lookup.

There is a naming confusion about vdso name, vdso buildid generated by
a 32-bit machine stores it with the name 'vdso', but when processing
buildid on a 64-bit machine with the same 'perf.data', perf will
search for vdso named as 'vdso32' and get failed.

This patch uses different names when storing the buildid, i.e. vdso64
for 64-bit machine and vdso32 for 32-bit machine, and eliminates this
naming confusion.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/util/vdso.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/vdso.h b/tools/perf/util/vdso.h
index cdc4fab..45e9ef4 100644
--- a/tools/perf/util/vdso.h
+++ b/tools/perf/util/vdso.h
@@ -4,10 +4,15 @@
 #include <linux/types.h>
 #include <string.h>
 #include <stdbool.h>
+#include "util.h"
 
 #define VDSO__MAP_NAME "[vdso]"
 
-#define DSO__NAME_VDSO    "[vdso]"
+#if BITS_PER_LONG == 64
+#define DSO__NAME_VDSO    "[vdso64]"
+#else
+#define DSO__NAME_VDSO    "[vdso32]"
+#endif
 #define DSO__NAME_VDSO32  "[vdso32]"
 #define DSO__NAME_VDSOX32 "[vdsox32]"
 
-- 
1.8.5.2

Re: [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform

[Adrian Hunter]

On 12/05/16 11:43, He Kuang wrote:
> This is a preparation for cross-platform vdso lookup.
> 
> There is a naming confusion about vdso name, vdso buildid generated by
> a 32-bit machine stores it with the name 'vdso', but when processing
> buildid on a 64-bit machine with the same 'perf.data', perf will
> search for vdso named as 'vdso32' and get failed.
> 
> This patch uses different names when storing the buildid, i.e. vdso64
> for 64-bit machine and vdso32 for 32-bit machine, and eliminates this
> naming confusion.

That looks like it will break existing perf.data files because they will
have a different name recorded in the buildid section.

Also it doesn't look like it would work the other way around i.e. recording
on a 64-bit machine and processing on a 32-bit machine.

> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/util/vdso.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/vdso.h b/tools/perf/util/vdso.h
> index cdc4fab..45e9ef4 100644
> --- a/tools/perf/util/vdso.h
> +++ b/tools/perf/util/vdso.h
> @@ -4,10 +4,15 @@
>  #include <linux/types.h>
>  #include <string.h>
>  #include <stdbool.h>
> +#include "util.h"
>  
>  #define VDSO__MAP_NAME "[vdso]"
>  
> -#define DSO__NAME_VDSO    "[vdso]"
> +#if BITS_PER_LONG == 64
> +#define DSO__NAME_VDSO    "[vdso64]"
> +#else
> +#define DSO__NAME_VDSO    "[vdso32]"
> +#endif
>  #define DSO__NAME_VDSO32  "[vdso32]"
>  #define DSO__NAME_VDSOX32 "[vdsox32]"
>  
> 

[PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform

[He Kuang]

There's a problem in machine__findnew_vdso(), vdso buildid generated
by a 32-bit machine stores it with the name 'vdso', but when
processing buildid on a 64-bit machine with the same 'perf.data', perf
will search for vdso named as 'vdso32' and get failed.

This patch tries to find the exsiting dsos in machine->dsos by thread
dso_type. 64-bit thread tries to find vdso with name 'vdso', because
all 64-bit vdso is named as that. 32-bit thread first tries to find
vdso with name 'vdso32' if this thread was run on 64-bit machine, if
failed, then it tries 'vdso' which indicates that the thread was run
on 32-bit machine when recording.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
index 44d440d..99f4a3d 100644
--- a/tools/perf/util/vdso.c
+++ b/tools/perf/util/vdso.c
@@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
 	return dso;
 }
 
-#if BITS_PER_LONG == 64
-
 static enum dso_type machine__thread_dso_type(struct machine *machine,
 					      struct thread *thread)
 {
@@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
 	return dso_type;
 }
 
+#if BITS_PER_LONG == 64
+
 static int vdso__do_copy_compat(FILE *f, int fd)
 {
 	char buf[4096];
@@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
 
 #endif
 
+static struct dso *machine__find_vdso(struct machine *machine,
+				      struct thread *thread)
+{
+	struct dso *dso = NULL;
+	enum dso_type dso_type;
+
+	dso_type = machine__thread_dso_type(machine, thread);
+	switch (dso_type) {
+	case DSO__TYPE_32BIT:
+		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
+		if (!dso)
+			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
+					   true);
+		break;
+	case DSO__TYPE_X32BIT:
+		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
+		if (!dso)
+			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
+					   true);
+		break;
+	case DSO__TYPE_64BIT:
+	case DSO__TYPE_UNKNOWN:
+	default:
+		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
+		break;
+	}
+
+	return dso;
+}
+
 struct dso *machine__findnew_vdso(struct machine *machine,
-				  struct thread *thread __maybe_unused)
+				  struct thread *thread)
 {
 	struct vdso_info *vdso_info;
 	struct dso *dso = NULL;
@@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
 	if (!vdso_info)
 		goto out_unlock;
 
+	dso = machine__find_vdso(machine, thread);
+	if (dso)
+		goto out_unlock;
+
 #if BITS_PER_LONG == 64
 	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
 		goto out_unlock;
-- 
1.8.5.2

Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform

[Arnaldo Carvalho de Melo]

Em Fri, May 13, 2016 at 08:51:49AM +0000, He Kuang escreveu:
> There's a problem in machine__findnew_vdso(), vdso buildid generated
> by a 32-bit machine stores it with the name 'vdso', but when
> processing buildid on a 64-bit machine with the same 'perf.data', perf
> will search for vdso named as 'vdso32' and get failed.
> 
> This patch tries to find the exsiting dsos in machine->dsos by thread
> dso_type. 64-bit thread tries to find vdso with name 'vdso', because
> all 64-bit vdso is named as that. 32-bit thread first tries to find
> vdso with name 'vdso32' if this thread was run on 64-bit machine, if
> failed, then it tries 'vdso' which indicates that the thread was run
> on 32-bit machine when recording.

Adrian, are you ok now?


- Arnaldo
 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> index 44d440d..99f4a3d 100644
> --- a/tools/perf/util/vdso.c
> +++ b/tools/perf/util/vdso.c
> @@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
>  	return dso;
>  }
>  
> -#if BITS_PER_LONG == 64
> -
>  static enum dso_type machine__thread_dso_type(struct machine *machine,
>  					      struct thread *thread)
>  {
> @@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
>  	return dso_type;
>  }
>  
> +#if BITS_PER_LONG == 64
> +
>  static int vdso__do_copy_compat(FILE *f, int fd)
>  {
>  	char buf[4096];
> @@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
>  
>  #endif
>  
> +static struct dso *machine__find_vdso(struct machine *machine,
> +				      struct thread *thread)
> +{
> +	struct dso *dso = NULL;
> +	enum dso_type dso_type;
> +
> +	dso_type = machine__thread_dso_type(machine, thread);
> +	switch (dso_type) {
> +	case DSO__TYPE_32BIT:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
> +		if (!dso)
> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> +					   true);
> +		break;
> +	case DSO__TYPE_X32BIT:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
> +		if (!dso)
> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> +					   true);
> +		break;
> +	case DSO__TYPE_64BIT:
> +	case DSO__TYPE_UNKNOWN:
> +	default:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
> +		break;
> +	}
> +
> +	return dso;
> +}
> +
>  struct dso *machine__findnew_vdso(struct machine *machine,
> -				  struct thread *thread __maybe_unused)
> +				  struct thread *thread)
>  {
>  	struct vdso_info *vdso_info;
>  	struct dso *dso = NULL;
> @@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
>  	if (!vdso_info)
>  		goto out_unlock;
>  
> +	dso = machine__find_vdso(machine, thread);
> +	if (dso)
> +		goto out_unlock;
> +
>  #if BITS_PER_LONG == 64
>  	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
>  		goto out_unlock;
> -- 
> 1.8.5.2

Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform

[Adrian Hunter]

On 16/05/16 16:32, Arnaldo Carvalho de Melo wrote:
> Em Fri, May 13, 2016 at 08:51:49AM +0000, He Kuang escreveu:
>> There's a problem in machine__findnew_vdso(), vdso buildid generated
>> by a 32-bit machine stores it with the name 'vdso', but when
>> processing buildid on a 64-bit machine with the same 'perf.data', perf
>> will search for vdso named as 'vdso32' and get failed.
>>
>> This patch tries to find the exsiting dsos in machine->dsos by thread
>> dso_type. 64-bit thread tries to find vdso with name 'vdso', because
>> all 64-bit vdso is named as that. 32-bit thread first tries to find
>> vdso with name 'vdso32' if this thread was run on 64-bit machine, if
>> failed, then it tries 'vdso' which indicates that the thread was run
>> on 32-bit machine when recording.
> 
> Adrian, are you ok now?

I sent a couple more comments.

Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform

[Adrian Hunter]

On 13/05/16 11:51, He Kuang wrote:
> There's a problem in machine__findnew_vdso(), vdso buildid generated
> by a 32-bit machine stores it with the name 'vdso', but when
> processing buildid on a 64-bit machine with the same 'perf.data', perf
> will search for vdso named as 'vdso32' and get failed.
> 
> This patch tries to find the exsiting dsos in machine->dsos by thread
> dso_type. 64-bit thread tries to find vdso with name 'vdso', because
> all 64-bit vdso is named as that. 32-bit thread first tries to find
> vdso with name 'vdso32' if this thread was run on 64-bit machine, if
> failed, then it tries 'vdso' which indicates that the thread was run
> on 32-bit machine when recording.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> index 44d440d..99f4a3d 100644
> --- a/tools/perf/util/vdso.c
> +++ b/tools/perf/util/vdso.c
> @@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
>  	return dso;
>  }
>  
> -#if BITS_PER_LONG == 64
> -
>  static enum dso_type machine__thread_dso_type(struct machine *machine,
>  					      struct thread *thread)
>  {
> @@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
>  	return dso_type;
>  }
>  
> +#if BITS_PER_LONG == 64
> +
>  static int vdso__do_copy_compat(FILE *f, int fd)
>  {
>  	char buf[4096];
> @@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
>  
>  #endif
>  
> +static struct dso *machine__find_vdso(struct machine *machine,
> +				      struct thread *thread)
> +{
> +	struct dso *dso = NULL;
> +	enum dso_type dso_type;
> +
> +	dso_type = machine__thread_dso_type(machine, thread);
> +	switch (dso_type) {
> +	case DSO__TYPE_32BIT:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
> +		if (!dso)
> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> +					   true);

So if we have not yet added the 32-bit vdso but have added the 64-bit vdso,
we will return the wrong one.

Can we check it? e.g.

		if (!dso)
			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
					   true);
			if (dso_type != dso__type(dso, machine))
				dso = NULL;

> +		break;
> +	case DSO__TYPE_X32BIT:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
> +		if (!dso)
> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> +					   true);

The x32 vdso is never called DSO__NAME_VDSO so this is not correct, but for
the same reason we don't need this __dsos__find() anyway.

> +		break;
> +	case DSO__TYPE_64BIT:
> +	case DSO__TYPE_UNKNOWN:
> +	default:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
> +		break;
> +	}
> +
> +	return dso;
> +}
> +
>  struct dso *machine__findnew_vdso(struct machine *machine,
> -				  struct thread *thread __maybe_unused)
> +				  struct thread *thread)
>  {
>  	struct vdso_info *vdso_info;
>  	struct dso *dso = NULL;
> @@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
>  	if (!vdso_info)
>  		goto out_unlock;
>  
> +	dso = machine__find_vdso(machine, thread);
> +	if (dso)
> +		goto out_unlock;
> +
>  #if BITS_PER_LONG == 64
>  	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
>  		goto out_unlock;
> 

[PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform

[He Kuang]

There's a problem in machine__findnew_vdso(), vdso buildid generated
by a 32-bit machine stores it with the name 'vdso', but when
processing buildid on a 64-bit machine with the same 'perf.data', perf
will search for vdso named as 'vdso32' and get failed.

This patch tries to find the exsiting dsos in machine->dsos by thread
dso_type. 64-bit thread tries to find vdso with name 'vdso', because
all 64-bit vdso is named as that. 32-bit thread first tries to find
vdso with name 'vdso32' if this thread was run on 64-bit machine, if
failed, then it tries 'vdso' which indicates that the thread was run
on 32-bit machine when recording.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
index 44d440d..8f81c41 100644
--- a/tools/perf/util/vdso.c
+++ b/tools/perf/util/vdso.c
@@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
 	return dso;
 }
 
-#if BITS_PER_LONG == 64
-
 static enum dso_type machine__thread_dso_type(struct machine *machine,
 					      struct thread *thread)
 {
@@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
 	return dso_type;
 }
 
+#if BITS_PER_LONG == 64
+
 static int vdso__do_copy_compat(FILE *f, int fd)
 {
 	char buf[4096];
@@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
 
 #endif
 
+static struct dso *machine__find_vdso(struct machine *machine,
+				      struct thread *thread)
+{
+	struct dso *dso = NULL;
+	enum dso_type dso_type;
+
+	dso_type = machine__thread_dso_type(machine, thread);
+	switch (dso_type) {
+	case DSO__TYPE_32BIT:
+		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
+		if (!dso) {
+			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
+					   true);
+			if (dso_type != dso__type(dso, machine))
+				dso = NULL;
+		}
+		break;
+	case DSO__TYPE_X32BIT:
+		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
+		break;
+	case DSO__TYPE_64BIT:
+	case DSO__TYPE_UNKNOWN:
+	default:
+		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
+		break;
+	}
+
+	return dso;
+}
+
 struct dso *machine__findnew_vdso(struct machine *machine,
-				  struct thread *thread __maybe_unused)
+				  struct thread *thread)
 {
 	struct vdso_info *vdso_info;
 	struct dso *dso = NULL;
@@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
 	if (!vdso_info)
 		goto out_unlock;
 
+	dso = machine__find_vdso(machine, thread);
+	if (dso)
+		goto out_unlock;
+
 #if BITS_PER_LONG == 64
 	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
 		goto out_unlock;
-- 
1.8.5.2

Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform

[Adrian Hunter]

On 17/05/16 12:04, He Kuang wrote:
> There's a problem in machine__findnew_vdso(), vdso buildid generated
> by a 32-bit machine stores it with the name 'vdso', but when
> processing buildid on a 64-bit machine with the same 'perf.data', perf
> will search for vdso named as 'vdso32' and get failed.
> 
> This patch tries to find the exsiting dsos in machine->dsos by thread
> dso_type. 64-bit thread tries to find vdso with name 'vdso', because
> all 64-bit vdso is named as that. 32-bit thread first tries to find
> vdso with name 'vdso32' if this thread was run on 64-bit machine, if
> failed, then it tries 'vdso' which indicates that the thread was run
> on 32-bit machine when recording.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>

Looks OK.

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> index 44d440d..8f81c41 100644
> --- a/tools/perf/util/vdso.c
> +++ b/tools/perf/util/vdso.c
> @@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
>  	return dso;
>  }
>  
> -#if BITS_PER_LONG == 64
> -
>  static enum dso_type machine__thread_dso_type(struct machine *machine,
>  					      struct thread *thread)
>  {
> @@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
>  	return dso_type;
>  }
>  
> +#if BITS_PER_LONG == 64
> +
>  static int vdso__do_copy_compat(FILE *f, int fd)
>  {
>  	char buf[4096];
> @@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
>  
>  #endif
>  
> +static struct dso *machine__find_vdso(struct machine *machine,
> +				      struct thread *thread)
> +{
> +	struct dso *dso = NULL;
> +	enum dso_type dso_type;
> +
> +	dso_type = machine__thread_dso_type(machine, thread);
> +	switch (dso_type) {
> +	case DSO__TYPE_32BIT:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
> +		if (!dso) {
> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> +					   true);
> +			if (dso_type != dso__type(dso, machine))
> +				dso = NULL;
> +		}
> +		break;
> +	case DSO__TYPE_X32BIT:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
> +		break;
> +	case DSO__TYPE_64BIT:
> +	case DSO__TYPE_UNKNOWN:
> +	default:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
> +		break;
> +	}
> +
> +	return dso;
> +}
> +
>  struct dso *machine__findnew_vdso(struct machine *machine,
> -				  struct thread *thread __maybe_unused)
> +				  struct thread *thread)
>  {
>  	struct vdso_info *vdso_info;
>  	struct dso *dso = NULL;
> @@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
>  	if (!vdso_info)
>  		goto out_unlock;
>  
> +	dso = machine__find_vdso(machine, thread);
> +	if (dso)
> +		goto out_unlock;
> +
>  #if BITS_PER_LONG == 64
>  	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
>  		goto out_unlock;
> 

Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform

[Arnaldo Carvalho de Melo]

Em Tue, May 17, 2016 at 12:17:29PM +0300, Adrian Hunter escreveu:
> On 17/05/16 12:04, He Kuang wrote:
> > There's a problem in machine__findnew_vdso(), vdso buildid generated
> > by a 32-bit machine stores it with the name 'vdso', but when
> > processing buildid on a 64-bit machine with the same 'perf.data', perf
> > will search for vdso named as 'vdso32' and get failed.

> > This patch tries to find the exsiting dsos in machine->dsos by thread
> > dso_type. 64-bit thread tries to find vdso with name 'vdso', because
> > all 64-bit vdso is named as that. 32-bit thread first tries to find
> > vdso with name 'vdso32' if this thread was run on 64-bit machine, if
> > failed, then it tries 'vdso' which indicates that the thread was run
> > on 32-bit machine when recording.

> > Signed-off-by: He Kuang <hekuang@huawei.com>
 
> Looks OK.
 
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied.

- Arnaldo

Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform

[Arnaldo Carvalho de Melo]

Em Tue, May 17, 2016 at 09:04:54AM +0000, He Kuang escreveu:
> There's a problem in machine__findnew_vdso(), vdso buildid generated
> by a 32-bit machine stores it with the name 'vdso', but when
> processing buildid on a 64-bit machine with the same 'perf.data', perf
> will search for vdso named as 'vdso32' and get failed.

Without looking at the code, just trying to understand your patch by
reading your description: Why would we look for names if we have
build-ids? All this type and name comparasions seems wrong if we have a
build-id, no?

Adrian?

- Arnaldo
 
> This patch tries to find the exsiting dsos in machine->dsos by thread
> dso_type. 64-bit thread tries to find vdso with name 'vdso', because
> all 64-bit vdso is named as that. 32-bit thread first tries to find
> vdso with name 'vdso32' if this thread was run on 64-bit machine, if
> failed, then it tries 'vdso' which indicates that the thread was run
> on 32-bit machine when recording.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> index 44d440d..8f81c41 100644
> --- a/tools/perf/util/vdso.c
> +++ b/tools/perf/util/vdso.c
> @@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
>  	return dso;
>  }
>  
> -#if BITS_PER_LONG == 64
> -
>  static enum dso_type machine__thread_dso_type(struct machine *machine,
>  					      struct thread *thread)
>  {
> @@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
>  	return dso_type;
>  }
>  
> +#if BITS_PER_LONG == 64
> +
>  static int vdso__do_copy_compat(FILE *f, int fd)
>  {
>  	char buf[4096];
> @@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
>  
>  #endif
>  
> +static struct dso *machine__find_vdso(struct machine *machine,
> +				      struct thread *thread)
> +{
> +	struct dso *dso = NULL;
> +	enum dso_type dso_type;
> +
> +	dso_type = machine__thread_dso_type(machine, thread);
> +	switch (dso_type) {
> +	case DSO__TYPE_32BIT:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
> +		if (!dso) {
> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> +					   true);
> +			if (dso_type != dso__type(dso, machine))
> +				dso = NULL;
> +		}
> +		break;
> +	case DSO__TYPE_X32BIT:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
> +		break;
> +	case DSO__TYPE_64BIT:
> +	case DSO__TYPE_UNKNOWN:
> +	default:
> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
> +		break;
> +	}
> +
> +	return dso;
> +}
> +
>  struct dso *machine__findnew_vdso(struct machine *machine,
> -				  struct thread *thread __maybe_unused)
> +				  struct thread *thread)
>  {
>  	struct vdso_info *vdso_info;
>  	struct dso *dso = NULL;
> @@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
>  	if (!vdso_info)
>  		goto out_unlock;
>  
> +	dso = machine__find_vdso(machine, thread);
> +	if (dso)
> +		goto out_unlock;
> +
>  #if BITS_PER_LONG == 64
>  	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
>  		goto out_unlock;
> -- 
> 1.8.5.2

Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform

[Hekuang]

hi

在 2016/6/15 21:34, Arnaldo Carvalho de Melo 写道:
> Em Tue, May 17, 2016 at 09:04:54AM +0000, He Kuang escreveu:
>> There's a problem in machine__findnew_vdso(), vdso buildid generated
>> by a 32-bit machine stores it with the name 'vdso', but when
>> processing buildid on a 64-bit machine with the same 'perf.data', perf
>> will search for vdso named as 'vdso32' and get failed.
> Without looking at the code, just trying to understand your patch by
> reading your description: Why would we look for names if we have
> build-ids? All this type and name comparasions seems wrong if we have a
> build-id, no?
>
> Adrian?
>
> - Arnaldo
>
We should find a binary with the {name, buildid} pair, the key is
name not buildid.

There're more than one vdso binaries in system, we store
parts/all of them when recording and should find out which one to
use for unwinding in the 'perf script' stage, by comparing the
name and the thread's dso_type.

Thank you.

>> This patch tries to find the exsiting dsos in machine->dsos by thread
>> dso_type. 64-bit thread tries to find vdso with name 'vdso', because
>> all 64-bit vdso is named as that. 32-bit thread first tries to find
>> vdso with name 'vdso32' if this thread was run on 64-bit machine, if
>> failed, then it tries 'vdso' which indicates that the thread was run
>> on 32-bit machine when recording.
>>
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> ---
>>   tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 37 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
>> index 44d440d..8f81c41 100644
>> --- a/tools/perf/util/vdso.c
>> +++ b/tools/perf/util/vdso.c
>> @@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
>>   	return dso;
>>   }
>>   
>> -#if BITS_PER_LONG == 64
>> -
>>   static enum dso_type machine__thread_dso_type(struct machine *machine,
>>   					      struct thread *thread)
>>   {
>> @@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
>>   	return dso_type;
>>   }
>>   
>> +#if BITS_PER_LONG == 64
>> +
>>   static int vdso__do_copy_compat(FILE *f, int fd)
>>   {
>>   	char buf[4096];
>> @@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
>>   
>>   #endif
>>   
>> +static struct dso *machine__find_vdso(struct machine *machine,
>> +				      struct thread *thread)
>> +{
>> +	struct dso *dso = NULL;
>> +	enum dso_type dso_type;
>> +
>> +	dso_type = machine__thread_dso_type(machine, thread);
>> +	switch (dso_type) {
>> +	case DSO__TYPE_32BIT:
>> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
>> +		if (!dso) {
>> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
>> +					   true);
>> +			if (dso_type != dso__type(dso, machine))
>> +				dso = NULL;
>> +		}
>> +		break;
>> +	case DSO__TYPE_X32BIT:
>> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
>> +		break;
>> +	case DSO__TYPE_64BIT:
>> +	case DSO__TYPE_UNKNOWN:
>> +	default:
>> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
>> +		break;
>> +	}
>> +
>> +	return dso;
>> +}
>> +
>>   struct dso *machine__findnew_vdso(struct machine *machine,
>> -				  struct thread *thread __maybe_unused)
>> +				  struct thread *thread)
>>   {
>>   	struct vdso_info *vdso_info;
>>   	struct dso *dso = NULL;
>> @@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
>>   	if (!vdso_info)
>>   		goto out_unlock;
>>   
>> +	dso = machine__find_vdso(machine, thread);
>> +	if (dso)
>> +		goto out_unlock;
>> +
>>   #if BITS_PER_LONG == 64
>>   	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
>>   		goto out_unlock;
>> -- 
>> 1.8.5.2

Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform

[Adrian Hunter]

On 15/06/16 16:34, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 17, 2016 at 09:04:54AM +0000, He Kuang escreveu:
>> There's a problem in machine__findnew_vdso(), vdso buildid generated
>> by a 32-bit machine stores it with the name 'vdso', but when
>> processing buildid on a 64-bit machine with the same 'perf.data', perf
>> will search for vdso named as 'vdso32' and get failed.
> 
> Without looking at the code, just trying to understand your patch by
> reading your description: Why would we look for names if we have
> build-ids? All this type and name comparasions seems wrong if we have a
> build-id, no?
> 
> Adrian?

We match maps to builds ids using the file name - consider
machine__findnew_[v]dso() called in map__new().  So in the context of a perf
data file, we consider the file name to be unique.

A vdso map does not have a file name - all we know is that it is vdso.  We
look at the thread to tell if it is 32-bit, 64-bit or x32.  Then we need to
get the build id which has been recorded using short name "[vdso]" or
"[vdso32]" or "[vdsox32]".

The problem is that on a 32-bit machine, we use the name "[vdso]".  If you
take a 32-bit perf data file to a 64-bit machine, it gets hard to figure out
if "[vdso]" is 32-bit or 64-bit.

This patch solves that problem.

I acked it in another email.

Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform

[Arnaldo Carvalho de Melo]

Em Thu, Jun 16, 2016 at 04:00:12PM +0300, Adrian Hunter escreveu:
> On 15/06/16 16:34, Arnaldo Carvalho de Melo wrote:
> > Em Tue, May 17, 2016 at 09:04:54AM +0000, He Kuang escreveu:
> >> There's a problem in machine__findnew_vdso(), vdso buildid generated
> >> by a 32-bit machine stores it with the name 'vdso', but when
> >> processing buildid on a 64-bit machine with the same 'perf.data', perf
> >> will search for vdso named as 'vdso32' and get failed.
> > 
> > Without looking at the code, just trying to understand your patch by
> > reading your description: Why would we look for names if we have
> > build-ids? All this type and name comparasions seems wrong if we have a
> > build-id, no?
> > 
> > Adrian?
> 
> We match maps to builds ids using the file name - consider
> machine__findnew_[v]dso() called in map__new().  So in the context of a perf
> data file, we consider the file name to be unique.
> 
> A vdso map does not have a file name - all we know is that it is vdso.  We
> look at the thread to tell if it is 32-bit, 64-bit or x32.  Then we need to
> get the build id which has been recorded using short name "[vdso]" or
> "[vdso32]" or "[vdsox32]".
> 
> The problem is that on a 32-bit machine, we use the name "[vdso]".  If you
> take a 32-bit perf data file to a 64-bit machine, it gets hard to figure out
> if "[vdso]" is 32-bit or 64-bit.
> 
> This patch solves that problem.
> 
> I acked it in another email.

Ok, somehow this got lost, I thought to have applied it already, but
re-reading the changeset comment, I got confused, thanks for providing
this explanation, that I will stick to the changeset log for this
message, to help understanding this when scratching our heads in the
future.

- Arnaldo

Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform

[Hekuang]

在 2016/5/17 15:33, Adrian Hunter 写道:
> On 13/05/16 11:51, He Kuang wrote:
>> There's a problem in machine__findnew_vdso(), vdso buildid generated
>> by a 32-bit machine stores it with the name 'vdso', but when
>> processing buildid on a 64-bit machine with the same 'perf.data', perf
>> will search for vdso named as 'vdso32' and get failed.
>>
>> This patch tries to find the exsiting dsos in machine->dsos by thread
>> dso_type. 64-bit thread tries to find vdso with name 'vdso', because
>> all 64-bit vdso is named as that. 32-bit thread first tries to find
>> vdso with name 'vdso32' if this thread was run on 64-bit machine, if
>> failed, then it tries 'vdso' which indicates that the thread was run
>> on 32-bit machine when recording.
>>
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> ---
>>   tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 37 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
>> index 44d440d..99f4a3d 100644
>> --- a/tools/perf/util/vdso.c
>> +++ b/tools/perf/util/vdso.c
>> @@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
>>   	return dso;
>>   }
>>   
>> -#if BITS_PER_LONG == 64
>> -
>>   static enum dso_type machine__thread_dso_type(struct machine *machine,
>>   					      struct thread *thread)
>>   {
>> @@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
>>   	return dso_type;
>>   }
>>   
>> +#if BITS_PER_LONG == 64
>> +
>>   static int vdso__do_copy_compat(FILE *f, int fd)
>>   {
>>   	char buf[4096];
>> @@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
>>   
>>   #endif
>>   
>> +static struct dso *machine__find_vdso(struct machine *machine,
>> +				      struct thread *thread)
>> +{
>> +	struct dso *dso = NULL;
>> +	enum dso_type dso_type;
>> +
>> +	dso_type = machine__thread_dso_type(machine, thread);
>> +	switch (dso_type) {
>> +	case DSO__TYPE_32BIT:
>> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
>> +		if (!dso)
>> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
>> +					   true);
> So if we have not yet added the 32-bit vdso but have added the 64-bit vdso,
> we will return the wrong one.
>
> Can we check it? e.g.
>
> 		if (!dso)
> 			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> 					   true);
> 			if (dso_type != dso__type(dso, machine))
> 				dso = NULL;
>
>> +		break;
>> +	case DSO__TYPE_X32BIT:
>> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
>> +		if (!dso)
>> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
>> +					   true);
> The x32 vdso is never called DSO__NAME_VDSO so this is not correct, but for
> the same reason we don't need this __dsos__find() anyway.

Thanks, update
>> +		break;
>> +	case DSO__TYPE_64BIT:
>> +	case DSO__TYPE_UNKNOWN:
>> +	default:
>> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
>> +		break;
>> +	}
>> +
>> +	return dso;
>> +}
>> +
>>   struct dso *machine__findnew_vdso(struct machine *machine,
>> -				  struct thread *thread __maybe_unused)
>> +				  struct thread *thread)
>>   {
>>   	struct vdso_info *vdso_info;
>>   	struct dso *dso = NULL;
>> @@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
>>   	if (!vdso_info)
>>   		goto out_unlock;
>>   
>> +	dso = machine__find_vdso(machine, thread);
>> +	if (dso)
>> +		goto out_unlock;
>> +
>>   #if BITS_PER_LONG == 64
>>   	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
>>   		goto out_unlock;
>>
>

Re: [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform

[Hekuang]

在 2016/5/12 18:06, Adrian Hunter 写道:
> On 12/05/16 11:43, He Kuang wrote:
>> This is a preparation for cross-platform vdso lookup.
>>
>> There is a naming confusion about vdso name, vdso buildid generated by
>> a 32-bit machine stores it with the name 'vdso', but when processing
>> buildid on a 64-bit machine with the same 'perf.data', perf will
>> search for vdso named as 'vdso32' and get failed.
>>
>> This patch uses different names when storing the buildid, i.e. vdso64
>> for 64-bit machine and vdso32 for 32-bit machine, and eliminates this
>> naming confusion.
> That looks like it will break existing perf.data files because they will
> have a different name recorded in the buildid section.
>
> Also it doesn't look like it would work the other way around i.e. recording
> on a 64-bit machine and processing on a 32-bit machine.

Yes, please have a look at the new patch.

Thanks
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> ---
>>   tools/perf/util/vdso.h | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/vdso.h b/tools/perf/util/vdso.h
>> index cdc4fab..45e9ef4 100644
>> --- a/tools/perf/util/vdso.h
>> +++ b/tools/perf/util/vdso.h
>> @@ -4,10 +4,15 @@
>>   #include <linux/types.h>
>>   #include <string.h>
>>   #include <stdbool.h>
>> +#include "util.h"
>>   
>>   #define VDSO__MAP_NAME "[vdso]"
>>   
>> -#define DSO__NAME_VDSO    "[vdso]"
>> +#if BITS_PER_LONG == 64
>> +#define DSO__NAME_VDSO    "[vdso64]"
>> +#else
>> +#define DSO__NAME_VDSO    "[vdso32]"
>> +#endif
>>   #define DSO__NAME_VDSO32  "[vdso32]"
>>   #define DSO__NAME_VDSOX32 "[vdsox32]"
>>   
>>
>

[PATCH v3 2/7] perf tools: Store vdso buildid unconditionally

[He Kuang]

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. But in most cases, the addresses of sample events
are not in vdso range, the buildid of a zero hit vdso won't be stored
into perf.data.

This patch stores vdso buildid regardless of whether the vdso is hit
or not.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/util/build-id.c | 2 +-
 tools/perf/util/dso.c      | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 0573c2e..bdc7580 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -256,7 +256,7 @@ static int machine__write_buildid_table(struct machine *machine, int fd)
 		size_t name_len;
 		bool in_kernel = false;
 
-		if (!pos->hit)
+		if (!pos->hit && !dso__is_vdso(pos))
 			continue;
 
 		if (dso__is_vdso(pos)) {
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 8e639543..b39b80c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -7,6 +7,7 @@
 #include "auxtrace.h"
 #include "util.h"
 #include "debug.h"
+#include "vdso.h"
 
 char dso__symtab_origin(const struct dso *dso)
 {
@@ -1169,7 +1170,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
 	struct dso *pos;
 
 	list_for_each_entry(pos, head, node) {
-		if (with_hits && !pos->hit)
+		if (with_hits && !pos->hit && !dso__is_vdso(pos))
 			continue;
 		if (pos->has_build_id) {
 			have_build_id = true;
-- 
1.8.5.2

Re: [PATCH v3 2/7] perf tools: Store vdso buildid unconditionally

[Arnaldo Carvalho de Melo]

Em Thu, May 12, 2016 at 08:43:11AM +0000, He Kuang escreveu:
> 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. But in most cases, the addresses of sample events
> are not in vdso range, the buildid of a zero hit vdso won't be stored
> into perf.data.
> 
> This patch stores vdso buildid regardless of whether the vdso is hit
> or not.

Looks ok, applied.
 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/util/build-id.c | 2 +-
>  tools/perf/util/dso.c      | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 0573c2e..bdc7580 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -256,7 +256,7 @@ static int machine__write_buildid_table(struct machine *machine, int fd)
>  		size_t name_len;
>  		bool in_kernel = false;
>  
> -		if (!pos->hit)
> +		if (!pos->hit && !dso__is_vdso(pos))
>  			continue;
>  
>  		if (dso__is_vdso(pos)) {
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 8e639543..b39b80c 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -7,6 +7,7 @@
>  #include "auxtrace.h"
>  #include "util.h"
>  #include "debug.h"
> +#include "vdso.h"
>  
>  char dso__symtab_origin(const struct dso *dso)
>  {
> @@ -1169,7 +1170,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
>  	struct dso *pos;
>  
>  	list_for_each_entry(pos, head, node) {
> -		if (with_hits && !pos->hit)
> +		if (with_hits && !pos->hit && !dso__is_vdso(pos))
>  			continue;
>  		if (pos->has_build_id) {
>  			have_build_id = true;
> -- 
> 1.8.5.2

[tip:perf/urgent] perf symbols: Store vdso buildid unconditionally

[tip-bot for He Kuang]

Commit-ID:  6ae98ba611ed1c11ddc5645475bc03b46a3c04e7
Gitweb:     http://git.kernel.org/tip/6ae98ba611ed1c11ddc5645475bc03b46a3c04e7
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Thu, 12 May 2016 08:43:11 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 16 May 2016 23:11:45 -0300

perf symbols: Store vdso buildid unconditionally

When unwinding callchains on a different machine, vdso info should be
available so the unwind process won't be interrupted if address falls
into vdso region. But in most cases, the addresses of sample events are
not in vdso range, the buildid of a zero hit vdso won't be stored into
perf.data.

This patch stores vdso buildid regardless of whether the vdso is hit or
not.

Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1463042596-61703-3-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/build-id.c | 2 +-
 tools/perf/util/dso.c      | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index bff425e..67e5966 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -256,7 +256,7 @@ static int machine__write_buildid_table(struct machine *machine, int fd)
 		size_t name_len;
 		bool in_kernel = false;
 
-		if (!pos->hit)
+		if (!pos->hit && !dso__is_vdso(pos))
 			continue;
 
 		if (dso__is_vdso(pos)) {
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 3357479..75b7561 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -7,6 +7,7 @@
 #include "auxtrace.h"
 #include "util.h"
 #include "debug.h"
+#include "vdso.h"
 
 char dso__symtab_origin(const struct dso *dso)
 {
@@ -1169,7 +1170,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
 	struct dso *pos;
 
 	list_for_each_entry(pos, head, node) {
-		if (with_hits && !pos->hit)
+		if (with_hits && !pos->hit && !dso__is_vdso(pos))
 			continue;
 		if (pos->has_build_id) {
 			have_build_id = true;

[PATCH v3 3/7] perf tools: Remove the logical that skip buildid cache if symfs is given

[He Kuang]

Symfs dir and buildid dir are two places that perf looks into for
symbols, currently, if symfs dir is given, buildid-cache is skipped.

In the cross-platform perf record/script scenario, we need vdsos in
buildid-cache dir and other libs in symfs dir at the same time. And
consider that the binaries indexed by buildid do not cause ambiguity,
this patch simply removes that logical.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/util/dso.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index b39b80c..a07166c5 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -64,8 +64,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		break;
 	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
 		/* skip the locally configured cache if a symfs is given */
-		if (symbol_conf.symfs[0] ||
-		    (dso__build_id_filename(dso, filename, size) == NULL))
+		if (dso__build_id_filename(dso, filename, size) == NULL)
 			ret = -1;
 		break;
 
-- 
1.8.5.2

Re: [PATCH v3 3/7] perf tools: Remove the logical that skip buildid cache if symfs is given

[Arnaldo Carvalho de Melo]

Em Thu, May 12, 2016 at 08:43:12AM +0000, He Kuang escreveu:
> Symfs dir and buildid dir are two places that perf looks into for
> symbols, currently, if symfs dir is given, buildid-cache is skipped.
> 
> In the cross-platform perf record/script scenario, we need vdsos in
> buildid-cache dir and other libs in symfs dir at the same time. And
> consider that the binaries indexed by buildid do not cause ambiguity,
> this patch simply removes that logical.

Makes perfect sense, David, do you have any concern? Can I have your
Acked-by?

- Arnaldo
 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/util/dso.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index b39b80c..a07166c5 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -64,8 +64,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
>  		break;
>  	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
>  		/* skip the locally configured cache if a symfs is given */
> -		if (symbol_conf.symfs[0] ||
> -		    (dso__build_id_filename(dso, filename, size) == NULL))
> +		if (dso__build_id_filename(dso, filename, size) == NULL)
>  			ret = -1;
>  		break;
>  
> -- 
> 1.8.5.2

Re: [PATCH v3 3/7] perf tools: Remove the logical that skip buildid cache if symfs is given

[David Ahern]

On 5/12/16 7:09 AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, May 12, 2016 at 08:43:12AM +0000, He Kuang escreveu:
>> Symfs dir and buildid dir are two places that perf looks into for
>> symbols, currently, if symfs dir is given, buildid-cache is skipped.
>>
>> In the cross-platform perf record/script scenario, we need vdsos in
>> buildid-cache dir and other libs in symfs dir at the same time. And
>> consider that the binaries indexed by buildid do not cause ambiguity,
>> this patch simply removes that logical.
>
> Makes perfect sense, David, do you have any concern? Can I have your
> Acked-by?

seems odd to me you want to look in the buildid-cache when a symfs is 
given. The point of symfs was "go look for everything under here."

I believe dso__load is going to hit DSO_BINARY_TYPE__BUILD_ID_CACHE 
before any of the others and there are probably cases where a stale 
cache entry would be hit before a build tree entry (e.g., symfs).

What about putting the build id cache under the symfs? so instead of 
dropping the symfs check and it to the path for the build id cache.

Re: [PATCH v3 3/7] perf tools: Remove the logical that skip buildid cache if symfs is given

[Arnaldo Carvalho de Melo]

Em Thu, May 12, 2016 at 02:23:37PM -0600, David Ahern escreveu:
> On 5/12/16 7:09 AM, Arnaldo Carvalho de Melo wrote:
> > Em Thu, May 12, 2016 at 08:43:12AM +0000, He Kuang escreveu:
> > > Symfs dir and buildid dir are two places that perf looks into for
> > > symbols, currently, if symfs dir is given, buildid-cache is skipped.

> > > In the cross-platform perf record/script scenario, we need vdsos in
> > > buildid-cache dir and other libs in symfs dir at the same time. And
> > > consider that the binaries indexed by buildid do not cause ambiguity,
> > > this patch simply removes that logical.

> > Makes perfect sense, David, do you have any concern? Can I have your
> > Acked-by?

> seems odd to me you want to look in the buildid-cache when a symfs is given.
> The point of symfs was "go look for everything under here."

> I believe dso__load is going to hit DSO_BINARY_TYPE__BUILD_ID_CACHE before
> any of the others and there are probably cases where a stale cache entry
> would be hit before a build tree entry (e.g., symfs).

Stale cache entry? How'd that be? Its content based, i.e. if there is an
entry for a given build id _anywhere_ why wouldn't we want to use it?
 
> What about putting the build id cache under the symfs? so instead of
> dropping the symfs check and it to the path for the build id cache.

Well, the use case of look at both still looks sane to me, but maybe the
way you suggest works for He, He?

- Arnaldo

Re: [PATCH v3 3/7] perf tools: Remove the logical that skip buildid cache if symfs is given

[Hekuang]

hi

在 2016/5/13 4:23, David Ahern 写道:
> On 5/12/16 7:09 AM, Arnaldo Carvalho de Melo wrote:
>> Em Thu, May 12, 2016 at 08:43:12AM +0000, He Kuang escreveu:
>>> Symfs dir and buildid dir are two places that perf looks into for
>>> symbols, currently, if symfs dir is given, buildid-cache is skipped.
>>>
>>> In the cross-platform perf record/script scenario, we need vdsos in
>>> buildid-cache dir and other libs in symfs dir at the same time. And
>>> consider that the binaries indexed by buildid do not cause ambiguity,
>>> this patch simply removes that logical.
>>
>> Makes perfect sense, David, do you have any concern? Can I have your
>> Acked-by?
>
> seems odd to me you want to look in the buildid-cache when a symfs is 
> given. The point of symfs was "go look for everything under here."
>
> I believe dso__load is going to hit DSO_BINARY_TYPE__BUILD_ID_CACHE 
> before any of the others and there are probably cases where a stale 
> cache entry would be hit before a build tree entry (e.g., symfs).

Build id entries recorded in perf.data reflect the current dso,
so if buildid is matched , how can it be a stale one?

>
> What about putting the build id cache under the symfs? so instead of 
> dropping the symfs check and it to the path for the build id cache.
>
>
I think your intention is to reference symbol files in one place
instead of two. So there're two possible approaches, one is all
in buildid-cache, but in practice, I found lots of binaries in
symfs even not contains valid buildid, so this way is not work.

The other one is all in symfs. It seems ok, but one problem I
should point out, with my test environment as an example, the
symfsdir is $(TARGET_ROOTFS),and by default buildid_dir is
$(TARGET_ROOTFS)/$(HOME)/.debug/, host perf does not know
$(HOME) folder in target and we should copy the debug folder
  to $(TARGET_ROOTFS), which is readonly in the target. For me, it's
easier to use 'buildid-cache -a vdso-xxxx' to add that into host
buildid-cache than copy debug folder from $(HOME) to readonly
$(TARGET_ROOTFS).

Without the stale concern, I prefer the two places(buildid-dir in
host and target symfs) way.

Thanks.

Re: [PATCH v3 3/7] perf tools: Remove the logical that skip buildid cache if symfs is given

[David Ahern]

On 5/13/16 1:19 AM, Hekuang wrote:
>> What about putting the build id cache under the symfs? so instead of
>> dropping the symfs check and it to the path for the build id cache.
>>
>>
> I think your intention is to reference symbol files in one place
> instead of two. So there're two possible approaches, one is all
> in buildid-cache, but in practice, I found lots of binaries in
> symfs even not contains valid buildid, so this way is not work.
>
> The other one is all in symfs. It seems ok, but one problem I
> should point out, with my test environment as an example, the
> symfsdir is $(TARGET_ROOTFS),and by default buildid_dir is
> $(TARGET_ROOTFS)/$(HOME)/.debug/, host perf does not know
> $(HOME) folder in target and we should copy the debug folder
>  to $(TARGET_ROOTFS), which is readonly in the target. For me, it's
> easier to use 'buildid-cache -a vdso-xxxx' to add that into host
> buildid-cache than copy debug folder from $(HOME) to readonly
> $(TARGET_ROOTFS).
>
> Without the stale concern, I prefer the two places(buildid-dir in
> host and target symfs) way.

The intention of symfs is every single file opened by perf is relative 
to that directory. As I recall when I added that option in early 2011 I 
made sure that statement is true. I think it is best to maintain that 
design.

Re: [PATCH v3 3/7] perf tools: Remove the logical that skip buildid cache if symfs is given

[Arnaldo Carvalho de Melo]

Em Fri, May 13, 2016 at 08:27:29AM -0600, David Ahern escreveu:
> On 5/13/16 1:19 AM, Hekuang wrote:
> > > What about putting the build id cache under the symfs? so instead of
> > > dropping the symfs check and it to the path for the build id cache.

> > I think your intention is to reference symbol files in one place
> > instead of two. So there're two possible approaches, one is all
> > in buildid-cache, but in practice, I found lots of binaries in
> > symfs even not contains valid buildid, so this way is not work.

> > The other one is all in symfs. It seems ok, but one problem I
> > should point out, with my test environment as an example, the
> > symfsdir is $(TARGET_ROOTFS),and by default buildid_dir is
> > $(TARGET_ROOTFS)/$(HOME)/.debug/, host perf does not know
> > $(HOME) folder in target and we should copy the debug folder
> >  to $(TARGET_ROOTFS), which is readonly in the target. For me, it's
> > easier to use 'buildid-cache -a vdso-xxxx' to add that into host
> > buildid-cache than copy debug folder from $(HOME) to readonly
> > $(TARGET_ROOTFS).

> > Without the stale concern, I prefer the two places(buildid-dir in
> > host and target symfs) way.
> 
> The intention of symfs is every single file opened by perf is relative to
> that directory. As I recall when I added that option in early 2011 I made
> sure that statement is true. I think it is best to maintain that design.

Ok, so we can introduce --dso-prefix for Hekuang's use case, i.e. be able
to lookup by build-id, fallbacking to a (possibly read-only) directory.

This way the existing --symfs semantic remains written in stone.

- Arnaldo

[PATCH v3 3/7 UPDATE] perf tools: Add option for the path of buildid dsos under symfs

[He Kuang]

In the cross-platform perf record/script scenario, we need vdsos in
buildid-cache dir and other libs in symfs dir at the same time. For
the reason that to have every single file opened by perf is relative
to symfs dirctory, perf skips the buildid dir if symfs is given.

This patch references the buildid dir under symfs if '--symfs' is
used, and adds new option '--dso-prefix' to specify the subdir path in
symfs which contains the buildid dsos.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/builtin-script.c |  2 ++
 tools/perf/util/config.c    | 10 ++++++++++
 tools/perf/util/dso.c       |  6 ++++--
 tools/perf/util/symbol.c    |  1 +
 tools/perf/util/symbol.h    |  1 +
 tools/perf/util/util.h      |  1 +
 6 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 8f6ab2a..52526e8 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2043,6 +2043,8 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 			"Enable symbol demangling"),
 	OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
 			"Enable kernel symbol demangling"),
+	OPT_STRING(0, "dso-prefix", &symbol_conf.dso_prefix, "direcotry",
+			"Look for dsos relative to this directory under symfs"),
 
 	OPT_END()
 	};
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 664490b..17bee62 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -553,3 +553,13 @@ void set_buildid_dir(const char *dir)
 	/* for communicating with external commands */
 	setenv("PERF_BUILDID_DIR", buildid_dir, 1);
 }
+
+static bool buildid_dir_under_symfs_flag;
+void set_buildid_dir_under_symfs(void)
+{
+	if (!buildid_dir_under_symfs_flag && symbol_conf.symfs[0]) {
+		__symbol__join_symfs(buildid_dir, MAXPATHLEN-1,
+				     symbol_conf.dso_prefix);
+		buildid_dir_under_symfs_flag = true;
+	}
+}
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index b39b80c..fb34274 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -64,8 +64,10 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		break;
 	case DSO_BINARY_TYPE__BUILD_ID_CACHE:
 		/* skip the locally configured cache if a symfs is given */
-		if (symbol_conf.symfs[0] ||
-		    (dso__build_id_filename(dso, filename, size) == NULL))
+		if (symbol_conf.symfs[0])
+			set_buildid_dir_under_symfs();
+
+		if (dso__build_id_filename(dso, filename, size) == NULL)
 			ret = -1;
 		break;
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index e7588dc..7aa34ca 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -40,6 +40,7 @@ struct symbol_conf symbol_conf = {
 	.show_hist_headers	= true,
 	.symfs			= "",
 	.event_group		= true,
+	.dso_prefix		= "",
 };
 
 static enum dso_binary_type binary_type_symtab[] = {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index c8b7544..c0fefdb 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -136,6 +136,7 @@ struct symbol_conf {
 	struct intlist	*pid_list,
 			*tid_list;
 	const char	*symfs;
+	const char	*dso_prefix;
 };
 
 extern struct symbol_conf symbol_conf;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 3bf3de8..f8e5582 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -142,6 +142,7 @@ void set_warning_routine(void (*routine)(const char *err, va_list params));
 
 int prefixcmp(const char *str, const char *prefix);
 void set_buildid_dir(const char *dir);
+void set_buildid_dir_under_symfs(void);
 
 #ifdef __GLIBC_PREREQ
 #if __GLIBC_PREREQ(2, 1)
-- 
1.8.5.2

Re: [PATCH v3 3/7 UPDATE] perf tools: Add option for the path of buildid dsos under symfs

[David Ahern]

On 5/14/16 2:19 AM, He Kuang wrote:
> In the cross-platform perf record/script scenario, we need vdsos in
> buildid-cache dir and other libs in symfs dir at the same time. For
> the reason that to have every single file opened by perf is relative
> to symfs dirctory, perf skips the buildid dir if symfs is given.
>
> This patch references the buildid dir under symfs if '--symfs' is
> used, and adds new option '--dso-prefix' to specify the subdir path in
> symfs which contains the buildid dsos.

In the previous version of this patch you just wanted to drop the symfs 
check. That means there is a path that perf searches for buildid files 
and reading it worked for you. Why is adding symsfs to that path not 
enough? ie., Why do you need to specify a different location under the 
symfs?

Re: [PATCH v3 3/7 UPDATE] perf tools: Add option for the path of buildid dsos under symfs

[Hekuang]

在 2016/5/14 22:43, David Ahern 写道:
> On 5/14/16 2:19 AM, He Kuang wrote:
>> In the cross-platform perf record/script scenario, we need vdsos in
>> buildid-cache dir and other libs in symfs dir at the same time. For
>> the reason that to have every single file opened by perf is relative
>> to symfs dirctory, perf skips the buildid dir if symfs is given.
>>
>> This patch references the buildid dir under symfs if '--symfs' is
>> used, and adds new option '--dso-prefix' to specify the subdir path in
>> symfs which contains the buildid dsos.
>
> In the previous version of this patch you just wanted to drop the 
> symfs check. That means there is a path that perf searches for buildid 
> files and reading it worked for you. Why is adding symsfs to that path 
> not enough? ie., Why do you need to specify a different location under 
> the symfs?
>
In previous patch, I use 'perf buildid-cache -a' to add vdso
binary into the HOST buildid dir.

Currently, $(BUILDID_ROOT)/.debug/.buildid has a directory
structure organized by the buildid value, like this:

  .build-id
  - 3a
  |    e5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd
  - 84
       dbd75729adba57cc42f5544b25de571c0c8731

perf searchs for buildid binaries by the buildid value.

And symfs is a normal file tree and perf searchs for binaryies
by the file name.

  lib
  - libc.so
  - ld.so

Tt's inappropriate to add symfs to .build-id dir becuase they
have different dirctory structure and perf searchs for them by
differnt ways.

So in this patch I add .buildid to symfs and got dirctory tree
like this:

  symfs
  - $(DSO_PREFIX)
      - .build-id
            - 3a
            - 84
  - libc.so
  - ld.so

Thanks

Re: [PATCH v3 3/7 UPDATE] perf tools: Add option for the path of buildid dsos under symfs

[David Ahern]

On 5/15/16 7:30 PM, Hekuang wrote:
> In previous patch, I use 'perf buildid-cache -a' to add vdso
> binary into the HOST buildid dir.

So 'perf buildid-cache' needs the symfs option?

Re: [PATCH v3 3/7 UPDATE] perf tools: Add option for the path of buildid dsos under symfs

[Hekuang]

在 2016/5/16 10:50, David Ahern 写道:
> On 5/15/16 7:30 PM, Hekuang wrote:
>> In previous patch, I use 'perf buildid-cache -a' to add vdso
>> binary into the HOST buildid dir.
>
> So 'perf buildid-cache' needs the symfs option?
>
>

No, for the host, we don't reference any files in buildid-cache
if symfs is givin.

Re: [PATCH v3 3/7 UPDATE] perf tools: Add option for the path of buildid dsos under symfs

[Hekuang]

在 2016/5/16 10:50, David Ahern 写道:
> On 5/15/16 7:30 PM, Hekuang wrote:
>> In previous patch, I use 'perf buildid-cache -a' to add vdso
>> binary into the HOST buildid dir.
>
> So 'perf buildid-cache' needs the symfs option?
>
>
With this patch 'PATCH v3 3/7 UPDATE', the tree of symfs dir is
like this:

├── debug($(dso-prefix))
│   ├── .build-id
│   │   ├── 3a
│   │   │   └── e5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd -> 
../../[kernel.kallsyms]/3ae5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd
│   │   └── 84
│   │       └── dbd75729adba57cc42f5544b25de571c0c8731 -> 
../../[vdso32]/84dbd75729adba57cc42f5544b25de571c0c8731
│   ├── [kernel.kallsyms]
│   │   └── 3ae5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd
│   ├── [vdso]
│   │   └── 84dbd75729adba57cc42f5544b25de571c0c8731
│   └── [vdso32]
│       └── 84dbd75729adba57cc42f5544b25de571c0c8731
├── lib
│   ├── ld-2.22.so
│   └── libc-2.22.so
├── tmp
│   └── hello
└── xxx

So all binaries we need are included in the symfs dir. I think
this is consistent with your idea explained in previous mails.

With this symfs, we do not need buildid dir anymore and what's
your idea on 'perf buildid-cache' needs symfs option? after all,
that only effects on buildid dir.

Thanks.

Re: [PATCH v3 3/7 UPDATE] perf tools: Add option for the path of buildid dsos under symfs

[David Ahern]

On 5/17/16 7:47 PM, Hekuang wrote:
>
>
> 在 2016/5/16 10:50, David Ahern 写道:
>> On 5/15/16 7:30 PM, Hekuang wrote:
>>> In previous patch, I use 'perf buildid-cache -a' to add vdso
>>> binary into the HOST buildid dir.
>>
>> So 'perf buildid-cache' needs the symfs option?
>>
>>
> With this patch 'PATCH v3 3/7 UPDATE', the tree of symfs dir is
> like this:
>
> ├── debug($(dso-prefix))
> │   ├── .build-id
> │   │   ├── 3a
> │   │   │   └── e5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd ->
> ../../[kernel.kallsyms]/3ae5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd
> │   │   └── 84
> │   │       └── dbd75729adba57cc42f5544b25de571c0c8731 ->
> ../../[vdso32]/84dbd75729adba57cc42f5544b25de571c0c8731
> │   ├── [kernel.kallsyms]
> │   │   └── 3ae5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd
> │   ├── [vdso]
> │   │   └── 84dbd75729adba57cc42f5544b25de571c0c8731
> │   └── [vdso32]
> │       └── 84dbd75729adba57cc42f5544b25de571c0c8731
> ├── lib
> │   ├── ld-2.22.so
> │   └── libc-2.22.so
> ├── tmp
> │   └── hello
> └── xxx
>
> So all binaries we need are included in the symfs dir. I think
> this is consistent with your idea explained in previous mails.
>
> With this symfs, we do not need buildid dir anymore and what's
> your idea on 'perf buildid-cache' needs symfs option? after all,
> that only effects on buildid dir.

I don't understand why dso-prefix option is needed? Why make me type yet 
more options to the analysis command? Why can't the directory be located 
under the symfs tree in a known location and populated the same way it 
is without symfs?

Re: [PATCH v3 3/7 UPDATE] perf tools: Add option for the path of buildid dsos under symfs

[Hekuang]

在 2016/5/18 9:51, David Ahern 写道:
> On 5/17/16 7:47 PM, Hekuang wrote:
>>
>>
>> 在 2016/5/16 10:50, David Ahern 写道:
>>> On 5/15/16 7:30 PM, Hekuang wrote:
>>>> In previous patch, I use 'perf buildid-cache -a' to add vdso
>>>> binary into the HOST buildid dir.
>>>
>>> So 'perf buildid-cache' needs the symfs option?
>>>
>>>
>> With this patch 'PATCH v3 3/7 UPDATE', the tree of symfs dir is
>> like this:
>>
>> ├── debug($(dso-prefix))
>> │   ├── .build-id
>> │   │   ├── 3a
>> │   │   │   └── e5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd ->
>> ../../[kernel.kallsyms]/3ae5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd
>> │   │   └── 84
>> │   │       └── dbd75729adba57cc42f5544b25de571c0c8731 ->
>> ../../[vdso32]/84dbd75729adba57cc42f5544b25de571c0c8731
>> │   ├── [kernel.kallsyms]
>> │   │   └── 3ae5ba6d4e532ad529e43ccf1ce1ddf8a64a4fdd
>> │   ├── [vdso]
>> │   │   └── 84dbd75729adba57cc42f5544b25de571c0c8731
>> │   └── [vdso32]
>> │       └── 84dbd75729adba57cc42f5544b25de571c0c8731
>> ├── lib
>> │   ├── ld-2.22.so
>> │   └── libc-2.22.so
>> ├── tmp
>> │   └── hello
>> └── xxx
>>
>> So all binaries we need are included in the symfs dir. I think
>> this is consistent with your idea explained in previous mails.
>>
>> With this symfs, we do not need buildid dir anymore and what's
>> your idea on 'perf buildid-cache' needs symfs option? after all,
>> that only effects on buildid dir.
>
> I don't understand why dso-prefix option is needed? Why make me type 
> yet more options to the analysis command? Why can't the directory be 
> located under the symfs tree in a known location and populated the 
> same way it is without symfs?
>
>
Because the default buidid folder path is $HOME/.debug/.buildid,
and this $HOME is on the target machine, not the same as $HOME
on the host. Without that option, we need to copy $HOME/.debug/.buildid
to the 'known location in symfs', that's also an extra work.

Re: [PATCH v3 3/7 UPDATE] perf tools: Add option for the path of buildid dsos under symfs

[David Ahern]

On 5/17/16 8:48 PM, Hekuang wrote:
>> I don't understand why dso-prefix option is needed? Why make me type
>> yet more options to the analysis command? Why can't the directory be
>> located under the symfs tree in a known location and populated the
>> same way it is without symfs?
>>
>>
> Because the default buidid folder path is $HOME/.debug/.buildid,
> and this $HOME is on the target machine, not the same as $HOME
> on the host. Without that option, we need to copy $HOME/.debug/.buildid
> to the 'known location in symfs', that's also an extra work.
>

My argument for symfs is that $HOME is not relevant or if it is the path 
is symfs/$HOME. The use case is dealing with countless images -- some 
development, some production. I should be able to nuke the symfs when 
the analysis is done and everything related to it is gone. With the 
$HOME/.debug path it just grows on and on with no real means of pruning it.

If the vdsos are for a particular symfs then why aren't the vdso's under 
it in a known location?

[PATCH v3 4/7] perf tools: Promote proper messages for cross-platform unwind

[He Kuang]

Currently, perf script uses host unwind methods to parse perf.data
callchain info regardless of the target architecture. So we get wrong
result and no promotion when unwinding callchains of x86(32-bit) on
x86(64-bit) machine.

This patch shows proper error messages when we do remote unwind
x86(32-bit) on other machines.

Same thing for other platforms will be added in next patches.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/config/Makefile         |  6 ++++++
 tools/perf/util/thread.c           |  2 ++
 tools/perf/util/unwind-libunwind.c | 33 +++++++++++++++++++++++++++++++++
 tools/perf/util/unwind.h           |  5 +++++
 4 files changed, 46 insertions(+)

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 1e46277..a86b864 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -345,6 +345,12 @@ ifeq ($(ARCH),powerpc)
 endif
 
 ifndef NO_LIBUNWIND
+  ifeq ($(feature-libunwind-x86), 1)
+    LIBUNWIND_LIBS += -lunwind-x86
+    $(call detected,CONFIG_LIBUNWIND_X86)
+    CFLAGS += -DHAVE_LIBUNWIND_X86_SUPPORT
+  endif
+
   ifneq ($(feature-libunwind), 1)
     msg := $(warning No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR);
     NO_LIBUNWIND := 1
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index dfd00c6..244c4f6 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -186,6 +186,8 @@ void thread__insert_map(struct thread *thread, struct map *map)
 {
 	map_groups__fixup_overlappings(thread->mg, map, stderr);
 	map_groups__insert(thread->mg, map);
+
+	unwind__get_arch(thread, map);
 }
 
 static int thread__clone_map_groups(struct thread *thread,
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 63687d3..d464c35 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -680,3 +680,36 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 
 	return get_entries(&ui, cb, arg, max_stack);
 }
+
+void unwind__get_arch(struct thread *thread, struct map *map)
+{
+	char *arch;
+	enum dso_type dso_type;
+
+	if (!thread->mg->machine->env)
+		return;
+
+	dso_type = dso__type(map->dso, thread->mg->machine);
+	if (dso_type == DSO__TYPE_UNKNOWN)
+		return;
+
+	if (thread->addr_space)
+		pr_debug("Thread map already set, 64bit is %d, dso=%s\n",
+			 dso_type == DSO__TYPE_64BIT, map->dso->name);
+
+	arch = thread->mg->machine->env->arch;
+
+	if (!strcmp(arch, "x86_64")
+		   || !strcmp(arch, "x86")
+		   || !strcmp(arch, "i686")) {
+		pr_debug("Thread map is X86, 64bit is %d\n",
+			 dso_type == DSO__TYPE_64BIT);
+		if (dso_type != DSO__TYPE_64BIT)
+#ifdef HAVE_LIBUNWIND_X86_SUPPORT
+			pr_err("target platform=%s is not implemented!\n",
+			       arch);
+#else
+			pr_err("target platform=%s is not supported!\n", arch);
+#endif
+	}
+}
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index 12790cf..889d630 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -24,6 +24,7 @@ int libunwind__arch_reg_id(int regnum);
 int unwind__prepare_access(struct thread *thread);
 void unwind__flush_access(struct thread *thread);
 void unwind__finish_access(struct thread *thread);
+void unwind__get_arch(struct thread *thread, struct map *map);
 #else
 static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
 {
@@ -32,6 +33,8 @@ static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
 
 static inline void unwind__flush_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__finish_access(struct thread *thread __maybe_unused) {}
+static inline void unwind__get_arch(struct thread *thread __maybe_unused,
+				    struct map *map __maybe_unused) {}
 #endif
 #else
 static inline int
@@ -51,5 +54,7 @@ static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
 
 static inline void unwind__flush_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__finish_access(struct thread *thread __maybe_unused) {}
+static inline void unwind__get_arch(struct thread *thread __maybe_unused,
+				    struct map *map __maybe_unused) {}
 #endif /* HAVE_DWARF_UNWIND_SUPPORT */
 #endif /* __UNWIND_H */
-- 
1.8.5.2

[PATCH v3 5/7] perf callchain: Add support for cross-platform unwind

[He Kuang]

Use thread specific unwind ops to unwind cross-platform callchains.

Before this patch, unwind methods is suitable for local unwind, this
patch changes the fixed methods to thread/map related. Each time a map
is inserted, we find the target arch and see if this platform can be
remote unwind. In this patch, we test for x86 platform and only show
proper messages. The real unwind methods are not implemented, will be
introduced in next patch.

Common function used by both local unwind and remote unwind are
separated into new file 'unwind-libunwind_common.c'.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/config/Makefile                |  19 +++++-
 tools/perf/util/Build                     |   3 +-
 tools/perf/util/thread.c                  |   5 +-
 tools/perf/util/thread.h                  |  14 ++++-
 tools/perf/util/unwind-libunwind.c        |  74 +++++++++++-----------
 tools/perf/util/unwind-libunwind_common.c | 101 ++++++++++++++++++++++++++++++
 tools/perf/util/unwind.h                  |  35 ++++++++---
 7 files changed, 197 insertions(+), 54 deletions(-)
 create mode 100644 tools/perf/util/unwind-libunwind_common.c

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index a86b864..16f14b1 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -345,14 +345,24 @@ ifeq ($(ARCH),powerpc)
 endif
 
 ifndef NO_LIBUNWIND
+  have_libunwind =
   ifeq ($(feature-libunwind-x86), 1)
-    LIBUNWIND_LIBS += -lunwind-x86
     $(call detected,CONFIG_LIBUNWIND_X86)
     CFLAGS += -DHAVE_LIBUNWIND_X86_SUPPORT
+    LDFLAGS += -lunwind -lunwind-x86
+    have_libunwind = 1
   endif
 
   ifneq ($(feature-libunwind), 1)
     msg := $(warning No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR);
+    NO_LOCAL_LIBUNWIND := 1
+  else
+    have_libunwind = 1
+    CFLAGS += -DHAVE_LIBUNWIND_LOCAL_SUPPORT
+    $(call detected,CONFIG_LOCAL_LIBUNWIND)
+  endif
+
+  ifneq ($(have_libunwind), 1)
     NO_LIBUNWIND := 1
   endif
 endif
@@ -392,7 +402,7 @@ else
   NO_DWARF_UNWIND := 1
 endif
 
-ifndef NO_LIBUNWIND
+ifndef NO_LOCAL_LIBUNWIND
   ifeq ($(ARCH),$(filter $(ARCH),arm arm64))
     $(call feature_check,libunwind-debug-frame)
     ifneq ($(feature-libunwind-debug-frame), 1)
@@ -403,12 +413,15 @@ ifndef NO_LIBUNWIND
     # non-ARM has no dwarf_find_debug_frame() function:
     CFLAGS += -DNO_LIBUNWIND_DEBUG_FRAME
   endif
-  CFLAGS  += -DHAVE_LIBUNWIND_SUPPORT
   EXTLIBS += $(LIBUNWIND_LIBS)
   CFLAGS  += $(LIBUNWIND_CFLAGS)
   LDFLAGS += $(LIBUNWIND_LDFLAGS)
 endif
 
+ifndef NO_LIBUNWIND
+  CFLAGS  += -DHAVE_LIBUNWIND_SUPPORT
+endif
+
 ifndef NO_LIBAUDIT
   ifneq ($(feature-libaudit), 1)
     msg := $(warning No libaudit.h found, disables 'trace' tool, please install audit-libs-devel or libaudit-dev);
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index ea4ac03..2e21529 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -97,7 +97,8 @@ libperf-$(CONFIG_DWARF) += probe-finder.o
 libperf-$(CONFIG_DWARF) += dwarf-aux.o
 
 libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
-libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind.o
+libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
+libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind_common.o
 
 libperf-$(CONFIG_LIBBABELTRACE) += data-convert-bt.o
 
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 244c4f6..2274263 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -41,9 +41,6 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 		thread->cpu = -1;
 		INIT_LIST_HEAD(&thread->comm_list);
 
-		if (unwind__prepare_access(thread) < 0)
-			goto err_thread;
-
 		comm_str = malloc(32);
 		if (!comm_str)
 			goto err_thread;
@@ -57,6 +54,8 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 		list_add(&comm->list, &thread->comm_list);
 		atomic_set(&thread->refcnt, 1);
 		RB_CLEAR_NODE(&thread->rb_node);
+
+		register_null_unwind_libunwind_ops(thread);
 	}
 
 	return thread;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index e214207..6f2d4cd 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -15,6 +15,17 @@
 
 struct thread_stack;
 
+struct unwind_entry;
+typedef int (*unwind_entry_cb_t)(struct unwind_entry *entry, void *arg);
+struct unwind_libunwind_ops {
+	int (*prepare_access)(struct thread *thread);
+	void (*flush_access)(struct thread *thread);
+	void (*finish_access)(struct thread *thread);
+	int (*get_entries)(unwind_entry_cb_t cb, void *arg,
+			   struct thread *thread,
+			   struct perf_sample *data, int max_stack);
+};
+
 struct thread {
 	union {
 		struct rb_node	 rb_node;
@@ -36,7 +47,8 @@ struct thread {
 	void			*priv;
 	struct thread_stack	*ts;
 #ifdef HAVE_LIBUNWIND_SUPPORT
-	unw_addr_space_t	addr_space;
+	unw_addr_space_t		addr_space;
+	struct unwind_libunwind_ops	*unwind_libunwind_ops;
 #endif
 };
 
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index d464c35..2a8d24e 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -22,6 +22,9 @@
 #include <unistd.h>
 #include <sys/mman.h>
 #include <linux/list.h>
+#ifdef ARCH_UNWIND_LIBUNWIND
+#include "libunwind-arch.h"
+#endif
 #include <libunwind.h>
 #include <libunwind-ptrace.h>
 #include "callchain.h"
@@ -34,6 +37,22 @@
 #include "debug.h"
 #include "asm/bug.h"
 
+#ifndef ARCH_UNWIND_LIBUNWIND
+  #define LIBUNWIND__ARCH_REG_ID libunwind__arch_reg_id
+  #define LOCAL_UNWIND_LIBUNWIND
+  #undef UNWT_OBJ
+  #define UNWT_OBJ(x) _##x
+#else
+  #undef NO_LIBUNWIND_DEBUG_FRAME
+  #if defined(LIBUNWIND_ARM) && !defined(NO_LIBUNWIND_DEBUG_FRAME_ARM)
+  #elif defined(LIBUNWIND_AARCH64) &&                    \
+	  defined(NO_LIBUNWIND_DEBUG_FRAME_ARM_AARCH64)
+  #else
+    #define NO_LIBUNWIND_DEBUG_FRAME
+  #endif
+#endif
+
+
 extern int
 UNW_OBJ(dwarf_search_unwind_table) (unw_addr_space_t as,
 				    unw_word_t ip,
@@ -579,7 +598,7 @@ static unw_accessors_t accessors = {
 	.get_proc_name		= get_proc_name,
 };
 
-int unwind__prepare_access(struct thread *thread)
+static int UNWT_OBJ(_unwind__prepare_access)(struct thread *thread)
 {
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return 0;
@@ -594,7 +613,7 @@ int unwind__prepare_access(struct thread *thread)
 	return 0;
 }
 
-void unwind__flush_access(struct thread *thread)
+static void UNWT_OBJ(_unwind__flush_access)(struct thread *thread)
 {
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return;
@@ -602,7 +621,7 @@ void unwind__flush_access(struct thread *thread)
 	unw_flush_cache(thread->addr_space, 0, 0);
 }
 
-void unwind__finish_access(struct thread *thread)
+static void UNWT_OBJ(_unwind__finish_access)(struct thread *thread)
 {
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return;
@@ -662,9 +681,10 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
 	return ret;
 }
 
-int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
-			struct thread *thread,
-			struct perf_sample *data, int max_stack)
+static int UNWT_OBJ(_unwind__get_entries)(unwind_entry_cb_t cb, void *arg,
+					 struct thread *thread,
+					 struct perf_sample *data,
+					 int max_stack)
 {
 	struct unwind_info ui = {
 		.sample       = data,
@@ -681,35 +701,17 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 	return get_entries(&ui, cb, arg, max_stack);
 }
 
-void unwind__get_arch(struct thread *thread, struct map *map)
-{
-	char *arch;
-	enum dso_type dso_type;
-
-	if (!thread->mg->machine->env)
-		return;
-
-	dso_type = dso__type(map->dso, thread->mg->machine);
-	if (dso_type == DSO__TYPE_UNKNOWN)
-		return;
+struct unwind_libunwind_ops
+UNWT_OBJ(unwind_libunwind_ops) = {
+	.prepare_access = UNWT_OBJ(_unwind__prepare_access),
+	.flush_access   = UNWT_OBJ(_unwind__flush_access),
+	.finish_access  = UNWT_OBJ(_unwind__finish_access),
+	.get_entries    = UNWT_OBJ(_unwind__get_entries),
+};
 
-	if (thread->addr_space)
-		pr_debug("Thread map already set, 64bit is %d, dso=%s\n",
-			 dso_type == DSO__TYPE_64BIT, map->dso->name);
-
-	arch = thread->mg->machine->env->arch;
-
-	if (!strcmp(arch, "x86_64")
-		   || !strcmp(arch, "x86")
-		   || !strcmp(arch, "i686")) {
-		pr_debug("Thread map is X86, 64bit is %d\n",
-			 dso_type == DSO__TYPE_64BIT);
-		if (dso_type != DSO__TYPE_64BIT)
-#ifdef HAVE_LIBUNWIND_X86_SUPPORT
-			pr_err("target platform=%s is not implemented!\n",
-			       arch);
-#else
-			pr_err("target platform=%s is not supported!\n", arch);
-#endif
-	}
+#ifdef LOCAL_UNWIND_LIBUNWIND
+void register_local_unwind_libunwind_ops(struct thread *thread)
+{
+	thread->unwind_libunwind_ops = &UNWT_OBJ(unwind_libunwind_ops);
 }
+#endif
diff --git a/tools/perf/util/unwind-libunwind_common.c b/tools/perf/util/unwind-libunwind_common.c
new file mode 100644
index 0000000..f67707d
--- /dev/null
+++ b/tools/perf/util/unwind-libunwind_common.c
@@ -0,0 +1,101 @@
+#include "thread.h"
+#include "session.h"
+#include "unwind.h"
+#include "symbol.h"
+#include "debug.h"
+
+static int __null__prepare_access(struct thread *thread __maybe_unused)
+{
+	return 0;
+}
+
+static void __null__flush_access(struct thread *thread __maybe_unused)
+{
+}
+
+static void __null__finish_access(struct thread *thread __maybe_unused)
+{
+}
+
+static int __null__get_entries(unwind_entry_cb_t cb __maybe_unused,
+			       void *arg __maybe_unused,
+			       struct thread *thread __maybe_unused,
+			       struct perf_sample *data __maybe_unused,
+			       int max_stack __maybe_unused)
+{
+	return 0;
+}
+
+static struct unwind_libunwind_ops null_unwind_libunwind_ops = {
+	.prepare_access = __null__prepare_access,
+	.flush_access   = __null__flush_access,
+	.finish_access  = __null__finish_access,
+	.get_entries    = __null__get_entries,
+};
+
+void register_null_unwind_libunwind_ops(struct thread *thread)
+{
+	thread->unwind_libunwind_ops = &null_unwind_libunwind_ops;
+	if (thread->mg)
+		pr_err("unwind: target platform=%s unwind unsupported\n",
+		       thread->mg->machine->env->arch);
+}
+
+void register_unwind_libunwind_ops(struct unwind_libunwind_ops *ops,
+				   struct thread *thread)
+{
+	thread->unwind_libunwind_ops = ops;
+}
+
+void unwind__flush_access(struct thread *thread)
+{
+	thread->unwind_libunwind_ops->flush_access(thread);
+}
+
+void unwind__finish_access(struct thread *thread)
+{
+	thread->unwind_libunwind_ops->finish_access(thread);
+}
+
+void unwind__get_arch(struct thread *thread, struct map *map)
+{
+	char *arch;
+	enum dso_type dso_type;
+	int use_local_unwind = 0;
+
+	if (!thread->mg->machine->env)
+		return;
+
+	dso_type = dso__type(map->dso, thread->mg->machine);
+	if (dso_type == DSO__TYPE_UNKNOWN)
+		return;
+
+	if (thread->addr_space)
+		pr_debug("unwind: thread map already set, 64bit is %d, dso=%s\n",
+			 dso_type == DSO__TYPE_64BIT, map->dso->name);
+
+	arch = thread->mg->machine->env->arch;
+
+	if (!strcmp(arch, "x86_64")
+		   || !strcmp(arch, "x86")
+		   || !strcmp(arch, "i686")) {
+		pr_debug("unwind: thread map is X86, 64bit is %d\n",
+			 dso_type == DSO__TYPE_64BIT);
+		if (dso_type != DSO__TYPE_64BIT) {
+#ifdef HAVE_LIBUNWIND_X86_SUPPORT
+			pr_err("unwind: target platform=%s is not implemented\n",
+			       arch);
+#endif
+			register_null_unwind_libunwind_ops(thread);
+		} else
+			use_local_unwind = 1;
+	} else {
+		use_local_unwind = 1;
+	}
+
+	if (use_local_unwind)
+		register_local_unwind_libunwind_ops(thread);
+
+	if (thread->unwind_libunwind_ops->prepare_access(thread) < 0)
+		return;
+}
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index 889d630..e045600 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -21,20 +21,38 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 /* libunwind specific */
 #ifdef HAVE_LIBUNWIND_SUPPORT
 int libunwind__arch_reg_id(int regnum);
-int unwind__prepare_access(struct thread *thread);
 void unwind__flush_access(struct thread *thread);
 void unwind__finish_access(struct thread *thread);
 void unwind__get_arch(struct thread *thread, struct map *map);
-#else
-static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
-{
-	return 0;
+void register_unwind_libunwind_ops(struct unwind_libunwind_ops *ops,
+				   struct thread *thread);
+void register_null_unwind_libunwind_ops(struct thread *thread);
+
+#ifndef HAVE_LIBUNWIND_LOCAL_SUPPORT
+static inline void
+register_local_unwind_libunwind_ops(struct thread *thread) {
+	register_null_unwind_libunwind_ops(thread);
 }
+#else
+void register_local_unwind_libunwind_ops(struct thread *thread);
+#endif
 
+#define unwind__get_entries(cb, arg,					\
+			    thread,					\
+			    data, max_stack)				\
+	thread->unwind_libunwind_ops->get_entries(cb,			\
+						  arg,			\
+						  thread,		\
+						  data,			\
+						  max_stack)
+
+#else
 static inline void unwind__flush_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__finish_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__get_arch(struct thread *thread __maybe_unused,
 				    struct map *map __maybe_unused) {}
+static inline void
+register_null_unwind_libunwind_ops(struct thread *thread __maybe_unused) {}
 #endif
 #else
 static inline int
@@ -47,14 +65,11 @@ unwind__get_entries(unwind_entry_cb_t cb __maybe_unused,
 	return 0;
 }
 
-static inline int unwind__prepare_access(struct thread *thread __maybe_unused)
-{
-	return 0;
-}
-
 static inline void unwind__flush_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__finish_access(struct thread *thread __maybe_unused) {}
 static inline void unwind__get_arch(struct thread *thread __maybe_unused,
 				    struct map *map __maybe_unused) {}
+static inline void
+register_null_unwind_libunwind_ops(struct thread *thread __maybe_unused) {}
 #endif /* HAVE_DWARF_UNWIND_SUPPORT */
 #endif /* __UNWIND_H */
-- 
1.8.5.2

[PATCH v3 6/7] perf callchain: Support x86 target platform

[He Kuang]

Support x86(32-bit) cross platform callchain unwind.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 .../arch/x86/include/libunwind/libunwind-arch.h    | 18 ++++++++++
 tools/perf/arch/x86/util/unwind-libunwind.c        | 42 ++++++++++++++++++++++
 tools/perf/util/Build                              |  6 ++++
 tools/perf/util/unwind-libunwind.c                 |  2 +-
 tools/perf/util/unwind-libunwind_common.c          |  7 ++--
 tools/perf/util/unwind.h                           |  5 +++
 6 files changed, 76 insertions(+), 4 deletions(-)
 create mode 100644 tools/perf/arch/x86/include/libunwind/libunwind-arch.h

diff --git a/tools/perf/arch/x86/include/libunwind/libunwind-arch.h b/tools/perf/arch/x86/include/libunwind/libunwind-arch.h
new file mode 100644
index 0000000..265f14d
--- /dev/null
+++ b/tools/perf/arch/x86/include/libunwind/libunwind-arch.h
@@ -0,0 +1,18 @@
+#ifndef _LIBUNWIND_ARCH_H
+#define _LIBUNWIND_ARCH_H
+
+#include <libunwind-x86.h>
+#include <../perf_regs.h>
+#include <../../../../../../arch/x86/include/uapi/asm/perf_regs.h>
+
+#define LIBUNWIND_X86_32
+int libunwind__x86_reg_id(int regnum);
+
+#include <../../../x86/util/unwind-libunwind.c>
+
+#define LIBUNWIND__ARCH_REG_ID libunwind__x86_reg_id
+
+#define UNWT_PREFIX	UNW_PASTE(UNW_PASTE(_U, x86), _)
+#define UNWT_OBJ(fn)	UNW_PASTE(UNWT_PREFIX, fn)
+
+#endif /* _LIBUNWIND_ARCH_H */
diff --git a/tools/perf/arch/x86/util/unwind-libunwind.c b/tools/perf/arch/x86/util/unwind-libunwind.c
index db25e93..d422fbf 100644
--- a/tools/perf/arch/x86/util/unwind-libunwind.c
+++ b/tools/perf/arch/x86/util/unwind-libunwind.c
@@ -5,6 +5,7 @@
 #include "../../util/unwind.h"
 #include "../../util/debug.h"
 
+#ifndef LIBUNWIND_X86_32
 #ifdef HAVE_ARCH_X86_64_SUPPORT
 int libunwind__arch_reg_id(int regnum)
 {
@@ -110,3 +111,44 @@ int libunwind__arch_reg_id(int regnum)
 	return id;
 }
 #endif /* HAVE_ARCH_X86_64_SUPPORT */
+#else
+int libunwind__x86_reg_id(int regnum)
+{
+	int id;
+
+	switch (regnum) {
+	case UNW_X86_EAX:
+		id = PERF_REG_X86_AX;
+		break;
+	case UNW_X86_EDX:
+		id = PERF_REG_X86_DX;
+		break;
+	case UNW_X86_ECX:
+		id = PERF_REG_X86_CX;
+		break;
+	case UNW_X86_EBX:
+		id = PERF_REG_X86_BX;
+		break;
+	case UNW_X86_ESI:
+		id = PERF_REG_X86_SI;
+		break;
+	case UNW_X86_EDI:
+		id = PERF_REG_X86_DI;
+		break;
+	case UNW_X86_EBP:
+		id = PERF_REG_X86_BP;
+		break;
+	case UNW_X86_ESP:
+		id = PERF_REG_X86_SP;
+		break;
+	case UNW_X86_EIP:
+		id = PERF_REG_X86_IP;
+		break;
+	default:
+		pr_err("unwind: invalid reg id %d\n", regnum);
+		return -EINVAL;
+	}
+
+	return id;
+}
+#endif /* LIBUNWIND_X86_32 */
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 2e21529..2dd3939 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -1,3 +1,5 @@
+include ../scripts/Makefile.include
+
 libperf-y += alias.o
 libperf-y += annotate.o
 libperf-y += build-id.o
@@ -99,6 +101,10 @@ libperf-$(CONFIG_DWARF) += dwarf-aux.o
 libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
 libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
 libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind_common.o
+libperf-$(CONFIG_LIBUNWIND_X86)      += unwind-libunwind_x86_32.o
+
+$(OUTPUT)util/unwind-libunwind_x86_32.o: util/unwind-libunwind.c arch/x86/util/unwind-libunwind.c
+	$(QUIET_CC)$(CC) $(CFLAGS) -DARCH_UNWIND_LIBUNWIND -Iarch/x86/include/libunwind -c -o $@ util/unwind-libunwind.c
 
 libperf-$(CONFIG_LIBBABELTRACE) += data-convert-bt.o
 
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 2a8d24e..1844431 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -527,7 +527,7 @@ static int access_reg(unw_addr_space_t __maybe_unused as,
 		return 0;
 	}
 
-	id = libunwind__arch_reg_id(regnum);
+	id = LIBUNWIND__ARCH_REG_ID(regnum);
 	if (id < 0)
 		return -EINVAL;
 
diff --git a/tools/perf/util/unwind-libunwind_common.c b/tools/perf/util/unwind-libunwind_common.c
index f67707d..dbc44b1 100644
--- a/tools/perf/util/unwind-libunwind_common.c
+++ b/tools/perf/util/unwind-libunwind_common.c
@@ -83,10 +83,11 @@ void unwind__get_arch(struct thread *thread, struct map *map)
 			 dso_type == DSO__TYPE_64BIT);
 		if (dso_type != DSO__TYPE_64BIT) {
 #ifdef HAVE_LIBUNWIND_X86_SUPPORT
-			pr_err("unwind: target platform=%s is not implemented\n",
-			       arch);
-#endif
+			register_unwind_libunwind_ops(
+				&_Ux86_unwind_libunwind_ops, thread);
+#else
 			register_null_unwind_libunwind_ops(thread);
+#endif
 		} else
 			use_local_unwind = 1;
 	} else {
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index e045600..8f7c4357 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -54,6 +54,11 @@ static inline void unwind__get_arch(struct thread *thread __maybe_unused,
 static inline void
 register_null_unwind_libunwind_ops(struct thread *thread __maybe_unused) {}
 #endif
+
+#ifdef HAVE_LIBUNWIND_X86_SUPPORT
+extern struct unwind_libunwind_ops _Ux86_unwind_libunwind_ops;
+#endif
+
 #else
 static inline int
 unwind__get_entries(unwind_entry_cb_t cb __maybe_unused,
-- 
1.8.5.2

[PATCH v3 7/7] perf callchain: Support aarch64 cross-platform

[He Kuang]

Support aarch64 cross platform callchain unwind.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 .../perf/arch/arm64/include/libunwind/libunwind-arch.h | 18 ++++++++++++++++++
 tools/perf/arch/arm64/util/unwind-libunwind.c          |  5 ++++-
 tools/perf/config/Makefile                             | 12 ++++++++++++
 tools/perf/util/Build                                  |  4 ++++
 tools/perf/util/unwind-libunwind_common.c              | 12 ++++++++++++
 tools/perf/util/unwind.h                               |  3 +++
 6 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/arch/arm64/include/libunwind/libunwind-arch.h

diff --git a/tools/perf/arch/arm64/include/libunwind/libunwind-arch.h b/tools/perf/arch/arm64/include/libunwind/libunwind-arch.h
new file mode 100644
index 0000000..7bc8a00
--- /dev/null
+++ b/tools/perf/arch/arm64/include/libunwind/libunwind-arch.h
@@ -0,0 +1,18 @@
+#ifndef _LIBUNWIND_ARCH_H
+#define _LIBUNWIND_ARCH_H
+
+#include <libunwind-aarch64.h>
+#include <../perf_regs.h>
+#include <../../../../../../arch/arm64/include/uapi/asm/perf_regs.h>
+
+#define LIBUNWIND_AARCH64
+int libunwind__aarch64_reg_id(int regnum);
+
+#include <../../../arm64/util/unwind-libunwind.c>
+
+#define LIBUNWIND__ARCH_REG_ID libunwind__aarch64_reg_id
+
+#define UNWT_PREFIX	UNW_PASTE(UNW_PASTE(_U, aarch64), _)
+#define UNWT_OBJ(fn)	UNW_PASTE(UNWT_PREFIX, fn)
+
+#endif /* _LIBUNWIND_ARCH_H */
diff --git a/tools/perf/arch/arm64/util/unwind-libunwind.c b/tools/perf/arch/arm64/util/unwind-libunwind.c
index a87afa9..5b557a5 100644
--- a/tools/perf/arch/arm64/util/unwind-libunwind.c
+++ b/tools/perf/arch/arm64/util/unwind-libunwind.c
@@ -1,11 +1,14 @@
 
 #include <errno.h>
-#include <libunwind.h>
 #include "perf_regs.h"
 #include "../../util/unwind.h"
 #include "../../util/debug.h"
 
+#ifndef LIBUNWIND_AARCH64
 int libunwind__arch_reg_id(int regnum)
+#else
+int libunwind__aarch64_reg_id(int regnum)
+#endif
 {
 	switch (regnum) {
 	case UNW_AARCH64_X0:
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 16f14b1..c1bfc5e 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -353,6 +353,18 @@ ifndef NO_LIBUNWIND
     have_libunwind = 1
   endif
 
+  ifeq ($(feature-libunwind-aarch64), 1)
+    $(call detected,CONFIG_LIBUNWIND_AARCH64)
+    CFLAGS += -DHAVE_LIBUNWIND_AARCH64_SUPPORT
+    LDFLAGS += -lunwind -lunwind-aarch64
+    have_libunwind = 1
+    $(call feature_check,libunwind-debug-frame-aarch64)
+    ifneq ($(feature-libunwind-debug-frame-aarch64), 1)
+      msg := $(warning No debug_frame support found in libunwind-aarch64);
+      CFLAGS += -DNO_LIBUNWIND_DEBUG_FRAME_AARCH64
+    endif
+  endif
+
   ifneq ($(feature-libunwind), 1)
     msg := $(warning No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR);
     NO_LOCAL_LIBUNWIND := 1
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 2dd3939..10e42ad 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -102,10 +102,14 @@ libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
 libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
 libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind_common.o
 libperf-$(CONFIG_LIBUNWIND_X86)      += unwind-libunwind_x86_32.o
+libperf-$(CONFIG_LIBUNWIND_AARCH64)  += unwind-libunwind_arm64.o
 
 $(OUTPUT)util/unwind-libunwind_x86_32.o: util/unwind-libunwind.c arch/x86/util/unwind-libunwind.c
 	$(QUIET_CC)$(CC) $(CFLAGS) -DARCH_UNWIND_LIBUNWIND -Iarch/x86/include/libunwind -c -o $@ util/unwind-libunwind.c
 
+$(OUTPUT)util/unwind-libunwind_arm64.o: util/unwind-libunwind.c arch/arm64/util/unwind-libunwind.c
+	$(QUIET_CC)$(CC) $(CFLAGS) -DARCH_UNWIND_LIBUNWIND -Iarch/arm64/include/libunwind -c -o $@ util/unwind-libunwind.c
+
 libperf-$(CONFIG_LIBBABELTRACE) += data-convert-bt.o
 
 libperf-y += scripting-engines/
diff --git a/tools/perf/util/unwind-libunwind_common.c b/tools/perf/util/unwind-libunwind_common.c
index dbc44b1..14a209d 100644
--- a/tools/perf/util/unwind-libunwind_common.c
+++ b/tools/perf/util/unwind-libunwind_common.c
@@ -90,6 +90,18 @@ void unwind__get_arch(struct thread *thread, struct map *map)
 #endif
 		} else
 			use_local_unwind = 1;
+	} else if (!strcmp(arch, "aarch64") || !strncmp(arch, "arm", 3)) {
+		pr_debug("Thread map is ARM, 64bit is %d, dso=%s\n",
+			 dso_type == DSO__TYPE_64BIT, map->dso->name);
+		if (dso_type == DSO__TYPE_64BIT) {
+#ifdef HAVE_LIBUNWIND_AARCH64_SUPPORT
+			register_unwind_libunwind_ops(
+				&_Uaarch64_unwind_libunwind_ops, thread);
+#else
+			register_null_unwind_libunwind_ops(thread);
+#endif
+		} else
+			use_local_unwind = 1;
 	} else {
 		use_local_unwind = 1;
 	}
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index 8f7c4357..741a53f 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -58,6 +58,9 @@ register_null_unwind_libunwind_ops(struct thread *thread __maybe_unused) {}
 #ifdef HAVE_LIBUNWIND_X86_SUPPORT
 extern struct unwind_libunwind_ops _Ux86_unwind_libunwind_ops;
 #endif
+#ifdef HAVE_LIBUNWIND_AARCH64_SUPPORT
+extern struct unwind_libunwind_ops _Uaarch64_unwind_libunwind_ops;
+#endif
 
 #else
 static inline int
-- 
1.8.5.2

[PATCH 0/5] Fixes on perf unwind

[He Kuang]

hi,

Patch 1-3 fix wrongly used PERF_REG_SP/IP by redefining
those macros in the wrapper file according to the target platform, for
example in "util/libunwind/x86_32.c".
The first 3 patches have been acked-by "Jiri Olsa" and not touched.

Patch 4 catches an error on python build_ext build.

Patch 5 fixes a NULL pointer deference which can cause segfault when
the desired dso is not found.

Thank you.

He Kuang (5):
  perf unwind: Change macro names of perf register
  perf unwind: Fix wrongly used regs for x86_32 unwind
  perf unwind: Fix wrongly used regs for aarch64 unwind
  perf tools: Let python use correct gcc for build_ext
  perf tools: Fix NULL pointer deference when vdso not found

 tools/perf/Makefile.perf                 | 3 ++-
 tools/perf/util/libunwind/arm64.c        | 5 +++++
 tools/perf/util/libunwind/x86_32.c       | 6 ++++++
 tools/perf/util/unwind-libunwind-local.c | 6 ++++--
 tools/perf/util/unwind.h                 | 9 +++++++++
 tools/perf/util/vdso.c                   | 2 +-
 6 files changed, 27 insertions(+), 4 deletions(-)

-- 
1.8.5.2

[PATCH 1/5] perf unwind: Change macro names of perf register

[He Kuang]

Use macro name prefixed with "LIBUNWIND_ARCH" for better understanding
that the regs used by callbacks of libunwind are arch specific. The
real regs used should be defined in the wrapper file of
"unwind-libunwind-local.c" for each supported arch.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/unwind-libunwind-local.c | 6 ++++--
 tools/perf/util/unwind.h                 | 9 +++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/unwind-libunwind-local.c b/tools/perf/util/unwind-libunwind-local.c
index 01c2e86..97c0f8f 100644
--- a/tools/perf/util/unwind-libunwind-local.c
+++ b/tools/perf/util/unwind-libunwind-local.c
@@ -462,7 +462,8 @@ static int access_mem(unw_addr_space_t __maybe_unused as,
 		return 0;
 	}
 
-	ret = perf_reg_value(&start, &ui->sample->user_regs, PERF_REG_SP);
+	ret = perf_reg_value(&start, &ui->sample->user_regs,
+			     LIBUNWIND__ARCH_REG_SP);
 	if (ret)
 		return ret;
 
@@ -621,7 +622,8 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
 	unw_cursor_t c;
 	int ret, i = 0;
 
-	ret = perf_reg_value(&val, &ui->sample->user_regs, PERF_REG_IP);
+	ret = perf_reg_value(&val, &ui->sample->user_regs,
+			     LIBUNWIND__ARCH_REG_IP);
 	if (ret)
 		return ret;
 
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index b074662..84c6d44 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -32,6 +32,15 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 #ifndef LIBUNWIND__ARCH_REG_ID
 #define LIBUNWIND__ARCH_REG_ID(regnum) libunwind__arch_reg_id(regnum)
 #endif
+
+#ifndef LIBUNWIND__ARCH_REG_SP
+#define LIBUNWIND__ARCH_REG_SP PERF_REG_SP
+#endif
+
+#ifndef LIBUNWIND__ARCH_REG_IP
+#define LIBUNWIND__ARCH_REG_IP PERF_REG_IP
+#endif
+
 int LIBUNWIND__ARCH_REG_ID(int regnum);
 int unwind__prepare_access(struct thread *thread, struct map *map);
 void unwind__flush_access(struct thread *thread);
-- 
1.8.5.2

[tip:perf/core] perf unwind: Change macro names of perf register

[tip-bot for He Kuang]

Commit-ID:  78ff1d6d8bf6bb3ee2b3781bbd88355a322435a4
Gitweb:     http://git.kernel.org/tip/78ff1d6d8bf6bb3ee2b3781bbd88355a322435a4
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Wed, 22 Jun 2016 06:57:02 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 23 Jun 2016 10:30:17 -0300

perf unwind: Change macro names of perf register

Use macro name prefixed with "LIBUNWIND_ARCH" for better understanding
that the regs used by callbacks of libunwind are arch specific. The real
regs used should be defined in the wrapper file of
"unwind-libunwind-local.c" for each supported arch.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1466578626-92406-2-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/unwind-libunwind-local.c | 6 ++++--
 tools/perf/util/unwind.h                 | 9 +++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/unwind-libunwind-local.c b/tools/perf/util/unwind-libunwind-local.c
index 01c2e86..97c0f8f 100644
--- a/tools/perf/util/unwind-libunwind-local.c
+++ b/tools/perf/util/unwind-libunwind-local.c
@@ -462,7 +462,8 @@ static int access_mem(unw_addr_space_t __maybe_unused as,
 		return 0;
 	}
 
-	ret = perf_reg_value(&start, &ui->sample->user_regs, PERF_REG_SP);
+	ret = perf_reg_value(&start, &ui->sample->user_regs,
+			     LIBUNWIND__ARCH_REG_SP);
 	if (ret)
 		return ret;
 
@@ -621,7 +622,8 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
 	unw_cursor_t c;
 	int ret, i = 0;
 
-	ret = perf_reg_value(&val, &ui->sample->user_regs, PERF_REG_IP);
+	ret = perf_reg_value(&val, &ui->sample->user_regs,
+			     LIBUNWIND__ARCH_REG_IP);
 	if (ret)
 		return ret;
 
diff --git a/tools/perf/util/unwind.h b/tools/perf/util/unwind.h
index b074662..84c6d44 100644
--- a/tools/perf/util/unwind.h
+++ b/tools/perf/util/unwind.h
@@ -32,6 +32,15 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
 #ifndef LIBUNWIND__ARCH_REG_ID
 #define LIBUNWIND__ARCH_REG_ID(regnum) libunwind__arch_reg_id(regnum)
 #endif
+
+#ifndef LIBUNWIND__ARCH_REG_SP
+#define LIBUNWIND__ARCH_REG_SP PERF_REG_SP
+#endif
+
+#ifndef LIBUNWIND__ARCH_REG_IP
+#define LIBUNWIND__ARCH_REG_IP PERF_REG_IP
+#endif
+
 int LIBUNWIND__ARCH_REG_ID(int regnum);
 int unwind__prepare_access(struct thread *thread, struct map *map);
 void unwind__flush_access(struct thread *thread);

[PATCH 2/5] perf unwind: Fix wrongly used regs for x86_32 unwind

[He Kuang]

By default, "unwind-libunwind-local.c" gets SP/IP register number
according to the host platform, for remote unwind, we should use
register number for target platform. Fix this by define
LIBUNWIND_ARCH_REG_SP/IP in the wrapper file of x86_32 platform.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/libunwind/x86_32.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/libunwind/x86_32.c b/tools/perf/util/libunwind/x86_32.c
index d98c17e..957ffff 100644
--- a/tools/perf/util/libunwind/x86_32.c
+++ b/tools/perf/util/libunwind/x86_32.c
@@ -12,7 +12,13 @@
  */
 
 #define REMOTE_UNWIND_LIBUNWIND
+
+/* Define arch specific functions & regs for libunwind, should be
+ * defined before including "unwind.h"
+ */
 #define LIBUNWIND__ARCH_REG_ID(regnum) libunwind__x86_reg_id(regnum)
+#define LIBUNWIND__ARCH_REG_IP PERF_REG_X86_IP
+#define LIBUNWIND__ARCH_REG_SP PERF_REG_X86_SP
 
 #include "unwind.h"
 #include "debug.h"
-- 
1.8.5.2

[tip:perf/core] perf unwind: Fix wrongly used regs for x86_32 unwind

[tip-bot for He Kuang]

Commit-ID:  5dafea097ac65bd01cc86801c399ae41dce79756
Gitweb:     http://git.kernel.org/tip/5dafea097ac65bd01cc86801c399ae41dce79756
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Wed, 22 Jun 2016 06:57:03 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 23 Jun 2016 10:30:21 -0300

perf unwind: Fix wrongly used regs for x86_32 unwind

By default, "unwind-libunwind-local.c" gets SP/IP register number
according to the host platform, for remote unwind, we should use
register number for target platform. Fix this by define
LIBUNWIND_ARCH_REG_SP/IP in the wrapper file of x86_32 platform.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1466578626-92406-3-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/libunwind/x86_32.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/libunwind/x86_32.c b/tools/perf/util/libunwind/x86_32.c
index d98c17e..957ffff 100644
--- a/tools/perf/util/libunwind/x86_32.c
+++ b/tools/perf/util/libunwind/x86_32.c
@@ -12,7 +12,13 @@
  */
 
 #define REMOTE_UNWIND_LIBUNWIND
+
+/* Define arch specific functions & regs for libunwind, should be
+ * defined before including "unwind.h"
+ */
 #define LIBUNWIND__ARCH_REG_ID(regnum) libunwind__x86_reg_id(regnum)
+#define LIBUNWIND__ARCH_REG_IP PERF_REG_X86_IP
+#define LIBUNWIND__ARCH_REG_SP PERF_REG_X86_SP
 
 #include "unwind.h"
 #include "debug.h"

[PATCH 3/5] perf unwind: Fix wrongly used regs for aarch64 unwind

[He Kuang]

By default, "unwind-libunwind-local.c" gets SP/IP register number
according to the host platform, for remote unwind, we should use
register number for target platform. Fix this by define
LIBUNWIND_ARCH_REG_SP/IP in the wrapper file of aarch64 platform.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/libunwind/arm64.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/libunwind/arm64.c b/tools/perf/util/libunwind/arm64.c
index 4fb5395..6559bc5 100644
--- a/tools/perf/util/libunwind/arm64.c
+++ b/tools/perf/util/libunwind/arm64.c
@@ -13,7 +13,12 @@
 
 #define REMOTE_UNWIND_LIBUNWIND
 
+/* Define arch specific functions & regs for libunwind, should be
+ * defined before including "unwind.h"
+ */
 #define LIBUNWIND__ARCH_REG_ID(regnum) libunwind__arm64_reg_id(regnum)
+#define LIBUNWIND__ARCH_REG_IP PERF_REG_ARM64_PC
+#define LIBUNWIND__ARCH_REG_SP PERF_REG_ARM64_SP
 
 #include "unwind.h"
 #include "debug.h"
-- 
1.8.5.2

[tip:perf/core] perf unwind: Fix wrongly used regs for aarch64 unwind

[tip-bot for He Kuang]

Commit-ID:  3bd03c9583bfb22cb82eeb09d8445bb79d27ae78
Gitweb:     http://git.kernel.org/tip/3bd03c9583bfb22cb82eeb09d8445bb79d27ae78
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Wed, 22 Jun 2016 06:57:04 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 23 Jun 2016 10:30:31 -0300

perf unwind: Fix wrongly used regs for aarch64 unwind

By default, "unwind-libunwind-local.c" gets SP/IP register number
according to the host platform, for remote unwind, we should use
register number for target platform. Fix this by define
LIBUNWIND_ARCH_REG_SP/IP in the wrapper file of aarch64 platform.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1466578626-92406-4-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/libunwind/arm64.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/libunwind/arm64.c b/tools/perf/util/libunwind/arm64.c
index 4fb5395..6559bc5 100644
--- a/tools/perf/util/libunwind/arm64.c
+++ b/tools/perf/util/libunwind/arm64.c
@@ -13,7 +13,12 @@
 
 #define REMOTE_UNWIND_LIBUNWIND
 
+/* Define arch specific functions & regs for libunwind, should be
+ * defined before including "unwind.h"
+ */
 #define LIBUNWIND__ARCH_REG_ID(regnum) libunwind__arm64_reg_id(regnum)
+#define LIBUNWIND__ARCH_REG_IP PERF_REG_ARM64_PC
+#define LIBUNWIND__ARCH_REG_SP PERF_REG_ARM64_SP
 
 #include "unwind.h"
 #include "debug.h"

[PATCH 4/5] perf tools: Let python use correct gcc for build_ext

[He Kuang]

Currently, python uses host gcc instead of cross-compile gcc in the
last step of compiling build_ext(remove '--quiet' to show verbose):

  cross-gcc ...
  cross-gcc ...
  creating ~/out/python_ext_build/lib
  gcc -pthread -shared -Wl,-z ...

This is wrong but may not cause any errors unless the features detected
by cross-compiler do not match those for host compiler, and causes the
following errors:

  /usr/lib64/gcc/bin/ld: cannot find -lunwind-x86
  collect2: error: ld returned 1 exit status
  error: command 'gcc' failed with exit status 1
  cp: cannot stat ‘~/out/python_ext_build/lib/perf.so’: No such file or directory
  Makefile.perf:257: recipe for target '~/out/python/perf.so' failed
  make[1]: *** [~/out/python/perf.so] Error 1
  Makefile:68: recipe for target 'all' failed
  make: *** [all] Error 2

This issue is also reported and anwsered on stackoverflow.
Link: http://stackoverflow.com/questions/5986256/python-distutils-gcc-path

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/Makefile.perf | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index bde8cba..d0a2cb1 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -254,7 +254,8 @@ PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
 PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBAPI)
 
 $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS) $(LIBTRACEEVENT_DYNAMIC_LIST)
-	$(QUIET_GEN)CFLAGS='$(CFLAGS)' LDFLAGS='$(LDFLAGS) $(LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS)' \
+	$(QUIET_GEN)LDSHARED="$(CC) -pthread -shared" \
+        CFLAGS='$(CFLAGS)' LDFLAGS='$(LDFLAGS) $(LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS)' \
 	  $(PYTHON_WORD) util/setup.py \
 	  --quiet build_ext; \
 	mkdir -p $(OUTPUT)python && \
-- 
1.8.5.2

[tip:perf/core] perf tools: Let python use correct gcc for build_ext

[tip-bot for He Kuang]

Commit-ID:  48d8d5db4ac454e590ef7d440f456743d6cbaa94
Gitweb:     http://git.kernel.org/tip/48d8d5db4ac454e590ef7d440f456743d6cbaa94
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Wed, 22 Jun 2016 06:57:05 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 22 Jun 2016 16:11:42 -0300

perf tools: Let python use correct gcc for build_ext

Currently, python uses host gcc instead of cross-compile gcc in the last
step of compiling build_ext(remove '--quiet' to show verbose):

  cross-gcc ...
  cross-gcc ...
  creating ~/out/python_ext_build/lib
  gcc -pthread -shared -Wl,-z ...

This is wrong but may not cause any errors unless the features detected
by cross-compiler do not match those for host compiler, and causes the
following errors:

  /usr/lib64/gcc/bin/ld: cannot find -lunwind-x86
  collect2: error: ld returned 1 exit status
  error: command 'gcc' failed with exit status 1
  cp: cannot stat ‘~/out/python_ext_build/lib/perf.so’: No such file or directory
  Makefile.perf:257: recipe for target '~/out/python/perf.so' failed
  make[1]: *** [~/out/python/perf.so] Error 1
  Makefile:68: recipe for target 'all' failed
  make: *** [all] Error 2

This issue is also reported and anwsered on stackoverflow.
Link: http://stackoverflow.com/questions/5986256/python-distutils-gcc-path

Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1466578626-92406-5-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile.perf | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index bde8cba..d0a2cb1 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -254,7 +254,8 @@ PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
 PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBAPI)
 
 $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS) $(LIBTRACEEVENT_DYNAMIC_LIST)
-	$(QUIET_GEN)CFLAGS='$(CFLAGS)' LDFLAGS='$(LDFLAGS) $(LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS)' \
+	$(QUIET_GEN)LDSHARED="$(CC) -pthread -shared" \
+        CFLAGS='$(CFLAGS)' LDFLAGS='$(LDFLAGS) $(LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS)' \
 	  $(PYTHON_WORD) util/setup.py \
 	  --quiet build_ext; \
 	mkdir -p $(OUTPUT)python && \

[PATCH 5/5] perf tools: Fix NULL pointer deference when vdso not found

[He Kuang]

We should check if 'dso' is a null pointer before passing it to the
function dso__type(), otherwise a segfault will be raised in
dso__data_get_fd(). In function machine__find_vdso(), the return value
checking of 'dso' is missed and this patch fixes this issue.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/util/vdso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
index 8f81c41..7bdcad4 100644
--- a/tools/perf/util/vdso.c
+++ b/tools/perf/util/vdso.c
@@ -296,7 +296,7 @@ static struct dso *machine__find_vdso(struct machine *machine,
 		if (!dso) {
 			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
 					   true);
-			if (dso_type != dso__type(dso, machine))
+			if (dso && dso_type != dso__type(dso, machine))
 				dso = NULL;
 		}
 		break;
-- 
1.8.5.2

Re: [PATCH 5/5] perf tools: Fix NULL pointer deference when vdso not found

[Wangnan (F)]

Hi,

This patch fixes a real crash problem when we do 'perf report'
on an arm64 platform with arm32 program.
It is introduced by commit f9b2bdf228 ("perf tools: Find vdso
with the consider of cross-platform"). From dmesg report, perf
crashes in dso__type() because dso is NULL.

Still don't know why on x86 it never crash, but it is obviously
that we need to check the return vaule from __dso__find(): it can
be NULL.

So please consider pulling.

Thank you.

On 2016/6/22 14:57, He Kuang wrote:
> We should check if 'dso' is a null pointer before passing it to the
> function dso__type(), otherwise a segfault will be raised in
> dso__data_get_fd(). In function machine__find_vdso(), the return value
> checking of 'dso' is missed and this patch fixes this issue.
>
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>   tools/perf/util/vdso.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> index 8f81c41..7bdcad4 100644
> --- a/tools/perf/util/vdso.c
> +++ b/tools/perf/util/vdso.c
> @@ -296,7 +296,7 @@ static struct dso *machine__find_vdso(struct machine *machine,
>   		if (!dso) {
>   			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
>   					   true);
> -			if (dso_type != dso__type(dso, machine))
> +			if (dso && dso_type != dso__type(dso, machine))
>   				dso = NULL;
>   		}
>   		break;

Re: [PATCH 5/5] perf tools: Fix NULL pointer deference when vdso not found

[Hekuang]

在 2016/6/23 10:02, Wangnan (F) 写道:
> Hi,
>
> This patch fixes a real crash problem when we do 'perf report'
> on an arm64 platform with arm32 program.
> It is introduced by commit f9b2bdf228 ("perf tools: Find vdso
> with the consider of cross-platform"). From dmesg report, perf
> crashes in dso__type() because dso is NULL.
>
> Still don't know why on x86 it never crash, but it is obviously

This is because the fault only occured while
dso_type==DSO__TYPE_32BIT, run 64bit executable won't enter the
fault branch, but if we run a 32bit executable on x86_64, this
bug can be reproduced easily.

   # file ~/hello
   ~/hello: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), 
dynamically linked (uses shared libs), for GNU/Linux 4.5.0, not stripped

   # perf record -g hello
   Segmentation fault

Thank you.

> that we need to check the return vaule from __dso__find(): it can
> be NULL.
>
> So please consider pulling.
>
> Thank you.
>
> On 2016/6/22 14:57, He Kuang wrote:
>> We should check if 'dso' is a null pointer before passing it to the
>> function dso__type(), otherwise a segfault will be raised in
>> dso__data_get_fd(). In function machine__find_vdso(), the return value
>> checking of 'dso' is missed and this patch fixes this issue.
>>
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> ---
>>   tools/perf/util/vdso.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
>> index 8f81c41..7bdcad4 100644
>> --- a/tools/perf/util/vdso.c
>> +++ b/tools/perf/util/vdso.c
>> @@ -296,7 +296,7 @@ static struct dso *machine__find_vdso(struct 
>> machine *machine,
>>           if (!dso) {
>>               dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
>>                          true);
>> -            if (dso_type != dso__type(dso, machine))
>> +            if (dso && dso_type != dso__type(dso, machine))
>>                   dso = NULL;
>>           }
>>           break;
>
>
>

Re: [PATCH 5/5] perf tools: Fix NULL pointer deference when vdso not found

[Arnaldo Carvalho de Melo]

Em Thu, Jun 23, 2016 at 10:02:39AM +0800, Wangnan (F) escreveu:
> Hi,
> 
> This patch fixes a real crash problem when we do 'perf report'
> on an arm64 platform with arm32 program.
> It is introduced by commit f9b2bdf228 ("perf tools: Find vdso
> with the consider of cross-platform"). From dmesg report, perf
> crashes in dso__type() because dso is NULL.

Ok, I removed f9b2bdf228 because of this crash, Ingo reported it, now
that I see this new patch and checked that the crash was introduced by
f9b2bdf228 I'll just combine those two and test again using a 32-bit
hackbench on top of f9b2bdf228, to reproduce the crash, then on top of
both patches combined.

- Arnaldo
 
> Still don't know why on x86 it never crash, but it is obviously
> that we need to check the return vaule from __dso__find(): it can
> be NULL.
> 
> So please consider pulling.
> 
> Thank you.
> 
> On 2016/6/22 14:57, He Kuang wrote:
> > We should check if 'dso' is a null pointer before passing it to the
> > function dso__type(), otherwise a segfault will be raised in
> > dso__data_get_fd(). In function machine__find_vdso(), the return value
> > checking of 'dso' is missed and this patch fixes this issue.
> > 
> > Signed-off-by: He Kuang <hekuang@huawei.com>
> > ---
> >   tools/perf/util/vdso.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> > index 8f81c41..7bdcad4 100644
> > --- a/tools/perf/util/vdso.c
> > +++ b/tools/perf/util/vdso.c
> > @@ -296,7 +296,7 @@ static struct dso *machine__find_vdso(struct machine *machine,
> >   		if (!dso) {
> >   			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> >   					   true);
> > -			if (dso_type != dso__type(dso, machine))
> > +			if (dso && dso_type != dso__type(dso, machine))
> >   				dso = NULL;
> >   		}
> >   		break;
> 

Re: [PATCH 5/5] perf tools: Fix NULL pointer deference when vdso not found

[Arnaldo Carvalho de Melo]

Em Thu, Jun 23, 2016 at 09:07:04AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jun 23, 2016 at 10:02:39AM +0800, Wangnan (F) escreveu:
> > Hi,
> > 
> > This patch fixes a real crash problem when we do 'perf report'
> > on an arm64 platform with arm32 program.
> > It is introduced by commit f9b2bdf228 ("perf tools: Find vdso
> > with the consider of cross-platform"). From dmesg report, perf
> > crashes in dso__type() because dso is NULL.
> 
> Ok, I removed f9b2bdf228 because of this crash, Ingo reported it, now
> that I see this new patch and checked that the crash was introduced by
> f9b2bdf228 I'll just combine those two and test again using a 32-bit
> hackbench on top of f9b2bdf228, to reproduce the crash, then on top of
> both patches combined.

Tested, reproduced, ammended, fixed, applied, thanks!

- Arnaldo
 
> - Arnaldo
>  
> > Still don't know why on x86 it never crash, but it is obviously
> > that we need to check the return vaule from __dso__find(): it can
> > be NULL.
> > 
> > So please consider pulling.
> > 
> > Thank you.
> > 
> > On 2016/6/22 14:57, He Kuang wrote:
> > > We should check if 'dso' is a null pointer before passing it to the
> > > function dso__type(), otherwise a segfault will be raised in
> > > dso__data_get_fd(). In function machine__find_vdso(), the return value
> > > checking of 'dso' is missed and this patch fixes this issue.
> > > 
> > > Signed-off-by: He Kuang <hekuang@huawei.com>
> > > ---
> > >   tools/perf/util/vdso.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> > > index 8f81c41..7bdcad4 100644
> > > --- a/tools/perf/util/vdso.c
> > > +++ b/tools/perf/util/vdso.c
> > > @@ -296,7 +296,7 @@ static struct dso *machine__find_vdso(struct machine *machine,
> > >   		if (!dso) {
> > >   			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> > >   					   true);
> > > -			if (dso_type != dso__type(dso, machine))
> > > +			if (dso && dso_type != dso__type(dso, machine))
> > >   				dso = NULL;
> > >   		}
> > >   		break;
> > 

[tip:perf/core] perf tools: Find right DSO taking into account if binary is 32 or 64-bit

[tip-bot for He Kuang]

Commit-ID:  76c588f1f6b560c510953b390bc0a26c27cbfbd0
Gitweb:     http://git.kernel.org/tip/76c588f1f6b560c510953b390bc0a26c27cbfbd0
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Tue, 17 May 2016 09:04:54 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 23 Jun 2016 10:25:58 -0300

perf tools: Find right DSO taking into account if binary is 32 or 64-bit

There's a problem in machine__findnew_vdso(), vdso buildid generated by a
32-bit machine stores it with the name 'vdso', but when processing buildid on a
64-bit machine with the same 'perf.data', perf will search for vdso named as
'vdso32' and get failed.

This patch tries to find the existing dsos in machine->dsos by thread dso_type.
64-bit thread tries to find vdso with name 'vdso', because all 64-bit vdso is
named as that. 32-bit thread first tries to find vdso with name 'vdso32' if
this thread was run on 64-bit machine, if failed, then it tries 'vdso' which
indicates that the thread was run on 32-bit machine when recording.

Committer note:

Additional explanation by Adrian Hunter:

We match maps to builds ids using the file name - consider
machine__findnew_[v]dso() called in map__new().  So in the context of a perf
data file, we consider the file name to be unique.

A vdso map does not have a file name - all we know is that it is vdso.  We look
at the thread to tell if it is 32-bit, 64-bit or x32.  Then we need to get the
build id which has been recorded using short name "[vdso]" or "[vdso32]" or
"[vdsox32]".

The problem is that on a 32-bit machine, we use the name "[vdso]".  If you take
a 32-bit perf data file to a 64-bit machine, it gets hard to figure out if
"[vdso]" is 32-bit or 64-bit.

This patch solves that problem.

 ----

This also merges a followup patch fixing a problem introduced by the
original submission of this patch, that would crash 'perf record' when
recording samples for a 32-bit app on a 64-bit system.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1463475894-163531-1-git-send-email-hekuang@huawei.com
Link: http://lkml.kernel.org/r/1466578626-92406-6-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
index 44d440d..7bdcad4 100644
--- a/tools/perf/util/vdso.c
+++ b/tools/perf/util/vdso.c
@@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
 	return dso;
 }
 
-#if BITS_PER_LONG == 64
-
 static enum dso_type machine__thread_dso_type(struct machine *machine,
 					      struct thread *thread)
 {
@@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
 	return dso_type;
 }
 
+#if BITS_PER_LONG == 64
+
 static int vdso__do_copy_compat(FILE *f, int fd)
 {
 	char buf[4096];
@@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
 
 #endif
 
+static struct dso *machine__find_vdso(struct machine *machine,
+				      struct thread *thread)
+{
+	struct dso *dso = NULL;
+	enum dso_type dso_type;
+
+	dso_type = machine__thread_dso_type(machine, thread);
+	switch (dso_type) {
+	case DSO__TYPE_32BIT:
+		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
+		if (!dso) {
+			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
+					   true);
+			if (dso && dso_type != dso__type(dso, machine))
+				dso = NULL;
+		}
+		break;
+	case DSO__TYPE_X32BIT:
+		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
+		break;
+	case DSO__TYPE_64BIT:
+	case DSO__TYPE_UNKNOWN:
+	default:
+		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
+		break;
+	}
+
+	return dso;
+}
+
 struct dso *machine__findnew_vdso(struct machine *machine,
-				  struct thread *thread __maybe_unused)
+				  struct thread *thread)
 {
 	struct vdso_info *vdso_info;
 	struct dso *dso = NULL;
@@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
 	if (!vdso_info)
 		goto out_unlock;
 
+	dso = machine__find_vdso(machine, thread);
+	if (dso)
+		goto out_unlock;
+
 #if BITS_PER_LONG == 64
 	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
 		goto out_unlock;

Re: [PATCH 0/5] Fixes on perf unwind

[Arnaldo Carvalho de Melo]

Em Wed, Jun 22, 2016 at 06:57:01AM +0000, He Kuang escreveu:
> hi,
> 
> Patch 1-3 fix wrongly used PERF_REG_SP/IP by redefining
> those macros in the wrapper file according to the target platform, for
> example in "util/libunwind/x86_32.c".
> The first 3 patches have been acked-by "Jiri Olsa" and not touched.
> 
> Patch 4 catches an error on python build_ext build.
> 
> Patch 5 fixes a NULL pointer deference which can cause segfault when
> the desired dso is not found.
> 
> Thank you.

Thanks, all applied.

- Arnaldo