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

[Coleen Phillimore]

On Tue, 8 Jul 2025 07:10:24 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> In [JDK-8186092](https://bugs.openjdk.org/browse/JDK-8186092) (way back in JDK 10) we elided loader constraint checks for overpass methods related to default methods by skipping them when initializing the itable for the interface. But that was the wrong thing to do. The overpass method is setup when there is a resolution/selection error so that the correct exception is thrown if the problematic method is invoked (like the ICCE reporting conflicting methods) and by eliding that entry we instead get the `AbstractMethhodError`.
>> 
>> The fix here is to revert that change from [JDK-8186092](https://bugs.openjdk.org/browse/JDK-8186092), and to address the loader constraint problem by adding the same check for overpass methods in `klassItable::check_constraints` that exists for `klassVtable::check_constraints`.
>> 
>> Testing:
>>  - modified existing regression test
>>  - tiers 1-4
>> 
>> EDIT: originally there was a new regression test for this, but this area is already covered by the `vmTestBase` "`defmeth` tests. That test was missing the necessary invocation modes to expose the bug, so they have been added.
>> 
>> Thanks
>> 
>> PS. The diff is much smaller if you disable whitespace differences.
>
> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Replaced new test by updating existing defmeth case that was missing the invokeinterface variants of the
>   test scenario. Also updated all tests therein to use `throwsExact` so that the wrong kind of ICCE does not
>   cause the test to pass by mistake.

Still so many questions...

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.

src/hotspot/share/oops/klassVtable.cpp line 1342:

> 1340:     if (target == nullptr || !target->is_public() || target->is_abstract()) {
> 1341:       // Entry does not resolve. Leave it empty for AbstractMethodError or other error.
> 1342:       if (!(target == nullptr) && !target->is_public()) {

Since you're here, can you fix this line to be target != nullptr ?  This part makes sense.  The reason to have overpass methods is to participate in resolution and selection.

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

PR Review: https://git.openjdk.org/jdk/pull/26122#pullrequestreview-2997364151
PR Review Comment: https://git.openjdk.org/jdk/pull/26122#discussion_r2192364443
PR Review Comment: https://git.openjdk.org/jdk/pull/26122#discussion_r2192339867