[hs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control
Boris Ulasevich
boris.ulasevich at bell-sw.com
Thu May 17 09:23:53 UTC 2018
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! :)
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
>>>>>
-------------- next part --------------
diff -r 3d98842c8677 src/hotspot/cpu/arm/templateTable_arm.cpp
--- a/src/hotspot/cpu/arm/templateTable_arm.cpp Tue May 15 05:33:26 2018 -0400
+++ b/src/hotspot/cpu/arm/templateTable_arm.cpp Thu May 17 12:03:52 2018 +0300
@@ -4276,25 +4276,42 @@
const Register Rinterf = R5_tmp;
const Register Rindex = R4_tmp;
const Register Rflags = R3_tmp;
- const Register Rklass = R3_tmp;
+ const Register Rklass = R2_tmp; // Note! Same register with Rrecv
prepare_invoke(byte_no, Rinterf, Rmethod, Rrecv, Rflags);
+ // First check for Object case, then private interface method,
+ // then regular interface method.
+
// Special case of invokeinterface called for virtual method of
- // java.lang.Object. See cpCacheOop.cpp for details.
- // This code isn't produced by javac, but could be produced by
- // another compliant java compiler.
- Label notMethod;
- __ tbz(Rflags, ConstantPoolCacheEntry::is_forced_virtual_shift, notMethod);
-
+ // java.lang.Object. See cpCache.cpp for details.
+ Label notObjectMethod;
+ __ tbz(Rflags, ConstantPoolCacheEntry::is_forced_virtual_shift, notObjectMethod);
invokevirtual_helper(Rmethod, Rrecv, Rflags);
- __ bind(notMethod);
+ __ bind(notObjectMethod);
// Get receiver klass into Rklass - also a null check
__ load_klass(Rklass, Rrecv);
+ // Check for private method invocation - indicated by vfinal
Label no_such_interface;
+ Label notVFinal;
+ __ tbz(Rflags, ConstantPoolCacheEntry::is_vfinal_shift, notVFinal);
+
+ Label subtype;
+ __ check_klass_subtype(Rklass, Rinterf, R1_tmp, R3_tmp, noreg, subtype);
+
+ // If we get here the typecheck failed
+ __ b(no_such_interface);
+ __ bind(subtype);
+
+ // do the call - the index is actually the method to call
+ __ profile_final_call(R0_tmp);
+ __ jump_from_interpreted(Rmethod);
+
+ __ bind(notVFinal);
+
// Receiver subtype check against REFC.
__ lookup_interface_method(// inputs: rec. class, interface
Rklass, Rinterf, noreg,
More information about the hotspot-dev
mailing list