Hi Dan,
<trimming>
On 21/04/2017 4:43 AM, Dan Smith wrote:
Security risk
The JEP text should acknowledge that, while this does allow compilers to grant
finer-grained access to members shared by nestmates, it also pushes compilers
to grant broader access to members that were previously kept private. It's a
trade-off, and presumably a good one because nestmates are completely trusted,
while package-mates might sometimes be suspect.
I'm not following you here. What broader access is being granted to members
that were previously kept private? This effort allows private-only access to
members that were previously package-accessible.
Old approach is to widen access to private members to the package level on a
per-member basis.
New approach is to widen access to private members to the nest level on a
per-class basis.
Nest is narrower than package (good!), but per-class is more permissive than
per-member (bad). Some members that are compiled as totally private in JDK 9 will be
incidentally shared with all nest members in <future JDK version>.
I'm not sure how you are coming at this. For a private member in 9 to be incidentally
shared in <future-JDK> the class of the member has to be added to a nest. With
no context as to how this may happen I can't really comment further. From the Java
language perspective if the class is in a nest then such access to private members
already exists.
Example:
class Outer {
private int x;
private int y;
class Inner { private int z = x; }
}
In JDK 9, this compiles to:
class Outer {
private int x;
private int y;
Outer() {}
static int access$000(Outer arg) { return x; }
}
class Outer$Inner {
private int z;
final Outer this$0;
Outer$Inner(Outer arg) {
this$0 = arg;
z = Outer.access$000(this$0);
}
}
In <future-JDK>, this compiles to:
class Outer [NestMembers:{Outer$Inner}] {
private int x;
private int y;
Outer() {}
}
class Outer$Inner [MemberOfNest:Outer] {
private int z;
final Outer this$0;
Outer$Inner(Outer arg) {
this$0 = arg;
z = this$0.x;
}
}
Notice that in JDK 9, Outer shares access to y with nobody, while in
<future-JDK>, Outer shares access to y with its NestMembers.
Only because in JDK9 javac doesn't generate all potential accessors
(which it could). The current language rules say that y is accessible to
nestmates. As none try to actually access it javac doesn't bother
generating the accessor.
In:
"If getfield or putfield is used to access a protected field referenced in a
superclass ..."
and following, you changed "declared" to read "referenced". Again I do not understand
what this is supposed to mean. It is the declaration site of the field or method that is needed to determine
whether that member is in the same or another package. To me "declared in a superclass ..." was
exactly correct. Your note does not make the change any clearer to me.
Good catch. It's important that the rule be limited to cases that reference a
superclass*; but it's also important that the declaring class be used for the
package check.
(*Here's why: if I reference my subclass, and that resolves to a field declared
in my superclass, the verifier will perform no check, because it may not know
that the referenced class is a subclass.)
Revised:
"If `getfield` or `putfield` is used to access a `protected` field by reference to a
superclass, and that field is declared in a different run-time package than the current
class or interface, then the type of the class instance being accessed must be assignable
to the current class or interface."
I still think "declared in" is the correct terminology. There's a nuance to how
you are expressing this that I'm just not getting.
Example:
public class p1.A {
protected int i;
}
public class p2.B extends p1.A {
int test(p2.C arg) {
aload_1
getfield p2.C.i:I
ireturn
}
}
public class p2.C extends p2.B {
}
When the verifier checks the getfield, it will apply the rules in 4.10.1.8. The check
will trivially pass, because p2.C is not a superclass of the current class (p2.B). There
will be no test that p2.C is "assignable to the current class or interface".
Thus, p2.C need not be loaded during verification.
Strictly reading the old rule, "field declared in a superclass" means that the
verifier would have to load p2.C and resolve (or simulate resolution of) p2.C.i:I in
order to determine whether the field is declared in a superclass of the current class
(p2.B).
I'm not 100% sure of what needs loading when, but don't know why this is
an issue either. The package check is needed to see if the more specific
protected-access check is needed. As p2.C.i actually refers to the 'i'
declared in p1.A, then we should be checking if B and A are in the same
package.
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).
Example:
invokespecial SomeOtherClass.foo(I)V
Verification relies on the descriptor to tell it that this method expects an
int and returns nothing. Those kinds of checks can be performed locally.
The descriptor doesn't tell you if the method is private. Only way to find out is to
resolve SomeOtherClass, then resolve "foo(I)V".
If we wanted to do the check at verification time, we'd have to load every
class named by an invokespecial instruction.
Now I am confused. I thought the access modifier of the method described by the
Method_Ref was available to us. If it is not then how can all these
verification rules be expressed in terms of non-private when we do not have the
means at verification time to determine if the access is private or not ???
The rule is:
1) if the referenced method name is not <init> (cheap), and
2) if the referenced class name is the name of a superclass or direct
superinterface (cheap—superclass chain is already loaded), and
3) if the loaded referenced class actually is a superclass or direct
superinterface (almost always true and cheap, unless there's a name clash), and
4) if the resolved method is not private (pretty cheap, because referenced
class is already loaded)
*then* check that the stack type is assignable to the current class type.
This is, essentially, the same logic being employed by the old protected check:
if the reference is to a superclass, find the field/method and decide if it's
protected; if the reference is to some other class, don't worry about it.
So here's the problem, in the spec 5.4 states:
"Linking a class or interface involves verifying and preparing that
class or interface, its direct superclass, its direct superinterfaces,
and its element type (if it is an array type), if necessary. Resolution
of symbolic references in the class or interface is an optional part of
linking."
So resolution is _optional_ at link time! But your updated spec requires
resolution to happen before we can complete verification!
When we do the verification of invokespecial in the VM it is before
resolution and we do not know if the target method is private or not.
David
-----
—Dan
