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

[Coleen Phillimore]

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.

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