On Oct 9, 2013, at 11:49 AM, David Chase <david.r.ch...@oracle.com> wrote:

> bug: https://bugs.openjdk.java.net/browse/JDK-8022718
> 
> webrev: http://cr.openjdk.java.net/~drchase/8022718/webrev.00/
> 
> Problem: Needed implementation for change to the spec for JVM behavior (from 
> the bug report):
> 
>> JSR 335 spec, chapter "15.28.2 Run-time Evaluation of Method References" 
>> contains following assertion:
>> 
>> If the method reference has the form ExpressionName :: 
>> NonWildTypeArgumentsopt Identifier or Primary :: NonWildTypeArgumentsopt 
>> Identifier, the body of the invocation method has the effect of invoking the 
>> compile-time declaration of the method reference, as described in 15.12.4.3, 
>> 15.12.4.4, 15.12.4.5.[jsr335-15.28.2-41]
>> 
>> This assertions refers to chapter "15.12.4.3. Check Accessibility of Type 
>> and Method" which contains following assertions:
>> 
>> Let C be the class containing the method invocation, and let T be the 
>> qualifying type of the method invocation (§13.1), and let m be the name of 
>> the method as determined at compile-time (§15.12.3).
>> ...
>> If T is in a different package than C, and T is protected, then T is 
>> accessible if and only if C is a subclass of T.
>> ...

JSR 292 doesn't implement the Java language spec; it implements the JVM spec.  
In this case there is a difference.

Mixing language and JVM spec. can lead to confusion down the road.

The error is that Class.getModifiers (which is correctly saying "protected") is 
information about the source code.  The JVM does not look at this for checking 
access; it looks at the more obscure Reflection.getClassAccessFlags.  Those 
flags will say "public" (correctly) for a class compiled as "protected".

In order to align exactly with JVM behavior (not Java semantics) the 
Reflection.getClassAccessFlags need to be looked at.  So I think this fix is 
wrong.  I think the root cause of this problem was me reaching for 
Class.getModifiers when I should have grabbed Reflection.getClassAccessFlags.

About BogoLoader:  I would prefer to see it hoisted into a place where more 
tests can use it.  Consider test/java/lang/invoke/indify/Indify.java as a 
model; name it test/java/lang/invoke/loader/BogoLoader.java.

— John

> Note that this cannot be expressed in compatibly compiled Java (yet).  
> Testing requires either monkeying with change and recompilation, or doctoring 
> bytecodes at runtime.
> 
> Fix:
> Turns out that all that was needed was the missing case of checking the 
> conditions for protected access,
> despite suggestions in the bug report that something different might be 
> needed (the different version of the
> access flags was not suitable, and not using them makes for a smaller change 
> anyway).  The new behavior
> needed to be mentioned in the Javadoc, so I did.
> 
> Testing:
> 1) wrote new self-contained test to verify behavior (*)
> 2) jtreg of jdk/lambda java/lang/invoke java/util/stream  (passed)
> 3) running ute on vm.quick.testlist
> 4) running JPRT on testset core (well, trying to run JPRT).
> 
> Issues:
> 
> - It's highly likely I botched the Javadoc changes; I'm sure we have 
> standards, and I'm sure I don't know them.
> 
> - (*) The bug, as reported, describes as "wrong" the behavior obtained if the 
> -PUBLIC +PROTECTED access mode appears on the innerclass attributes, not on 
> the class attributes.  I explored this space while developing the test, and 
> if the class itself is marked protected, the failure occurs much earlier in 
> the game when an attempt is made to load the subclass.  The test is written 
> to mimic standalone (with bytecode rewriting) what was seen in the test.
> 
> - I decided to put the test in its own subdirectory of test/java/lang/invoke 
> with its own informative name, and not directly in that directory, because 
> all these invocation/protection tests have overlapping and near-overlapping 
> class names, and so it would be confusing not to do this.  Unfortunately, 
> that means there's now two copies of the BogoLoader checked in to the test 
> tree.
> 
> David
> 

_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Reply via email to