Re: A "RecursiveConstants" attribute

[Dan Smith]

> On Jan 31, 2018, at 2:50 PM, Brian Goetz  wrote:
> 
> OK, so the goal is clearly to *prohibit* cyclic constants by enforcing a 
> depth constraint on condy-in-condy nesting.  I like this goal much better, 
> and I see how the solution addresses it.
> 
> Still, I have to wonder whether its worth the complexity of capturing it 
> explicitly with a new (stackmap-like) attribute, which must be specified, and 
> gives us more chances for the classfile to be out of sync with itself, for 
> the goal of preventing something that can't usefully happen anyway -- as we 
> discussed previously, trying to actually resolve a cyclic constant will 
> likely result in SOE, so such classfiles are pretty much useless and 
> therefore not likely to spread very far.

This is a reasonable critique. Another critique is that the cost of tracking 
down and parsing an extra attribute will undo a lot of the performance benefit 
of having it, perhaps even causing a net slowdown.

As we've discussed the idea a little more, my inclination (raised in the 
meeting this week) is to give up on trying to add structural restrictions on 
constant pools designed to facilitate cycle detection. It seems like we can, in 
fact, detect cycles without extra guidance, and with minimal overhead in most 
cases. Some details below.

Big picture, the most attractive path forward seems to be:
- In 11, a CONSTANT_Dynamic with a cycle is an "error", reported via SOE at 
resolution time
- When we introduce lazy condy resolution, we move the error detection to class 
load time, with a check that we're comfortable isn't disrupting startup time
- As other potentially-cyclical constants are introduced, we use the same load 
time checks
- We never require extra information from the compiler, or reject classes that 
are hard to prove cycle-free

One nice thing about that last point is that we don't have to worry about 
incompatible support for "hard" classes in different releases.

I'd love to hear about some experiments with this. We're talking about 
performance impact, and all discussion to this point has been hypothetical. 
Would be good to get some concrete feedback.

—Dan

-

Details:

We started with a rule for format checking (JVMS 4.8) that says it's illegal 
for a CONSTANT_Dynamic (or, in the future, other potentially-cyclical 
constants) to refer, via static arguments, to itself—directly or indirectly. 
But we quickly decided this sounded expensive, and we didn't want a heavy 
validation step to mess with startup time.

Hence, we explored various ways to prohibit otherwise-valid classes that would 
be hard to prove cycle-free. Something like the RecursiveConstants attributes 
seems to be the best option in that space.

However, scrutinizing the original assumption, it seems reasonable that we can 
optimize the check so that it's extremely lightweight when references are 
"backward" (to previous entries in the constant pool), and only performs more 
expensive analysis when references are "forward". Most classes should naturally 
use backward references exclusively, since entries tend to be generated 
bottom-up. (In theory. If there are counter examples produced by any mainstream 
bytecode generators, that would be good to know.)

So a dumb (worst case O(n^2)) but fast (very low constant cost) check could 
look something like:

void checkCP(int i) {
switch (tag(i)) {
case CONSTANT_Dynamic:
...
for (int j : condyStaticArgs(i)) {
if (j >= i && tag(j) == CONSTANT_Dynamic) {
assertUnreachable(i, j); // can't reach i from j
}
}
...
...
}
}

It's already necessary to check that constant pool references are well-typed, 
so everything up to 'assertUnreachable' here is practically free*. The 
'assertUnreachable' call might be somewhat expensive, but it will rarely be 
called in practice (and even then, we don't expect many structures to be 
particularly deep).

(*In the particular case of CONSTANT_Dynamic, there's an indirection through 
BootstrapMethods. If a class heavily shares BootstrapMethods entries, then 
there's a new cost here, iterating over the same static arguments multiple 
times. Some optimization could be done to speed that up. For example, if there 
are no forward references on the first pass, there will be no forward 
references on later passes, when i is a larger number.)

A good worst-case test would be a constant pool without cycles, built entirely 
out of forward references, consisting of lots of deep CONSTANT_Dynamic trees. 
Needless to say, we don't expect to encounter classes like this in the wild. 
But, if we do, the big benefit here is that, while they'll be a little slower 
to load, they'll still work.

Re: API Updates: 8191116: [Nestmates] Update core reflection, MethodHandle and varhandle APIs to allow for nestmate access

[David Holmes]

Specdiffs now available again:

http://cr.openjdk.java.net/~dholmes/8010319/specs/java.lang/java/lang/Class.html

http://cr.openjdk.java.net/~dholmes/8010319/specs/java.lang.invoke/java/lang/invoke/MethodHandle.html

http://cr.openjdk.java.net/~dholmes/8010319/specs/java.lang.invoke/java/lang/invoke/MethodHandles.Lookup.html

http://cr.openjdk.java.net/~dholmes/8010319/specs/java.lang.reflect/java/lang/reflect/AccessibleObject.html

http://cr.openjdk.java.net/~dholmes/8010319/specs/java.lang.reflect/java/lang/reflect/Method.html

David

On 16/02/2018 7:20 PM, David Holmes wrote:

Hi Karen,

On 16/02/2018 8:01 AM, Karen Kinnear wrote:

David,

I think that is a much better solution. Let the description of each 
Lookup mode be precise, and you have already updated PRIVATE mode to

include nestmates.


Okay I've deleted that sentence.

Unfortunately something has broken specdiff so I can't regenerate the 
docs at the moment - and I've lost the ones I had generated.




I brought this concern up in the EG meeting  yesterday and wanted to 
clarify the difference
between handling of inner/outer classes for backward compatibility and 
general nestmate handling.


Many thanks for that detailed walk through.

David
-



Assumptions:
1. MethodHandle/VarHandle behavior is modeled on bytecode behavior.
2. Nestmates have the added capability of access to private members of 
their nestmates. Period.
3. In future we expect to use nestmates for more than inner/outer 
classes.
4. Inner/outer classes will continue to have the InnerClasses 
attribute, and starting in JDK11, javac

will also generate NestHost and NestMember attributes.
5. With Nestmates, javac will not generate the default (package) 
trampolines to allow inner/outer
classes to access each other’s private members. Note that today this 
is only done for members that

have compile time accesses.
(6. With Nestmates, bridges for protected members will still be 
generated unchanged.)


For nestmates in general, the modifications you have made below allow 
a nestmate to access private

members of their nestmates to match the bytecode behavior.

Prior to nestmates, there is a special workaround in 
MethodHandles.Lookup.in() to allow inner/outer
classes to access any member of any class that shares its top level 
class to emulate the generated trampolines.


https://docs.oracle.com/javase/9/docs/api/java/lang/invoke/MethodHandles.Lookup.html#lookupModes-- 



In some cases, access between nested classes is obtained by the Java 
compiler by creating an wrapper method to access a private method of 
another class in the same top-level declaration. For example, a nested 
class |C.D| can access private members within other related classes 
such as |C|, |C.D.E|, or |C.B|, but the Java compiler may need to 
generate wrapper methods in those related classes. In such cases, a 
|Lookup| object on |C.E| would be unable to those private members. A 
workaround for this limitation is the |Lookup.in| 
 method, 
which can transform a lookup on |C.E| into one on any of those other 
classes, without special elevation of privilege.


This workaround will continue to be supported going forward explicitly 
for inner/outer classes.
A side-effect of this workaround is the ability of the returned Lookup 
to access not only private methods in
the “related” class, but also protected and inherited members of that 
class which are defined in other packages.
So this workaround will continue to work for inner/outer classes that 
are also nestmates for backward

compatibility.

Going forward, for nestmates in general, the goal is to provide access 
to private members, which
can be done via the access check to match bytecode behavior, and does 
not require special Lookup.in() workarounds.


If at some point in the future we decide we want increased access for 
nestmates, we can widen
this. Let’s just say that the complexity there is challenging and that 
it is better to err on the side of

starting out more restrictive.

Summary - I agree with David. We can leave the documentation as is, 
with the explicit changes to
access checking modified below for private members accessible to 
nestmates.


thanks David!
Karen

On Feb 14, 2018, at 8:36 PM, David Holmes > wrote:


Hi Karen,

Thanks for looking at this.

On 15/02/2018 1:16 AM, Karen Kinnear wrote:

David,
Re-reading these I had one suggestion:

- java/lang/invoke/MethodHandles.java
  * 
- * In some cases, access between nested classes is obtained by 
the Java compiler by creating

- * an wrapper method to access a private method of another class
- * in the same top-level declaration.
+ * Since JDK 11 the relationship between nested types can be 
expressed directly through the

+ * {@code NestHost} and {@code NestMembers} attributes.
+ * (See the Java 

Re: API Updates: 8191116: [Nestmates] Update core reflection, MethodHandle and varhandle APIs to allow for nestmate access

[David Holmes]

Hi Karen,

On 16/02/2018 8:01 AM, Karen Kinnear wrote:

David,

I think that is a much better solution. Let the description of each 
Lookup mode be precise, and you have already updated PRIVATE mode to

include nestmates.


Okay I've deleted that sentence.

Unfortunately something has broken specdiff so I can't regenerate the 
docs at the moment - and I've lost the ones I had generated.




I brought this concern up in the EG meeting  yesterday and wanted to 
clarify the difference
between handling of inner/outer classes for backward compatibility and 
general nestmate handling.


Many thanks for that detailed walk through.

David
-



Assumptions:
1. MethodHandle/VarHandle behavior is modeled on bytecode behavior.
2. Nestmates have the added capability of access to private members of 
their nestmates. Period.

3. In future we expect to use nestmates for more than inner/outer classes.
4. Inner/outer classes will continue to have the InnerClasses attribute, 
and starting in JDK11, javac

will also generate NestHost and NestMember attributes.
5. With Nestmates, javac will not generate the default (package) 
trampolines to allow inner/outer
classes to access each other’s private members. Note that today this is 
only done for members that

have compile time accesses.
(6. With Nestmates, bridges for protected members will still be 
generated unchanged.)


For nestmates in general, the modifications you have made below allow a 
nestmate to access private

members of their nestmates to match the bytecode behavior.

Prior to nestmates, there is a special workaround in 
MethodHandles.Lookup.in() to allow inner/outer
classes to access any member of any class that shares its top level 
class to emulate the generated trampolines.


https://docs.oracle.com/javase/9/docs/api/java/lang/invoke/MethodHandles.Lookup.html#lookupModes--

In some cases, access between nested classes is obtained by the Java 
compiler by creating an wrapper method to access a private method of 
another class in the same top-level declaration. For example, a nested 
class |C.D| can access private members within other related classes such 
as |C|, |C.D.E|, or |C.B|, but the Java compiler may need to generate 
wrapper methods in those related classes. In such cases, a 
|Lookup| object on |C.E| would be unable to those private members. A 
workaround for this limitation is the |Lookup.in| 
 method, 
which can transform a lookup on |C.E| into one on any of those other 
classes, without special elevation of privilege.


This workaround will continue to be supported going forward explicitly 
for inner/outer classes.
A side-effect of this workaround is the ability of the returned Lookup 
to access not only private methods in
the “related” class, but also protected and inherited members of that 
class which are defined in other packages.
So this workaround will continue to work for inner/outer classes that 
are also nestmates for backward

compatibility.

Going forward, for nestmates in general, the goal is to provide access 
to private members, which
can be done via the access check to match bytecode behavior, and does 
not require special Lookup.in() workarounds.


If at some point in the future we decide we want increased access for 
nestmates, we can widen
this. Let’s just say that the complexity there is challenging and that 
it is better to err on the side of

starting out more restrictive.

Summary - I agree with David. We can leave the documentation as is, with 
the explicit changes to

access checking modified below for private members accessible to nestmates.

thanks David!
Karen

On Feb 14, 2018, at 8:36 PM, David Holmes > wrote:


Hi Karen,

Thanks for looking at this.

On 15/02/2018 1:16 AM, Karen Kinnear wrote:

David,
Re-reading these I had one suggestion:

- java/lang/invoke/MethodHandles.java
  * 
- * In some cases, access between nested classes is obtained by 
the Java compiler by creating

- * an wrapper method to access a private method of another class
- * in the same top-level declaration.
+ * Since JDK 11 the relationship between nested types can be 
expressed directly through the

+ * {@code NestHost} and {@code NestMembers} attributes.
+ * (See the Java Virtual Machine Specification, sections 4.7.28 
and 4.7.29.)
+ * In that case, the lookup class has direct access to private 
members of all its nestmates, and

+ * that is true of the associated {@code Lookup} object as well.
+ * Otherwise, access between nested classes is obtained by the 
Java compiler creating
+ * a wrapper method to access a private method of another class 
in the same nest.

  * For example, a nested class {@code C.D}

Updated the nested classes description to cover legacy approach and 
new nestmate approach.


- * {@code C.E} would be