RFR: 8260009: InstanceKlass::has_as_permitted_subclass() fails if subclass was redefined [v2]

[Harold Seigel]

On Thu, 21 Jan 2021 22:15:36 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Harold Seigel has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8260009: remove comment
>
> Hi Harold,
> 
> This makes me wonder whether we can have a similar issue with nestmate checking?
> 
> Just curious: is there any way to determine that a Klass k2 is actually a redefinition of Klass k1? I
> 
> One nit below, otherwise this seems fine.
> 
> Thanks,
> David

Hi David,

I agree that this is not an issue for nestmates.  Class redefinition loads the redefined class, and class loading calls verify_member_access(), which does a nestmate check for private access.  But, class loading only calls verify_member_access() for accessing non-private methods, when doing method override checking in check_final_method_override().

Thanks, Harold
________________________________
From: mlbridge[bot] <notifications at github.com>
Sent: Thursday, January 21, 2021 5:25 PM
To: openjdk/jdk <jdk at noreply.github.com>
Cc: Harold Seigel <harold.seigel at oracle.com>; Mention <mention at noreply.github.com>
Subject: [External] : Re: [openjdk/jdk] 8260009: InstanceKlass::has_as_permitted_subclass() fails if subclass was redefined (#2184)


Mailing list message from David Holmes<mailto:david.holmes at oracle.com> on hotspot-dev<mailto:hotspot-dev at openjdk.java.net>:

On 22/01/2021 8:19 am, David Holmes wrote:

On Thu, 21 Jan 2021 13:41:44 GMT, Harold Seigel <hseigel at openjdk.org> wrote:

Please review this fix for JDK-8260009. The fix removes the "if (k2 == k)" check for a valid permitted subclass because the check does not work if "k" is being redefined. It fails because 'k' is a temporary InstanceKlass created when loading the redefined class.

Instead, the code relies on a name check to determine if 'k' is a permitted subclass.

The fix was tested with Mach5 tiers 1-2 on Linux, Mac OS, and Windows, tiers 3-5 on Linux x64, and JCK Lang and VM tests.

Thanks, Harold

Hi Harold,

This makes me wonder whether we can have a similar issue with nestmate checking?

Thinking more, I think not. IIUC the issue here is that the permitted
subclass check happens as part of redefinition - correct? Whereas there
is no nestmate access check as part of redefinition.

David

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/2184*issuecomment-764980526__;Iw!!GqivPVa7Brio!K9lG_bCi-kUDG0EXHNUs42PuDTpuzkrpKpRfThkaBWcD9ct3VwlvM19wzyIVcnwrKA$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AOXZ7OWMVJPHR72ZNL2TXKLS3CSTZANCNFSM4WM4FQFQ__;!!GqivPVa7Brio!K9lG_bCi-kUDG0EXHNUs42PuDTpuzkrpKpRfThkaBWcD9ct3VwlvM19wzyJL1nMdLA$>.

> src/hotspot/share/oops/instanceKlass.cpp line 255:
> 
>> 253:   // Do not check for a resolved cp entry, because that check can fail if
>> 254:   // the class is being redefined.  Just do a name check.
>> 255:   // We don't want to resolve any class other than the one being checked.
> 
> You don't resolve any classes at all now so this comment can go.

comment was deleted.

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

PR: https://git.openjdk.java.net/jdk/pull/2184