RFR (M) JDK-8033150 (#2): invokestatic: IncompatibleClassChangeError trying to invoke static method from a parent in presence of conflicting defaults

Coleen Phillimore coleen.phillimore at oracle.com
Fri Apr 11 14:50:27 UTC 2014

On 4/11/14, 10:09 AM, Lois Foltan wrote:
> On 4/11/2014 10:02 AM, Coleen Phillimore wrote:
>> This code looks good.  The MethodLookupMode enum is a nice 
>> improvement.  It's a good warning of the complexity of this code.
>> Did something part of the build complain about MethodLookupMode not 
>> being in vmStructs?
>> I'd prefer the serviceability agent not be tempted to duplicate the 
>> method lookup code in the JVM, so not have this change.
> Hi Coleen,
> Thanks for the review.  The vmStructs.cpp change was a cautionary move 
> on my part to include the new enum MethodLookupMode.  The build did 
> not complain without it.  From your comments sounds like it would be 
> better to leave this change to vmStructs.cpp out.  I can do that and 
> run through some minor testing.  Are you okay with me going forward 
> with this change and not sending it out for another round of review?

Yes, I'm fine with the change if you revert the change to vmStructs.   I 
don't need another round of review.  If vmStructs actually needed this 
new type, then there'd be changes in the serviceability agent required.  
I'm glad that isn't the case.


> Lois
>> Thanks,
>> Coleen
>> On 4/11/14, 9:04 AM, Lois Foltan wrote:
>>> Please review the updated fix, webrev at:
>>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8033150.1/
>>> This includes Keith's suggestion below.  All testing has been rerun 
>>> successfully.
>>> Thank you,
>>> Lois
>>> On 3/31/2014 3:51 PM, Lois Foltan wrote:
>>>> On 3/31/2014 2:08 PM, Keith McGuigan wrote:
>>>>> Hi Lois,
>>>>> I think that looks good.  I like it much better than doing the 
>>>>> static method check in the default method processing.
>>>>> My only suggestion is that it would be nice to encode new 
>>>>> parameter to uncached_lookup_method to be some sort of enum 
>>>>> instead of a boolean, so that it is obvious from the call-site 
>>>>> what the behavior should be (having just "false" in the parameter 
>>>>> list requires you to reference back to the declaration to figure 
>>>>> out what that "false" means).
>>>>> So instead of:
>>>>>    uncached_lookup_method(field_name, field_sig, false);
>>>>> It'd be:
>>>>>   uncached_lookup_method(field_name, field_sig, NORMAL); or
>>>>>   uncached_lookup_method(field_name, field_sig, IGNORE_OVERPASS);
>>>>> (or something like that -- I'm no good at names).
>>>> Thank you Keith.  Good suggestion.  I will implement and repost an 
>>>> updated webrev for review.
>>>> Lois
>>>>> --
>>>>> - Keith
>>>>> On Mon, Mar 31, 2014 at 1:25 PM, Lois Foltan 
>>>>> <lois.foltan at oracle.com <mailto:lois.foltan at oracle.com>> wrote:
>>>>>     Hi,
>>>>>     Please review the following fix:
>>>>>     Webrev:
>>>>>     http://cr.openjdk.java.net/~lfoltan/bug_jdk8033150/
>>>>>     <http://cr.openjdk.java.net/%7Elfoltan/bug_jdk8033150/>
>>>>>     Bug: invokestatic: IncompatibleClassChangeError trying to
>>>>>     invoke static method from a parent in presence of conflicting
>>>>>     defaults
>>>>>     https://bugs.openjdk.java.net/browse/JDK-8033150
>>>>>     Summary of fix:
>>>>>     During default method processing, determine_target(), is
>>>>>     responsible for making decisions on whether
>>>>>     to create and add an overpass method to the current class for
>>>>>     issues that have been encountered during the walk
>>>>>     of the default methods.  The routine
>>>>>     defaultMethods.cpp/has_matching_static() helped determine this
>>>>>     decision by looking within the current class for a static
>>>>>     method that should be preferred during method
>>>>>     resolution over an overpass method being created.  However,
>>>>>     has_matching_static() did not continue to
>>>>>     look for a static method in current class' superclasses which
>>>>>     JDK-8033150 exposed.
>>>>>     After looking at the code more closely, the has
>>>>>     _matching_static() concept is being moved out out of default
>>>>>     method processing and into method resolution processing.  The
>>>>>     primary reason for this is to allow
>>>>>     default method processing to match method selection where
>>>>>     statics are and should be ignored.   According
>>>>>     to JVMS, static methods should only be preferred over an
>>>>>     overpass method at method and interface
>>>>>     method resolution time.  To enable method resolution to
>>>>>     potentially find a static method over an overpass
>>>>>     method, a new parameter "ignore_overpass" was added to
>>>>>     InstanceKlass::uncached_lookup_method().
>>>>>     It has the affect of indicating to find_method_index() to
>>>>>     ignore overpass methods and continue the search
>>>>>     in case a static method of the same name and signature is
>>>>>     present in the current class or its superclasses.
>>>>>     Tests:
>>>>>         - jtreg hotspot/test/*, JDK java.lang & java.util,
>>>>>     vm.quick.testlist, JCK lang & vm, defmeth tests
>>>>>     - Test will be added to the defmeth test system
>>>>>     Thank you,
>>>>>     Lois

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140411/478a4404/attachment.html>

More information about the hotspot-runtime-dev mailing list