[hs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Mon Jun 4 09:10:39 UTC 2018
Hi David,
It looks good.
Nice approach to fix these tests.
Thanks,
Serguei
On 6/4/18 00:15, David Holmes wrote:
> This update fixes some tests that were being excluded temporarily, but
> which can now run under nestmates.
>
> Incremental hotspot webrev:
> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.hotspot.v5-incr/
>
> Full hotspot webrev:
> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.hotspot.v5/
>
> Change summary:
>
> - test/hotspot/jtreg/vmTestbase/nsk/stress/except/except004.java (see:
> 8203046):
>
> The test expected an IllegalAccessException using reflection to access
> a private field of a nested class. That's actually a reflection bug
> that nestmates fixes. So relocated the Abra.PRIVATE_FIELD to a
> top-level package-access class Ext
>
> - all other tests involve class redefinition (see: 8199450):
>
> These tests were failing because the RedefineClassHelper, if passed a
> string containing "class A$B { ...}" doesn't define a nested class but
> a top-level class called A$B (which is perfectly legal). The
> redefinition itself would fail as the old class called A$B was a
> nested class and you're not allowed to change the nest attributes in
> class redefinition or transformation.
>
> The fix is simply to factor out the A$B class being redefined to being
> a top-level package access class in the same source file, called A_B,
> and with all references to "B" suitable adjusted.
>
> [The alternate fix considered would be to update the
> RedefineClassHelper and its use of the InMemoryJavaCompiler so that
> the tests would pass in a string like "class A { class B { ... } }"
> and then read back the bytes for A$B with nest attributes intact. But
> that is a non-trivial task and it isn't really significant that the
> classes used in these tests were in fact nested.]
>
>
> Thanks,
> David
>
>
> On 28/05/2018 9:20 PM, David Holmes wrote:
>> I've added some missing JNI tests for the basic access checks. Given
>> JNI ignores access it should go without saying that JNI access to
>> nestmates will work fine, but it doesn't hurt to verify that.
>>
>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.hotspot.v4-incr/
>>
>>
>> Thanks,
>> David
>>
>> On 24/05/2018 7:48 PM, David Holmes wrote:
>>> Here are the further updates based on review comments and rebasing
>>> to get the vmTestbase updates for which some closed test changes now
>>> have to be applied to the open versions.
>>>
>>> Incremental hotspot webrev:
>>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.hotspot.v3-incr/
>>>
>>>
>>> Full hotspot webrev:
>>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.hotspot.v3/
>>>
>>> Change summary:
>>>
>>> test/hotspot/jtreg/ProblemList.txt
>>> - Exclude vmTestbase/nsk/stress/except/except004.java under 8203046
>>>
>>> test/hotspot/jtreg/vmTestbase/vm/runtime/defmeth/BasicTest.java
>>> test/hotspot/jtreg/vmTestbase/vm/runtime/defmeth/PrivateMethodsTest.java
>>>
>>> - updated to work with new invokeinterface rules and nestmate changes
>>> - misc cleanups
>>>
>>> src/hotspot/share/runtime/reflection.?pp
>>> - rename verify_field_access to verify_member_access (it's always
>>> been mis-named and I nearly forgot to do this cleanup!) and rename
>>> field_class to member_class
>>> - add TRAPS to verify_member_access to allow use with CHECK macros
>>>
>>> src/hotspot/share/ci/ciField.cpp
>>> src/hotspot/share/classfile/classFileParser.cpp
>>> src/hotspot/share/interpreter/linkResolver.cpp
>>> - updated to use THREAD/CHECK with verify_member_access
>>> - for ciField rename thread to THREAD so it can be used with
>>> HAS_PENDING_EXCEPTION
>>>
>>> src/hotspot/share/oops/instanceKlass.cpp
>>> - use CHECK_false when calling nest_host()
>>> - fix indent near nestmate code
>>>
>>> src/hotspot/share/oops/instanceKlass.hpp
>>> - make has_nest_member private
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>> On 23/05/2018 4:57 PM, David Holmes wrote:
>>>> Here are the updates so far in response to all the review comments.
>>>>
>>>> Incremental webrev:
>>>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.hotspot.v2-incr/
>>>>
>>>>
>>>> Full webrev:
>>>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.hotspot.v2/
>>>>
>>>> Change summary:
>>>>
>>>> test/runtime/Nestmates/reflectionAPI/*
>>>> - moved to java/lang/reflect/Nestmates
>>>>
>>>> src/hotspot/cpu/arm/templateTable_arm.cpp
>>>> - Fixed ARM invocation logic as provided by Boris.
>>>>
>>>> src/hotspot/share/interpreter/linkResolver.cpp
>>>> - expanded comment regarding exceptions
>>>> - Removed leftover debugging code
>>>>
>>>> src/hotspot/share/oops/instanceKlass.cpp
>>>> - Removed FIXME comments
>>>> - corrected incorrect comment
>>>> - Fixed if/else formatting
>>>>
>>>> src/hotspot/share/oops/instanceKlass.hpp
>>>> - removed unused debug method
>>>>
>>>> src/hotspot/share/oops/klassVtable.cpp
>>>> - added comment by request of Karen
>>>>
>>>> src/hotspot/share/runtime/reflection.cpp
>>>> - Removed FIXME comments
>>>> - expanded comments in places
>>>> - used CHECK_false
>>>>
>>>> Thanks,
>>>> 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 hotspot - webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.hotspot.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
>>>>>
More information about the hotspot-dev
mailing list