[svc] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

[David Holmes]

Hi Serguei,

Thanks for that review. I will fix that typo when I advance everything 
to .v2 and regenerate the specdiffs for the CSR.

Meanwhile I just updated the webrev by adding in the missing JDI test 
for rejecting the nestmate attribute changes during class redefinition. 
I copied the existing instrumentation RedefineNestmateAttr test and 
adapted it to JDI - which was quite an educational experience ;-) So 
please take a look at that when you can.

Thanks,
David

On 17/05/2018 5:52 PM, serguei.spitsyn at oracle.com wrote:
> Hi David,
> 
> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.serviceability.v1/src/java.instrument/share/classes/java/lang/instrument/Instrumentation.java.udiff.html
> 
> A small typo: retransformation => redefinition
> 
> - * The *redefinition* may change method bodies, the constant pool and 
> attributes.
> + * The *redefinition* may change method bodies, the constant pool and 
> attributes
> + * (unless explicitly prohibited).
>        * The redefinition must not add, remove or rename fields or methods, change the
> - * signatures of methods, or change inheritance. These restrictions 
> maybe be
> - * lifted in future versions. The class file bytes are not checked, 
> verified and installed
> + * signatures of methods, or change inheritance.
> + * The *retransformation* must not change the <code>NestHost</code> or
> + * <code>NestMembers</code> attributes.
> + * These restrictions maybe be lifted in future versions.
> + * The class file bytes are not checked, verified and installed
> 
> 
> Otherwise, the changes look good.
> No need in another webrev if the above is fixed.
> 
> Thank you for taking care about this!
> 
> Thanks,
> Serguei
> 
> 
> On 5/16/18 23:33, David Holmes wrote:
>> FYI I updated the webrev with an additional change to the JDI/JDWP 
>> version - which needed updating to 11. This was done under "nestmates" 
>> as:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8203333
>>
>> and the patch has now been included here. Only two files with trivial 
>> updates:
>>
>>  src/jdk.jdi/share/classes/com/sun/tools/jdi/VirtualMachineManagerImpl.java 
>>
>>  src/jdk.jdwp.agent/share/native/libjdwp/VirtualMachineImpl.c
>>
>> David
>>
>> On 15/05/2018 10:52 AM, David Holmes wrote:
>>> This review is being spread across four groups: langtools, core-libs, 
>>> hotspot and serviceability. This is the specific review thread for 
>>> serviceability - webrev:
>>>
>>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.serviceability.v1/ 
>>>
>>>
>>> See below for full details - including annotated full webrev guiding 
>>> the review.
>>>
>>> The intent is to have JEP-181 targeted and integrated by the end of 
>>> this month.
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>> The nestmates project (JEP-181) introduces new classfile attributes 
>>> to identify classes and interfaces in the same nest, so that the VM 
>>> can perform access control based on those attributes and so allow 
>>> direct private access between nestmates without requiring javac to 
>>> generate synthetic accessor methods. These access control changes 
>>> also extend to core reflection and the MethodHandle.Lookup contexts.
>>>
>>> Direct private calls between nestmates requires a more general 
>>> calling context than is permitted by invokespecial, and so the JVMS 
>>> is updated to allow, and javac updated to use, invokevirtual and 
>>> invokeinterface for private class and interface method calls 
>>> respectively. These changed semantics also extend to MethodHandle 
>>> findXXX operations.
>>>
>>> At this time we are only concerned with static nest definitions, 
>>> which map to a top-level class/interface as the nest-host and all its 
>>> nested types as nest-members.
>>>
>>> Please see the JEP for further details.
>>>
>>> JEP: https://bugs.openjdk.java.net/browse/JDK-8046171
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8010319
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8197445
>>>
>>> All of the specification changes have been previously been worked out 
>>> by the Valhalla Project Expert Group, and the implementation reviewed 
>>> by the various contributors and discussed on the valhalla-dev mailing 
>>> list.
>>>
>>> Acknowledgments and contributions: Alex Buckley, Maurizio Cimadamore, 
>>> Mandy Chung, Tobias Hartmann, Vladimir Ivanov, Karen Kinnear, 
>>> Vladimir Kozlov, John Rose, Dan Smith, Serguei Spitsyn, Kumar Srinivasan
>>>
>>> Master webrev of all changes:
>>>
>>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v1/
>>>
>>> Annotated master webrev index:
>>>
>>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/jep181-webrev.html
>>>
>>> Performance: this is expected to be performance neutral in a general 
>>> sense. Benchmarking and performance runs are about to start.
>>>
>>> Testing Discussion:
>>> ------------------
>>>
>>> The testing for nestmates can be broken into four main groups:
>>>
>>> -  New tests specifically related to nestmates and currently in the 
>>> runtime/Nestmates directory
>>>
>>> - New tests to complement existing tests by adding in testcases not 
>>> previously expressible.
>>>    -  For example java/lang/invoke/SpecialInterfaceCall.java tests 
>>> use of invokespecial for private interface methods and performing 
>>> receiver typechecks, so we add 
>>> java/lang/invoke/PrivateInterfaceCall.java to do similar tests for 
>>> invokeinterface.
>>>
>>> -  New JVM TI tests to verify the spec changes related to nest 
>>> attributes.
>>>
>>> -  Existing tests significantly affected by the nestmates changes, 
>>> primarily:
>>>     -  runtime/SelectionResolution
>>>
>>>     In most cases the nestmate changes makes certain invocations that 
>>> were illegal, legal (e.g. not requiring invokespecial to invoke 
>>> private interface methods; allowing access to private members via 
>>> reflection/Methodhandles that were previously not allowed).
>>>
>>> - Existing tests incidentally affected by the nestmate changes
>>>
>>>    This includes tests of things utilising class 
>>> redefinition/retransformation to alter nested types but which 
>>> unintentionally alter nest relationships (which is not permitted).
>>>
>>> There are still a number of tests problem-listed with issues filed 
>>> against them to have them adapted to work with nestmates. Some of 
>>> these are intended to be addressed in the short-term, while some 
>>> (such as the runtime/SelectionResolution test changes) may not 
>>> eventuate.
>>>
>>> - https://bugs.openjdk.java.net/browse/JDK-8203033
>>> - https://bugs.openjdk.java.net/browse/JDK-8199450
>>> - https://bugs.openjdk.java.net/browse/JDK-8196855
>>> - https://bugs.openjdk.java.net/browse/JDK-8194857
>>> - https://bugs.openjdk.java.net/browse/JDK-8187655
>>>
>>> There is also further test work still to be completed (the JNI and 
>>> JDI invocation tests):
>>> - https://bugs.openjdk.java.net/browse/JDK-8191117
>>> which will continue in parallel with the main RFR.
>>>
>>> Pre-integration Testing:
>>>   - General:
>>>      - Mach5: hs/jdk tier1,2
>>>      - Mach5: hs-nightly (tiers 1 -3)
>>>   - Targetted
>>>     - nashorn (for asm changes)
>>>     - hotspot: runtime/*
>>>                serviceability/*
>>>                compiler/*
>>>                vmTestbase/*
>>>     - jdk: java/lang/invoke/*
>>>            java/lang/reflect/*
>>>            java/lang/instrument/*
>>>            java/lang/Class/*
>>>            java/lang/management/*
>>>    - langtools: tools/javac
>>>                 tools/javap
>>>
>