[PATCH perf 0/4] Build fixes for gcc 6

[PATCH perf 0/4] Build fixes for gcc 6

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

gcc 6 warns about various things in tools/perf  and with -Werror
these turn into build failures.  One of them is a real though not
very serious bug.

Ben.

Ben Hutchings (4):
  perf tools: Fix wrong indentation and build failure with gcc 6
  perf top: Fix behaviour of Shift-Tab in annotated view with nothing
    focussed
  perf tools: Fix unused variables: x86_{32,64}_regoffset_table
  perf tests: Delete mis-indented dead code that causes build failure
    with gcc 6

 tools/perf/arch/x86/tests/intel-cqm.c |  5 +++--
 tools/perf/arch/x86/util/dwarf-regs.c | 38 ++++++++++++++++-------------------
 tools/perf/ui/browsers/annotate.c     |  4 ++--
 tools/perf/util/pmu.c                 |  2 +-
 4 files changed, 23 insertions(+), 26 deletions(-)


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

[PATCH perf 1/4] perf tools: Fix wrong indentation and build failure with gcc 6

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

One line in perf_pmu__parse_unit() is indented wrongly, leading
to a warning (=> error) from gcc 6:

util/pmu.c:156:3: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
   sret = read(fd, alias->unit, UNIT_MAX_LEN);
   ^~~~

util/pmu.c:153:2: note: ...this 'if' clause, but it is not
  if (fd == -1)
  ^~

Fixes: 410136f5dd96 ("tools/perf/stat: Add event unit and scale support")
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 tools/perf/util/pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index e4b173d..c900b66 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -153,7 +153,7 @@ static int perf_pmu__parse_unit(struct perf_pmu_alias *alias, char *dir, char *n
 	if (fd == -1)
 		return -1;
 
-		sret = read(fd, alias->unit, UNIT_MAX_LEN);
+	sret = read(fd, alias->unit, UNIT_MAX_LEN);
 	if (sret < 0)
 		goto error;
 


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

[PATCH perf 2/4] perf top: Fix behaviour of Shift-Tab in annotated view with nothing focussed

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]

gcc 6 catches a case of missing braces:

ui/browsers/annotate.c: In function 'annotate_browser__run':
ui/browsers/annotate.c:760:5: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
     if (nd == NULL)
     ^~

ui/browsers/annotate.c:758:4: note: ...this 'if' clause, but it is not
    if (nd != NULL)
    ^~

Fixes: c97cf42219b7 ("perf top: Live TUI Annotation")
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 tools/perf/ui/browsers/annotate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index d4d7cc2..718bd46 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -755,11 +755,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
 				nd = browser->curr_hot;
 			break;
 		case K_UNTAB:
-			if (nd != NULL)
+			if (nd != NULL) {
 				nd = rb_next(nd);
 				if (nd == NULL)
 					nd = rb_first(&browser->entries);
-			else
+			} else
 				nd = browser->curr_hot;
 			break;
 		case K_F1:


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

[PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2585 bytes --]

gcc 5 doesn't seem to care about these, but gcc 6 does and that
results in a build failure.

Fixes: bbbe6bf6037d ("perf tools: Introduce regs_query_register_offset() ...")
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 tools/perf/arch/x86/util/dwarf-regs.c | 38 ++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c
index 9223c16..fe1e516 100644
--- a/tools/perf/arch/x86/util/dwarf-regs.c
+++ b/tools/perf/arch/x86/util/dwarf-regs.c
@@ -55,26 +55,10 @@ struct pt_regs_offset {
 
 #define REG_OFFSET_END {.name = NULL, .offset = 0}
 
+/* TODO: switching by dwarf address size */
 #ifdef __x86_64__
-# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
-# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = -1}
-#else
-# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = -1}
-# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
-#endif
-
-static const struct pt_regs_offset x86_32_regoffset_table[] = {
-	REG_OFFSET_NAME_32("%ax",	eax),
-	REG_OFFSET_NAME_32("%cx",	ecx),
-	REG_OFFSET_NAME_32("%dx",	edx),
-	REG_OFFSET_NAME_32("%bx",	ebx),
-	REG_OFFSET_NAME_32("$stack",	esp),	/* Stack address instead of %sp */
-	REG_OFFSET_NAME_32("%bp",	ebp),
-	REG_OFFSET_NAME_32("%si",	esi),
-	REG_OFFSET_NAME_32("%di",	edi),
-	REG_OFFSET_END,
-};
 
+#define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
 static const struct pt_regs_offset x86_64_regoffset_table[] = {
 	REG_OFFSET_NAME_64("%ax",	rax),
 	REG_OFFSET_NAME_64("%dx",	rdx),
@@ -94,12 +78,24 @@ static const struct pt_regs_offset x86_64_regoffset_table[] = {
 	REG_OFFSET_NAME_64("%r15",	r15),
 	REG_OFFSET_END,
 };
-
-/* TODO: switching by dwarf address size */
-#ifdef __x86_64__
 #define regoffset_table x86_64_regoffset_table
+
 #else
+
+#define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
+static const struct pt_regs_offset x86_32_regoffset_table[] = {
+	REG_OFFSET_NAME_32("%ax",	eax),
+	REG_OFFSET_NAME_32("%cx",	ecx),
+	REG_OFFSET_NAME_32("%dx",	edx),
+	REG_OFFSET_NAME_32("%bx",	ebx),
+	REG_OFFSET_NAME_32("$stack",	esp),	/* Stack address instead of %sp */
+	REG_OFFSET_NAME_32("%bp",	ebp),
+	REG_OFFSET_NAME_32("%si",	esi),
+	REG_OFFSET_NAME_32("%di",	edi),
+	REG_OFFSET_END,
+};
 #define regoffset_table x86_32_regoffset_table
+
 #endif
 
 /* Minus 1 for the ending REG_OFFSET_END */


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

[PATCH perf 4/4] perf tests: Delete mis-indented dead code that causes build failure with gcc 6

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

gcc 6 complains:

arch/x86/tests/intel-cqm.c: In function 'spawn':
arch/x86/tests/intel-cqm.c:21:3: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
   sleep(5);
   ^~~~~

arch/x86/tests/intel-cqm.c:20:2: note: ...this 'while' clause, but it is not
  while(1);
  ^~~~~

Fixes: 035827e9f2bd ("perf tests: Add Intel CQM test")
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 tools/perf/arch/x86/tests/intel-cqm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/arch/x86/tests/intel-cqm.c b/tools/perf/arch/x86/tests/intel-cqm.c
index d28c1b6..bd123a1 100644
--- a/tools/perf/arch/x86/tests/intel-cqm.c
+++ b/tools/perf/arch/x86/tests/intel-cqm.c
@@ -17,8 +17,9 @@ static pid_t spawn(void)
 	if (pid)
 		return pid;
 
-	while(1);
-		sleep(5);
+	while (1)
+		;
+
 	return 0;
 }
 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH perf 0/4] Build fixes for gcc 6

[Markus Trippelsdorf]

On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote:
> gcc 6 warns about various things in tools/perf  and with -Werror
> these turn into build failures.  One of them is a real though not
> very serious bug.

I've already send patches for 1,2 and 4. See:
https://lkml.org/lkml/2015/12/14/460

Not sure what happened with them. Also your patch number 4 is wrong, you
should just delete the semicolon.


-- 
Markus

Re: [PATCH perf 0/4] Build fixes for gcc 6

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote:
> On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote:
> > gcc 6 warns about various things in tools/perf  and with -Werror
> > these turn into build failures.  One of them is a real though not
> > very serious bug.
> 
> I've already send patches for 1,2 and 4. See:
> https://lkml.org/lkml/2015/12/14/460
> 
> Not sure what happened with them. Also your patch number 4 is wrong, you
> should just delete the semicolon.

I think that the busy-wait, intentional or not, may be a necessary
part of the test case.

Ben.

-- 
Ben Hutchings
When in doubt, use brute force. - Ken Thompson

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH perf 0/4] Build fixes for gcc 6

[Markus Trippelsdorf]

On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote:
> On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote:
> > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote:
> > > gcc 6 warns about various things in tools/perf  and with -Werror
> > > these turn into build failures.  One of them is a real though not
> > > very serious bug.
> > 
> > I've already send patches for 1,2 and 4. See:
> > https://lkml.org/lkml/2015/12/14/460
> > 
> > Not sure what happened with them. Also your patch number 4 is wrong, you
> > should just delete the semicolon.
> 
> I think that the busy-wait, intentional or not, may be a necessary
> part of the test case.

Well, the author of the code thinks otherwise:

https://lkml.org/lkml/2015/12/14/269


-- 
Markus

Re: [PATCH perf 0/4] Build fixes for gcc 6

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

On Tue, 2016-01-19 at 23:00 +0100, Markus Trippelsdorf wrote:
> On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote:
> > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote:
> > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote:
> > > > gcc 6 warns about various things in tools/perf  and with -Werror
> > > > these turn into build failures.  One of them is a real though not
> > > > very serious bug.
> > > 
> > > I've already send patches for 1,2 and 4. See:
> > > https://lkml.org/lkml/2015/12/14/460
> > > 
> > > Not sure what happened with them. Also your patch number 4 is wrong, you
> > > should just delete the semicolon.
> > 
> > I think that the busy-wait, intentional or not, may be a necessary
> > part of the test case.
> 
> Well, the author of the code thinks otherwise:
> 
> https://lkml.org/lkml/2015/12/14/269

Oh I see, thanks.

Ben.

-- 
Ben Hutchings
Horngren's Observation:
                   Among economists, the real world is often a special case.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH perf 0/4] Build fixes for gcc 6

[Arnaldo Carvalho de Melo]

Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu:
> On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote:
> > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote:
> > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote:
> > > > gcc 6 warns about various things in tools/perf  and with -Werror
> > > > these turn into build failures.  One of them is a real though not
> > > > very serious bug.
> > > 
> > > I've already send patches for 1,2 and 4. See:
> > > https://lkml.org/lkml/2015/12/14/460
> > > 
> > > Not sure what happened with them. Also your patch number 4 is wrong, you
> > > should just delete the semicolon.
> > 
> > I think that the busy-wait, intentional or not, may be a necessary
> > part of the test case.
> 
> Well, the author of the code thinks otherwise:
> 
> https://lkml.org/lkml/2015/12/14/269

Right, I saw those and I think I haven't processed them because I was
waiting for those to be broken up in separate patches after I read
Ingo's comment about one of them fixing up a real bug, a part that the
original autor, mfleming even acked, could you please break it down into
multiple patches?

- Arnaldo

Re: [PATCH perf 0/4] Build fixes for gcc 6

[Arnaldo Carvalho de Melo]

Em Tue, Jan 19, 2016 at 07:28:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu:
> > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote:
> > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote:
> > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote:
> > > > > gcc 6 warns about various things in tools/perf  and with -Werror
> > > > > these turn into build failures.  One of them is a real though not
> > > > > very serious bug.
> > > > 
> > > > I've already send patches for 1,2 and 4. See:
> > > > https://lkml.org/lkml/2015/12/14/460
> > > > 
> > > > Not sure what happened with them. Also your patch number 4 is wrong, you
> > > > should just delete the semicolon.
> > > 
> > > I think that the busy-wait, intentional or not, may be a necessary
> > > part of the test case.
> > 
> > Well, the author of the code thinks otherwise:
> > 
> > https://lkml.org/lkml/2015/12/14/269
> 
> Right, I saw those and I think I haven't processed them because I was
> waiting for those to be broken up in separate patches after I read
> Ingo's comment about one of them fixing up a real bug, a part that the
> original autor, mfleming even acked, could you please break it down into
> multiple patches?

Alternatively I can do it for the patch acked by Matt and use the other
patches from Ben, that even have the Fixes: tags (yay, those are
appreciated!).

- Arnaldo

Re: [PATCH perf 0/4] Build fixes for gcc 6

[Markus Trippelsdorf]

On 2016.01.19 at 19:28 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu:
> > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote:
> > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote:
> > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote:
> > > > > gcc 6 warns about various things in tools/perf  and with -Werror
> > > > > these turn into build failures.  One of them is a real though not
> > > > > very serious bug.
> > > > 
> > > > I've already send patches for 1,2 and 4. See:
> > > > https://lkml.org/lkml/2015/12/14/460
> > > > 
> > > > Not sure what happened with them. Also your patch number 4 is wrong, you
> > > > should just delete the semicolon.
> > > 
> > > I think that the busy-wait, intentional or not, may be a necessary
> > > part of the test case.
> > 
> > Well, the author of the code thinks otherwise:
> > 
> > https://lkml.org/lkml/2015/12/14/269
> 
> Right, I saw those and I think I haven't processed them because I was
> waiting for those to be broken up in separate patches after I read
> Ingo's comment about one of them fixing up a real bug, a part that the
> original autor, mfleming even acked, could you please break it down into
> multiple patches?

https://lkml.org/lkml/2015/12/14/460
https://lkml.org/lkml/2015/12/14/461
https://lkml.org/lkml/2015/12/14/465

-- 
Markus

Re: [PATCH perf 0/4] Build fixes for gcc 6

[Markus Trippelsdorf]

On 2016.01.19 at 19:30 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 19, 2016 at 07:28:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu:
> > > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote:
> > > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote:
> > > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote:
> > > > > > gcc 6 warns about various things in tools/perf  and with -Werror
> > > > > > these turn into build failures.  One of them is a real though not
> > > > > > very serious bug.
> > > > > 
> > > > > I've already send patches for 1,2 and 4. See:
> > > > > https://lkml.org/lkml/2015/12/14/460
> > > > > 
> > > > > Not sure what happened with them. Also your patch number 4 is wrong, you
> > > > > should just delete the semicolon.
> > > > 
> > > > I think that the busy-wait, intentional or not, may be a necessary
> > > > part of the test case.
> > > 
> > > Well, the author of the code thinks otherwise:
> > > 
> > > https://lkml.org/lkml/2015/12/14/269
> > 
> > Right, I saw those and I think I haven't processed them because I was
> > waiting for those to be broken up in separate patches after I read
> > Ingo's comment about one of them fixing up a real bug, a part that the
> > original autor, mfleming even acked, could you please break it down into
> > multiple patches?
> 
> Alternatively I can do it for the patch acked by Matt and use the other
> patches from Ben, that even have the Fixes: tags (yay, those are
> appreciated!).

Just use what is easiest for you to work with. I don't really mind.

-- 
Markus

Re: [PATCH perf 0/4] Build fixes for gcc 6

[Arnaldo Carvalho de Melo]

Em Tue, Jan 19, 2016 at 11:31:01PM +0100, Markus Trippelsdorf escreveu:
> On 2016.01.19 at 19:28 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jan 19, 2016 at 11:00:50PM +0100, Markus Trippelsdorf escreveu:
> > > On 2016.01.19 at 21:58 +0000, Ben Hutchings wrote:
> > > > On Tue, Jan 19, 2016 at 10:40:18PM +0100, Markus Trippelsdorf wrote:
> > > > > On 2016.01.19 at 21:32 +0000, Ben Hutchings wrote:
> > > > > > gcc 6 warns about various things in tools/perf  and with -Werror
> > > > > > these turn into build failures.  One of them is a real though not
> > > > > > very serious bug.
> > > > > 
> > > > > I've already send patches for 1,2 and 4. See:
> > > > > https://lkml.org/lkml/2015/12/14/460
> > > > > 
> > > > > Not sure what happened with them. Also your patch number 4 is wrong, you
> > > > > should just delete the semicolon.
> > > > 
> > > > I think that the busy-wait, intentional or not, may be a necessary
> > > > part of the test case.
> > > 
> > > Well, the author of the code thinks otherwise:
> > > 
> > > https://lkml.org/lkml/2015/12/14/269
> > 
> > Right, I saw those and I think I haven't processed them because I was
> > waiting for those to be broken up in separate patches after I read
> > Ingo's comment about one of them fixing up a real bug, a part that the
> > original autor, mfleming even acked, could you please break it down into
> > multiple patches?
> 
> https://lkml.org/lkml/2015/12/14/460
> https://lkml.org/lkml/2015/12/14/461
> https://lkml.org/lkml/2015/12/14/465

And you had done that already, my bad, going thru them now.

- Arnaldo

Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table

[Arnaldo Carvalho de Melo]

Em Tue, Jan 19, 2016 at 09:33:06PM +0000, Ben Hutchings escreveu:
> gcc 5 doesn't seem to care about these, but gcc 6 does and that
> results in a build failure.

Ben, please CC the people on the CC list for the patch that introduces
the problem, Wang, He, can I have your Acked-by?

- Arnaldo
 
> Fixes: bbbe6bf6037d ("perf tools: Introduce regs_query_register_offset() ...")
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>  tools/perf/arch/x86/util/dwarf-regs.c | 38 ++++++++++++++++-------------------
>  1 file changed, 17 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c
> index 9223c16..fe1e516 100644
> --- a/tools/perf/arch/x86/util/dwarf-regs.c
> +++ b/tools/perf/arch/x86/util/dwarf-regs.c
> @@ -55,26 +55,10 @@ struct pt_regs_offset {
>  
>  #define REG_OFFSET_END {.name = NULL, .offset = 0}
>  
> +/* TODO: switching by dwarf address size */
>  #ifdef __x86_64__
> -# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
> -# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = -1}
> -#else
> -# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = -1}
> -# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
> -#endif
> -
> -static const struct pt_regs_offset x86_32_regoffset_table[] = {
> -	REG_OFFSET_NAME_32("%ax",	eax),
> -	REG_OFFSET_NAME_32("%cx",	ecx),
> -	REG_OFFSET_NAME_32("%dx",	edx),
> -	REG_OFFSET_NAME_32("%bx",	ebx),
> -	REG_OFFSET_NAME_32("$stack",	esp),	/* Stack address instead of %sp */
> -	REG_OFFSET_NAME_32("%bp",	ebp),
> -	REG_OFFSET_NAME_32("%si",	esi),
> -	REG_OFFSET_NAME_32("%di",	edi),
> -	REG_OFFSET_END,
> -};
>  
> +#define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
>  static const struct pt_regs_offset x86_64_regoffset_table[] = {
>  	REG_OFFSET_NAME_64("%ax",	rax),
>  	REG_OFFSET_NAME_64("%dx",	rdx),
> @@ -94,12 +78,24 @@ static const struct pt_regs_offset x86_64_regoffset_table[] = {
>  	REG_OFFSET_NAME_64("%r15",	r15),
>  	REG_OFFSET_END,
>  };
> -
> -/* TODO: switching by dwarf address size */
> -#ifdef __x86_64__
>  #define regoffset_table x86_64_regoffset_table
> +
>  #else
> +
> +#define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
> +static const struct pt_regs_offset x86_32_regoffset_table[] = {
> +	REG_OFFSET_NAME_32("%ax",	eax),
> +	REG_OFFSET_NAME_32("%cx",	ecx),
> +	REG_OFFSET_NAME_32("%dx",	edx),
> +	REG_OFFSET_NAME_32("%bx",	ebx),
> +	REG_OFFSET_NAME_32("$stack",	esp),	/* Stack address instead of %sp */
> +	REG_OFFSET_NAME_32("%bp",	ebp),
> +	REG_OFFSET_NAME_32("%si",	esi),
> +	REG_OFFSET_NAME_32("%di",	edi),
> +	REG_OFFSET_END,
> +};
>  #define regoffset_table x86_32_regoffset_table
> +
>  #endif
>  
>  /* Minus 1 for the ending REG_OFFSET_END */
> 

Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table

[Wangnan (F)]

On 2016/1/20 21:59, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 19, 2016 at 09:33:06PM +0000, Ben Hutchings escreveu:
>> gcc 5 doesn't seem to care about these, but gcc 6 does and that
>> results in a build failure.
> Ben, please CC the people on the CC list for the patch that introduces
> the problem, Wang, He, can I have your Acked-by?
>
> - Arnaldo
>   

This patch lead me find a bug in original code.

If both perf and target ELF binary is x86_64, following command works okay:

  # perf probe -v -n --exec /tmp/oxygen_root/lib64/libc.so.6 pselect 
data exceptfds readfds writefds nfds sigmask tval timeout
  <SNIP>
  Opening /sys/kernel/debug/tracing//uprobe_events write=1
  Writing event: p:probe_libc/pselect 
/home/w00229757/oxygen_root-w00229757/lib64/libc-2.18.so:0xdfef0 
data=-216(%sp):u64 exceptfds=%cx:u64 readfds=%si:u64 writefds=%dx:u64 
nfds=%di:s32 sigmask=%r9:u64 tval=-232(%sp):u64 timeout=%r8:u64
  <SNIP>

But if the library is x86_32, result is incorrect:

   # perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect 
data exceptfds readfds writefds nfds sigmask tval
   <SNIP>
   Writing event: p:probe_libc/pselect 
/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64 
exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32 
nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64 
timeout=+20(%si):u32
   <SNIP>

We know that (%si) is used to passing arguments. Here we should see 
'%sp' or '$stack'.

Use a x86_32 perf we get currect result:

  # ~/perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect 
data exceptfds readfds writefds nfds sigmask tval
  <SNIP>
  Writing event: p:probe_libc/pselect 
/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 
data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32 
writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32 
tval=-180($stack):u64
  <SNIP>


Use a small test program to check the result:

  #include <sys/time.h>
  #include <sys/types.h>
  #include <unistd.h>
  #include <memory.h>

  static struct {
         fd_set r, w, e;
         struct timespec ts;
         sigset_t m;
  } s;

  int main()
  {
         memset(&s, '\0', sizeof(s));

         pselect(0, &s.r, &s.w, &s.e, &s.ts, &s.m);
         return 0;
  }

# gcc -m32 -g ./test_pselect.c

Use x86_32 perf:

# ./perf probe -v  --exec /tmp/oxygen_root/lib32/libc.so.6 pselect data 
exceptfds readfds writefds nfds sigmask tval
Writing event: p:probe_libc/pselect 
/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 
data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32 
writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32 
tval=-180($stack):u64
Added new event:
   probe_libc:pselect   (on pselect in 
/tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds 
readfds writefds nfds sigmask tval)

You can now use it in all perf tools, such as:

     perf record -e probe_libc:pselect -aR sleep 1

# ./perf record -e probe_libc:pselect ./a.out
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ]
# ./perf script
            a.out 25336 [006] 64588.457597: probe_libc:pselect: 
(f7663330) data=0xf772e00000000000 exceptfds=0x8049880 readfds=0x8049780 
writefds=0x8049800 nfds=0 sigmask=0x8049908 tval=0x0

Switch to x86_64 perf:

  # ./perf probe -v  --exec /tmp/oxygen_root/lib32/libc.so.6 pselect 
data exceptfds readfds writefds nfds sigmask tval
  <SNIP>
  Opening /sys/kernel/debug/tracing//uprobe_events write=1
Writing event: p:probe_libc/pselect 
/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64 
exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32 
nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64
Added new event:
   probe_libc:pselect   (on pselect in 
/tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds 
readfds writefds nfds sigmask tval)

You can now use it in all perf tools, such as:

     perf record -e probe_libc:pselect -aR sleep 1

# ./perf record -e probe_libc:pselect ./a.out
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ]
# ./perf script
            a.out 25599 [002] 64759.743554: probe_libc:pselect: 
(f76e7330) data=0x0 exceptfds=0x0 readfds=0x0 writefds=0x0 nfds=0 
sigmask=0x0 tval=0x0

Sad...

I think this problem is not introduced by my patch. In fact
there's a fundamental problem in get_arch_regstr() that it is
impossible to switch sub ISA.

Not only x86_64 and x86_32, I think on arm64 we also have this
problem when we try to setup uprobes on arm32 code. For me the
later problem is more important because there are many legacy arm32
applications on Android platform (and I have already seen the buggy
unwind result in this case. It is another problem though).

So I suggest us to solve this problem first before considering
gcc 6 Werror. At least x86_32_regoffset_table and x86_64_regoffset_table
should both be compiled no matter which ISA we select for perf.

Thank you.

Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table

[Arnaldo Carvalho de Melo]

Em Thu, Jan 21, 2016 at 12:43:04PM +0800, Wangnan (F) escreveu:
> I think this problem is not introduced by my patch. In fact
> there's a fundamental problem in get_arch_regstr() that it is
> impossible to switch sub ISA.

> Not only x86_64 and x86_32, I think on arm64 we also have this
> problem when we try to setup uprobes on arm32 code. For me the
> later problem is more important because there are many legacy arm32
> applications on Android platform (and I have already seen the buggy
> unwind result in this case. It is another problem though).

Humm, and possibly to do something with arm code on a x86 workstation,
even if just analysis, yeah, I think these functions should take as an
argument the desired architecture instead of assuming it is the one in
the machine issuing the commands.
 
> So I suggest us to solve this problem first before considering
> gcc 6 Werror. At least x86_32_regoffset_table and x86_64_regoffset_table
> should both be compiled no matter which ISA we select for perf.
> 
> Thank you.

Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table

[Arnaldo Carvalho de Melo]

Em Thu, Jan 21, 2016 at 12:38:48PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 21, 2016 at 12:43:04PM +0800, Wangnan (F) escreveu:
> > I think this problem is not introduced by my patch. In fact
> > there's a fundamental problem in get_arch_regstr() that it is
> > impossible to switch sub ISA.
> 
> > Not only x86_64 and x86_32, I think on arm64 we also have this
> > problem when we try to setup uprobes on arm32 code. For me the
> > later problem is more important because there are many legacy arm32
> > applications on Android platform (and I have already seen the buggy
> > unwind result in this case. It is another problem though).
> 
> Humm, and possibly to do something with arm code on a x86 workstation,
> even if just analysis, yeah, I think these functions should take as an
> argument the desired architecture instead of assuming it is the one in
> the machine issuing the commands.
>  
> > So I suggest us to solve this problem first before considering
> > gcc 6 Werror. At least x86_32_regoffset_table and x86_64_regoffset_table

But... I think that the unflexible original code has a bug, one that makes it
not work when using gcc6 :-\

So I think we should make it build in gcc6, using that patch (or does it
have some other problem?) so that at least doing what we can do now can
be done for those using gcc6.

Then fix these shortcomings you detected.

- Arnaldo

> > should both be compiled no matter which ISA we select for perf.
> > 
> > Thank you.

Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table

[Wangnan (F)]

On 2016/1/21 23:41, Arnaldo Carvalho de Melo wrote:
>
> But... I think that the unflexible original code has a bug, one that makes it
> not work when using gcc6 :-\
>
> So I think we should make it build in gcc6, using that patch (or does it
> have some other problem?) so that at least doing what we can do now can
> be done for those using gcc6.
>
> Then fix these shortcomings you detected.

OK. His patch does what it claims to do. Please merge it first, then
let's look into my problem.

Thank you.

RE: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table

[平松雅巳 / HIRAMATU，MASAMI]

>From: Wangnan (F) [mailto:wangnan0@huawei.com]
>
>On 2016/1/20 21:59, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Jan 19, 2016 at 09:33:06PM +0000, Ben Hutchings escreveu:
>>> gcc 5 doesn't seem to care about these, but gcc 6 does and that
>>> results in a build failure.
>> Ben, please CC the people on the CC list for the patch that introduces
>> the problem, Wang, He, can I have your Acked-by?
>>
>> - Arnaldo
>>
>
>This patch lead me find a bug in original code.
>
>If both perf and target ELF binary is x86_64, following command works okay:
>
>  # perf probe -v -n --exec /tmp/oxygen_root/lib64/libc.so.6 pselect
>data exceptfds readfds writefds nfds sigmask tval timeout
>  <SNIP>
>  Opening /sys/kernel/debug/tracing//uprobe_events write=1
>  Writing event: p:probe_libc/pselect
>/home/w00229757/oxygen_root-w00229757/lib64/libc-2.18.so:0xdfef0
>data=-216(%sp):u64 exceptfds=%cx:u64 readfds=%si:u64 writefds=%dx:u64
>nfds=%di:s32 sigmask=%r9:u64 tval=-232(%sp):u64 timeout=%r8:u64
>  <SNIP>
>
>But if the library is x86_32, result is incorrect:
>
>   # perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect
>data exceptfds readfds writefds nfds sigmask tval
>   <SNIP>
>   Writing event: p:probe_libc/pselect
>/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64
>exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32
>nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64
>timeout=+20(%si):u32
>   <SNIP>
>
>We know that (%si) is used to passing arguments. Here we should see
>'%sp' or '$stack'.
>
>Use a x86_32 perf we get currect result:
>
>  # ~/perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect
>data exceptfds readfds writefds nfds sigmask tval
>  <SNIP>
>  Writing event: p:probe_libc/pselect
>/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330
>data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32
>writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32
>tval=-180($stack):u64
>  <SNIP>

Ah, I see. Uprobes may not check the target binary is in 32bit mode.
Since the stack of x86-64 and x86-32 on pt_regs are different,
(regs->sp points stack on x86-64, &(regs->pt) points stack on x86-32)
uprobes would better checking and change the behavior.

But anyway, it is also fixed by changing perf's register table.

>
>
>Use a small test program to check the result:
>
>  #include <sys/time.h>
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <memory.h>
>
>  static struct {
>         fd_set r, w, e;
>         struct timespec ts;
>         sigset_t m;
>  } s;
>
>  int main()
>  {
>         memset(&s, '\0', sizeof(s));
>
>         pselect(0, &s.r, &s.w, &s.e, &s.ts, &s.m);
>         return 0;
>  }
>
># gcc -m32 -g ./test_pselect.c
>
>Use x86_32 perf:
>
># ./perf probe -v  --exec /tmp/oxygen_root/lib32/libc.so.6 pselect data
>exceptfds readfds writefds nfds sigmask tval
>Writing event: p:probe_libc/pselect
>/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330
>data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32
>writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32
>tval=-180($stack):u64
>Added new event:
>   probe_libc:pselect   (on pselect in
>/tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds
>readfds writefds nfds sigmask tval)
>
>You can now use it in all perf tools, such as:
>
>     perf record -e probe_libc:pselect -aR sleep 1
>
># ./perf record -e probe_libc:pselect ./a.out
>[ perf record: Woken up 1 times to write data ]
>[ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ]
># ./perf script
>            a.out 25336 [006] 64588.457597: probe_libc:pselect:
>(f7663330) data=0xf772e00000000000 exceptfds=0x8049880 readfds=0x8049780
>writefds=0x8049800 nfds=0 sigmask=0x8049908 tval=0x0
>
>Switch to x86_64 perf:
>
>  # ./perf probe -v  --exec /tmp/oxygen_root/lib32/libc.so.6 pselect
>data exceptfds readfds writefds nfds sigmask tval
>  <SNIP>
>  Opening /sys/kernel/debug/tracing//uprobe_events write=1
>Writing event: p:probe_libc/pselect
>/tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64
>exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32
>nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64
>Added new event:
>   probe_libc:pselect   (on pselect in
>/tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds
>readfds writefds nfds sigmask tval)
>
>You can now use it in all perf tools, such as:
>
>     perf record -e probe_libc:pselect -aR sleep 1
>
># ./perf record -e probe_libc:pselect ./a.out
>[ perf record: Woken up 1 times to write data ]
>[ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ]
># ./perf script
>            a.out 25599 [002] 64759.743554: probe_libc:pselect:
>(f76e7330) data=0x0 exceptfds=0x0 readfds=0x0 writefds=0x0 nfds=0
>sigmask=0x0 tval=0x0
>
>Sad...
>
>I think this problem is not introduced by my patch. In fact
>there's a fundamental problem in get_arch_regstr() that it is
>impossible to switch sub ISA.

Right, but I guess this can fixed by switching %sp (for x86-64)
and +0(%sp) (for x86-32) instead of $stack.
 
Thanks!


>
>Not only x86_64 and x86_32, I think on arm64 we also have this
>problem when we try to setup uprobes on arm32 code. For me the
>later problem is more important because there are many legacy arm32
>applications on Android platform (and I have already seen the buggy
>unwind result in this case. It is another problem though).
>
>So I suggest us to solve this problem first before considering
>gcc 6 Werror. At least x86_32_regoffset_table and x86_64_regoffset_table
>should both be compiled no matter which ISA we select for perf.
>
>Thank you.

Re: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table

[Wangnan (F)]

On 2016/1/22 13:56, 平松雅巳 / HIRAMATU，MASAMI wrote:
>> From: Wangnan (F) [mailto:wangnan0@huawei.com]
>>
>> On 2016/1/20 21:59, Arnaldo Carvalho de Melo wrote:
>>> Em Tue, Jan 19, 2016 at 09:33:06PM +0000, Ben Hutchings escreveu:
>>>> gcc 5 doesn't seem to care about these, but gcc 6 does and that
>>>> results in a build failure.
>>> Ben, please CC the people on the CC list for the patch that introduces
>>> the problem, Wang, He, can I have your Acked-by?
>>>
>>> - Arnaldo
>>>
>> This patch lead me find a bug in original code.
>>
>> If both perf and target ELF binary is x86_64, following command works okay:
>>
>>   # perf probe -v -n --exec /tmp/oxygen_root/lib64/libc.so.6 pselect
>> data exceptfds readfds writefds nfds sigmask tval timeout
>>   <SNIP>
>>   Opening /sys/kernel/debug/tracing//uprobe_events write=1
>>   Writing event: p:probe_libc/pselect
>> /home/w00229757/oxygen_root-w00229757/lib64/libc-2.18.so:0xdfef0
>> data=-216(%sp):u64 exceptfds=%cx:u64 readfds=%si:u64 writefds=%dx:u64
>> nfds=%di:s32 sigmask=%r9:u64 tval=-232(%sp):u64 timeout=%r8:u64
>>   <SNIP>
>>
>> But if the library is x86_32, result is incorrect:
>>
>>    # perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect
>> data exceptfds readfds writefds nfds sigmask tval
>>    <SNIP>
>>    Writing event: p:probe_libc/pselect
>> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64
>> exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32
>> nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64
>> timeout=+20(%si):u32
>>    <SNIP>
>>
>> We know that (%si) is used to passing arguments. Here we should see
>> '%sp' or '$stack'.
>>
>> Use a x86_32 perf we get currect result:
>>
>>   # ~/perf probe -v -n --exec /tmp/oxygen_root/lib32/libc.so.6 pselect
>> data exceptfds readfds writefds nfds sigmask tval
>>   <SNIP>
>>   Writing event: p:probe_libc/pselect
>> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330
>> data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32
>> writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32
>> tval=-180($stack):u64
>>   <SNIP>
> Ah, I see. Uprobes may not check the target binary is in 32bit mode.
> Since the stack of x86-64 and x86-32 on pt_regs are different,
> (regs->sp points stack on x86-64, &(regs->pt) points stack on x86-32)
> uprobes would better checking and change the behavior.
>
> But anyway, it is also fixed by changing perf's register table.
>
>>
>> Use a small test program to check the result:
>>
>>   #include <sys/time.h>
>>   #include <sys/types.h>
>>   #include <unistd.h>
>>   #include <memory.h>
>>
>>   static struct {
>>          fd_set r, w, e;
>>          struct timespec ts;
>>          sigset_t m;
>>   } s;
>>
>>   int main()
>>   {
>>          memset(&s, '\0', sizeof(s));
>>
>>          pselect(0, &s.r, &s.w, &s.e, &s.ts, &s.m);
>>          return 0;
>>   }
>>
>> # gcc -m32 -g ./test_pselect.c
>>
>> Use x86_32 perf:
>>
>> # ./perf probe -v  --exec /tmp/oxygen_root/lib32/libc.so.6 pselect data
>> exceptfds readfds writefds nfds sigmask tval
>> Writing event: p:probe_libc/pselect
>> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330
>> data=-172($stack):u64 exceptfds=+16($stack):u32 readfds=+8($stack):u32
>> writefds=+12($stack):u32 nfds=+4($stack):s32 sigmask=+24($stack):u32
>> tval=-180($stack):u64
>> Added new event:
>>    probe_libc:pselect   (on pselect in
>> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds
>> readfds writefds nfds sigmask tval)
>>
>> You can now use it in all perf tools, such as:
>>
>>      perf record -e probe_libc:pselect -aR sleep 1
>>
>> # ./perf record -e probe_libc:pselect ./a.out
>> [ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ]
>> # ./perf script
>>             a.out 25336 [006] 64588.457597: probe_libc:pselect:
>> (f7663330) data=0xf772e00000000000 exceptfds=0x8049880 readfds=0x8049780
>> writefds=0x8049800 nfds=0 sigmask=0x8049908 tval=0x0
>>
>> Switch to x86_64 perf:
>>
>>   # ./perf probe -v  --exec /tmp/oxygen_root/lib32/libc.so.6 pselect
>> data exceptfds readfds writefds nfds sigmask tval
>>   <SNIP>
>>   Opening /sys/kernel/debug/tracing//uprobe_events write=1
>> Writing event: p:probe_libc/pselect
>> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so:0xd1330 data=-172(%si):u64
>> exceptfds=+16(%si):u32 readfds=+8(%si):u32 writefds=+12(%si):u32
>> nfds=+4(%si):s32 sigmask=+24(%si):u32 tval=-180(%si):u64
>> Added new event:
>>    probe_libc:pselect   (on pselect in
>> /tmp/oxygen_root-w00229757/lib32/libc-2.18.so with data exceptfds
>> readfds writefds nfds sigmask tval)
>>
>> You can now use it in all perf tools, such as:
>>
>>      perf record -e probe_libc:pselect -aR sleep 1
>>
>> # ./perf record -e probe_libc:pselect ./a.out
>> [ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 0.011 MB perf.data (1 samples) ]
>> # ./perf script
>>             a.out 25599 [002] 64759.743554: probe_libc:pselect:
>> (f76e7330) data=0x0 exceptfds=0x0 readfds=0x0 writefds=0x0 nfds=0
>> sigmask=0x0 tval=0x0
>>
>> Sad...
>>
>> I think this problem is not introduced by my patch. In fact
>> there's a fundamental problem in get_arch_regstr() that it is
>> impossible to switch sub ISA.
> Right, but I guess this can fixed by switching %sp (for x86-64)
> and +0(%sp) (for x86-32) instead of $stack.
>   

It may not work.

No matter how we change regoffset_table, when get_arch_regstr()
get a register number 4, how can it know whether we are looking
for x86_32 register and return $stack or return %si for x86_64?

The fundamental problem is: we need a way to map dwarf's register
number to register names so uprobe know which register we are
looking for, but the API we designed for this can't distinguish
the sub ISAs dwarf are using.

Thanks.

RE: [PATCH perf 3/4] perf tools: Fix unused variables: x86_{32,64}_regoffset_table

[平松雅巳 / HIRAMATU，MASAMI]

>From: Wangnan (F) [mailto:wangnan0@huawei.com]
>On 2016/1/22 13:56, 平松雅巳 / HIRAMATU，MASAMI wrote:
>>> From: Wangnan (F) [mailto:wangnan0@huawei.com]
>>> I think this problem is not introduced by my patch. In fact
>>> there's a fundamental problem in get_arch_regstr() that it is
>>> impossible to switch sub ISA.
>> Right, but I guess this can fixed by switching %sp (for x86-64)
>> and +0(%sp) (for x86-32) instead of $stack.
>>
>
>It may not work.
>
>No matter how we change regoffset_table, when get_arch_regstr()
>get a register number 4, how can it know whether we are looking
>for x86_32 register and return $stack or return %si for x86_64?

I think we can also use elf header to get the ISA of the
target binary.

>The fundamental problem is: we need a way to map dwarf's register
>number to register names so uprobe know which register we are
>looking for, but the API we designed for this can't distinguish
>the sub ISAs dwarf are using.

As I said, we can read the elf header and get the Class or Machine
and switch the table according to it. Can't it?

Thanks,