RFR: 8233285: Demangling C++ symbols in jhsdb jstack --mixed
Chris Plummer
chris.plummer at oracle.com
Tue Nov 5 19:45:08 UTC 2019
That's good to know.
thanks,
Chris
On 11/4/19 11:47 PM, Yasumasa Suenaga wrote:
> Hi Chris,
>
> It works well when I added "git=1" to ~/.hgrc .
> "git=1" seems to be the most important for webrev.
>
> I added it to all my machine :)
>
>
> Yasumasa
>
>
> On 2019/11/05 16:06, Chris Plummer wrote:
>> Hi Yasumasa,
>>
>> How did you even find that changeset? It's not clear to me that it
>> was actually a move + modification. I could have been an rm + add. I
>> don't think the changeset URL gives you enough information. But
>> here's one I did that had a 4 moves. They do show up in the changeset
>> rm + add:
>>
>> https://hg.openjdk.java.net/jdk/jdk/rev/a64caa5269cf
>>
>> You can't tell from the chagneset that files were moved, and 1 of
>> them modified, but you can from the webrev:
>>
>>
>> http://cr.openjdk.java.net/~cjplummer/8227645/webrev.00/webrev.open/
>>
>> Notice 3 of the moved files show up as new files with no diff, but
>> they also says "0 lines changed" so that is why they had no diff. One
>> file was both moved and had a change, so it shows up with a diff and
>> "1 line changed". It looks like when you move a file mercurial
>> actually copies it and deletes the original. See "hg log -v -C"
>> output for example. Notice the "copies" section when you use -C:
>>
>> $ hg log -v -C
>> test/hotspot/jtreg/resourcehogs/serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java
>> changeset: 55952:a64caa5269cf
>> user: cjplummer
>> date: Fri Aug 09 11:27:08 2019 -0700
>> files: test/hotspot/jtreg/TEST.groups
>> test/hotspot/jtreg/resourcehogs/TEST.properties
>> test/hotspot/jtreg/resourcehogs/serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java
>> test/hotspot/jtreg/resourcehogs/serviceability/sa/LingeredAppWithLargeArray.java
>> test/hotspot/jtreg/resourcehogs/serviceability/sa/LingeredAppWithLargeStringArray.java
>> test/hotspot/jtreg/resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java
>> test/hotspot/jtreg/serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java
>> test/hotspot/jtreg/serviceability/sa/LingeredAppWithLargeArray.java
>> test/hotspot/jtreg/serviceability/sa/LingeredAppWithLargeStringArray.java
>> test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java
>> copies:
>> test/hotspot/jtreg/resourcehogs/serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java
>> (test/hotspot/jtreg/serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java)
>> test/hotspot/jtreg/resourcehogs/serviceability/sa/LingeredAppWithLargeArray.java
>> (test/hotspot/jtreg/serviceability/sa/LingeredAppWithLargeArray.java)
>> test/hotspot/jtreg/resourcehogs/serviceability/sa/LingeredAppWithLargeStringArray.java
>> (test/hotspot/jtreg/serviceability/sa/LingeredAppWithLargeStringArray.java)
>> test/hotspot/jtreg/resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java
>> (test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java)
>> description:
>> 8227645: Some tests in serviceability/sa run with fixed -Xmx values
>> and risk running out of memory
>> Summary: move tests to seprate directory
>> Reviewed-by: dtitov, jcbeyler, ctornqvi, sspitsyn
>>
>> So maybe since mercurial is just making a copy of the file when you
>> move it, newer versions don't even bother trying to make the local
>> edits appear to be a "move + mod" anymore.
>>
>> Chris
>>
>> On 11/4/19 9:26 PM, Yasumasa Suenaga wrote:
>>> Hi Chris,
>>>
>>> I do not have [diff] section both ~/.hgrc and <jdk src>/.hg/hgrc.
>>> In particular I've not edited .hg/hgrc (except defpath).
>>>
>>> It seems to be nature at least hg of OpenJDK.
>>> For example, [1] moves (and make some changes) BaseFileManager, but
>>> the changeset removes old file and adds new one.
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> [1] http://hg.openjdk.java.net/jdk10/master/rev/8f8e54a1fa20
>>>
>>>
>>> On 2019/11/05 13:50, Chris Plummer wrote:
>>>> On 11/4/19 8:47 PM, Chris Plummer wrote:
>>>>> On 11/4/19 5:19 PM, Yasumasa Suenaga wrote:
>>>>>> Hi Chris,
>>>>>>
>>>>>> On 2019/11/05 2:28, Chris Plummer wrote:
>>>>>>> On 11/2/19 5:33 AM, Yasumasa Suenaga wrote:
>>>>>>>> Hi Chris,
>>>>>>>>
>>>>>>>> On 2019/11/02 7:07, Chris Plummer wrote:
>>>>>>>>> Hi Yasumasa,
>>>>>>>>>
>>>>>>>>> I can't comment on the build changes. I don't know how the
>>>>>>>>> build works well enough.
>>>>>>>>
>>>>>>>> I ensured it on submit repo
>>>>>>>> (mach5-one-ysuenaga-JDK-8233285-1-20191101-0746-6336009).
>>>>>>>> This change affects for Linux only. So I changed toolchain if
>>>>>>>> SA would be built for Linux only.
>>>>>>>>
>>>>>>>>
>>>>>>>>> LinuxDebuggerLocal.cpp should show up as a rename + diffs, not
>>>>>>>>> as a new file.
>>>>>>>>
>>>>>>>> I uploaded it:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8233285/webrev.00/LinuxDebuggerLocal.patch
>>>>>>>>
>>>>>>>>
>>>>>>>> If we can use Git, it can show "rename + diffs"...
>>>>>>>> I tried to use "hg rename" and "hg move", but the result did
>>>>>>>> not change.
>>>>>>> Hi Yasumasa,
>>>>>>>
>>>>>>> I just did an "hg mv" and then modified the file, and webrev did
>>>>>>> what it is suppose to do and showed just the diffs, but also
>>>>>>> indicated that the file was moved. Which version of webrev are
>>>>>>> you using?
>>>>>>
>>>>>> I'm using wevrev changeset: 24:8cd091802cd0 with Mercurial 4.5.3
>>>>>> on Ubuntu 18.04 LTS.
>>>>>>
>>>>>>> What do "hg diff" and "hg status" show?
>>>>>>
>>>>>> For example, rename Makefile to Makefile.orig:
>>>>>>
>>>>>> ```
>>>>>> $ hg mv Makefile Makefile.orig
>>>>>> $ hg status
>>>>>> A Makefile.orig
>>>>>> R Makefile
>>>>> This part looks correct.
>>>>>> $ hg diff
>>>>>> diff -r c41d1303a87c Makefile
>>>>>> --- a/Makefile Mon Nov 04 13:02:40 2019 -0800
>>>>>> +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
>>>>>> @@ -1,64 +0,0 @@
>>>>>> -#
>>>>>> -# Copyright (c) 2012, 2015, Oracle and/or its affiliates. All
>>>>>> rights reserved.
>>>>>> -# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>>> :
>>>>>> (snip)
>>>>>> ```
>>>>> This part is odd. Not sure why it says "diff -r". Mine looks like:
>>>>>
>>>>> $ hg status
>>>>> A test/hotspot/jtreg/serviceability/sa/CDSJMapClstats2.java
>>>>> R test/hotspot/jtreg/serviceability/sa/CDSJMapClstats.java
>>>>>
>>>>> $ hg diff
>>>>> diff --git
>>>>> a/test/hotspot/jtreg/serviceability/sa/CDSJMapClstats.java
>>>>> b/test/hotspot/jtreg/serviceability/sa/CDSJMapClstats2.java
>>>>> rename from test/hotspot/jtreg/serviceability/sa/CDSJMapClstats.java
>>>>> rename to test/hotspot/jtreg/serviceability/sa/CDSJMapClstats2.java
>>>>> --- a/test/hotspot/jtreg/serviceability/sa/CDSJMapClstats.java
>>>>> +++ b/test/hotspot/jtreg/serviceability/sa/CDSJMapClstats2.java
>>>>> @@ -40,7 +40,7 @@
>>>>> <snip>
>>>>>
>>>>> ..and the actual diff follows the above, which is just a one line
>>>>> edit. Do you have an override of "diff" in your .hgrc?
>>>> I should have mention what's in my .hgrc. because I just noticed
>>>> something:
>>>>
>>>> diff =
>>>> [diff]
>>>> git=1
>>>> nodates=1
>>>>
>>>> Don't ask me why I have this. I cloned someones .hgrc when openjdk
>>>> first moved to Mercurial, and have never touched this part of it.
>>>> At the very least the "git=1" would explain why my diff output says
>>>> "diff --git ...".
>>>>
>>>> Chris
>>>>>
>>>>> BTW, my Mercurial is 3.6.
>>>>>
>>>>> Chris
>>>>>
>>>>>
>>>>>>
>>>>>> It seems to be a problem in hg instead of webrev.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>> thanks,
>>>>>>>
>>>>>>> Chris
>>>>>>>>
>>>>>>>>> The rest of the changes look fine.
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>>> thanks,
>>>>>>>>>
>>>>>>>>> Chris
>>>>>>>>>
>>>>>>>>> On 11/1/19 1:56 AM, Yasumasa Suenaga wrote:
>>>>>>>>>> (Changed subject to review request)
>>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> I converted LinuxDebuggerLocal.c to C++ code, and it works
>>>>>>>>>> fine on submit repo.
>>>>>>>>>> (mach5-one-ysuenaga-JDK-8233285-1-20191101-0746-6336009)
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8233285/webrev.00/
>>>>>>>>>>
>>>>>>>>>> Could you review it?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2019/11/01 8:54, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi David,
>>>>>>>>>>>
>>>>>>>>>>> On 2019/11/01 7:55, David Holmes wrote:
>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>
>>>>>>>>>>>> New build dependencies cannot be added lightly. This
>>>>>>>>>>>> impacts everyone who maintains build/test farms.
>>>>>>>>>>>
>>>>>>>>>>> Ok, thanks for telling it.
>>>>>>>>>>>
>>>>>>>>>>>> We already use the C++ demangling capabilities in the VM.
>>>>>>>>>>>> Is there some way to export that for use by libsaproc ?
>>>>>>>>>>>>
>>>>>>>>>>>> Otherwise using C++ demangle may still be the better choice
>>>>>>>>>>>> given we already have it as a dependency.
>>>>>>>>>>>
>>>>>>>>>>> I found abi::__cxa_demangle() is used in
>>>>>>>>>>> ElfDecoder::demangle() at decoder_linux.cpp .
>>>>>>>>>>> It is similar with my original proposal.
>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/sa-demangle/
>>>>>>>>>>>
>>>>>>>>>>> I agree with David to use C++ demangle way.
>>>>>>>>>>> However we need to choice the fix from following:
>>>>>>>>>>>
>>>>>>>>>>> A. Convert LinuxDebuggerLocal.c to C++ code
>>>>>>>>>>> B. Add C++ code for libsaproc.so to demangle symbols.
>>>>>>>>>>>
>>>>>>>>>>> I've discussed with Chris about it in [1].
>>>>>>>>>>> Option A might be large change.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> [1]
>>>>>>>>>>> https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029716.html
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> David
>>>>>>>>>>>>
>>>>>>>>>>>> On 1/11/2019 12:58 am, Chris Plummer wrote:
>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Here's the failure during configure:
>>>>>>>>>>>>>
>>>>>>>>>>>>> [2019-10-31T06:07:45,131Z] checking demangle.h
>>>>>>>>>>>>> usability... no
>>>>>>>>>>>>> [2019-10-31T06:07:45,150Z] checking demangle.h presence... no
>>>>>>>>>>>>> [2019-10-31T06:07:45,150Z] checking for demangle.h... no
>>>>>>>>>>>>> [2019-10-31T06:07:45,151Z] configure: error: Could not
>>>>>>>>>>>>> find demangle.h! You might be able to fix this by running
>>>>>>>>>>>>> 'sudo yum install binutils-devel'.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 10/31/19 1:08 AM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I filed this enhancement to JBS:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8233285
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Also I pushed the changes to submit repo, but it was failed.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk/submit/rev/bfbc49233c26
>>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk/submit/rev/430e4f65ef25
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> According to the email from Mach 5, dependency errors
>>>>>>>>>>>>>> were occurred in jib.
>>>>>>>>>>>>>> Can someone share the details?
>>>>>>>>>>>>>> I'm not familiar in jib, so I want help.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> mach5-one-ysuenaga-JDK-8233285-20191031-0606-6301426
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2019/10/31 11:23, Chris Plummer wrote:
>>>>>>>>>>>>>>> You can change the configure script. I don't know if
>>>>>>>>>>>>>>> there's any concerns with using libiberty.a. That's
>>>>>>>>>>>>>>> possibly a legal question (GNU GPL). You might want to
>>>>>>>>>>>>>>> ask that on jdk-dev and/or build-dev.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 10/30/19 7:14 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>> Hi Chris,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks for quick reply!
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> If we convert LinuxDebuggerLocal.c to C++ code, we have
>>>>>>>>>>>>>>>> to convert a lot of JNI calls to C++ style.
>>>>>>>>>>>>>>>> For example:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> (*env)->FindClass(env, "java/lang/String")
>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>> env->FindClass("java/lang/String")
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Can it be accepted?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> OTOH I said in my email, to use cplus_demangle(), we
>>>>>>>>>>>>>>>> need to link libiberty.a which is provided by binutils.
>>>>>>>>>>>>>>>> Thus I think we need to check libiberty.a in configure
>>>>>>>>>>>>>>>> script. Is it ok?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I prefer to use cplus_demangle() if we can change
>>>>>>>>>>>>>>>> configure script.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 2019/10/31 11:03, Chris Plummer wrote:
>>>>>>>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I don't have concerns with adding C++ source to SA,
>>>>>>>>>>>>>>>>> but in order to do so you've put the new native code
>>>>>>>>>>>>>>>>> in its own file rather than in LinuxDebuggerLocal.c.
>>>>>>>>>>>>>>>>> I'd like to see that resolved. So either convert
>>>>>>>>>>>>>>>>> LinuxDebuggerLocal.c to C++, or use cplus_demangle().
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> thanks,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 10/30/19 6:54 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I saw C++ frames in `jhsdb jstack --mixed`, and they
>>>>>>>>>>>>>>>>>> were mangled as below:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> 0x00007ff255a8fa4c
>>>>>>>>>>>>>>>>>> _ZN9JavaCalls11call_helperEP9JavaValueRK12methodHandleP17JavaCallArgumentsP6Thread
>>>>>>>>>>>>>>>>>> + 0x6ac
>>>>>>>>>>>>>>>>>> 0x00007ff255a8cc1d
>>>>>>>>>>>>>>>>>> _ZN9JavaCalls12call_virtualEP9JavaValueP5KlassP6SymbolS5_P17JavaCallArgumentsP6Thread
>>>>>>>>>>>>>>>>>> + 0x33d
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> We can demangle them via c++filt, but I think it is
>>>>>>>>>>>>>>>>>> more convenience if jstack can show demangling symbols.
>>>>>>>>>>>>>>>>>> I think we can demangle in jstack with this patch. If
>>>>>>>>>>>>>>>>>> it is accepted, I will file it to JBS and send review
>>>>>>>>>>>>>>>>>> request.
>>>>>>>>>>>>>>>>>> What do you think?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/sa-demangle/
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> We can get the stack as below after applying this patch:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> 0x00007ff1aba20a4c JavaCalls::call_helper(JavaValue*,
>>>>>>>>>>>>>>>>>> methodHandle const&, JavaCallArguments*, Thread*) +
>>>>>>>>>>>>>>>>>> 0x6ac
>>>>>>>>>>>>>>>>>> 0x00007ff1aba1dc1d
>>>>>>>>>>>>>>>>>> JavaCalls::call_virtual(JavaValue*, Klass*, Symbol*,
>>>>>>>>>>>>>>>>>> Symbol*, JavaCallArguments*, Thread*) + 0x33d
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I use abi::__cxa_demangle() for demangling, so this
>>>>>>>>>>>>>>>>>> patch adds C++ source to SA.
>>>>>>>>>>>>>>>>>> If it is not comfortable, we can use cplus_demangle().
>>>>>>>>>>>>>>>>>> But this function is provided by libiberty.a, so we
>>>>>>>>>>>>>>>>>> need to link it to libsaproc and need to check
>>>>>>>>>>>>>>>>>> libiberty.a in configure script.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>
>>
More information about the serviceability-dev
mailing list