linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Nick Gasson <nick.gasson@arm.com>
Cc: Ian Rogers <irogers@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols
Date: Wed, 27 May 2020 11:20:57 -0300	[thread overview]
Message-ID: <20200527142057.GF14219@kernel.org> (raw)
In-Reply-To: <20200427061520.24905-4-nick.gasson@arm.com>

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;

  parent reply	other threads:[~2020-05-27 14:21 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200527142057.GF14219@kernel.org \
    --to=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=nick.gasson@arm.com \
    --cc=peterz@infradead.org \
    --subject='Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).