RFR (S) 8229900: RedefineDeleteJmethod.java fails with -Xcheck:jni

David Holmes david.holmes at oracle.com
Thu Oct 3 06:36:27 UTC 2019

Hi Coleen,

On 3/10/2019 8:11 am, coleen.phillimore at oracle.com wrote:
> I added more checks (as discussed below) and reran tier1 with -Xcheck:jni.
> http://cr.openjdk.java.net/~coleenp/2019/8229900.02/webrev/index.html

That aspect seems fine. Thanks for adding the extra checks.


> thanks,
> Coleen
> On 10/2/19 10:03 AM, coleen.phillimore at oracle.com wrote:
>> On 10/2/19 12:53 AM, David Holmes wrote:
>>> Hi Coleen,
>>> Sorry rather long-winded reply ...
>>> On 1/10/2019 11:52 pm, coleen.phillimore at oracle.com wrote:
>>>> Summary: Remove RedefineClasses adjustment and test, but improve 
>>>> checking for method/class matching.
>>>> Tested with tier1 with -Xcheck:jni locally, and tier1 on Oracle 
>>>> platforms.
>>>> open webrev at 
>>>> http://cr.openjdk.java.net/~coleenp/2019/8229900.01/webrev
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8229900
>>> I was troubled by this change because it made me think more deeply 
>>> about what should reasonably happen with respect to jmethodIDs and 
>>> class redefinition.
>>> I had a (simplistic?) mental model that a jmethodID is a "pointer" to 
>>> a specific method in a specific class (the defining class of the 
>>> method). As such a jmethodID should only be used when operating on 
>>> that class (or subclass thereof) or an instance of that class (or 
>>> subclass thereof). With that mental model it makes sense for 
>>> Xcheck:jni to validate the defining class is the target class, or a 
>>> superclass thereof. But it doesn't then make sense for class 
>>> redefinition to update jmethodIDs as the redefined class is not the 
>>> original class! If a class is redefined then it should, in my mental 
>>> model, invalidate all existing jmethodIDs that pertained to the 
>>> original class.
>>> However, that's not very user friendly in the face of redefinition of 
>>> a superclass as code that only works with the subclass may reasonably 
>>> expect jmethodIDs to remain valid even if they refer to an inherited 
>>> method that has been redefined. So we update them to refer to the 
>>> redefined method implementation.
>>> So in that regards the update to jniCheck::validate_call_class seems 
>>> correct. Though I wonder if we also need to check that obj->class is 
>>> a subtype of clazz? As far as I can see we never actually validate 
>>> that! We use the jmethodID to find the method and we then find the 
>>> vtable index wrt. the method->holder class, and then we use that 
>>> vtable index to lookup a method in the receiver object's class - 
>>> which could lead to a random method being selected in a different class!
>> I just added this check and am testing it now.
>> Coleen
>>> Continuing on, if we do expect jmethodIDs to get updated upon class 
>>> redefinition then it makes sense to me to keep the logic that handles 
>>> deleted methods, by redirecting them to a method that throws NSME. 
>>> The fact that method is in Unsafe is unfortunate but it is what we do 
>>> elsewhere in the VM. I'm assuming the problem here is that the 
>>> augmented jniCheck::validate_call_class will fail in such cases? That 
>>> is a problem, but I think I'd rather see it special-cased than change 
>>> the existing behaviour:
>>>   if (obj != NULL) {
>>>     jniCheck::validate_object(thr, obj);
>>>   }
>>> + if (m == Universe::throw_no_such_method_error())
>>> +   return;  // skip class checks in this case
>>> then the test could also remain.
>>> Also note that while we generally require JNI programmers to ensure 
>>> everything is called correctly, jmethodIDs are also used by JVM TI 
>>> and we tend to want JVM TI to have well defined semantics. I'm 
>>> unclear now what happens if we invoke a deleted method through JVM TI ?
>>> Thanks,
>>> David
>>> ----
>>>> Thanks,
>>>> Coleen

More information about the hotspot-runtime-dev mailing list