RFR(S): Fix regression test for 8012015 (Use PROT_NONE when reserving memory)

[David Holmes]

Looks okay - no further comments.

Thanks,
David

On 18/05/2013 7:02 AM, Mikael Vidstedt wrote:
>
> David,
>
> Thanks for the comments! New webrev available here (and some comments
> below):
>
> http://cr.openjdk.java.net/~mikael/webrevs/8013726/webrev.04/webrev
>
> On 2013-05-16 18:54, David Holmes wrote:
>> On 17/05/2013 2:39 AM, Mikael Vidstedt wrote:
>>>
>>> Please review the below change:
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~mikael/webrevs/8013726/webrev.03/webrev/
>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8013726
>>
>> src/share/vm/prims/whitebox.cpp
>>
>> Typo: elimnated
>
> Fixed.
>
>> Couldn't you just return the value read to ensure the compiler doesn't
>> eliminate the read?
>
> I could, but I'm not sure it makes the code significantly better so I'd
> probably rather keep it the way it is if that's ok with you.
>
>> What if p is NULL? (Unlikely I know but ...)
>
> The new version now checks explicitly for that and throws a helpful
> exception.
>
>> ---
>>
>> Otherwise changes seem to address problems described.
>>
>> What testing has now been carried out? Presumably OSX was not tested
>> previously.
>
> My mistake last time was to not realize that the new test was not picked
> up and run as part of the normal jprt submit. As part of fixing the test
> I manually ran/verified the test on the (other) platforms and I have
> also explicitly run the test through JPRT (using -rtests
> '*-*-*-runtime/memory').
>
> Anything recommendations on additional things to test?
>
> Cheers,
> Mikael
>
>>
>> Thanks,
>> David
>>
>>
>>>
>>> Background (see bug report for some additional information):
>>>
>>> When changing the protection for reserved memory [1][2] I also added a
>>> regression test. The test is supposed to reserve a memory range and
>>> explicitly provoke a crash by accessing that memory. Unfortunately the
>>> test is severely broken and does not work on all platforms. Specifically
>>> it has the following problems:
>>>
>>> * It tries to reserve a 4kb memory range, which is not always allowed -
>>> os::vm_allocation_granularity() is the memory allocation granularity.
>>> For example, on my windows machine the granularity is 64kb. This
>>> triggers the assert in the bug report.
>>>
>>> * The test also uses Unsafe to do the memory access, but signals raised
>>> by Unsafe operations are handled slightly differently from normal memory
>>> accesses. Specifically, a SIGBUS signal is translated into a Java
>>> exception. Furthermore, on OSX a memory access in reserved memory raises
>>> a SIGBUS whereas on other platforms (Linux, Solaris) it is a SIGSEGV.
>>>
>>>
>>> About the change:
>>>
>>> The change resolves the assert by using os::vm_allocation_granularity()
>>> when specifying the size of the reserved memory.
>>>
>>> In order to avoid the Unsafe specific behavior in combination with the
>>> platform specific signal raised the memory access has also been moved
>>> into the native whitebox code. The function now both does the memory
>>> reservation and touches the memory and it has been renamed to reflect
>>> that. One alternative to that would be to have a separate whitebox
>>> function to read from a memory address, but that did not immediately
>>> make sense. Future tests may want to revisit that.
>>>
>>> Thanks,
>>> Mikael
>>>
>>> [1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012015
>>> [2] http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/f32b6c267d2e
>>>
>