RFR(S) 8205965: SIGSEGV on write to NativeCallStack::EMPTY_STACK

[Martin Buchholz]

Whether or not you use my suggestions, I am your second reviewer.  Approved.

On Mon, Jul 2, 2018 at 12:46 PM, Zhengyu Gu <zgu at redhat.com> wrote:

> Hi,
>
> Could I get second review?
>
> Thanks,
>
> -Zhengyu
>
>
> On 07/01/2018 08:18 AM, Thomas Stüfe wrote:
>
>> Hi Zhengyu,
>>
>> On Fri, Jun 29, 2018 at 10:17 PM, Zhengyu Gu <zgu at redhat.com> wrote:
>>
>>> Hi Thomas,
>>>
>>> On 06/29/2018 03:56 PM, Thomas Stüfe wrote:
>>>
>>>>
>>>> Hi Zhengyu,
>>>>
>>>> do I understand the problem right that the static initialization of
>>>> EMPTY_STACK can be preceded by a call to MemTracker::tracking_level()?
>>>> Otherwise I do not understand the placement new in
>>>> MemTracker::tracking_level().
>>>>
>>>> Correct.
>>>
>>> If yes, how? Do we really run that much complex code as part of C++
>>>> static initialization?
>>>>
>>>
>>>
>>> Because there are other static objects that call os::malloc/new in their
>>> constructors, and they may be initialized prior to EMPTY_STACK.
>>>
>>>
>> this is terrible :)
>>
>> We should probably fix that sometime, but in the meantime your change
>> makes sense to me. Reviewed.
>>
>> Thanks, Thomas
>>
>>
>> Thanks,
>>>
>>> -Zhengyu
>>>
>>>
>>>
>>>
>>>
>>>
>>>> Thanks, Thomas
>>>>
>>>>
>>>> On Fri, Jun 29, 2018 at 3:04 PM, Zhengyu Gu <zgu at redhat.com> wrote:
>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> clang-6.0 and above, can deduce that NativeCallStack::EMPTY_STACK is
>>>>> all
>>>>> zeros, and since it is a static constant, it places the object in the
>>>>> read-only BSS data section.
>>>>>
>>>>> To workaround static initialization ordering issue, NMT has to ensure
>>>>> EMPTY_STACK is initialized before turns itself on, which can happen in
>>>>> the
>>>>> middle of initialization of other static objects. In this case, it
>>>>> causes
>>>>> SIGSEGV while try to write to the read-only memory.
>>>>>
>>>>> The solution is to make EMPTY_STACk private and non-constant, but hands
>>>>> out
>>>>> constant version.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8205965
>>>>> Webrev: http://cr.openjdk.java.net/~zgu/8205965/webrev.00/
>>>>>
>>>>> Test:
>>>>>
>>>>>     hotspot_nmt on Linux 64 (fastdebug and release)
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Zhengyu
>>>>>
>>>>