* [PATCH Resent] perf annotate: Fix s390 target function disassembly @ 2018-03-06 12:39 Thomas Richter 2018-03-06 14:04 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 4+ messages in thread From: Thomas Richter @ 2018-03-06 12:39 UTC (permalink / raw) To: linux-kernel, linux-perf-users, acme Cc: brueckner, schwidefsky, heiko.carstens, Thomas Richter Perf annotate displays function call assembler instructions with a right arrow. Hitting enter on this line/instruction causes the browser to disassemble this target function and show it on the screen. On s390 this results in an error message 'The called function was not found.' The function call assembly line parsing does not handle the s390 bras and brasl instructions. Function call__parse expects the target as first operand: callq e9140 <__fxstat> S390 has a register number as first operand: brasl %r14,41d60 <abort> Therefore the target addresses on s390 are always zero which is an invalid address. Fix this by skipping the first operand on s390. Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> Reviewed-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com> --- tools/perf/util/annotate.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 49ff825f745c..feb6006b676d 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -192,6 +192,14 @@ static int call__parse(struct arch *arch, struct ins_operands *ops, struct map * }; ops->target.addr = strtoull(ops->raw, &endptr, 16); + if (!strcmp(arch->name, "s390")) { + /* s390 function call 1st operand is register */ + tok = strchr(ops->raw, ','); + if (tok) + ops->target.addr = strtoull(tok + 1, &endptr, 16); + else + ops->target.addr = 0; + } else + ops->target.addr = strtoull(ops->raw, &endptr, 16); name = strchr(endptr, '<'); if (name == NULL) -- 2.14.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH Resent] perf annotate: Fix s390 target function disassembly 2018-03-06 12:39 [PATCH Resent] perf annotate: Fix s390 target function disassembly Thomas Richter @ 2018-03-06 14:04 ` Arnaldo Carvalho de Melo 2018-03-06 14:52 ` Thomas-Mich Richter 0 siblings, 1 reply; 4+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-03-06 14:04 UTC (permalink / raw) To: Thomas Richter Cc: linux-kernel, linux-perf-users, brueckner, schwidefsky, heiko.carstens Em Tue, Mar 06, 2018 at 01:39:55PM +0100, Thomas Richter escreveu: > Perf annotate displays function call assembler instructions > with a right arrow. Hitting enter on this line/instruction > causes the browser to disassemble this target function and > show it on the screen. On s390 this results in an error > message 'The called function was not found.' > > The function call assembly line parsing does not handle > the s390 bras and brasl instructions. Function call__parse > expects the target as first operand: > callq e9140 <__fxstat> > S390 has a register number as first operand: > brasl %r14,41d60 <abort> > Therefore the target addresses on s390 are always zero > which is an invalid address. > > Fix this by skipping the first operand on s390. > > Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com> > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > Reviewed-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com> > --- > tools/perf/util/annotate.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index 49ff825f745c..feb6006b676d 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -192,6 +192,14 @@ static int call__parse(struct arch *arch, struct ins_operands *ops, struct map * > }; > > ops->target.addr = strtoull(ops->raw, &endptr, 16); > + if (!strcmp(arch->name, "s390")) { > + /* s390 function call 1st operand is register */ > + tok = strchr(ops->raw, ','); > + if (tok) > + ops->target.addr = strtoull(tok + 1, &endptr, 16); > + else > + ops->target.addr = 0; > + } else > + ops->target.addr = strtoull(ops->raw, &endptr, 16); Do we have to do this here? Can't we have a s390_call__parse() and set that in the s/390 instructions? We should avoid arch specific stuff in the common code, and we could even have a call_reg__parse() perhaps? One that knows that the first operand is a reg and it should _always_ skip te first one, be it on s/390 or elsewhere? - Arnaldo > name = strchr(endptr, '<'); > if (name == NULL) > -- > 2.14.3 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH Resent] perf annotate: Fix s390 target function disassembly 2018-03-06 14:04 ` Arnaldo Carvalho de Melo @ 2018-03-06 14:52 ` Thomas-Mich Richter 2018-03-06 15:20 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 4+ messages in thread From: Thomas-Mich Richter @ 2018-03-06 14:52 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, linux-perf-users, brueckner, schwidefsky, heiko.carstens On 03/06/2018 03:04 PM, Arnaldo Carvalho de Melo wrote: > Em Tue, Mar 06, 2018 at 01:39:55PM +0100, Thomas Richter escreveu: >> Perf annotate displays function call assembler instructions >> with a right arrow. Hitting enter on this line/instruction >> causes the browser to disassemble this target function and >> show it on the screen. On s390 this results in an error >> message 'The called function was not found.' >> >> The function call assembly line parsing does not handle >> the s390 bras and brasl instructions. Function call__parse >> expects the target as first operand: >> callq e9140 <__fxstat> >> S390 has a register number as first operand: >> brasl %r14,41d60 <abort> >> Therefore the target addresses on s390 are always zero >> which is an invalid address. >> >> Fix this by skipping the first operand on s390. >> >> Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com> >> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> >> Reviewed-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com> >> --- >> tools/perf/util/annotate.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >> index 49ff825f745c..feb6006b676d 100644 >> --- a/tools/perf/util/annotate.c >> +++ b/tools/perf/util/annotate.c >> @@ -192,6 +192,14 @@ static int call__parse(struct arch *arch, struct ins_operands *ops, struct map * >> }; >> >> ops->target.addr = strtoull(ops->raw, &endptr, 16); >> + if (!strcmp(arch->name, "s390")) { >> + /* s390 function call 1st operand is register */ >> + tok = strchr(ops->raw, ','); >> + if (tok) >> + ops->target.addr = strtoull(tok + 1, &endptr, 16); >> + else >> + ops->target.addr = 0; >> + } else >> + ops->target.addr = strtoull(ops->raw, &endptr, 16); > > Do we have to do this here? Can't we have a s390_call__parse() and set > that in the s/390 instructions? Originally I wanted to add an architecture specific weak function to handle s390 This did not work because file util/annotate.c includes the architecture specific versions around line 100: #include "arch/arm/annotate/instructions.c" #include "arch/arm64/annotate/instructions.c" #include "arch/x86/annotate/instructions.c" #include "arch/powerpc/annotate/instructions.c" #include "arch/s390/annotate/instructions.c" This includes the C file for s390 so we end up having function call__parse defined twice, one with a weak definition and one without which results in a compiler error. I will use a s390 specific call__parse function and sent another patch. >> -- >> 2.14.3 > -- > To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany -- Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH Resent] perf annotate: Fix s390 target function disassembly 2018-03-06 14:52 ` Thomas-Mich Richter @ 2018-03-06 15:20 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 4+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-03-06 15:20 UTC (permalink / raw) To: Thomas-Mich Richter Cc: linux-kernel, linux-perf-users, brueckner, schwidefsky, heiko.carstens Em Tue, Mar 06, 2018 at 03:52:35PM +0100, Thomas-Mich Richter escreveu: > On 03/06/2018 03:04 PM, Arnaldo Carvalho de Melo wrote: > > Em Tue, Mar 06, 2018 at 01:39:55PM +0100, Thomas Richter escreveu: > >> Perf annotate displays function call assembler instructions > >> with a right arrow. Hitting enter on this line/instruction > >> causes the browser to disassemble this target function and > >> show it on the screen. On s390 this results in an error > >> message 'The called function was not found.' > >> > >> The function call assembly line parsing does not handle > >> the s390 bras and brasl instructions. Function call__parse > >> expects the target as first operand: > >> callq e9140 <__fxstat> > >> S390 has a register number as first operand: > >> brasl %r14,41d60 <abort> > >> Therefore the target addresses on s390 are always zero > >> which is an invalid address. > >> > >> Fix this by skipping the first operand on s390. > >> > >> Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com> > >> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > >> Reviewed-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com> > >> --- > >> tools/perf/util/annotate.c | 8 ++++++++ > >> 1 file changed, 8 insertions(+) > >> > >> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > >> index 49ff825f745c..feb6006b676d 100644 > >> --- a/tools/perf/util/annotate.c > >> +++ b/tools/perf/util/annotate.c > >> @@ -192,6 +192,14 @@ static int call__parse(struct arch *arch, struct ins_operands *ops, struct map * > >> }; > >> > >> ops->target.addr = strtoull(ops->raw, &endptr, 16); > >> + if (!strcmp(arch->name, "s390")) { > >> + /* s390 function call 1st operand is register */ > >> + tok = strchr(ops->raw, ','); > >> + if (tok) > >> + ops->target.addr = strtoull(tok + 1, &endptr, 16); > >> + else > >> + ops->target.addr = 0; > >> + } else > >> + ops->target.addr = strtoull(ops->raw, &endptr, 16); > > > > Do we have to do this here? Can't we have a s390_call__parse() and set > > that in the s/390 instructions? > > > Originally I wanted to add an architecture specific weak function to handle s390 > This did not work because file util/annotate.c includes the architecture specific versions > around line 100: > > #include "arch/arm/annotate/instructions.c" > #include "arch/arm64/annotate/instructions.c" > #include "arch/x86/annotate/instructions.c" > #include "arch/powerpc/annotate/instructions.c" > #include "arch/s390/annotate/instructions.c" > > This includes the C file for s390 so we end up having function call__parse defined twice, > one with a weak definition and one without which results in a compiler error. > > I will use a s390 specific call__parse function and sent another patch. So, in tools/perf/arch/s390/annotate/instructions.c you have that function s390__associate_ins_ops() where it sets the handler for the "brasl" asm instruction to be 'call__ops', what I am proposing is that instead you provide a brasl__ops and use it, this ops knows that it is a 'call', but the target is not where call__ops expects it to be, thus we have a separate ops for that, got it? - Arnaldo > >> -- > >> 2.14.3 > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > -- > Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany > -- > Vorsitzende des Aufsichtsrats: Martina Koederitz > Geschäftsführung: Dirk Wittkopp > Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-06 15:20 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-06 12:39 [PATCH Resent] perf annotate: Fix s390 target function disassembly Thomas Richter 2018-03-06 14:04 ` Arnaldo Carvalho de Melo 2018-03-06 14:52 ` Thomas-Mich Richter 2018-03-06 15:20 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).