PING: RFR: JDK-8151815: Could not parse core image with JSnap.
Yasumasa Suenaga
yasuenag at gmail.com
Mon Jun 20 12:40:58 UTC 2016
PING: What do you think about this change?
Point of troubleshooting, I want to fix this issue.
Thanks,
Yasumasa
On 2016/05/25 20:58, Yasumasa Suenaga wrote:
> Hi Dmitry,
>
>>>> I understand the problem, but I'm concerned of storing pointers to
>>>> invalid memory region just for coredump. Sorry!
>
> The latest webrev [1] holds pointers for PerfMemory in _saved_* fields.
> These pointers are valid because they are not unmap'ed.
>
> I think that it is a bug not to parse PerfMemory in coredump.
> SA tools should be able to handle data in coredump.
>
> Can we fix this issue?
>
>
> Thanks,
>
> Yasumasa
>
>
> [1] http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/
>
>
> On 2016/05/06 22:39, Yasumasa Suenaga wrote:
>> Dmitry,
>>
>>>> The solution might be in another areas, e.g. print value of performance
>>>> counters to hs_err.log on crash.
>>>
>>> If so, we have to use native debugger.
>>> For Java developers and troubleshooters, I want to support this feature
>>> with JDK tool.
>>
>> If we encounter native stack overflow error, we cannot get hs_err log.
>> I posted a bug report for this issue as JDK-7109520, but it was rejected.
>>
>> In all crash case, we might collect core image.
>> If I collect it, I want to check performance counter in it.
>>
>> Thus I want to merge this patch to JDK 9.
>> Is it difficult?
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2016/05/06 21:42, Yasumasa Suenaga wrote:
>>> Hi Dmitry,
>>>
>>> On 2016/05/06 21:22, Dmitry Samersoff wrote:
>>>> Yasumasa,
>>>>
>>>> I understand the problem, but I'm concerned of storing pointers to
>>>> invalid memory region just for coredump. Sorry!
>>>
>>> I do not think that it is NOT invalid memory in webrev.02 .
>>> PerfMemory is mmap'ed, and it is not unmap'ed.
>>>
>>> In addition, webrev.02 adds several pointers to store addresses which are
>>> related to PerfMemory.
>>> It is not disruptive against current memory management which is related to
>>> PerfMemory.
>>>
>>> Coredump will include all virtual memory and thread contexts.
>>> If pointers ( _saved_* in webrev.02) are invalid, it is not affect JVM behavior,
>>> but JSnap will fail.
>>> (This pointer is referred from JSnap only.)
>>>
>>>
>>>> The solution might be in another areas, e.g. print value of performance
>>>> counters to hs_err.log on crash.
>>>
>>> If so, we have to use native debugger.
>>> For Java developers and troubleshooters, I want to support this feature
>>> with JDK tool.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>> -Dmitry
>>>>
>>>>
>>>> On 2016-05-06 14:42, Yasumasa Suenaga wrote:
>>>>> PING: Have you ever reviewed it?
>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/
>>>>>
>>>>> I've sent review request to parse core image with JSnap.
>>>>> If you have unclear point about this change, please tell me.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2016/04/20 21:21, Dmitry Samersoff wrote:
>>>>>> Yasumasa,
>>>>>>
>>>>>> I need some more time to check what is happening with performance
>>>>>> counters and what side effect this fix can have.
>>>>>>
>>>>>> Sorry about it.
>>>>>>
>>>>>> -Dmitry
>>>>>>
>>>>>>
>>>>>> On 2016-04-20 15:14, Yasumasa Suenaga wrote:
>>>>>>> PING: Could you review it?
>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/
>>>>>>>
>>>>>>> This changes is based on jdk9/hs-rt.
>>>>>>> But I confirmed this patch works fine on jdk9/hs .
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2016/04/13 22:22, Yasumasa Suenaga wrote:
>>>>>>>> Hi Dmitry,
>>>>>>>>
>>>>>>>> Thank you for your comment.
>>>>>>>> I uploaded new webrev. Could you review again?
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2016/04/13 21:41, Dmitry Samersoff wrote:
>>>>>>>>> Yasumasa,
>>>>>>>>>
>>>>>>>>> Yes. It's better.
>>>>>>>>>
>>>>>>>>> Please place all _saved_* declarations together and add a comment
>>>>>>>>> explaining what is the purpose of this variables.
>>>>>>>>>
>>>>>>>>> -Dmitry
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2016-04-13 15:20, Yasumasa Suenaga wrote:
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> Could you review and sponsor it?
>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.01/
>>>>>>>>>>
>>>>>>>>>> If it is not accepted, I think that we can add new field to
>>>>>>>>>> PerfMemory
>>>>>>>>>> for using in JSnap:
>>>>>>>>>> -----------------------
>>>>>>>>>> diff -r 4823056a5bbd src/share/vm/runtime/perfMemory.cpp
>>>>>>>>>> --- a/src/share/vm/runtime/perfMemory.cpp Tue Apr 12 09:08:48
>>>>>>>>>> 2016 +0000
>>>>>>>>>> +++ b/src/share/vm/runtime/perfMemory.cpp Wed Apr 13 14:18:15
>>>>>>>>>> 2016 +0900
>>>>>>>>>> @@ -45,11 +45,16 @@
>>>>>>>>>> UINT_CHARS + 1;
>>>>>>>>>>
>>>>>>>>>> char* PerfMemory::_start = NULL;
>>>>>>>>>> +char* PerfMemory::_saved_start = NULL;
>>>>>>>>>> char* PerfMemory::_end = NULL;
>>>>>>>>>> +char* PerfMemory::_saved_end = NULL;
>>>>>>>>>> char* PerfMemory::_top = NULL;
>>>>>>>>>> +char* PerfMemory::_saved_top = NULL;
>>>>>>>>>> size_t PerfMemory::_capacity = 0;
>>>>>>>>>> +size_t PerfMemory::_saved_capacity = 0;
>>>>>>>>>> jint PerfMemory::_initialized = false;
>>>>>>>>>> PerfDataPrologue* PerfMemory::_prologue = NULL;
>>>>>>>>>> +PerfDataPrologue* PerfMemory::_saved_prologue = NULL;
>>>>>>>>>>
>>>>>>>>>> void perfMemory_init() {
>>>>>>>>>>
>>>>>>>>>> @@ -103,6 +108,8 @@
>>>>>>>>>>
>>>>>>>>>> // allocate PerfData memory region
>>>>>>>>>> create_memory_region(capacity);
>>>>>>>>>> + _saved_start = _start;
>>>>>>>>>> + _saved_capacity = _capacity;
>>>>>>>>>>
>>>>>>>>>> if (_start == NULL) {
>>>>>>>>>>
>>>>>>>>>> @@ -132,8 +139,11 @@
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> _prologue = (PerfDataPrologue *)_start;
>>>>>>>>>> + _saved_prologue = _prologue;
>>>>>>>>>> _end = _start + _capacity;
>>>>>>>>>> + _saved_end = _end;
>>>>>>>>>> _top = _start + sizeof(PerfDataPrologue);
>>>>>>>>>> + _saved_top = _top;
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> assert(_prologue != NULL, "prologue pointer must be
>>>>>>>>>> initialized");
>>>>>>>>>> -----------------------
>>>>>>>>>>
>>>>>>>>>> If it is better, I will upload a webrev.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2016/04/06 12:42, Yasumasa Suenaga wrote:
>>>>>>>>>>> Hi Dmitry,
>>>>>>>>>>>
>>>>>>>>>>> Thanks for your comment.
>>>>>>>>>>>
>>>>>>>>>>> I uploaded a new webrev. Could you review again?
>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.01/
>>>>>>>>>>>
>>>>>>>>>>> On 2016/04/06 0:31, Dmitry Samersoff wrote:
>>>>>>>>>>>> Yasumasa,
>>>>>>>>>>>>
>>>>>>>>>>>> 1. It's better to change JSnap code to produce meaningful error
>>>>>>>>>>>> message
>>>>>>>>>>>> instead of NPE.
>>>>>>>>>>>
>>>>>>>>>>> I added null check and set message to PerfMemory.java .
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> 2. We should check that no other consumer of perf counters rely on
>>>>>>>>>>>> the
>>>>>>>>>>>> fact it's NULL after call to destroy(). I'm not sure that this
>>>>>>>>>>>> part of
>>>>>>>>>>>> the fix is not dangerous.
>>>>>>>>>>>
>>>>>>>>>>> I added new flag (_destroyed) in PerfMemory class.
>>>>>>>>>>> This flag set true at PerfMemory::destroy().
>>>>>>>>>>>
>>>>>>>>>>> _start, _end, and more field which I removed from destroy() are
>>>>>>>>>>> private member of
>>>>>>>>>>> PerfMemory. I implemented that the getter functions of them check
>>>>>>>>>>> _destroyed flag,
>>>>>>>>>>> and returns same value before this change.
>>>>>>>>>>>
>>>>>>>>>>> So I think this change does not affect other place.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> -Dmitry
>>>>>>>>>>>>
>>>>>>>>>>>> On 2016-03-29 15:02, Yasumasa Suenaga wrote:
>>>>>>>>>>>>> PING: Could you review it?
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2016/03/22 21:24, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>> PING: Could you review it?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2016/03/14 23:39 "Yasumasa Suenaga" <yasuenag at gmail.com
>>>>>>>>>>>>>> <mailto:yasuenag at gmail.com>>:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> When I use `jhsdb jsnap` to get PerfCounter from core
>>>>>>>>>>>>>> images, I
>>>>>>>>>>>>>> encountered NPE:
>>>>>>>>>>>>>> -------------
>>>>>>>>>>>>>> Exception in thread "main" java.lang.NullPointerException
>>>>>>>>>>>>>> at sun.jvm.hotspot.tools.JSnap.run(JSnap.java:45)
>>>>>>>>>>>>>> at
>>>>>>>>>>>>>> sun.jvm.hotspot.tools.Tool.startInternal(Tool.java:260)
>>>>>>>>>>>>>> at sun.jvm.hotspot.tools.Tool.start(Tool.java:223)
>>>>>>>>>>>>>> at
>>>>>>>>>>>>>> sun.jvm.hotspot.tools.Tool.execute(Tool.java:118)
>>>>>>>>>>>>>> at sun.jvm.hotspot.tools.JSnap.main(JSnap.java:67)
>>>>>>>>>>>>>> at
>>>>>>>>>>>>>> sun.jvm.hotspot.SALauncher.runJSNAP(SALauncher.java:352)
>>>>>>>>>>>>>> at
>>>>>>>>>>>>>> sun.jvm.hotspot.SALauncher.main(SALauncher.java:404)
>>>>>>>>>>>>>> -------------
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> PerfMemory::destroy() clears all members which are used in
>>>>>>>>>>>>>> JSnap.
>>>>>>>>>>>>>> Thus NPE is occurred.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I uploaded webrev for this issue.
>>>>>>>>>>>>>> Could you review it?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.00/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I cannot access JPRT.
>>>>>>>>>>>>>> So I need a Sponsor.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
More information about the serviceability-dev
mailing list