RFR(S): Fix regression test for 8012015 (Use PROT_NONE when reserving memory)
Mikael Vidstedt
mikael.vidstedt at oracle.com
Tue May 21 09:45:32 PDT 2013
Christian, David - thanks for the reviews!
Cheers,
Mikael
On 5/20/2013 9:55 PM, David Holmes wrote:
> 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
>>>>
>>
More information about the hotspot-runtime-dev
mailing list