RFR: 8233285: Demangling C++ symbols in jhsdb jstack --mixed
Chris Plummer
chris.plummer at oracle.com
Tue Nov 5 07:06:42 UTC 2019
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