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

[Mikael Vidstedt]

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
>>