RFR: 8356941: AbstractMethodError in HotSpot Due to Incorrect Handling of Private Method [v2]

[Coleen Phillimore]

On Wed, 16 Jul 2025 06:51:55 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> src/hotspot/share/classfile/defaultMethods.cpp line 686:
>> 
>>> 684:           }
>>> 685:           Method* impl = klass->lookup_method(m->name(), m->signature());
>>> 686:           if (impl == nullptr || impl->is_overpass() || impl->is_static() || impl->is_private()) {
>> 
>> I still haven't figured out why this code fixes this issue, but why doesn't line 657 need the same change?
>
>> why doesn't line 657 need the same change?
> 
> Are you asking why it doesn't need the change to address the current bug? That would be because that block of code has nothing at all to do with the current bug. That block is looking at superclass overpass and static methods, to see if they may be candidates for replacing by a default method. For each of those methods it checks if there is an implementation in the current class that matches the name and sig and is a real method, and if so it doesn't create an empty vtable slot for it for default method processing to fill in. I spent a long time trying to create a scenario that would hit this block and the condition you want changed, but failed. Lets say we are looking at a slot for method `void m()` for simplicity. First you need a superclass that declares as static `void m()` method. Then you need a subclass that implements a non-static `void m()` method. But you also need to have the subclass implement an interface with a default `void m()` method, but in that case we will never
  reach the block of code because we will find that default method is already present in the list of miranda methods that we already processed! (During this phase of processing, default methods are considered mirandas - pass 1).
> 
> Now as to the actual fix ... we have a subclass that we are processing, and a superclass that has a private `void m()` method, and a super-super-interface that defines the default `void m()` method. We find `void m()` in the superclass's default methods - hence we reach the block of code I modified. For this slot to be processed  for default method handling (i.e. to get the correct entry so we invoke the true default method) we need to decide to create an empty vtable slot. The current logic (mirroring that for superclass methods - including the comment block that may or may not be accurate) checks to see if the current class has a concrete implementation of `void m()` as that would replace use of the default implementation (class instance methods "win"), except that is only true for non-private concrete implementations - a private class method is never considered the implementation of an interface method. So if the method is private we need to create the empty vtable slot - hence 
 the fix. 
> 
> Note that `klass->lookup_method` looks up the method in the current class and all superclasses, and will find overpass and private methods - so an alternative fix would be to change the lookup to skip private methods (and ...

I see why your fix fixes the bug in your test case.  I was asking about the block above with the identical comment and code.  Like you, I was trying to find a scenario where the block above your fix might also need the same condition added and couldn't find one.  Because if there's a private non-static function hiding a static function in a super class, the default method is then processed and will get the vtable slot added.
So your fix is good.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26302#discussion_r2210066182