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

Boris Ulasevich boris.ulasevich at bell-sw.com
Wed May 16 15:05:47 UTC 2018


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	Wed May 16 17:35:40 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, R2_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