Draft JVMS changes for Nestmates

[John Rose]

On Apr 18, 2017, at 11:40 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> 4.7.26 The MemberOfNest Attribute
> 
> "the class is implicitly a member of its own nest"
> 
> I suggest changing "a" to "the sole". Otherwise "a" implies there may be other members. As these are static properties being defined I don't think we need to be accounting for some future dynamic expansion of a nest.

I think you're missing the point of the spec. logic here.  If there is a
non-empty NestMembers attribute, then in fact there *are* other members.
Therefore "a" is correct, and "the sole" would be incorrect.

Separately, we do need to leave wiggle room for dynamic injection.
Dan, I think that would be done by allowing a dynamic injection
operation to implicitly extend the NestMembers list to make a
"ticket" for the injected class, and implicitly add (if necessary)
a MemberOfNest to the injected class.

...
> 
> 4.10.1 Verification by Type Checking
> 
> Does the order in the rules mandate the order in which the VM must actually check things? classHasValidNest seems much more complex than the actual runtime process of validation (though it may just be exposing distinct steps that are lumped together at runtime).

Agree.  I think this logic can be simplified.  (Will take that to a separate sub-thread.)

> ---
> 
> 4.10.1.8 Type Checking for Restricted Member References
> 
> I'm not very good at reading or understanding these rules, but I'm surprised that the invokespecial rules seem to make no mention of nestmates at all:
> 
> "A method reference is allowed by an invokespecial instruction if it is not a restricted invokespecial reference, ..."
> 
> This seems to open things up to all private method references??

Well, the whole point of nestmates is to extend, in a regular way,
all access rules that pertain to self-access and privacy.
If the extension is truly regular, it is not surprising that the spec.
changes are subtle like this.  We're not trying to do an ad hoc
patch on some set of particular behaviors, but rather extend
pre-existing notions so they apply in more conditions, including
invokespecial restrictions.

> 
> ---
> 
> 5.4.4 Access Control
> 
> I don't think you need to restate:
> 
> "A class with a MemberOfNest attribute belongs to the nest hosted by the referenced host class.
> 
> A class without a MemberOfNest attribute implicitly belongs to a nest hosted by the class itself. (If the class also lacks a NestMembers attribute, then the nest has only one member.)"
> 
> The rule is simply stated as is: "belonging to the same nest as D" - belonging to the same nest is, or should be, already defined elsewhere.

Actually, there is no "elsewhere"; this is where the condition is defined.
We don't need a special section for "nests"; we don't have a special
section for "packages" either.  So that part reads fine to me.

> I'm avoiding commenting on the protected access changes but do want to raise a concern that there are no changes to Method resolution (5.4.3.3) yet resolution relies on the definition of access control to prune/discard inaccessible methods. It appears now that we're allowing more potential successful resolutions, then using the additional rules on the invoke* bytecodes to try and discard any undesirable results.
> 
> ---
> 
> 6.5 getfield (and others)
> 
> You note "These rules are redundant: verification already guarantees them " but this brings me back to an area I keep raising concerns about: the split between static verification based checks and dynamic runtime checks. Yes the verifier precludes certain things but if we run without verification the rules expressed for the bytecodes are considered to be still required at runtime. If you delete them because they overlap with verifier rules we have no way to tell which rules must be enforced regardless of verification status.
> 
> ---
> 
> 6.5 invokespecial
> 
> You note regarding the IllegalAccessError:
> 
> "This replaces a VerifyError previously specified by 4.9.2. The check must be delayed until after resolution in order to determine whether the referenced method is private."
> 
> We know the access flags for the referenced method at verification time - shouldn't that be the sole basis for the verifier check? Afterall it is verifying the static properties of the classfile and bytecode. If the actual resolved method has different access to the referenced method then that may lead to ICCE (depending on the exact nature of the change - a private method made public may not be an issue for example).
> 
> ---
> 
> 5.4.3.3 Method resolution
> 
> You did not make any changes here, but as per my comment in the bug report we do require, IMHO, further tightening here to ensure nestmate invokespecial invocations do not resolve to method implementations that they should not. The example here is a hierarchy of nestmates (C extends B extends A) where A and C both declare a private method "void m()" and we have an invocation c.m() where c is an instance of class C. We then "separately compile" C to remove the definition of m(). At runtime method resolution will locate A.m, even though the method reference was for C.m. Normally the accessibility rules would exclude A.m from being a viable candidate but here the classes are nestmates so A.m is accessible. But it would be wrong to invoke A.m. This case should throw ICCE or NSME.
> 
> Thanks,
> David
> -----