RFR: 8356942: invokeinterface Throws AbstractMethodError Instead of IncompatibleClassChangeError [v2]

[Coleen Phillimore]

On Tue, 8 Jul 2025 21:28:57 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> src/hotspot/share/oops/klassVtable.cpp line 1185:
>> 
>>> 1183:       // Do not check loader constraints for overpass methods because overpass
>>> 1184:       // methods are created by the jvm to throw exceptions.
>>> 1185:       if (!target->is_overpass()) {
>> 
>> Why are loader constraints not checked for overpass methods?  In your description it says they _are_ checked?  But not here?  If the two methods that are duplicate on the inheritance path and should get ICCE have a loader constraint violation with one of their parameter or return type, they should get a LinkageError, but not ICCE.
>> 
>> What's confusing is that there are two check_constraints - I was looking at the wrong one that also has the !target->is_overpass() condition.  Are these mostly the same now?  Maybe after this change is backported they can be refactored so they share more code.
>
>> Why are loader constraints not checked for overpass methods? In your description it says they are checked? 
> 
> I don't follow. They were checked prior to [JDK-8186092](https://bugs.openjdk.org/browse/JDK-8186092) but they should not be - you need to read [JDK-8186092](https://bugs.openjdk.org/browse/JDK-8186092) for the "why". I'm simply changing how we decide to skip them for overpass methods - using the same technique as is already applied to the vtable methods.
> 
>> What's confusing is that there are two check_constraints 
> 
> Yes one on the `klassVtable` and one on the `klassItable`. They do the same high-level job but are quite different in structure.

Okay.  Now I see (thanks for the explanation).  The wrong thing to do wasn't skipping loader constraint checks, it was where we skip them.  Now that these methods look the same, the consistency is helpful for understanding this.

>> test/hotspot/jtreg/vmTestbase/vm/runtime/defmeth/ConflictingDefaultsTest.java line 86:
>> 
>>> 84:      * TEST: C c = new C(); c.m() ==> ICCE
>>> 85:      * TEST: I c = new C(); c.m() ==> ICCE
>>> 86:      * TEST: J c = new C(); c.m() ==> ICCE
>> 
>> So this is the generator code that generates the defmeth tests, but iirc, the tests are regenerated and the generated tests were the ones checked in and run.
>> 
>> And shouldn't the test from commit 87e30fd80172c5bd6748f140ddf6cd19adb62764 now fail?
>
> I'm not aware of any checked in generated tests. I added the new tests here and they run.
> 
> Why should the test for loader-constraints fail? I fixed that issue just in a different way to the original fix. Hence the test passes as it should.

Got it now.

Thanks for verifying that the new tests run.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26122#discussion_r2193547047
PR Review Comment: https://git.openjdk.org/jdk/pull/26122#discussion_r2193547246