RFR: 8356941: AbstractMethodError in HotSpot Due to Incorrect Handling of Private Method [v3]
Coleen Phillimore
coleenp at openjdk.org
Fri Jul 18 14:51:54 UTC 2025
On Fri, 18 Jul 2025 14:13:06 GMT, Dan Heidinga <heidinga at openjdk.org> wrote:
>> src/hotspot/share/classfile/defaultMethods.cpp line 667:
>>
>>> 665: if (!already_in_vtable_slots(slots, m)) {
>>> 666: Method* impl = klass->lookup_method(m->name(), m->signature());
>>> 667: if (impl == nullptr || impl->is_overpass() || impl->is_static() || impl->is_private()) {
>>
>> Trying to capture your and Coleen's discussion in a comment. Does this correctly cover the gist of the discussion?
>>
>> Suggestion:
>>
>> // Unless the klass provides a non-static public or package-private method for this name
>> // & sig combo, we need to add the EmptyVtableSlot so default method processing finds
>> // the correct candidate to fill in the slot later.
>> if (impl == nullptr || impl->is_overpass() || impl->is_static() || impl->is_private()) {
>
> Alternatively, this same series of checks occurs in `FindMethodsByErasedSig::visit()` with a long comment there about the set of possible methods. Is it worth replacing both sets of checks with a helper method that encapsulates the set of decisions?
>
>
> // Private interface methods are not candidates for default methods.
> // invokespecial to private interface methods doesn't use default method logic.
> // Private class methods are not candidates for default methods.
> // Private methods do not override default methods, so need to perform
> // default method inheritance without including private methods.
> // The overpasses are your supertypes' errors, we do not include them.
> static bool is_not_excluded_method(Method* m) {
> if (m != nullptr && !m->is_static() && !m->is_overpass() && !m->is_private()) {
> return true;
> }
> return false;
> }
>
>
> The name isn't great - feel free to replace with something better - but the idea is to capture the requirements in one place
I like this idea and I don't think there's harm in using this condition in the upper loop also, even though we couldn't find a case of a private method not getting to the code to create the empty vtable slots. I tested the upper loop with that case and even though we couldn't prove it's needed, I think it's also harmless to make it the same.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26302#discussion_r2216251232
More information about the hotspot-runtime-dev
mailing list