On Mar 14, 2013, at 8:31 PM, Christian Thalinger <christian.thalin...@oracle.com> wrote:
> [This is the HotSpot part of JEP 176] > > http://cr.openjdk.java.net/~twisti/7198429 > > 7198429: need checked categorization of caller-sensitive methods in the JDK > Reviewed-by: Over all, great work on a tricky problem. I'd add a few asserts and tweak a couple of lines; see below. Reviewed as is or with suggested changes. — John --- Method::is_ignored_by_security_stack_walk I would like to see reflect_invoke_cache go away some day. Would it be possible to strengthen the asserts to prove that it is an expensive alias for an intrinsic_id check? (I realize this is a question mainly for folks working on JVMTI.) --- JVM_GetCallerClass Suggest an assert for vfst.method() == NULL. Should not happen, and previous code would apparently have crashed already, but... (The corner case I'm thinking of is a compiled frame with nmethod::method returning null after nmethod::make_unloaded. Should not happen.) --- JVM_GetClassContext What do these lines do: + // Collect method holders + GrowableArray<KlassHandle>* klass_array = new GrowableArray<KlassHandle>(); It looks like a paste-o from another source base. --- LibraryCallKit::inline_native_Reflection_getCallerClass I believe this assertion, but I would prefer to see it checked more forcibly: + // NOTE: Start the loop at depth 1 because the current JVM state does + // not include the Reflection.getCallerClass() frame. Not sure there is a good way to do this. But, perhaps put the comment here: case 0: // ...comment... ShouldNotReachHere(); Also, if something goes wrong with caller sensitivity, we just get a "return false". Perhaps do a "caller_jvm=NULL;break" to branch to the pretty failure message? That makes it slightly easier to see what happened. The LogCompilation switch should leave a "paper trail". Actually, I see that LogCompilation doesn't mention failed intrinsic inlines. Rats. At least PrintInlining or PrintIntrinsics (diagnostic flags) will give us some leverage if we need to debug. --- JVM_RegisterUnsafeMethods That's an improvement. Thanks. (A nagging worry: How big are those static tables getting?) --- vframeStreamCommon::security_get_caller_frame This now does something odd if depth < 0. Suggest an assert.
