RFR: 8087342: crash in klassItable_initialize_itable_for_interface

[Lois Foltan]

On 7/15/2015 12:40 PM, Karen Kinnear wrote:
> Please review for JDK9:
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8087342
> webrev: http://cr.openjdk.java.net/~acorn/8087342.2/webrev/
>
> Crash occurs in product when we should through IncompatibleClassChangeError.

Hi Karen,
I think this looks good and I really like how straight forward 
klassVtable::is_miranda now is.  Some minor clarification comments:

- src/share/vm/instanceKlass.cpp
comments for the find_local_method* were changed to state:

+// note that the local methods array can have up to one overpass, one static
+// and one instance (private or not) with the same name/signature

I think there are two combinations that the code depends on not 
occurring, they are:
       1. all 3 are in existence in the local methods array (one 
overpass, one static and one instance)
       2. the combination of one static and one instance (private or not)
In other words there has to be an overpass to cause more than one method 
with the same name/signature within the local methods array. And it is 
either an overpass and a static or an overpass and an instance, but not 
all 3.  Correct me if I am wrong.

- src/share/vm/oops/klassVtable.cpp
Thank you for adding the improved comments ahead of is_miranda.  My read 
is that overpass methods are not considered miranda methods and I agree 
with that statement.  Yet, Klass::find_overpass is specified in the 
code.  I think the code is correct, but based on the comment I would 
have thought Klass::skip_overpass should have been specified?  Much like 
skip_static and skip_private.  So based on your later statement that 
"Abstract methods may already have been handled via overpasses" it 
implies that overpass methods, although not miranda methods, can satisfy 
or stand in for an miranda during pass 2.  So they must be found, did I 
understand that correctly?

Again, looks good.  I don't need to see another review.  My comments 
were merely clarification based.

Thanks,
Lois








>
> testing:
> internal tests: Defmeth (updated), SelectionResolution - product and fastdebug
> jprt
> jck
> jtreg/hotspot_all
> jtreg/jdk_streams
> test,noncolo.testlist, -atk quick
>
> (jck and macosx testing in progress)
>
> thanks,
> Karen
>