PING: RFR: JDK-8151815: Could not parse core image with JSnap.

[Yasumasa Suenaga]

Hi David,

> With your changes you no longer null out _prologue so the assertion would
> now not fail and we'd proceed to access the deleted memory region!

On Linux, PerfMemory::delete_memory_region() does not call munmap()
for PerfMemory.


> I'm unclear why you no longer clear all the fields set during
> initialization?

PerfMemory.java in jdk.hotspot.agent needs these field values.
`jhsdb jsnap --core` is failed if they are cleared.


> But it seems to me that there are various checks of
> _prologue that should really be checking is_initialized() and/or
> is_destroyed() as a guard.

Should I change all assertions for _prologue?


Thanks,

Yasumasa


2017-10-18 10:53 GMT+09:00 David Holmes <david.holmes at oracle.com>:
> Hi Yasumasa,
>
> By chance we ran into this bug which I analysed yesterday:
>
> https://bugs.openjdk.java.net/browse/JDK-8189390
>
> We hit the assertion:
>
> #  Internal Error (/open/src/hotspot/share/runtime/perfMemory.cpp:216),
> pid=17874, tid=17875
> #  assert(_prologue != __null) failed: called before initialization
> #
>
> which is misleading because it can fail if called before initialization, or
> after PerfMemory::destroy has been called.
>
> With your changes you no longer null out _prologue so the assertion would
> now not fail and we'd proceed to access the deleted memory region!
>
> I'm unclear why you no longer clear all the fields set during
> initialization? But it seems to me that there are various checks of
> _prologue that should really be checking is_initialized() and/or
> is_destroyed() as a guard.
>
> Thanks,
> David
>
>
> On 16/10/2017 11:25 PM, Yasumasa Suenaga wrote:
>>
>> PING:
>>
>> Could you review it?
>>
>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.05/
>>
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2017/10/03 13:18, Yasumasa Suenaga wrote:
>>>
>>> Hi all,
>>>
>>> I added gtest unit test case for this change in new webrev:
>>>
>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.05/
>>>
>>> Could you review it?
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>
>>> 2017-09-27 0:01 GMT+09:00 Yasumasa Suenaga <yasuenag at gmail.com>:
>>>>
>>>> Hi all,
>>>>
>>>> I uploaded new webrev to be adapted to jdk10/hs:
>>>>
>>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.04/
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2017/09/21 7:45, Yasumasa Suenaga wrote:
>>>>>
>>>>>
>>>>> PING:
>>>>>
>>>>> Have you checked this issue?
>>>>>
>>>>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.03/
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2017/07/01 23:43, Yasumasa Suenaga wrote:
>>>>>>
>>>>>>
>>>>>> PING:
>>>>>>
>>>>>> Have you checked this issue?
>>>>>>
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2017/06/13 14:10, Yasumasa Suenaga wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> I want to discuss about JDK-8151815: Could not parse core image with
>>>>>>> JSnap.
>>>>>>>
>>>>>>>
>>>>>>> In last year, I found JSnap cannot parse coredump and I've sent
>>>>>>> review
>>>>>>> request for it as JDK-8151815. However it has not been reviewed yet
>>>>>>> [1].
>>>>>>>
>>>>>>> We've discussed about safety implementation, but we could not get
>>>>>>> consensus.
>>>>>>> IMHO all SA tools should be handled java processes and core images,
>>>>>>> and PerfCounter value is useful. So I fix this issue.
>>>>>>>
>>>>>>> I uploaded new webrev for this issue. I think this patch is safety
>>>>>>> because new flag PerfMemory::_destroyed guards double free, and all
>>>>>>> members in PerfMemory is accessible (they are not munmap'ed)
>>>>>>>
>>>>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.03/
>>>>>>>
>>>>>>>
>>>>>>> Can you cooperate?
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> [1]
>>>>>>>
>>>>>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-April/019480.html
>>>>>>>
>>>>
>