[Nestmates] RFR: 8197395: [Nestmates] VerifyAccess.isMemberAccessible must not allow private access between legacy nested types
david.holmes at oracle.com
Fri Feb 9 06:46:46 UTC 2018
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
> 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
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
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
More information about the valhalla-dev