RFR (S) 8210512: vmTestbase/nsk/jdi/ObjectReference/referringObjects/referringObjects002/referringObjects002.java fails with unexpected size of ClassLoaderReference.referringObjects

[dean.long at oracle.com]

On 9/10/18 1:40 AM, David Holmes wrote:
> Hi Dean,
>
> On 10/09/2018 5:31 PM, dean.long at oracle.com wrote:
>> Hi David.  If a class references its own fields or methods, and that 
>> code is executed by the interpreter, then we should expect that 
>> constant pool entry to be resolved, shouldn't we?
>
> Yes
>
>> Likewise the compiler will treat it as resolved.  If we treat is as 
>> unresolved, we run into the case where we executed compiled code for 
>> a method that would have resolved the constant pool entry, but JVMTI 
>> says we didn't.
>
> I don't think the this_klass reference necessarily falls under that 
> categorization though. In an empty class (as used in the test case) 
> there's no bytecode that references any CP index for the current 
> class. It's simply an artifact of the classfile format.
>
>> Isn't the test in error for assuming no eager resolution?
>
> Hmmm. Yes I suppose that is right. If we did eager resolution then 
> even the this_klass CP entry would have been resolved and the test 
> would have then encountered it from day one.
>
> What bothers me is the self-referential nature of this ... and the 
> obscurity of it. I don't think anyone looking at the various heap 
> reference API's would think about a reference-to-self.
>

Maybe for an empty class, but in general I would expect self references 
to be considered.

>> By the way, it looks like we always eagerly resolve this_class for 
>> unsafe-anonymous classes.
>
> Can you point me at that code please?
>

http://hg.openjdk.java.net/jdk/jdk/file/4e99f412148f/src/hotspot/share/classfile/classFileParser.cpp#l5594

dl

> Thanks,
> David
>
>> dl
>>
>> On 9/9/18 10:51 PM, David Holmes wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210512
>>> Webrev: http://cr.openjdk.java.net/~dholmes/8210512/webrev/
>>>
>>> After the fix for JDK-8209361 where we modified JVM-TI to treat an 
>>> unresolved CP klass entry to a loaded klass as a resolved CP entry, 
>>> the listed test starting failing due to finding an extra reference 
>>> to the test class. As outlined in the bug report this extra reference:
>>>
>>> 17: instance of java.lang.Class(reflected 
>>> class=nsk.share.jdi.TestClass1, id=792)
>>>
>>> actually comes from the class itself. Every classfile has a 
>>> self-referential entry in the constant pool (this_klass in JVMS 4.1) 
>>> and that is what we were encountering here.
>>>
>>> I would argue that such an unresolved reference from a class to 
>>> itself should never be treated as a "real" reference from a JVM TI 
>>> perspective, and so we ship skip it - which is what the fix does:
>>>
>>> - if (klass == NULL) {
>>> + if (klass == NULL || klass == ik) {
>>>                continue;
>>>
>>> Testing: the test concerned
>>>          joatc testing of test that failed in 8209361 (TBD)
>>>          mach5 tiers 1-3 (TBD)
>>>          jvmti (TBD)
>>>
>>> As noted testing is still TBD other then actual test but there are 
>>> technical delays with that so I'll get the RFR out anyway.
>>>
>>> Thanks,
>>> David
>>