RFR: 8022718 : Runtime accessibility checking: protected class, if extended, should be accessible from another package

[David Chase]

How about this one?

http://cr.openjdk.java.net/~drchase/8022718/webrev.01/

 This version tests both versions of the modifiers for "public" --
ref.getModifiers apparently sets public in some cases where the other one does not.
Not 100% sure what those cases are, I suspect arrays from the cryptic failure message.

Retested so far on the added test, and on j/l/i/MethodHandlesTest, which has been a fruitful source of problems so far.

David

On 2013-10-09, at 3:22 PM, John Rose <john.r.rose at oracle.com> wrote:
> 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
>> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Message signed with OpenPGP using GPGMail
Url : http://mail.openjdk.java.net/pipermail/mlvm-dev/attachments/20131009/e04146aa/signature.asc