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

[serguei.spitsyn at oracle.com]

On 7/30/14 5:49 PM, Coleen Phillimore wrote:
>
> Thank you, Serguei!  I thought the code removed was something 
> cut/pasted from post_field_modification, which does something for each 
> case.  I thought it looked strange because it has all cases except 
> vtos, so essentially it's asserting that the cpCache doesn't get 
> initialized to void for getfield, which should be asserted somewhere 
> else if it's a useful test.   It was visual noise.

Agreed.

Thanks,
Serguei

>
> Thanks!
> Coleen
>
>
> On 7/30/14, 7:28 PM, serguei.spitsyn at oracle.com wrote:
>> 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
>>>>>
>>>>>
>>>>
>>>
>>
>