[hs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control
David Holmes
david.holmes at oracle.com
Wed May 23 06:57:54 UTC 2018
Hi Boris,
On 17/05/2018 7:23 PM, Boris Ulasevich wrote:
> Hi David,
>
> You are right! My bad, R2_tmp parameter conflicts with Rklass on
> check_klass_subtype(..) call. See correct patch in attach. Now all
> runtime/Nestmates tests passed! :)
I went to aplpy your patch and found it's not a diff against my patch
but against the original non-nestmate code, so I want to be clear on the
changes. AFAICS the differences are:
- const Register Rklass = R3_tmp;
+ const Register Rklass = R2_tmp; // Note! Same register with Rrecv
This initially concerned me as we stomp on Rrecv when we do the
load_klass, but then you moved the:
__ load_klass(Rklass, Rrecv);
to after the object case, which used Rrecv. I had assumed Rrecv was
somehow used when we actually do the call, but I'm assuming
jump_from_interpreted is done in such as way that the receiver is still
available on the interpreter stack and is used from there.
- __ check_klass_subtype(Rklass, Rinterf, R1_tmp, R0_tmp, subtype);
+ __ check_klass_subtype(Rklass, Rinterf, R1_tmp, R3_tmp, noreg, subtype);
Okay - reworking of tmp regs.
- __ jump_from_interpreted(Rindex);
+ __ jump_from_interpreted(Rmethod);
I'm going to trust this is okay :) It's not at all clear to me how the
f2 Method* gets passed through on different platforms - sometimes its in
the "index" and sometimes the "method" registers. (I _really_ wish there
was consistency in terminology across the different platforms - this
code is awful for trying to compare the different platforms to figure
out what to do on a new one.)
Thanks,
David
> regards,
> Boris
>
> On 17.05.2018 11:24, David Holmes wrote:
>> On 17/05/2018 6:13 PM, Boris Ulasevich wrote:
>>> Hi David,
>>>
>>> I see three tests failing:
>>> > NullPointerException at
>>> TestInterfaceMethodSelection.doInvoke(TestInterfaceMethodSelection.java:191)
>>>
>>> > NullPointerException at TestInvoke.access_priv(TestInvoke.java:54)
>>> > InvocationTargetException at
>>> TestReflection.access_priv(TestReflection.java:61)
>>>
>>> I will send you test details in a separate mail.
>>
>> Ok. This indicates a bug in the assembly code. The NPE's will likely
>> be SEGVs caused by a zero register.
>>
>> Thanks,
>> David
>>
>>
>>> Boris
>>>
>>> On 17.05.2018 00:23, David Holmes wrote:
>>>> Hi Boris,
>>>>
>>>> Many thanks for looking at this and working through the ARM code.
>>>>
>>>> I will study your patch in detail but am concerned by the "passes
>>>> most of runtime/Nestmates tests Ok."! What tests are not passing?
>>>>
>>>> David
>>>>
>>>> On 17/05/2018 1:05 AM, Boris Ulasevich wrote:
>>>>> Hi David,
>>>>>
>>>>> Let us look to the change in templateTable_arm.cpp. I have several
>>>>> notes.
>>>>>
>>>>> 1. We have compilation error because check_klass_subtype call needs
>>>>> one more temporary register parameter. I think correct change is:
>>>>> check_klass_subtype(Rklass, Rinterf, R1_tmp, R0_tmp, subtype);
>>>>> ->
>>>>> check_klass_subtype(Rklass, Rinterf, R1_tmp, R0_tmp, noreg, subtype);
>>>>>
>>>>> 2. R0_tmp holds mdp used for profilation several lines below, so we
>>>>> can't spoil it. I think correct change is:
>>>>> check_klass_subtype(Rklass, Rinterf, R1_tmp, R0_tmp, noreg, subtype);
>>>>> ->
>>>>> check_klass_subtype(Rklass, Rinterf, R1_tmp, R2_tmp, noreg, subtype);
>>>>>
>>>>> 3. We can't jump to Rindex. I believe it was supposed to jump to
>>>>> Rmethod:
>>>>> jump_from_interpreted(Rindex);
>>>>> ->
>>>>> jump_from_interpreted(Rmethod);
>>>>>
>>>>> 4. Please note than Rklass and Rflags reuses same register. Before
>>>>> the change their life ranges had no intersection. I would propose
>>>>> to use R2 for Rklass (same with Rrecv) and move load_klass call
>>>>> after invokevirtual_helper call to avoid life range intersecton.
>>>>>
>>>>> See my patch against original templateTable_arm.cpp version in
>>>>> attach - with this change ARM build compiles and passes most of
>>>>> runtime/Nestmates tests Ok.
>>>>>
>>>>> regards,
>>>>> Boris
>>>>>
>>>>> On 15.05.2018 03:52, 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