[Nestmates] RFR: 8197395: [Nestmates] VerifyAccess.isMemberAccessible must not allow private access between legacy nested types (original) (raw)

David Holmes david.holmes at oracle.com
Fri Feb 9 06:46:46 UTC 2018

Hi Mandy,

Thanks for looking at this!

On 9/02/2018 10:13 AM, mandy chung wrote:

On 2/7/18 11:53 PM, David Holmes wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8197395 webrev: http://cr.openjdk.java.net/~dholmes/8197395/webrev/

With the fix for JDK-8196320 however, isSamePackageMember() (now renamed to areNestMates) first checks Reflection.areNestMates() and then if that is false, the old enclosing-class check. Consequently the check can now return true for both nestmate classfiles and earlier classfiles! That is wrong. To fix this we replace the call to VerifyAccess.areNestMates() with Reflection.areNestMates(). The fix looks fine.   Thanks for the clear description of the issue.  These two methods could cause confusion to the reader and always need to go the javadoc.  What about renaming VerifyAccess::areNestMates to areNestmatesOrPreNestmates?

Yes this needs renaming. We need to reserve "areNestMates" to mean they have matching NestHost attributes - as per Class.areNestMates() and Reflection.areNestMates(). I'm tempted to not only revert back to the original isSamePackageMember() but to also restore the original implementation with no Reflection.areNestMates() call - as nestmates will still pass the enclosing-class check. That also avoids the penalty that applies to pre-nestmate classfiles, by avoiding the guaranteed-to-fail nestmate check. My only reservation is that in the future, when nestmates need not originate in source code, and so the enclosing class check may not apply, then we would need to restore the direct nestmate check. - and again be confronted with an awkwardly named method.

I think I will turn this around and rename the method to havePrivateAccess(). I'm also going to reverse the order of checking so that the actual nestmate check comes last.

Updated webrev: http://cr.openjdk.java.net/~dholmes/8197395/webrev.v2/

There is also a temporary psuedo-assertion verifying that if we've granted private nestmate access then refc==defc (which should be assured by the resolution of the member being accessed).

Why do you want myassert?

I wanted an always-on assertion independent of the language assert statement. This will either go in the final version of be replaced by a regular assert.

Thanks, David

Mandy

More information about the valhalla-dev mailing list