PING: RFR: JDK-8151815: Could not parse core image with JSnap.
Yasumasa Suenaga
yasuenag at gmail.com
Fri May 6 12:42:24 UTC 2016
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