On 3/19/2013 5:29 PM, Christian Thalinger wrote:
On Mar 19, 2013, at 1:14 PM, Mandy Chung <mandy.ch...@oracle.com> wrote:
I do a partial review in particular to make sure the jdk and hotspot change are
in sync.
javaClasses.hpp - MN_CALLER_SENSITIVE and MN_SEARCH_SUPERCLASSES have the same
value. Should they be different?
1057 MN_CALLER_SENSITIVE = 0x00100000, // @CallerSensitive annotation
detected
1061 MN_SEARCH_SUPERCLASSES = 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 MN_SEARCH_*
guys don't add much value and might be removed.
Thanks. That's fine.
method.hpp - Is caller_sensitive 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
JVM_GetCallerClass 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
JVM_GetCallerClass(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
