RFR 8051398: jvmti tests fieldacc002, fieldmod002 fail in nightly with errors: (watch#0) wrong location

[serguei.spitsyn at oracle.com]

Coleen,

The fix looks good to me.
I only  had the same minor comments that Dan already asked below.
The code removed in the interpreterRuntime.cpp looks like an assert but 
I'm not sure how useful it is.

Thanks,
Serguei

On 7/30/14 1:20 PM, Coleen Phillimore wrote:
>
> On 7/30/14, 3:58 PM, Daniel D. Daugherty wrote:
>> On 7/30/14 1:20 PM, Coleen Phillimore wrote:
>>> Summary: Didn't handle NULL bcp for native methods
>>>
>>> bcp is set to NULL in the interpreter frame for native methods. x86 
>>> generate_native_entry() contains a call_VM that sets the bcp address 
>>> to the beginning of code, but sparc doesn't.  I don't think ppc does 
>>> either.   The code I changed, doesn't handle a null bcp which 
>>> post_field_access and post_field_modification tests call from JNI 
>>> for a native method.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8051398/
>>
>> src/share/vm/interpreter/interpreterRuntime.cpp
>>     I'm not seeing the reason for the code deletion here.
>>     Just re-read the bug and I'm still not seeing it.
>>     Could be that I've been away from this code for too long.
>
> I was debugging this and found this useless piece of code, so while 
> not directly related to the cause of the bug, it was in the path of 
> the bug.
>>
>> src/share/vm/oops/method.hpp
>>     line 652:   address bcp_from(address bci) const;
>>         Should the prototype parameter name be 'bcp' instead
>>         of 'bci' since the type is address?
>>
>
> You're right.  I will make that bcp.
>> src/share/vm/oops/method.cpp
>>     line 287:   if (is_native() && bcp == 0) {
>>     line 288:     return code_base() + (intptr_t)bcp;
>>         Why add '(intptr_t)bcp' since you know it is zero?
>>
>
> True.  I don't need to add bcp.  That saves a cast.
>
> Thanks!
> Coleen
>
>> src/share/vm/runtime/frame.cpp
>>     No comments.
>>
>>
>> Dan
>>
>>
>>
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8051398
>>>
>>> Tested with jck vm/jvmti, jtreg, and NSK internal tests.  No test 
>>> added because there's a test that already tests this.
>>>
>>> Thanks,
>>> Coleen
>>>
>>>
>>
>