* [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent
@ 2020-04-27 6:15 Nick Gasson
2020-04-27 6:15 ` [PATCH 1/3] perf jvmti: Fix jitdump for methods without debug info Nick Gasson
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Nick Gasson @ 2020-04-27 6:15 UTC (permalink / raw)
To: Alexander Shishkin, Arnaldo Carvalho de Melo, Ingo Molnar,
Jiri Olsa, Mark Rutland, Namhyung Kim, Peter Zijlstra
Cc: Nick Gasson, linux-kernel
Hi,
These three patches fix a couple of issues I ran into while using the
jitdump JVMTI agent to profile the SPECjbb benchmark.
Tested with OpenJDK 8 and 14.
Thanks,
Nick
Nick Gasson (3):
perf jvmti: Fix jitdump for methods without debug info
perf jvmti: Do not report error when missing debug information
perf jvmti: Fix demangling Java symbols
tools/perf/jvmti/libjvmti.c | 24 +++++++--------
tools/perf/tests/Build | 1 +
tools/perf/tests/builtin-test.c | 4 +++
tools/perf/tests/demangle-java-test.c | 42 +++++++++++++++++++++++++++
tools/perf/tests/tests.h | 1 +
tools/perf/util/demangle-java.c | 13 +++++----
6 files changed, 66 insertions(+), 19 deletions(-)
create mode 100644 tools/perf/tests/demangle-java-test.c
--
2.26.1
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/3] perf jvmti: Fix jitdump for methods without debug info 2020-04-27 6:15 [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent Nick Gasson @ 2020-04-27 6:15 ` Nick Gasson 2020-05-14 22:06 ` Ian Rogers 2020-04-27 6:15 ` [PATCH 2/3] perf jvmti: Do not report error when missing debug information Nick Gasson ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: Nick Gasson @ 2020-04-27 6:15 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim Cc: Nick Gasson, linux-kernel If a Java class is compiled with -g:none to omit debug information, the JVMTI plugin won't write jitdump entries for any method in this class and prints a lot of errors like: java: GetSourceFileName failed with JVMTI_ERROR_ABSENT_INFORMATION The call to GetSourceFileName is used to derive the file name `fn`, but this value is not actually used since commit ca58d7e64bdf ("perf jvmti: Generate correct debug information for inlined code") which moved the file name lookup into fill_source_filenames(). So the call to GetSourceFileName and related code can be safely removed. Signed-off-by: Nick Gasson <nick.gasson@arm.com> --- tools/perf/jvmti/libjvmti.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c index c441a34cb1c0..50ef524b5cd4 100644 --- a/tools/perf/jvmti/libjvmti.c +++ b/tools/perf/jvmti/libjvmti.c @@ -246,8 +246,6 @@ compiled_method_load_cb(jvmtiEnv *jvmti, char *class_sign = NULL; char *func_name = NULL; char *func_sign = NULL; - char *file_name = NULL; - char fn[PATH_MAX]; uint64_t addr = (uint64_t)(uintptr_t)code_addr; jvmtiError ret; int nr_lines = 0; /* in line_tab[] */ @@ -282,12 +280,6 @@ compiled_method_load_cb(jvmtiEnv *jvmti, } } - ret = (*jvmti)->GetSourceFileName(jvmti, decl_class, &file_name); - if (ret != JVMTI_ERROR_NONE) { - print_error(jvmti, "GetSourceFileName", ret); - goto error; - } - ret = (*jvmti)->GetClassSignature(jvmti, decl_class, &class_sign, NULL); if (ret != JVMTI_ERROR_NONE) { @@ -302,8 +294,6 @@ compiled_method_load_cb(jvmtiEnv *jvmti, goto error; } - copy_class_filename(class_sign, file_name, fn, PATH_MAX); - /* * write source line info record if we have it */ @@ -323,7 +313,6 @@ compiled_method_load_cb(jvmtiEnv *jvmti, (*jvmti)->Deallocate(jvmti, (unsigned char *)func_name); (*jvmti)->Deallocate(jvmti, (unsigned char *)func_sign); (*jvmti)->Deallocate(jvmti, (unsigned char *)class_sign); - (*jvmti)->Deallocate(jvmti, (unsigned char *)file_name); free(line_tab); while (line_file_names && (nr_lines > 0)) { if (line_file_names[nr_lines - 1]) { -- 2.26.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] perf jvmti: Fix jitdump for methods without debug info 2020-04-27 6:15 ` [PATCH 1/3] perf jvmti: Fix jitdump for methods without debug info Nick Gasson @ 2020-05-14 22:06 ` Ian Rogers 0 siblings, 0 replies; 19+ messages in thread From: Ian Rogers @ 2020-05-14 22:06 UTC (permalink / raw) To: Nick Gasson Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, LKML On Sun, Apr 26, 2020 at 11:16 PM Nick Gasson <nick.gasson@arm.com> wrote: > > If a Java class is compiled with -g:none to omit debug information, the > JVMTI plugin won't write jitdump entries for any method in this class > and prints a lot of errors like: > > java: GetSourceFileName failed with JVMTI_ERROR_ABSENT_INFORMATION > > The call to GetSourceFileName is used to derive the file name `fn`, but > this value is not actually used since commit ca58d7e64bdf ("perf jvmti: > Generate correct debug information for inlined code") which moved the > file name lookup into fill_source_filenames(). So the call to > GetSourceFileName and related code can be safely removed. > > Signed-off-by: Nick Gasson <nick.gasson@arm.com> Reviewed-by: Ian Rogers <irogers@google.com> Tested-by: Ian Rogers <irogers@google.com> Thanks! Ian > --- > tools/perf/jvmti/libjvmti.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c > index c441a34cb1c0..50ef524b5cd4 100644 > --- a/tools/perf/jvmti/libjvmti.c > +++ b/tools/perf/jvmti/libjvmti.c > @@ -246,8 +246,6 @@ compiled_method_load_cb(jvmtiEnv *jvmti, > char *class_sign = NULL; > char *func_name = NULL; > char *func_sign = NULL; > - char *file_name = NULL; > - char fn[PATH_MAX]; > uint64_t addr = (uint64_t)(uintptr_t)code_addr; > jvmtiError ret; > int nr_lines = 0; /* in line_tab[] */ > @@ -282,12 +280,6 @@ compiled_method_load_cb(jvmtiEnv *jvmti, > } > } > > - ret = (*jvmti)->GetSourceFileName(jvmti, decl_class, &file_name); > - if (ret != JVMTI_ERROR_NONE) { > - print_error(jvmti, "GetSourceFileName", ret); > - goto error; > - } > - > ret = (*jvmti)->GetClassSignature(jvmti, decl_class, > &class_sign, NULL); > if (ret != JVMTI_ERROR_NONE) { > @@ -302,8 +294,6 @@ compiled_method_load_cb(jvmtiEnv *jvmti, > goto error; > } > > - copy_class_filename(class_sign, file_name, fn, PATH_MAX); > - > /* > * write source line info record if we have it > */ > @@ -323,7 +313,6 @@ compiled_method_load_cb(jvmtiEnv *jvmti, > (*jvmti)->Deallocate(jvmti, (unsigned char *)func_name); > (*jvmti)->Deallocate(jvmti, (unsigned char *)func_sign); > (*jvmti)->Deallocate(jvmti, (unsigned char *)class_sign); > - (*jvmti)->Deallocate(jvmti, (unsigned char *)file_name); > free(line_tab); > while (line_file_names && (nr_lines > 0)) { > if (line_file_names[nr_lines - 1]) { > -- > 2.26.1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/3] perf jvmti: Do not report error when missing debug information 2020-04-27 6:15 [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent Nick Gasson 2020-04-27 6:15 ` [PATCH 1/3] perf jvmti: Fix jitdump for methods without debug info Nick Gasson @ 2020-04-27 6:15 ` Nick Gasson 2020-05-14 22:08 ` Ian Rogers 2020-05-27 14:07 ` Arnaldo Carvalho de Melo 2020-04-27 6:15 ` [PATCH 3/3] perf jvmti: Fix demangling Java symbols Nick Gasson 2020-04-27 10:35 ` [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent Jiri Olsa 3 siblings, 2 replies; 19+ messages in thread From: Nick Gasson @ 2020-04-27 6:15 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim Cc: Nick Gasson, linux-kernel If the Java sources are compiled with -g:none to disable debug information the perf JVMTI plugin reports a lot of errors like: java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION Instead if GetLineNumberTable returns JVMTI_ERROR_ABSENT_INFORMATION simply skip emitting line number information for that method. Unlike the previous patch these errors don't affect the jitdump generation, they just generate a lot of noise. Similarly for native methods which also don't have line tables. Signed-off-by: Nick Gasson <nick.gasson@arm.com> --- tools/perf/jvmti/libjvmti.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c index 50ef524b5cd4..a9a056d68416 100644 --- a/tools/perf/jvmti/libjvmti.c +++ b/tools/perf/jvmti/libjvmti.c @@ -41,7 +41,11 @@ do_get_line_numbers(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci, jvmtiError ret; ret = (*jvmti)->GetLineNumberTable(jvmti, m, &nr_lines, &loc_tab); - if (ret != JVMTI_ERROR_NONE) { + if (ret == JVMTI_ERROR_ABSENT_INFORMATION || ret == JVMTI_ERROR_NATIVE_METHOD) { + /* No debug information for this method */ + *nr = 0; + return JVMTI_ERROR_NONE; + } else if (ret != JVMTI_ERROR_NONE) { print_error(jvmti, "GetLineNumberTable", ret); return ret; } @@ -93,6 +97,9 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info, jvmti_line_info_t ** /* free what was allocated for nothing */ (*jvmti)->Deallocate(jvmti, (unsigned char *)lne); nr_total += (int)nr; + } else if (ret == JVMTI_ERROR_ABSENT_INFORMATION + || ret == JVMTI_ERROR_NATIVE_METHOD) { + /* No debug information for this method */ } else { print_error(jvmti, "GetLineNumberTable", ret); } @@ -262,7 +269,9 @@ compiled_method_load_cb(jvmtiEnv *jvmti, if (has_line_numbers && map && map_length) { ret = get_line_numbers(jvmti, compile_info, &line_tab, &nr_lines); if (ret != JVMTI_ERROR_NONE) { - warnx("jvmti: cannot get line table for method"); + if (ret != JVMTI_ERROR_NOT_FOUND) { + warnx("jvmti: cannot get line table for method"); + } nr_lines = 0; } else if (nr_lines > 0) { line_file_names = malloc(sizeof(char*) * nr_lines); -- 2.26.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] perf jvmti: Do not report error when missing debug information 2020-04-27 6:15 ` [PATCH 2/3] perf jvmti: Do not report error when missing debug information Nick Gasson @ 2020-05-14 22:08 ` Ian Rogers 2020-05-27 14:07 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 19+ messages in thread From: Ian Rogers @ 2020-05-14 22:08 UTC (permalink / raw) To: Nick Gasson Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, LKML On Sun, Apr 26, 2020 at 11:16 PM Nick Gasson <nick.gasson@arm.com> wrote: > > If the Java sources are compiled with -g:none to disable debug > information the perf JVMTI plugin reports a lot of errors like: > > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > > Instead if GetLineNumberTable returns JVMTI_ERROR_ABSENT_INFORMATION > simply skip emitting line number information for that method. Unlike the > previous patch these errors don't affect the jitdump generation, they > just generate a lot of noise. > > Similarly for native methods which also don't have line tables. > > Signed-off-by: Nick Gasson <nick.gasson@arm.com> Reviewed-by: Ian Rogers <irogers@google.com> Tested-by: Ian Rogers <irogers@google.com> Thanks! Ian > --- > tools/perf/jvmti/libjvmti.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c > index 50ef524b5cd4..a9a056d68416 100644 > --- a/tools/perf/jvmti/libjvmti.c > +++ b/tools/perf/jvmti/libjvmti.c > @@ -41,7 +41,11 @@ do_get_line_numbers(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci, > jvmtiError ret; > > ret = (*jvmti)->GetLineNumberTable(jvmti, m, &nr_lines, &loc_tab); > - if (ret != JVMTI_ERROR_NONE) { > + if (ret == JVMTI_ERROR_ABSENT_INFORMATION || ret == JVMTI_ERROR_NATIVE_METHOD) { > + /* No debug information for this method */ > + *nr = 0; > + return JVMTI_ERROR_NONE; > + } else if (ret != JVMTI_ERROR_NONE) { > print_error(jvmti, "GetLineNumberTable", ret); > return ret; > } > @@ -93,6 +97,9 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info, jvmti_line_info_t ** > /* free what was allocated for nothing */ > (*jvmti)->Deallocate(jvmti, (unsigned char *)lne); > nr_total += (int)nr; > + } else if (ret == JVMTI_ERROR_ABSENT_INFORMATION > + || ret == JVMTI_ERROR_NATIVE_METHOD) { > + /* No debug information for this method */ > } else { > print_error(jvmti, "GetLineNumberTable", ret); > } > @@ -262,7 +269,9 @@ compiled_method_load_cb(jvmtiEnv *jvmti, > if (has_line_numbers && map && map_length) { > ret = get_line_numbers(jvmti, compile_info, &line_tab, &nr_lines); > if (ret != JVMTI_ERROR_NONE) { > - warnx("jvmti: cannot get line table for method"); > + if (ret != JVMTI_ERROR_NOT_FOUND) { > + warnx("jvmti: cannot get line table for method"); > + } > nr_lines = 0; > } else if (nr_lines > 0) { > line_file_names = malloc(sizeof(char*) * nr_lines); > -- > 2.26.1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] perf jvmti: Do not report error when missing debug information 2020-04-27 6:15 ` [PATCH 2/3] perf jvmti: Do not report error when missing debug information Nick Gasson 2020-05-14 22:08 ` Ian Rogers @ 2020-05-27 14:07 ` Arnaldo Carvalho de Melo 2020-05-27 14:08 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-27 14:07 UTC (permalink / raw) To: Nick Gasson Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-kernel Em Mon, Apr 27, 2020 at 02:15:15PM +0800, Nick Gasson escreveu: > If the Java sources are compiled with -g:none to disable debug > information the perf JVMTI plugin reports a lot of errors like: > > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > > Instead if GetLineNumberTable returns JVMTI_ERROR_ABSENT_INFORMATION > simply skip emitting line number information for that method. Unlike the > previous patch these errors don't affect the jitdump generation, they > just generate a lot of noise. > > Similarly for native methods which also don't have line tables. > > Signed-off-by: Nick Gasson <nick.gasson@arm.com> > --- > tools/perf/jvmti/libjvmti.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c > index 50ef524b5cd4..a9a056d68416 100644 > --- a/tools/perf/jvmti/libjvmti.c > +++ b/tools/perf/jvmti/libjvmti.c > @@ -41,7 +41,11 @@ do_get_line_numbers(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci, > jvmtiError ret; > > ret = (*jvmti)->GetLineNumberTable(jvmti, m, &nr_lines, &loc_tab); > - if (ret != JVMTI_ERROR_NONE) { > + if (ret == JVMTI_ERROR_ABSENT_INFORMATION || ret == JVMTI_ERROR_NATIVE_METHOD) { > + /* No debug information for this method */ > + *nr = 0; > + return JVMTI_ERROR_NONE; > + } else if (ret != JVMTI_ERROR_NONE) { > print_error(jvmti, "GetLineNumberTable", ret); > return ret; > } > @@ -93,6 +97,9 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info, jvmti_line_info_t ** > /* free what was allocated for nothing */ > (*jvmti)->Deallocate(jvmti, (unsigned char *)lne); > nr_total += (int)nr; > + } else if (ret == JVMTI_ERROR_ABSENT_INFORMATION > + || ret == JVMTI_ERROR_NATIVE_METHOD) { Please add the || operator at the end of the line next time, I'm fixing this up this time, Applied, - Arnaldo > + /* No debug information for this method */ > } else { > print_error(jvmti, "GetLineNumberTable", ret); > } > @@ -262,7 +269,9 @@ compiled_method_load_cb(jvmtiEnv *jvmti, > if (has_line_numbers && map && map_length) { > ret = get_line_numbers(jvmti, compile_info, &line_tab, &nr_lines); > if (ret != JVMTI_ERROR_NONE) { > - warnx("jvmti: cannot get line table for method"); > + if (ret != JVMTI_ERROR_NOT_FOUND) { > + warnx("jvmti: cannot get line table for method"); > + } > nr_lines = 0; > } else if (nr_lines > 0) { > line_file_names = malloc(sizeof(char*) * nr_lines); > -- > 2.26.1 > -- - Arnaldo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] perf jvmti: Do not report error when missing debug information 2020-05-27 14:07 ` Arnaldo Carvalho de Melo @ 2020-05-27 14:08 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-27 14:08 UTC (permalink / raw) To: Nick Gasson Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-kernel Em Wed, May 27, 2020 at 11:07:53AM -0300, Arnaldo Carvalho de Melo escreveu: > Em Mon, Apr 27, 2020 at 02:15:15PM +0800, Nick Gasson escreveu: > > If the Java sources are compiled with -g:none to disable debug > > information the perf JVMTI plugin reports a lot of errors like: > > > > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > > java: GetLineNumberTable failed with JVMTI_ERROR_ABSENT_INFORMATION > > > > Instead if GetLineNumberTable returns JVMTI_ERROR_ABSENT_INFORMATION > > simply skip emitting line number information for that method. Unlike the > > previous patch these errors don't affect the jitdump generation, they > > just generate a lot of noise. > > > > Similarly for native methods which also don't have line tables. > > > > Signed-off-by: Nick Gasson <nick.gasson@arm.com> > > --- > > tools/perf/jvmti/libjvmti.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c > > index 50ef524b5cd4..a9a056d68416 100644 > > --- a/tools/perf/jvmti/libjvmti.c > > +++ b/tools/perf/jvmti/libjvmti.c > > @@ -41,7 +41,11 @@ do_get_line_numbers(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci, > > jvmtiError ret; > > > > ret = (*jvmti)->GetLineNumberTable(jvmti, m, &nr_lines, &loc_tab); > > - if (ret != JVMTI_ERROR_NONE) { > > + if (ret == JVMTI_ERROR_ABSENT_INFORMATION || ret == JVMTI_ERROR_NATIVE_METHOD) { > > + /* No debug information for this method */ > > + *nr = 0; > > + return JVMTI_ERROR_NONE; > > + } else if (ret != JVMTI_ERROR_NONE) { > > print_error(jvmti, "GetLineNumberTable", ret); > > return ret; > > } > > @@ -93,6 +97,9 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info, jvmti_line_info_t ** > > /* free what was allocated for nothing */ > > (*jvmti)->Deallocate(jvmti, (unsigned char *)lne); > > nr_total += (int)nr; > > + } else if (ret == JVMTI_ERROR_ABSENT_INFORMATION > > + || ret == JVMTI_ERROR_NATIVE_METHOD) { > > Please add the || operator at the end of the line next time, I'm fixing > this up this time, > > Applied, This: diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c index a9a056d68416..c5d30834a64c 100644 --- a/tools/perf/jvmti/libjvmti.c +++ b/tools/perf/jvmti/libjvmti.c @@ -97,8 +97,8 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info, jvmti_line_info_t ** /* free what was allocated for nothing */ (*jvmti)->Deallocate(jvmti, (unsigned char *)lne); nr_total += (int)nr; - } else if (ret == JVMTI_ERROR_ABSENT_INFORMATION - || ret == JVMTI_ERROR_NATIVE_METHOD) { + } else if (ret == JVMTI_ERROR_ABSENT_INFORMATION || + ret == JVMTI_ERROR_NATIVE_METHOD) { /* No debug information for this method */ } else { print_error(jvmti, "GetLineNumberTable", ret); ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/3] perf jvmti: Fix demangling Java symbols 2020-04-27 6:15 [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent Nick Gasson 2020-04-27 6:15 ` [PATCH 1/3] perf jvmti: Fix jitdump for methods without debug info Nick Gasson 2020-04-27 6:15 ` [PATCH 2/3] perf jvmti: Do not report error when missing debug information Nick Gasson @ 2020-04-27 6:15 ` Nick Gasson 2020-05-14 22:09 ` Ian Rogers ` (2 more replies) 2020-04-27 10:35 ` [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent Jiri Olsa 3 siblings, 3 replies; 19+ messages in thread From: Nick Gasson @ 2020-04-27 6:15 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim Cc: Nick Gasson, linux-kernel For a Java method signature like: Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V The demangler produces: void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) The arguments should be (java.lang.String, int, int) but the demangler interprets the "S" in String as the type code for "short". Correct this and two other minor things: - There is no "bool" type in Java, should be "boolean". - The demangler prepends "class" to every Java class name. This is not standard Java syntax and it wastes a lot of horizontal space if the signature is long. Remove this as there isn't any ambiguity between class names and primitives. Also added a test case. Signed-off-by: Nick Gasson <nick.gasson@arm.com> --- tools/perf/tests/Build | 1 + tools/perf/tests/builtin-test.c | 4 +++ tools/perf/tests/demangle-java-test.c | 42 +++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + tools/perf/util/demangle-java.c | 13 +++++---- 5 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 tools/perf/tests/demangle-java-test.c diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build index 1692529639b0..2c45ac4a9581 100644 --- a/tools/perf/tests/Build +++ b/tools/perf/tests/Build @@ -55,6 +55,7 @@ perf-y += mem2node.o perf-y += maps.o perf-y += time-utils-test.o perf-y += genelf.o +perf-y += demangle-java-test.o $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build $(call rule_mkdir) diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 54d9516c9839..03b362b37f97 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -309,6 +309,10 @@ static struct test generic_tests[] = { .desc = "maps__merge_in", .func = test__maps__merge_in, }, + { + .desc = "Demangle Java", + .func = test__demangle_java, + }, { .func = NULL, }, diff --git a/tools/perf/tests/demangle-java-test.c b/tools/perf/tests/demangle-java-test.c new file mode 100644 index 000000000000..8f3b90832fb0 --- /dev/null +++ b/tools/perf/tests/demangle-java-test.c @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <string.h> +#include <stdlib.h> +#include <stdio.h> +#include "tests.h" +#include "session.h" +#include "debug.h" +#include "demangle-java.h" + +int test__demangle_java(struct test *test __maybe_unused, int subtest __maybe_unused) +{ + int ret = TEST_OK; + char *buf = NULL; + size_t i; + + struct { + const char *mangled, *demangled; + } test_cases[] = { + { "Ljava/lang/StringLatin1;equals([B[B)Z", + "boolean java.lang.StringLatin1.equals(byte[], byte[])" }, + { "Ljava/util/zip/ZipUtils;CENSIZ([BI)J", + "long java.util.zip.ZipUtils.CENSIZ(byte[], int)" }, + { "Ljava/util/regex/Pattern$BmpCharProperty;match(Ljava/util/regex/Matcher;ILjava/lang/CharSequence;)Z", + "boolean java.util.regex.Pattern$BmpCharProperty.match(java.util.regex.Matcher, int, java.lang.CharSequence)" }, + { "Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V", + "void java.lang.AbstractStringBuilder.appendChars(java.lang.String, int, int)" }, + { "Ljava/lang/Object;<init>()V", + "void java.lang.Object<init>()" }, + }; + + for (i = 0; i < sizeof(test_cases) / sizeof(test_cases[0]); i++) { + buf = java_demangle_sym(test_cases[i].mangled, 0); + if (strcmp(buf, test_cases[i].demangled)) { + pr_debug("FAILED: %s: %s != %s\n", test_cases[i].mangled, + buf, test_cases[i].demangled); + ret = TEST_FAIL; + } + free(buf); + } + + return ret; +} diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h index 9a160fef47c9..49b791d978f6 100644 --- a/tools/perf/tests/tests.h +++ b/tools/perf/tests/tests.h @@ -111,6 +111,7 @@ int test__mem2node(struct test *t, int subtest); int test__maps__merge_in(struct test *t, int subtest); int test__time_utils(struct test *t, int subtest); int test__jit_write_elf(struct test *test, int subtest); +int test__demangle_java(struct test *test, int subtest); bool test__bp_signal_is_supported(void); bool test__bp_account_is_supported(void); diff --git a/tools/perf/util/demangle-java.c b/tools/perf/util/demangle-java.c index 6fb7f34c0814..39c05200ed65 100644 --- a/tools/perf/util/demangle-java.c +++ b/tools/perf/util/demangle-java.c @@ -15,7 +15,7 @@ enum { MODE_CLASS = 1, MODE_FUNC = 2, MODE_TYPE = 3, - MODE_CTYPE = 3, /* class arg */ + MODE_CTYPE = 4, /* class arg */ }; #define BASE_ENT(c, n) [c - 'A']=n @@ -27,7 +27,7 @@ static const char *base_types['Z' - 'A' + 1] = { BASE_ENT('I', "int" ), BASE_ENT('J', "long" ), BASE_ENT('S', "short" ), - BASE_ENT('Z', "bool" ), + BASE_ENT('Z', "boolean" ), }; /* @@ -59,15 +59,16 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int switch (*q) { case 'L': - if (mode == MODE_PREFIX || mode == MODE_CTYPE) { - if (mode == MODE_CTYPE) { + if (mode == MODE_PREFIX || mode == MODE_TYPE) { + if (mode == MODE_TYPE) { if (narg) rlen += scnprintf(buf + rlen, maxlen - rlen, ", "); narg++; } - rlen += scnprintf(buf + rlen, maxlen - rlen, "class "); if (mode == MODE_PREFIX) mode = MODE_CLASS; + else + mode = MODE_CTYPE; } else buf[rlen++] = *q; break; @@ -120,7 +121,7 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int if (mode != MODE_CLASS && mode != MODE_CTYPE) goto error; /* safe because at least one other char to process */ - if (isalpha(*(q + 1))) + if (isalpha(*(q + 1)) && mode == MODE_CLASS) rlen += scnprintf(buf + rlen, maxlen - rlen, "."); if (mode == MODE_CLASS) mode = MODE_FUNC; -- 2.26.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols 2020-04-27 6:15 ` [PATCH 3/3] perf jvmti: Fix demangling Java symbols Nick Gasson @ 2020-05-14 22:09 ` Ian Rogers 2020-05-27 14:10 ` Arnaldo Carvalho de Melo 2020-05-27 14:20 ` Arnaldo Carvalho de Melo 2 siblings, 0 replies; 19+ messages in thread From: Ian Rogers @ 2020-05-14 22:09 UTC (permalink / raw) To: Nick Gasson Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, LKML On Sun, Apr 26, 2020 at 11:16 PM Nick Gasson <nick.gasson@arm.com> wrote: > > For a Java method signature like: > > Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V > > The demangler produces: > > void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) > > The arguments should be (java.lang.String, int, int) but the demangler > interprets the "S" in String as the type code for "short". Correct this > and two other minor things: > > - There is no "bool" type in Java, should be "boolean". > > - The demangler prepends "class" to every Java class name. This is not > standard Java syntax and it wastes a lot of horizontal space if the > signature is long. Remove this as there isn't any ambiguity between > class names and primitives. > > Also added a test case. > > Signed-off-by: Nick Gasson <nick.gasson@arm.com> Reviewed-by: Ian Rogers <irogers@google.com> Tested-by: Ian Rogers <irogers@google.com> Thanks! Ian > --- > tools/perf/tests/Build | 1 + > tools/perf/tests/builtin-test.c | 4 +++ > tools/perf/tests/demangle-java-test.c | 42 +++++++++++++++++++++++++++ > tools/perf/tests/tests.h | 1 + > tools/perf/util/demangle-java.c | 13 +++++---- > 5 files changed, 55 insertions(+), 6 deletions(-) > create mode 100644 tools/perf/tests/demangle-java-test.c > > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build > index 1692529639b0..2c45ac4a9581 100644 > --- a/tools/perf/tests/Build > +++ b/tools/perf/tests/Build > @@ -55,6 +55,7 @@ perf-y += mem2node.o > perf-y += maps.o > perf-y += time-utils-test.o > perf-y += genelf.o > +perf-y += demangle-java-test.o > > $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build > $(call rule_mkdir) > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > index 54d9516c9839..03b362b37f97 100644 > --- a/tools/perf/tests/builtin-test.c > +++ b/tools/perf/tests/builtin-test.c > @@ -309,6 +309,10 @@ static struct test generic_tests[] = { > .desc = "maps__merge_in", > .func = test__maps__merge_in, > }, > + { > + .desc = "Demangle Java", > + .func = test__demangle_java, > + }, > { > .func = NULL, > }, > diff --git a/tools/perf/tests/demangle-java-test.c b/tools/perf/tests/demangle-java-test.c > new file mode 100644 > index 000000000000..8f3b90832fb0 > --- /dev/null > +++ b/tools/perf/tests/demangle-java-test.c > @@ -0,0 +1,42 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <string.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include "tests.h" > +#include "session.h" > +#include "debug.h" > +#include "demangle-java.h" > + > +int test__demangle_java(struct test *test __maybe_unused, int subtest __maybe_unused) > +{ > + int ret = TEST_OK; > + char *buf = NULL; > + size_t i; > + > + struct { > + const char *mangled, *demangled; > + } test_cases[] = { > + { "Ljava/lang/StringLatin1;equals([B[B)Z", > + "boolean java.lang.StringLatin1.equals(byte[], byte[])" }, > + { "Ljava/util/zip/ZipUtils;CENSIZ([BI)J", > + "long java.util.zip.ZipUtils.CENSIZ(byte[], int)" }, > + { "Ljava/util/regex/Pattern$BmpCharProperty;match(Ljava/util/regex/Matcher;ILjava/lang/CharSequence;)Z", > + "boolean java.util.regex.Pattern$BmpCharProperty.match(java.util.regex.Matcher, int, java.lang.CharSequence)" }, > + { "Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V", > + "void java.lang.AbstractStringBuilder.appendChars(java.lang.String, int, int)" }, > + { "Ljava/lang/Object;<init>()V", > + "void java.lang.Object<init>()" }, > + }; > + > + for (i = 0; i < sizeof(test_cases) / sizeof(test_cases[0]); i++) { > + buf = java_demangle_sym(test_cases[i].mangled, 0); > + if (strcmp(buf, test_cases[i].demangled)) { > + pr_debug("FAILED: %s: %s != %s\n", test_cases[i].mangled, > + buf, test_cases[i].demangled); > + ret = TEST_FAIL; > + } > + free(buf); > + } > + > + return ret; > +} > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h > index 9a160fef47c9..49b791d978f6 100644 > --- a/tools/perf/tests/tests.h > +++ b/tools/perf/tests/tests.h > @@ -111,6 +111,7 @@ int test__mem2node(struct test *t, int subtest); > int test__maps__merge_in(struct test *t, int subtest); > int test__time_utils(struct test *t, int subtest); > int test__jit_write_elf(struct test *test, int subtest); > +int test__demangle_java(struct test *test, int subtest); > > bool test__bp_signal_is_supported(void); > bool test__bp_account_is_supported(void); > diff --git a/tools/perf/util/demangle-java.c b/tools/perf/util/demangle-java.c > index 6fb7f34c0814..39c05200ed65 100644 > --- a/tools/perf/util/demangle-java.c > +++ b/tools/perf/util/demangle-java.c > @@ -15,7 +15,7 @@ enum { > MODE_CLASS = 1, > MODE_FUNC = 2, > MODE_TYPE = 3, > - MODE_CTYPE = 3, /* class arg */ > + MODE_CTYPE = 4, /* class arg */ > }; > > #define BASE_ENT(c, n) [c - 'A']=n > @@ -27,7 +27,7 @@ static const char *base_types['Z' - 'A' + 1] = { > BASE_ENT('I', "int" ), > BASE_ENT('J', "long" ), > BASE_ENT('S', "short" ), > - BASE_ENT('Z', "bool" ), > + BASE_ENT('Z', "boolean" ), > }; > > /* > @@ -59,15 +59,16 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int > > switch (*q) { > case 'L': > - if (mode == MODE_PREFIX || mode == MODE_CTYPE) { > - if (mode == MODE_CTYPE) { > + if (mode == MODE_PREFIX || mode == MODE_TYPE) { > + if (mode == MODE_TYPE) { > if (narg) > rlen += scnprintf(buf + rlen, maxlen - rlen, ", "); > narg++; > } > - rlen += scnprintf(buf + rlen, maxlen - rlen, "class "); > if (mode == MODE_PREFIX) > mode = MODE_CLASS; > + else > + mode = MODE_CTYPE; > } else > buf[rlen++] = *q; > break; > @@ -120,7 +121,7 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int > if (mode != MODE_CLASS && mode != MODE_CTYPE) > goto error; > /* safe because at least one other char to process */ > - if (isalpha(*(q + 1))) > + if (isalpha(*(q + 1)) && mode == MODE_CLASS) > rlen += scnprintf(buf + rlen, maxlen - rlen, "."); > if (mode == MODE_CLASS) > mode = MODE_FUNC; > -- > 2.26.1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols 2020-04-27 6:15 ` [PATCH 3/3] perf jvmti: Fix demangling Java symbols Nick Gasson 2020-05-14 22:09 ` Ian Rogers @ 2020-05-27 14:10 ` Arnaldo Carvalho de Melo 2020-05-27 14:20 ` Arnaldo Carvalho de Melo 2 siblings, 0 replies; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-27 14:10 UTC (permalink / raw) To: Nick Gasson Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-kernel Em Mon, Apr 27, 2020 at 02:15:16PM +0800, Nick Gasson escreveu: > For a Java method signature like: > > Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V > > The demangler produces: > > void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) Cool, one more 'perf test' entry, thanks a lot! Applied, - Arnaldo > The arguments should be (java.lang.String, int, int) but the demangler > interprets the "S" in String as the type code for "short". Correct this > and two other minor things: > > - There is no "bool" type in Java, should be "boolean". > > - The demangler prepends "class" to every Java class name. This is not > standard Java syntax and it wastes a lot of horizontal space if the > signature is long. Remove this as there isn't any ambiguity between > class names and primitives. > > Also added a test case. > > Signed-off-by: Nick Gasson <nick.gasson@arm.com> > --- > tools/perf/tests/Build | 1 + > tools/perf/tests/builtin-test.c | 4 +++ > tools/perf/tests/demangle-java-test.c | 42 +++++++++++++++++++++++++++ > tools/perf/tests/tests.h | 1 + > tools/perf/util/demangle-java.c | 13 +++++---- > 5 files changed, 55 insertions(+), 6 deletions(-) > create mode 100644 tools/perf/tests/demangle-java-test.c > > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build > index 1692529639b0..2c45ac4a9581 100644 > --- a/tools/perf/tests/Build > +++ b/tools/perf/tests/Build > @@ -55,6 +55,7 @@ perf-y += mem2node.o > perf-y += maps.o > perf-y += time-utils-test.o > perf-y += genelf.o > +perf-y += demangle-java-test.o > > $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build > $(call rule_mkdir) > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > index 54d9516c9839..03b362b37f97 100644 > --- a/tools/perf/tests/builtin-test.c > +++ b/tools/perf/tests/builtin-test.c > @@ -309,6 +309,10 @@ static struct test generic_tests[] = { > .desc = "maps__merge_in", > .func = test__maps__merge_in, > }, > + { > + .desc = "Demangle Java", > + .func = test__demangle_java, > + }, > { > .func = NULL, > }, > diff --git a/tools/perf/tests/demangle-java-test.c b/tools/perf/tests/demangle-java-test.c > new file mode 100644 > index 000000000000..8f3b90832fb0 > --- /dev/null > +++ b/tools/perf/tests/demangle-java-test.c > @@ -0,0 +1,42 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <string.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include "tests.h" > +#include "session.h" > +#include "debug.h" > +#include "demangle-java.h" > + > +int test__demangle_java(struct test *test __maybe_unused, int subtest __maybe_unused) > +{ > + int ret = TEST_OK; > + char *buf = NULL; > + size_t i; > + > + struct { > + const char *mangled, *demangled; > + } test_cases[] = { > + { "Ljava/lang/StringLatin1;equals([B[B)Z", > + "boolean java.lang.StringLatin1.equals(byte[], byte[])" }, > + { "Ljava/util/zip/ZipUtils;CENSIZ([BI)J", > + "long java.util.zip.ZipUtils.CENSIZ(byte[], int)" }, > + { "Ljava/util/regex/Pattern$BmpCharProperty;match(Ljava/util/regex/Matcher;ILjava/lang/CharSequence;)Z", > + "boolean java.util.regex.Pattern$BmpCharProperty.match(java.util.regex.Matcher, int, java.lang.CharSequence)" }, > + { "Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V", > + "void java.lang.AbstractStringBuilder.appendChars(java.lang.String, int, int)" }, > + { "Ljava/lang/Object;<init>()V", > + "void java.lang.Object<init>()" }, > + }; > + > + for (i = 0; i < sizeof(test_cases) / sizeof(test_cases[0]); i++) { > + buf = java_demangle_sym(test_cases[i].mangled, 0); > + if (strcmp(buf, test_cases[i].demangled)) { > + pr_debug("FAILED: %s: %s != %s\n", test_cases[i].mangled, > + buf, test_cases[i].demangled); > + ret = TEST_FAIL; > + } > + free(buf); > + } > + > + return ret; > +} > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h > index 9a160fef47c9..49b791d978f6 100644 > --- a/tools/perf/tests/tests.h > +++ b/tools/perf/tests/tests.h > @@ -111,6 +111,7 @@ int test__mem2node(struct test *t, int subtest); > int test__maps__merge_in(struct test *t, int subtest); > int test__time_utils(struct test *t, int subtest); > int test__jit_write_elf(struct test *test, int subtest); > +int test__demangle_java(struct test *test, int subtest); > > bool test__bp_signal_is_supported(void); > bool test__bp_account_is_supported(void); > diff --git a/tools/perf/util/demangle-java.c b/tools/perf/util/demangle-java.c > index 6fb7f34c0814..39c05200ed65 100644 > --- a/tools/perf/util/demangle-java.c > +++ b/tools/perf/util/demangle-java.c > @@ -15,7 +15,7 @@ enum { > MODE_CLASS = 1, > MODE_FUNC = 2, > MODE_TYPE = 3, > - MODE_CTYPE = 3, /* class arg */ > + MODE_CTYPE = 4, /* class arg */ > }; > > #define BASE_ENT(c, n) [c - 'A']=n > @@ -27,7 +27,7 @@ static const char *base_types['Z' - 'A' + 1] = { > BASE_ENT('I', "int" ), > BASE_ENT('J', "long" ), > BASE_ENT('S', "short" ), > - BASE_ENT('Z', "bool" ), > + BASE_ENT('Z', "boolean" ), > }; > > /* > @@ -59,15 +59,16 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int > > switch (*q) { > case 'L': > - if (mode == MODE_PREFIX || mode == MODE_CTYPE) { > - if (mode == MODE_CTYPE) { > + if (mode == MODE_PREFIX || mode == MODE_TYPE) { > + if (mode == MODE_TYPE) { > if (narg) > rlen += scnprintf(buf + rlen, maxlen - rlen, ", "); > narg++; > } > - rlen += scnprintf(buf + rlen, maxlen - rlen, "class "); > if (mode == MODE_PREFIX) > mode = MODE_CLASS; > + else > + mode = MODE_CTYPE; > } else > buf[rlen++] = *q; > break; > @@ -120,7 +121,7 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int > if (mode != MODE_CLASS && mode != MODE_CTYPE) > goto error; > /* safe because at least one other char to process */ > - if (isalpha(*(q + 1))) > + if (isalpha(*(q + 1)) && mode == MODE_CLASS) > rlen += scnprintf(buf + rlen, maxlen - rlen, "."); > if (mode == MODE_CLASS) > mode = MODE_FUNC; > -- > 2.26.1 > -- - Arnaldo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols 2020-04-27 6:15 ` [PATCH 3/3] perf jvmti: Fix demangling Java symbols Nick Gasson 2020-05-14 22:09 ` Ian Rogers 2020-05-27 14:10 ` Arnaldo Carvalho de Melo @ 2020-05-27 14:20 ` Arnaldo Carvalho de Melo 2020-05-27 16:23 ` Arnaldo Carvalho de Melo 2 siblings, 1 reply; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-27 14:20 UTC (permalink / raw) To: Nick Gasson Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-kernel Em Mon, Apr 27, 2020 at 02:15:16PM +0800, Nick Gasson escreveu: > For a Java method signature like: > > Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V > > The demangler produces: > > void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) > > The arguments should be (java.lang.String, int, int) but the demangler > interprets the "S" in String as the type code for "short". Correct this > and two other minor things: > > - There is no "bool" type in Java, should be "boolean". > > - The demangler prepends "class" to every Java class name. This is not > standard Java syntax and it wastes a lot of horizontal space if the > signature is long. Remove this as there isn't any ambiguity between > class names and primitives. > > Also added a test case. So, I took this and split into a patch for the new 'perf test java' and then the fix, so that we can see the problem being detected and then apply the fix and see it fixed, the last patch in this series thus became: commit 341e11c1d445999932da3f5d626c9fe096949ae3 Author: Nick Gasson <nick.gasson@arm.com> Date: Mon Apr 27 14:15:16 2020 +0800 perf jvmti: Fix demangling Java symbols For a Java method signature like: Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V The demangler produces: void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) The arguments should be (java.lang.String, int, int) but the demangler interprets the "S" in String as the type code for "short". Correct this and two other minor things: - There is no "bool" type in Java, should be "boolean". - The demangler prepends "class" to every Java class name. This is not standard Java syntax and it wastes a lot of horizontal space if the signature is long. Remove this as there isn't any ambiguity between class names and primitives. Committer notes: This was split from a larger patch that also added a java demangler 'perf test' entry, that, before this patch shows the error being fixed by it: $ perf test java 65: Demangle Java : FAILED! $ perf test -v java Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc 65: Demangle Java : --- start --- test child forked, pid 307264 FAILED: Ljava/lang/StringLatin1;equals([B[B)Z: bool class java.lang.StringLatin1.equals(byte[], byte[]) != boolean java.lang.StringLatin1.equals(byte[], byte[]) FAILED: Ljava/util/zip/ZipUtils;CENSIZ([BI)J: long class java.util.zip.ZipUtils.CENSIZ(byte[], int) != long java.util.zip.ZipUtils.CENSIZ(byte[], int) FAILED: Ljava/util/regex/Pattern$BmpCharProperty;match(Ljava/util/regex/Matcher;ILjava/lang/CharSequence;)Z: bool class java.util.regex.Pattern$BmpCharProperty.match(class java.util.regex.Matcher., int, class java.lang., charhar, shortequence) != boolean java.util.regex.Pattern$BmpCharProperty.match(java.util.regex.Matcher, int, java.lang.CharSequence) FAILED: Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V: void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) != void java.lang.AbstractStringBuilder.appendChars(java.lang.String, int, int) FAILED: Ljava/lang/Object;<init>()V: void class java.lang.Object<init>() != void java.lang.Object<init>() test child finished with -1 ---- end ---- Demangle Java: FAILED! $ After applying this patch: $ perf test java 65: Demangle Java : Ok $ Signed-off-by: Nick Gasson <nick.gasson@arm.com> Reviewed-by: Ian Rogers <irogers@google.com> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Tested-by: Ian Rogers <irogers@google.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lore.kernel.org/lkml/20200427061520.24905-4-nick.gasson@arm.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> diff --git a/tools/perf/util/demangle-java.c b/tools/perf/util/demangle-java.c index 6fb7f34c0814..39c05200ed65 100644 --- a/tools/perf/util/demangle-java.c +++ b/tools/perf/util/demangle-java.c @@ -15,7 +15,7 @@ enum { MODE_CLASS = 1, MODE_FUNC = 2, MODE_TYPE = 3, - MODE_CTYPE = 3, /* class arg */ + MODE_CTYPE = 4, /* class arg */ }; #define BASE_ENT(c, n) [c - 'A']=n @@ -27,7 +27,7 @@ static const char *base_types['Z' - 'A' + 1] = { BASE_ENT('I', "int" ), BASE_ENT('J', "long" ), BASE_ENT('S', "short" ), - BASE_ENT('Z', "bool" ), + BASE_ENT('Z', "boolean" ), }; /* @@ -59,15 +59,16 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int switch (*q) { case 'L': - if (mode == MODE_PREFIX || mode == MODE_CTYPE) { - if (mode == MODE_CTYPE) { + if (mode == MODE_PREFIX || mode == MODE_TYPE) { + if (mode == MODE_TYPE) { if (narg) rlen += scnprintf(buf + rlen, maxlen - rlen, ", "); narg++; } - rlen += scnprintf(buf + rlen, maxlen - rlen, "class "); if (mode == MODE_PREFIX) mode = MODE_CLASS; + else + mode = MODE_CTYPE; } else buf[rlen++] = *q; break; @@ -120,7 +121,7 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int if (mode != MODE_CLASS && mode != MODE_CTYPE) goto error; /* safe because at least one other char to process */ - if (isalpha(*(q + 1))) + if (isalpha(*(q + 1)) && mode == MODE_CLASS) rlen += scnprintf(buf + rlen, maxlen - rlen, "."); if (mode == MODE_CLASS) mode = MODE_FUNC; ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols 2020-05-27 14:20 ` Arnaldo Carvalho de Melo @ 2020-05-27 16:23 ` Arnaldo Carvalho de Melo 2020-05-27 22:34 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-27 16:23 UTC (permalink / raw) To: Nick Gasson Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-kernel Em Wed, May 27, 2020 at 11:20:57AM -0300, Arnaldo Carvalho de Melo escreveu: > Em Mon, Apr 27, 2020 at 02:15:16PM +0800, Nick Gasson escreveu: > > For a Java method signature like: > > > > Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V > > > > The demangler produces: > > > > void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) > > > > The arguments should be (java.lang.String, int, int) but the demangler > > interprets the "S" in String as the type code for "short". Correct this > > and two other minor things: > > > > - There is no "bool" type in Java, should be "boolean". > > > > - The demangler prepends "class" to every Java class name. This is not > > standard Java syntax and it wastes a lot of horizontal space if the > > signature is long. Remove this as there isn't any ambiguity between > > class names and primitives. > > > > Also added a test case. > > So, I took this and split into a patch for the new 'perf test java' and > then the fix, so that we can see the problem being detected and then > apply the fix and see it fixed, the last patch in this series thus > became: This is in my tmp.perf/core branch pending a round of testing, after that it'll move to perf/core on its way to 5.8, thanks. - Arnaldo > > commit 341e11c1d445999932da3f5d626c9fe096949ae3 > Author: Nick Gasson <nick.gasson@arm.com> > Date: Mon Apr 27 14:15:16 2020 +0800 > > perf jvmti: Fix demangling Java symbols > > For a Java method signature like: > > Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V > > The demangler produces: > > void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) > > The arguments should be (java.lang.String, int, int) but the demangler > interprets the "S" in String as the type code for "short". Correct this > and two other minor things: > > - There is no "bool" type in Java, should be "boolean". > > - The demangler prepends "class" to every Java class name. This is not > standard Java syntax and it wastes a lot of horizontal space if the > signature is long. Remove this as there isn't any ambiguity between > class names and primitives. > > Committer notes: > > This was split from a larger patch that also added a java demangler > 'perf test' entry, that, before this patch shows the error being fixed > by it: > > $ perf test java > 65: Demangle Java : FAILED! > $ perf test -v java > Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc > 65: Demangle Java : > --- start --- > test child forked, pid 307264 > FAILED: Ljava/lang/StringLatin1;equals([B[B)Z: bool class java.lang.StringLatin1.equals(byte[], byte[]) != boolean java.lang.StringLatin1.equals(byte[], byte[]) > FAILED: Ljava/util/zip/ZipUtils;CENSIZ([BI)J: long class java.util.zip.ZipUtils.CENSIZ(byte[], int) != long java.util.zip.ZipUtils.CENSIZ(byte[], int) > FAILED: Ljava/util/regex/Pattern$BmpCharProperty;match(Ljava/util/regex/Matcher;ILjava/lang/CharSequence;)Z: bool class java.util.regex.Pattern$BmpCharProperty.match(class java.util.regex.Matcher., int, class java.lang., charhar, shortequence) != boolean java.util.regex.Pattern$BmpCharProperty.match(java.util.regex.Matcher, int, java.lang.CharSequence) > FAILED: Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V: void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) != void java.lang.AbstractStringBuilder.appendChars(java.lang.String, int, int) > FAILED: Ljava/lang/Object;<init>()V: void class java.lang.Object<init>() != void java.lang.Object<init>() > test child finished with -1 > ---- end ---- > Demangle Java: FAILED! > $ > > After applying this patch: > > $ perf test java > 65: Demangle Java : Ok > $ > > Signed-off-by: Nick Gasson <nick.gasson@arm.com> > Reviewed-by: Ian Rogers <irogers@google.com> > Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> > Tested-by: Ian Rogers <irogers@google.com> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Peter Zijlstra <peterz@infradead.org> > Link: http://lore.kernel.org/lkml/20200427061520.24905-4-nick.gasson@arm.com > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> > > diff --git a/tools/perf/util/demangle-java.c b/tools/perf/util/demangle-java.c > index 6fb7f34c0814..39c05200ed65 100644 > --- a/tools/perf/util/demangle-java.c > +++ b/tools/perf/util/demangle-java.c > @@ -15,7 +15,7 @@ enum { > MODE_CLASS = 1, > MODE_FUNC = 2, > MODE_TYPE = 3, > - MODE_CTYPE = 3, /* class arg */ > + MODE_CTYPE = 4, /* class arg */ > }; > > #define BASE_ENT(c, n) [c - 'A']=n > @@ -27,7 +27,7 @@ static const char *base_types['Z' - 'A' + 1] = { > BASE_ENT('I', "int" ), > BASE_ENT('J', "long" ), > BASE_ENT('S', "short" ), > - BASE_ENT('Z', "bool" ), > + BASE_ENT('Z', "boolean" ), > }; > > /* > @@ -59,15 +59,16 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int > > switch (*q) { > case 'L': > - if (mode == MODE_PREFIX || mode == MODE_CTYPE) { > - if (mode == MODE_CTYPE) { > + if (mode == MODE_PREFIX || mode == MODE_TYPE) { > + if (mode == MODE_TYPE) { > if (narg) > rlen += scnprintf(buf + rlen, maxlen - rlen, ", "); > narg++; > } > - rlen += scnprintf(buf + rlen, maxlen - rlen, "class "); > if (mode == MODE_PREFIX) > mode = MODE_CLASS; > + else > + mode = MODE_CTYPE; > } else > buf[rlen++] = *q; > break; > @@ -120,7 +121,7 @@ __demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int > if (mode != MODE_CLASS && mode != MODE_CTYPE) > goto error; > /* safe because at least one other char to process */ > - if (isalpha(*(q + 1))) > + if (isalpha(*(q + 1)) && mode == MODE_CLASS) > rlen += scnprintf(buf + rlen, maxlen - rlen, "."); > if (mode == MODE_CLASS) > mode = MODE_FUNC; -- - Arnaldo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols 2020-05-27 16:23 ` Arnaldo Carvalho de Melo @ 2020-05-27 22:34 ` Arnaldo Carvalho de Melo 2020-05-28 5:42 ` Nick Gasson 0 siblings, 1 reply; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-27 22:34 UTC (permalink / raw) To: Nick Gasson Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-kernel Em Wed, May 27, 2020 at 01:23:00PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Wed, May 27, 2020 at 11:20:57AM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Mon, Apr 27, 2020 at 02:15:16PM +0800, Nick Gasson escreveu: > > > For a Java method signature like: > > > > > > Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V > > > > > > The demangler produces: > > > > > > void class java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int, int) > > > > > > The arguments should be (java.lang.String, int, int) but the demangler > > > interprets the "S" in String as the type code for "short". Correct this > > > and two other minor things: > > > > > > - There is no "bool" type in Java, should be "boolean". > > > > > > - The demangler prepends "class" to every Java class name. This is not > > > standard Java syntax and it wastes a lot of horizontal space if the > > > signature is long. Remove this as there isn't any ambiguity between > > > class names and primitives. > > > > > > Also added a test case. > > > > So, I took this and split into a patch for the new 'perf test java' and > > then the fix, so that we can see the problem being detected and then > > apply the fix and see it fixed, the last patch in this series thus > > became: > > This is in my tmp.perf/core branch pending a round of testing, after > that it'll move to perf/core on its way to 5.8, thanks. All tests passed, moved to perf/core. - Arnaldo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols 2020-05-27 22:34 ` Arnaldo Carvalho de Melo @ 2020-05-28 5:42 ` Nick Gasson 0 siblings, 0 replies; 19+ messages in thread From: Nick Gasson @ 2020-05-28 5:42 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: linux-kernel On 05/28/20 06:34 AM, Arnaldo Carvalho de Melo wrote: >> >> This is in my tmp.perf/core branch pending a round of testing, after >> that it'll move to perf/core on its way to 5.8, thanks. > > All tests passed, moved to perf/core. > Great, thank you! -- Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent 2020-04-27 6:15 [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent Nick Gasson ` (2 preceding siblings ...) 2020-04-27 6:15 ` [PATCH 3/3] perf jvmti: Fix demangling Java symbols Nick Gasson @ 2020-04-27 10:35 ` Jiri Olsa 2020-05-14 8:56 ` Nick Gasson 3 siblings, 1 reply; 19+ messages in thread From: Jiri Olsa @ 2020-04-27 10:35 UTC (permalink / raw) To: Nick Gasson Cc: Alexander Shishkin, Arnaldo Carvalho de Melo, Ingo Molnar, Mark Rutland, Namhyung Kim, Peter Zijlstra, linux-kernel, Stephane Eranian On Mon, Apr 27, 2020 at 02:15:13PM +0800, Nick Gasson wrote: > Hi, adding Stephane to the loop jirka > > These three patches fix a couple of issues I ran into while using the > jitdump JVMTI agent to profile the SPECjbb benchmark. > > Tested with OpenJDK 8 and 14. > > Thanks, > Nick > > > Nick Gasson (3): > perf jvmti: Fix jitdump for methods without debug info > perf jvmti: Do not report error when missing debug information > perf jvmti: Fix demangling Java symbols > > tools/perf/jvmti/libjvmti.c | 24 +++++++-------- > tools/perf/tests/Build | 1 + > tools/perf/tests/builtin-test.c | 4 +++ > tools/perf/tests/demangle-java-test.c | 42 +++++++++++++++++++++++++++ > tools/perf/tests/tests.h | 1 + > tools/perf/util/demangle-java.c | 13 +++++---- > 6 files changed, 66 insertions(+), 19 deletions(-) > create mode 100644 tools/perf/tests/demangle-java-test.c > > -- > 2.26.1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent 2020-04-27 10:35 ` [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent Jiri Olsa @ 2020-05-14 8:56 ` Nick Gasson 2020-05-14 13:23 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 19+ messages in thread From: Nick Gasson @ 2020-05-14 8:56 UTC (permalink / raw) To: Jiri Olsa Cc: Alexander Shishkin, Arnaldo Carvalho de Melo, Ingo Molnar, Mark Rutland, Namhyung Kim, Peter Zijlstra, linux-kernel, Stephane Eranian On 04/27/20 18:35 pm, Jiri Olsa wrote: > > adding Stephane to the loop > > jirka > >> >> These three patches fix a couple of issues I ran into while using the >> jitdump JVMTI agent to profile the SPECjbb benchmark. >> Hi, any feedback on these patches? Thanks, Nick >> >> >> Nick Gasson (3): >> perf jvmti: Fix jitdump for methods without debug info >> perf jvmti: Do not report error when missing debug information >> perf jvmti: Fix demangling Java symbols >> >> tools/perf/jvmti/libjvmti.c | 24 +++++++-------- >> tools/perf/tests/Build | 1 + >> tools/perf/tests/builtin-test.c | 4 +++ >> tools/perf/tests/demangle-java-test.c | 42 +++++++++++++++++++++++++++ >> tools/perf/tests/tests.h | 1 + >> tools/perf/util/demangle-java.c | 13 +++++---- >> 6 files changed, 66 insertions(+), 19 deletions(-) >> create mode 100644 tools/perf/tests/demangle-java-test.c >> >> -- >> 2.26.1 >> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent 2020-05-14 8:56 ` Nick Gasson @ 2020-05-14 13:23 ` Arnaldo Carvalho de Melo 2020-05-14 22:41 ` Ian Rogers 0 siblings, 1 reply; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-14 13:23 UTC (permalink / raw) To: Stephane Eranian, Ian Rogers, Nick Gasson Cc: Jiri Olsa, Alexander Shishkin, Ingo Molnar, Mark Rutland, Namhyung Kim, Peter Zijlstra, linux-kernel Em Thu, May 14, 2020 at 04:56:07PM +0800, Nick Gasson escreveu: > On 04/27/20 18:35 pm, Jiri Olsa wrote: > > > > adding Stephane to the loop Stephane, Ian, can you guys please take a look at this one and provide a Reviewed-by or Acked-by? Thanks, - Arnaldo > > jirka > > > >> > >> These three patches fix a couple of issues I ran into while using the > >> jitdump JVMTI agent to profile the SPECjbb benchmark. > >> > > Hi, any feedback on these patches? > > Thanks, > Nick > > >> > >> > >> Nick Gasson (3): > >> perf jvmti: Fix jitdump for methods without debug info > >> perf jvmti: Do not report error when missing debug information > >> perf jvmti: Fix demangling Java symbols > >> > >> tools/perf/jvmti/libjvmti.c | 24 +++++++-------- > >> tools/perf/tests/Build | 1 + > >> tools/perf/tests/builtin-test.c | 4 +++ > >> tools/perf/tests/demangle-java-test.c | 42 +++++++++++++++++++++++++++ > >> tools/perf/tests/tests.h | 1 + > >> tools/perf/util/demangle-java.c | 13 +++++---- > >> 6 files changed, 66 insertions(+), 19 deletions(-) > >> create mode 100644 tools/perf/tests/demangle-java-test.c > >> > >> -- > >> 2.26.1 > >> > -- - Arnaldo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent 2020-05-14 13:23 ` Arnaldo Carvalho de Melo @ 2020-05-14 22:41 ` Ian Rogers 2020-05-15 7:45 ` Nick Gasson 0 siblings, 1 reply; 19+ messages in thread From: Ian Rogers @ 2020-05-14 22:41 UTC (permalink / raw) To: Nick Gasson Cc: Stephane Eranian, Jiri Olsa, Alexander Shishkin, Ingo Molnar, Mark Rutland, Namhyung Kim, Peter Zijlstra, linux-kernel, Arnaldo Carvalho de Melo On Thu, May 14, 2020 at 6:23 AM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Em Thu, May 14, 2020 at 04:56:07PM +0800, Nick Gasson escreveu: > > On 04/27/20 18:35 pm, Jiri Olsa wrote: > > > > > > adding Stephane to the loop > > Stephane, Ian, can you guys please take a look at this one and provide a > Reviewed-by or Acked-by? > > Thanks, > > - Arnaldo > > > > > jirka > > > > > >> > > >> These three patches fix a couple of issues I ran into while using the > > >> jitdump JVMTI agent to profile the SPECjbb benchmark. > > >> > > > > Hi, any feedback on these patches? > > > > Thanks, > > Nick > Thanks Nick, If you are looking at this code I believe there is a bug in that the loop handling jvmtiCompiledMethodLoadInlineRecord is writing out the entire line number table before a pc and not just the line number table at the pc. This loop in do_get_line_numbers: if (loc_tab[i].start_location < bci) { tab[lines].pc = (unsigned long)pc; tab[lines].line_number = loc_tab[i].line_number; tab[lines].discrim = 0; /* not yet used */ tab[lines].methodID = m; lines++; } else { It could possibly make sense if it were iterating over the inline data in the jvmtiCompiledMethodLoadInlineRecord rather than the line number table. Fixing this is toward the end of a list of things I need to look at. Thanks, Ian > > >> > > >> > > >> Nick Gasson (3): > > >> perf jvmti: Fix jitdump for methods without debug info > > >> perf jvmti: Do not report error when missing debug information > > >> perf jvmti: Fix demangling Java symbols > > >> > > >> tools/perf/jvmti/libjvmti.c | 24 +++++++-------- > > >> tools/perf/tests/Build | 1 + > > >> tools/perf/tests/builtin-test.c | 4 +++ > > >> tools/perf/tests/demangle-java-test.c | 42 +++++++++++++++++++++++++++ > > >> tools/perf/tests/tests.h | 1 + > > >> tools/perf/util/demangle-java.c | 13 +++++---- > > >> 6 files changed, 66 insertions(+), 19 deletions(-) > > >> create mode 100644 tools/perf/tests/demangle-java-test.c > > >> > > >> -- > > >> 2.26.1 > > >> > > > > -- > > - Arnaldo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent 2020-05-14 22:41 ` Ian Rogers @ 2020-05-15 7:45 ` Nick Gasson 0 siblings, 0 replies; 19+ messages in thread From: Nick Gasson @ 2020-05-15 7:45 UTC (permalink / raw) To: Ian Rogers Cc: Stephane Eranian, Jiri Olsa, Alexander Shishkin, Ingo Molnar, Mark Rutland, Namhyung Kim, Peter Zijlstra, linux-kernel, Arnaldo Carvalho de Melo On 05/15/20 06:41 am, Ian Rogers wrote: > > If you are looking at this code I believe there is a bug in that the > loop handling jvmtiCompiledMethodLoadInlineRecord is writing out the > entire line number table before a pc and not just the line number > table at the pc. This loop in do_get_line_numbers: > > if (loc_tab[i].start_location < bci) { > tab[lines].pc = (unsigned long)pc; > tab[lines].line_number = loc_tab[i].line_number; > tab[lines].discrim = 0; /* not yet used */ > tab[lines].methodID = m; > lines++; > } else { > > It could possibly make sense if it were iterating over the inline data > in the jvmtiCompiledMethodLoadInlineRecord rather than the line number > table. > Fixing this is toward the end of a list of things I need to look at. > OK sure, I'll have a look at this. Thanks for the reviews. -- Thanks, Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-05-28 5:42 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-27 6:15 [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent Nick Gasson 2020-04-27 6:15 ` [PATCH 1/3] perf jvmti: Fix jitdump for methods without debug info Nick Gasson 2020-05-14 22:06 ` Ian Rogers 2020-04-27 6:15 ` [PATCH 2/3] perf jvmti: Do not report error when missing debug information Nick Gasson 2020-05-14 22:08 ` Ian Rogers 2020-05-27 14:07 ` Arnaldo Carvalho de Melo 2020-05-27 14:08 ` Arnaldo Carvalho de Melo 2020-04-27 6:15 ` [PATCH 3/3] perf jvmti: Fix demangling Java symbols Nick Gasson 2020-05-14 22:09 ` Ian Rogers 2020-05-27 14:10 ` Arnaldo Carvalho de Melo 2020-05-27 14:20 ` Arnaldo Carvalho de Melo 2020-05-27 16:23 ` Arnaldo Carvalho de Melo 2020-05-27 22:34 ` Arnaldo Carvalho de Melo 2020-05-28 5:42 ` Nick Gasson 2020-04-27 10:35 ` [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent Jiri Olsa 2020-05-14 8:56 ` Nick Gasson 2020-05-14 13:23 ` Arnaldo Carvalho de Melo 2020-05-14 22:41 ` Ian Rogers 2020-05-15 7:45 ` Nick Gasson
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).