Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

dean.long
Adding runtime alias...

comments inlined below.


On 11/13/17 9:10 PM, jamsheed wrote:

> Hi, request for review, jbs:
> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
> changes equivalent to JDK-4454115 is done for windows.

It looks like "nm" can be uninitialized if "in_java" is false.

> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.

Can you narrow the scope of the unsafe access using something like
GuardUnsafeAccess, instead of marking the whole function as doing unsafe
access?

There are some risks with trying to  abort a copy function.

First, won't we get multiple exceptions until we finally hit the end of
the range?  What if the bad range is very large?

Second, what if the loop is using auto-increment instructions? Skipping
to the next instruction would mean we loop forever if the increment
never happens.

I think if we are going to safely abort copy functions then we need to
register them as a kind of CodeBlob that has a special abort entry point
or exception handler we can redirect to, or maybe pop the whole frame
and return.

Is there really a problem with these copy functions?  I'm wondering why
Mikael did not identify these as a problem in 8154592.

> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
> thread->thread_state() == _thread_in_vm checks from signal handler

How about adding a check for _thread_in_native instead of removing the
check entirely?

dl

> Best regards, Jamsheed
>

Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

Jamsheed C m
Hi Dean,

Thank you for the feedback,

On Tuesday 14 November 2017 11:57 PM, [hidden email] wrote:

> Adding runtime alias...
>
> comments inlined below.
>
>
> On 11/13/17 9:10 PM, jamsheed wrote:
>
>> Hi, request for review, jbs:
>> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
>> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
>> changes equivalent to JDK-4454115 is done for windows.
>
> It looks like "nm" can be uninitialized if "in_java" is false.
that was a miss, thank you for pointing that.
>
>> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
>> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.
>
> Can you narrow the scope of the unsafe access using something like
> GuardUnsafeAccess, instead of marking the whole function as doing
> unsafe access?
can do that, but i don't find an issue with putting guard for the whole
function.
>
> There are some risks with trying to  abort a copy function.
>
> First, won't we get multiple exceptions until we finally hit the end
> of the range?  What if the bad range is very large?
we may get multiple exception, and we may reach safepoint a bit late,
this should be case in compiled code too, where we use intrinsics
>
> Second, what if the loop is using auto-increment instructions?
> Skipping to the next instruction would mean we loop forever if the
> increment never happens.
we move to next instruction, so will exit loop, no backward branching
right ?

>
> I think if we are going to safely abort copy functions then we need to
> register them as a kind of CodeBlob that has a special abort entry
> point or exception handler we can redirect to, or maybe pop the whole
> frame and return.
>
> Is there really a problem with these copy functions?  I'm wondering
> why Mikael did not identify these as a problem in 8154592.
>
>> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
>> thread->thread_state() == _thread_in_vm checks from signal handler
>
> How about adding a check for _thread_in_native instead of removing the
> check entirely?
ok i will add that.
Best regards,
Jamsheed

>
> dl
>
>> Best regards, Jamsheed
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

Jamsheed C m
In reply to this post by dean.long
Hi Dean,

Thank you for the feedback,

On Tuesday 14 November 2017 11:57 PM, [hidden email] wrote:

> Adding runtime alias...
>
> comments inlined below.
>
>
> On 11/13/17 9:10 PM, jamsheed wrote:
>
>> Hi, request for review, jbs:
>> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
>> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
>> changes equivalent to JDK-4454115 is done for windows.
>
> It looks like "nm" can be uninitialized if "in_java" is false.
that was a miss, thank you for pointing that.
>
>> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
>> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.
>
> Can you narrow the scope of the unsafe access using something like
> GuardUnsafeAccess, instead of marking the whole function as doing
> unsafe access?
can do that, but i don't find an issue with putting guard for the whole
function.
>
> There are some risks with trying to  abort a copy function.
>
> First, won't we get multiple exceptions until we finally hit the end
> of the range?  What if the bad range is very large?
we may get multiple exception, and we may reach safepoint a bit late,
this should be case in compiled code too, where we use intrinsics
>
> Second, what if the loop is using auto-increment instructions?
> Skipping to the next instruction would mean we loop forever if the
> increment never happens.
we move to next instruction, so will exit loop, no backward branching
right ?

>
> I think if we are going to safely abort copy functions then we need to
> register them as a kind of CodeBlob that has a special abort entry
> point or exception handler we can redirect to, or maybe pop the whole
> frame and return.
>
> Is there really a problem with these copy functions?  I'm wondering
> why Mikael did not identify these as a problem in 8154592.
>
>> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
>> thread->thread_state() == _thread_in_vm checks from signal handler
>
> How about adding a check for _thread_in_native instead of removing the
> check entirely?
ok i will add that.
Best regards,
Jamsheed

>
> dl
>
>> Best regards, Jamsheed
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

Jamsheed C m
sorry, we do loop, even for autoincrement case only if both from and to
are invalid we may loop for ever, i didn't assume that case

Best regards,

Jamsheed


On Wednesday 15 November 2017 01:02 AM, jamsheed wrote:
> Second, what if the loop is using auto-increment instructions?
> Skipping to the next instruction would mean we loop forever if the
> increment never happens.

Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

Jamsheed C m
only problem i see in autoincrement is in  unsafe.setMemory(base, size,
(byte) 0), may be i will skip this case.

and maintain same implementation for copy and copyswap, as i assume
autoincrement  for count wouldn't happen in single instruction for these
two case.

Best regards,
Jamsheed
On Wednesday 15 November 2017 01:28 AM, jamsheed wrote:

> sorry, we do loop, even for autoincrement case only if both from and
> to are invalid we may loop for ever, i didn't assume that case
>
> Best regards,
>
> Jamsheed
>
>
> On Wednesday 15 November 2017 01:02 AM, jamsheed wrote:
>> Second, what if the loop is using auto-increment instructions?
>> Skipping to the next instruction would mean we loop forever if the
>> increment never happens.
>

Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

Jamsheed C m
In reply to this post by dean.long
Hi Dean,

revised webrev:

http://cr.openjdk.java.net/~jcm/6415680/webrev.01/

i agree to the comment that it is potentially unsafe to assume the
implementation, and count can be in autoincrement mode. so with this bug
i would like to deal with only

the single value access error handling.

revised webrev: http://cr.openjdk.java.net/~jcm/6415680/webrev.01/

Best regards,

Jamsheed



On Tuesday 14 November 2017 11:57 PM, [hidden email] wrote:

> Adding runtime alias...
>
> comments inlined below.
>
>
> On 11/13/17 9:10 PM, jamsheed wrote:
>
>> Hi, request for review, jbs:
>> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
>> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
>> changes equivalent to JDK-4454115 is done for windows.
>
> It looks like "nm" can be uninitialized if "in_java" is false.
>
>> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
>> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.
>
> Can you narrow the scope of the unsafe access using something like
> GuardUnsafeAccess, instead of marking the whole function as doing
> unsafe access?
>
> There are some risks with trying to  abort a copy function.
>
> First, won't we get multiple exceptions until we finally hit the end
> of the range?  What if the bad range is very large?
>
> Second, what if the loop is using auto-increment instructions?
> Skipping to the next instruction would mean we loop forever if the
> increment never happens.
>
> I think if we are going to safely abort copy functions then we need to
> register them as a kind of CodeBlob that has a special abort entry
> point or exception handler we can redirect to, or maybe pop the whole
> frame and return.
>
> Is there really a problem with these copy functions?  I'm wondering
> why Mikael did not identify these as a problem in 8154592.
>
>> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
>> thread->thread_state() == _thread_in_vm checks from signal handler
>
> How about adding a check for _thread_in_native instead of removing the
> check entirely?
>
> dl
>
>> Best regards, Jamsheed
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

David Holmes
In reply to this post by dean.long
On 15/11/2017 4:27 AM, [hidden email] wrote:
> Adding runtime alias...

Thanks Dean!

> comments inlined below.
>
>
> On 11/13/17 9:10 PM, jamsheed wrote:
>
>> Hi, request for review, jbs:
>> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
>> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
>> changes equivalent to JDK-4454115 is done for windows.
>
> It looks like "nm" can be uninitialized if "in_java" is false.
>
>> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
>> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.
>
> Can you narrow the scope of the unsafe access using something like
> GuardUnsafeAccess, instead of marking the whole function as doing unsafe
> access?

I tend to agree with this suggestion. The unsafe region should be as
narrow as possible.

> There are some risks with trying to  abort a copy function.
>
> First, won't we get multiple exceptions until we finally hit the end of
> the range?  What if the bad range is very large?
>
> Second, what if the loop is using auto-increment instructions? Skipping
> to the next instruction would mean we loop forever if the increment
> never happens.
>
> I think if we are going to safely abort copy functions then we need to
> register them as a kind of CodeBlob that has a special abort entry point
> or exception handler we can redirect to, or maybe pop the whole frame
> and return.

Jamsheed and I has some discussions on this on IM last week. I also
flagged the multiple exceptions as potentially problematic. I'm not sure
how things will behave if we trigger hundreds of faults in succession -
nor how long it will take. Aborting the whole loop, or frame, seems
preferable but I don't know how you would do that.

> Is there really a problem with these copy functions?  I'm wondering why
> Mikael did not identify these as a problem in 8154592.

Good question. :) It seems quite obvious to me that if a single
load/store needs protection then bulk load/store would also need
protection. And Jamsheeds tests confirmed that.

Cheers,
David

>> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
>> thread->thread_state() == _thread_in_vm checks from signal handler
>
> How about adding a check for _thread_in_native instead of removing the
> check entirely?
>
> dl
>
>> Best regards, Jamsheed
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

David Holmes
In reply to this post by Jamsheed C m
On 15/11/2017 7:11 AM, jamsheed wrote:

> Hi Dean,
>
> revised webrev:
>
> http://cr.openjdk.java.net/~jcm/6415680/webrev.01/
>
> i agree to the comment that it is potentially unsafe to assume the
> implementation, and count can be in autoincrement mode. so with this bug
> i would like to deal with only the single value access error handling.
>
> revised webrev: http://cr.openjdk.java.net/~jcm/6415680/webrev.01/

Okay so now this just brings to windows what currently exists on the
other platforms - right?

Seems reasonable for now. Longer term we should look at the bulk
operation problem.

Thanks,
David

> Best regards,
>
> Jamsheed
>
>
>
> On Tuesday 14 November 2017 11:57 PM, [hidden email] wrote:
>> Adding runtime alias...
>>
>> comments inlined below.
>>
>>
>> On 11/13/17 9:10 PM, jamsheed wrote:
>>
>>> Hi, request for review, jbs:
>>> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
>>> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
>>> changes equivalent to JDK-4454115 is done for windows.
>>
>> It looks like "nm" can be uninitialized if "in_java" is false.
>>
>>> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
>>> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.
>>
>> Can you narrow the scope of the unsafe access using something like
>> GuardUnsafeAccess, instead of marking the whole function as doing
>> unsafe access?
>>
>> There are some risks with trying to  abort a copy function.
>>
>> First, won't we get multiple exceptions until we finally hit the end
>> of the range?  What if the bad range is very large?
>>
>> Second, what if the loop is using auto-increment instructions?
>> Skipping to the next instruction would mean we loop forever if the
>> increment never happens.
>>
>> I think if we are going to safely abort copy functions then we need to
>> register them as a kind of CodeBlob that has a special abort entry
>> point or exception handler we can redirect to, or maybe pop the whole
>> frame and return.
>>
>> Is there really a problem with these copy functions?  I'm wondering
>> why Mikael did not identify these as a problem in 8154592.
>>
>>> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
>>> thread->thread_state() == _thread_in_vm checks from signal handler
>>
>> How about adding a check for _thread_in_native instead of removing the
>> check entirely?
>>
>> dl
>>
>>> Best regards, Jamsheed
>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

Jamsheed C m
In reply to this post by Jamsheed C m
Hi David,

Hope you too agree here.

Best regards,

Jamsheed


On Wednesday 15 November 2017 02:41 AM, jamsheed wrote:

> Hi Dean,
>
> revised webrev:
>
> http://cr.openjdk.java.net/~jcm/6415680/webrev.01/
>
> i agree to the comment that it is potentially unsafe to assume the
> implementation, and count can be in autoincrement mode. so with this
> bug i would like to deal with only
>
> the single value access error handling.
>
> revised webrev: http://cr.openjdk.java.net/~jcm/6415680/webrev.01/
>
> Best regards,
>
> Jamsheed
>
>
>
> On Tuesday 14 November 2017 11:57 PM, [hidden email] wrote:
>> Adding runtime alias...
>>
>> comments inlined below.
>>
>>
>> On 11/13/17 9:10 PM, jamsheed wrote:
>>
>>> Hi, request for review, jbs:
>>> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
>>> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
>>> changes equivalent to JDK-4454115 is done for windows.
>>
>> It looks like "nm" can be uninitialized if "in_java" is false.
>>
>>> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
>>> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.
>>
>> Can you narrow the scope of the unsafe access using something like
>> GuardUnsafeAccess, instead of marking the whole function as doing
>> unsafe access?
>>
>> There are some risks with trying to  abort a copy function.
>>
>> First, won't we get multiple exceptions until we finally hit the end
>> of the range?  What if the bad range is very large?
>>
>> Second, what if the loop is using auto-increment instructions?
>> Skipping to the next instruction would mean we loop forever if the
>> increment never happens.
>>
>> I think if we are going to safely abort copy functions then we need
>> to register them as a kind of CodeBlob that has a special abort entry
>> point or exception handler we can redirect to, or maybe pop the whole
>> frame and return.
>>
>> Is there really a problem with these copy functions?  I'm wondering
>> why Mikael did not identify these as a problem in 8154592.
>>
>>> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
>>> thread->thread_state() == _thread_in_vm checks from signal handler
>>
>> How about adding a check for _thread_in_native instead of removing
>> the check entirely?
>>
>> dl
>>
>>> Best regards, Jamsheed
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

Jamsheed C m
In reply to this post by David Holmes
Hi David,

On Wednesday 15 November 2017 02:48 AM, David Holmes wrote:

> On 15/11/2017 7:11 AM, jamsheed wrote:
>> Hi Dean,
>>
>> revised webrev:
>>
>> http://cr.openjdk.java.net/~jcm/6415680/webrev.01/
>>
>> i agree to the comment that it is potentially unsafe to assume the
>> implementation, and count can be in autoincrement mode. so with this
>> bug i would like to deal with only the single value access error
>> handling.
>>
>> revised webrev: http://cr.openjdk.java.net/~jcm/6415680/webrev.01/
>
> Okay so now this just brings to windows what currently exists on the
> other platforms - right?
Yes,
>
> Seems reasonable for now. Longer term we should look at the bulk
> operation problem.
Sure,
Best regards,
Jamsheed

>
> Thanks,
> David
>
>> Best regards,
>>
>> Jamsheed
>>
>>
>>
>> On Tuesday 14 November 2017 11:57 PM, [hidden email] wrote:
>>> Adding runtime alias...
>>>
>>> comments inlined below.
>>>
>>>
>>> On 11/13/17 9:10 PM, jamsheed wrote:
>>>
>>>> Hi, request for review, jbs:
>>>> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
>>>> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
>>>> changes equivalent to JDK-4454115 is done for windows.
>>>
>>> It looks like "nm" can be uninitialized if "in_java" is false.
>>>
>>>> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
>>>> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.
>>>
>>> Can you narrow the scope of the unsafe access using something like
>>> GuardUnsafeAccess, instead of marking the whole function as doing
>>> unsafe access?
>>>
>>> There are some risks with trying to  abort a copy function.
>>>
>>> First, won't we get multiple exceptions until we finally hit the end
>>> of the range?  What if the bad range is very large?
>>>
>>> Second, what if the loop is using auto-increment instructions?
>>> Skipping to the next instruction would mean we loop forever if the
>>> increment never happens.
>>>
>>> I think if we are going to safely abort copy functions then we need
>>> to register them as a kind of CodeBlob that has a special abort
>>> entry point or exception handler we can redirect to, or maybe pop
>>> the whole frame and return.
>>>
>>> Is there really a problem with these copy functions?  I'm wondering
>>> why Mikael did not identify these as a problem in 8154592.
>>>
>>>> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
>>>> thread->thread_state() == _thread_in_vm checks from signal handler
>>>
>>> How about adding a check for _thread_in_native instead of removing
>>> the check entirely?
>>>
>>> dl
>>>
>>>> Best regards, Jamsheed
>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

dean.long
In reply to this post by Jamsheed C m
This version looks good.  If you wanted you could also do the following:

CompiledMethod* nm = NULL;
JavaThread* thread = (JavaThread*)t;
if (in_java) {
CodeBlob* cb= CodeCache::find_blob_unsafe(pc);


because "cb" is only used in that inner block.

dl

On 11/14/17 1:11 PM, jamsheed wrote:

> Hi Dean,
>
> revised webrev:
>
> http://cr.openjdk.java.net/~jcm/6415680/webrev.01/
>
> i agree to the comment that it is potentially unsafe to assume the
> implementation, and count can be in autoincrement mode. so with this
> bug i would like to deal with only
>
> the single value access error handling.
>
> revised webrev: http://cr.openjdk.java.net/~jcm/6415680/webrev.01/
>
> Best regards,
>
> Jamsheed
>
>
>
> On Tuesday 14 November 2017 11:57 PM, [hidden email] wrote:
>> Adding runtime alias...
>>
>> comments inlined below.
>>
>>
>> On 11/13/17 9:10 PM, jamsheed wrote:
>>
>>> Hi, request for review, jbs:
>>> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
>>> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
>>> changes equivalent to JDK-4454115 is done for windows.
>>
>> It looks like "nm" can be uninitialized if "in_java" is false.
>>
>>> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
>>> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.
>>
>> Can you narrow the scope of the unsafe access using something like
>> GuardUnsafeAccess, instead of marking the whole function as doing
>> unsafe access?
>>
>> There are some risks with trying to  abort a copy function.
>>
>> First, won't we get multiple exceptions until we finally hit the end
>> of the range?  What if the bad range is very large?
>>
>> Second, what if the loop is using auto-increment instructions?
>> Skipping to the next instruction would mean we loop forever if the
>> increment never happens.
>>
>> I think if we are going to safely abort copy functions then we need
>> to register them as a kind of CodeBlob that has a special abort entry
>> point or exception handler we can redirect to, or maybe pop the whole
>> frame and return.
>>
>> Is there really a problem with these copy functions?  I'm wondering
>> why Mikael did not identify these as a problem in 8154592.
>>
>>> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
>>> thread->thread_state() == _thread_in_vm checks from signal handler
>>
>> How about adding a check for _thread_in_native instead of removing
>> the check entirely?
>>
>> dl
>>
>>> Best regards, Jamsheed
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [10]RFR: 6415680: (bf) MappedByteBuffer.get() can provoke crash with EXCEPTION_IN_PAGE_ERROR

Jamsheed C m
Thank you, Dean, David for review,

created a new CR for tracking bulk access failure graceful handling

https://bugs.openjdk.java.net/browse/JDK-8191278

On Wednesday 15 November 2017 04:08 AM, [hidden email] wrote:
>
> This version looks good.  If you wanted you could also do the following:
>
> CompiledMethod* nm = NULL;
> JavaThread* thread = (JavaThread*)t;
> if (in_java) {
> CodeBlob* cb= CodeCache::find_blob_unsafe(pc);
>
> because "cb" is only used in that inner block.
Ok

Best regards,
Jamsheed

>
> dl
>
> On 11/14/17 1:11 PM, jamsheed wrote:
>> Hi Dean,
>>
>> revised webrev:
>>
>> http://cr.openjdk.java.net/~jcm/6415680/webrev.01/
>>
>> i agree to the comment that it is potentially unsafe to assume the
>> implementation, and count can be in autoincrement mode. so with this
>> bug i would like to deal with only
>>
>> the single value access error handling.
>>
>> revised webrev: http://cr.openjdk.java.net/~jcm/6415680/webrev.01/
>>
>> Best regards,
>>
>> Jamsheed
>>
>>
>>
>> On Tuesday 14 November 2017 11:57 PM, [hidden email] wrote:
>>> Adding runtime alias...
>>>
>>> comments inlined below.
>>>
>>>
>>> On 11/13/17 9:10 PM, jamsheed wrote:
>>>
>>>> Hi, request for review, jbs:
>>>> https://bugs.openjdk.java.net/browse/JDK-6415680 webrev:
>>>> http://cr.openjdk.java.net/~jcm/6415680/webrev.00/ Description: 1)
>>>> changes equivalent to JDK-4454115 is done for windows.
>>>
>>> It looks like "nm" can be uninitialized if "in_java" is false.
>>>
>>>> 2) added guard to multiple value access sites, Unsafe_CopyMemory0,
>>>> Unsafe_SetMemory0 and Unsafe_CopySwapMemory0.
>>>
>>> Can you narrow the scope of the unsafe access using something like
>>> GuardUnsafeAccess, instead of marking the whole function as doing
>>> unsafe access?
>>>
>>> There are some risks with trying to  abort a copy function.
>>>
>>> First, won't we get multiple exceptions until we finally hit the end
>>> of the range?  What if the bad range is very large?
>>>
>>> Second, what if the loop is using auto-increment instructions?
>>> Skipping to the next instruction would mean we loop forever if the
>>> increment never happens.
>>>
>>> I think if we are going to safely abort copy functions then we need
>>> to register them as a kind of CodeBlob that has a special abort
>>> entry point or exception handler we can redirect to, or maybe pop
>>> the whole frame and return.
>>>
>>> Is there really a problem with these copy functions?  I'm wondering
>>> why Mikael did not identify these as a problem in 8154592.
>>>
>>>> 3) Unsafe_CopySwapMemory0 is JVM_LEAF so removed
>>>> thread->thread_state() == _thread_in_vm checks from signal handler
>>>
>>> How about adding a check for _thread_in_native instead of removing
>>> the check entirely?
>>>
>>> dl
>>>
>>>> Best regards, Jamsheed
>>>>
>>>
>>
>