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

Wed Oct 18 04:27:42 UTC 2017

Hi David,

2017-10-18 12:55 GMT+09:00 David Holmes <david.holmes at oracle.com>:
> On 18/10/2017 12:37 PM, Yasumasa Suenaga wrote:
>>
>> 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.
>
>
> Perhaps not but there are still other actions that happen and the point is
> we should not be able to continue to use PerfMemory once it has been
> destroyed (even if the destruction is only logical).

I received same comment from Dmitry in the past, but we couldn't
decide how should we do.

  http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-May/019728.html

In that discussion, I uploaded another webrev which adds other fields for JSnap.
Is it suitable?

  http://cr.openjdk.java.net/~ysuenaga/JDK-8151815/webrev.02/

>>> 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.
>
>
> I'm not familiar with these tools. When do we produce a core file after
> calling PerfMemory::destroy ?

PerfMemory::destroy() is called before aborting.

-----------------------
#0  perfMemory_exit ()
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/share/vm/runtime/perfMemory.cpp:80
#1  0x00007f99b091c949 in os::shutdown ()
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/os/linux/vm/os_linux.cpp:1483
#2  0x00007f99b091c980 in os::abort (dump_core=<optimized out>)
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/os/linux/vm/os_linux.cpp:1503
#3  0x00007f99b0b689c3 in VMError::report_and_die (
    this=this at entry=0x7ffcacf40b50)
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/share/vm/utilities/vmError.cpp:1060
#4  0x00007f99b0926f04 in JVM_handle_linux_signal (sig=sig at entry=11,
    info=info at entry=0x7ffcacf40df0, ucVoid=ucVoid at entry=0x7ffcacf40cc0,
    abort_if_unrecognized=abort_if_unrecognized at entry=1)
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.144-7.b01.fc26.x86_64/openjdk/hotspot/src/os_cpu/linux_x86/vm/os_linux_x86.cpp:541
-----------------------

Thanks,

Yasumasa

>>> 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?
>
>
> Assertions and direct guards. Checking _prologue is a placeholder for the
> real check.
>
>
> Thanks,
> David
>
>
>>
>> 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
>>>>>>>>>
>>>>>>
>>>
>