Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK

[Christian Thalinger]

On Mar 27, 2013, at 10:35 AM, Mandy Chung <mandy.chung at oracle.com> wrote:

> This is the JDK change for JEP 176: JEP 176: Mechanical Checking of Caller-Sensitive Methods [1].  Christian has posted the webrev for the hotspot VM change a couple weeks ago [2].
> 
> Webrev at:
> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/7198429/webrev.00/

src/share/classes/java/lang/ClassLoader.java:

+    static void checkClassLoaderPermission(ClassLoader cl, Class<?> caller) {

I think we should rename that method to:

+    static void checkGetClassLoaderPermission(ClassLoader cl, Class<?> caller) {

src/share/classes/java/lang/invoke/MethodHandleImpl.java:

+        @sun.reflect.CallerSensitive
+            Class<?> actual = sun.reflect.Reflection.getCallerClass();

Why are we not using imports here?

src/share/classes/java/util/logging/Logger.java:

             // 0: Reflection 1: Logger.demandLogger 2: Logger.getLogger 3: caller
             final int SKIP_FRAMES = 3;

Please remove these lines as well.

src/share/native/sun/reflect/Reflection.c:

Could you put back this comment:

+     // Let's do at least some basic handshaking:
+     const int depth = -1;

It makes it clearer why it's -1.

test/sun/reflect/GetCallerClass.sh:

Could you please don't use a shell script to copy the class file?  For example this test:

http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/file/tip/test/compiler/whitebox/DeoptimizeAllTest.java

does the same thing using a little Java program ClassFileInstaller:

http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/file/tip/test/testlibrary/ClassFileInstaller.java

-- Chris

> 
> While it touches many files, the fix is simple and straight-forward for review.
> 
> This fix annotates all methods that call Reflection.getCallerClass() method with @sun.reflect.CallerSensitive annotation so that it enables the VM to reliably enforce that methods looking up its immediate caller class are marked as caller-sensitive. The JVM will set a new caller-sensitive bit when resolving a MemberName and java.lang.invoke.MethodHandleNatives.isCallerSensitive is upgraded to query it directly.
> The hand-maintained method list in MethodHandleNatives is removed.
> 
> A couple things to mention:
> 1. I am working on a fix for 8007035 that proposes to deprecate SecurityManager.checkMemberAccess method as it requires the caller’s frame to be at a stack depth of four, which is fragile and difficult to enforce.
> 
> 2. NashornScriptEngineFactory.getAppClassLoader()
> 
> The change is to workaround the issue until 8009783 is resolved.
> 
> The current implementation walks the stack to find the classloader of the user context that NashornScriptEngine is running on which is fragile.  Also other script engine implementations may require similiar capability.  8009783 has been filed to revisit the scripting API to pass the user "context" to the script engine rather than relying the implementation to find it magically.
> 
> Thanks
> Mandy
> 
> [1] http://openjdk.java.net/jeps/176
> [2] http://mail.openjdk.java.net/pipermail/hotspot-dev/2013-March/008915.html