RFR (M): 7198429: need checked categorization of caller-sensitive methods in the JDK (original) (raw)

Mandy Chung mandy.chung at oracle.com
Wed Mar 20 01:02:16 UTC 2013

On 3/19/2013 5:29 PM, Christian Thalinger wrote:

On Mar 19, 2013, at 1:14 PM, Mandy Chung <mandy.chung at oracle.com> wrote:

I do a partial review in particular to make sure the jdk and hotspot change are in sync.

javaClasses.hpp - MNCALLERSENSITIVE and MNSEARCHSUPERCLASSES have the same value. Should they be different? 1057 MNCALLERSENSITIVE = 0x00100000, // @CallerSensitive annotation detected 1061 MNSEARCHSUPERCLASSES = 0x00100000, // walk super classes They can have the same value because they are used for different things in different places. I talked to John about this and he said that the MNSEARCH* guys don't add much value and might be removed.

Thanks. That's fine.

method.hpp - Is callersensitive set to true if @CallerSensitive annotation is present and must be loaded by null class loader? Does it only check annotations if the class of that method is defined by the null class loader? Per our offline discussion early, classes loaded by the extension class loader may also be caller-sensitive. Right. I forgot to add that code. Here is an incremental webrev: http://cr.openjdk.java.net/~twisti/7198429/edit/ And the full thing: http://cr.openjdk.java.net/~twisti/7198429/ Let me know if that works for you.

I'll take your patch and let you know.

If a method calls Reflection.getCallerClass but its class is defined by other class loader (non-null and not extension class loader), your fix will throw InternalError with the same error message even if that method is annotated with @CS. Is there a way to improve the error message so that we can differentiate this case (i.e. @CS is present but not supported)? Not easily. We set a flag on the method when parse the class. At that point we decide if the annotation is there or not. If the annotation is not allowed in parsed class because it's not loaded by the right class loader then it does not "exist".

701 THROW_MSG_NULL(vmSymbols::java_lang_InternalError(), err_msg("CallerSensitive annotation expected at frame %d", n));

Perhaps it could check the class loader of the class of the method or print out additional info? It should be rare to run into this error but it would greatly help diagnosing the problem if @CS method loaded by unexpected class loader.

jvm.cpp: have you considered adding a new entry point instead of having JVMGetCallerClass to behave differently depending on the existence of sun.reflect.CallerSensitive class? There are pros and cons of both options. Having a new entry point is cleaner and enables the opportunity to remove JVMGetCallerClass(int) in the future. I am fine with either approach but just to bring it up. Yes. I talked to Vladimir about that yesterday. The better solution seems to be to leave the old entry point. If we add a new one that kind of adds a new method to the "official" VM interface. Other VM implementors would have to implement that one as well because the JDK then links to this new method.

Yup, that's one of the cons of adding a new entry point. Other VM implementation may need to support JVM_GetCallerClass(-1) to run on our JDK library implementation in either approach.

Good and keep the code entry point. I'll send out the jdk change for code review.

Mandy

More information about the core-libs-dev mailing list