Thanks, Mandy - sounds fine.
webrev.03 all looks good to me.

-Brent
On 09/13/2016 05:04 PM, Mandy Chung wrote:
Yes that’s one option.

JVM_STACKWALK_FILL_CLASS_REFS_ONLY is not necessarily used to get caller class.  For 
example, AccessControlContext is interested in protection domains.  We could build a 
specialized impl class to walk the stack fetching Class<?> only whereas 
getCallerClass will stop walking after a few top frames.   So a different bit will 
enable future experiment.  Having said that, the assert should be reverted and minor 
adjustment:

@@ -140,6 +143,13 @@
        fill_stackframe(stackFrame, method, bci);
      } else {
        assert (use_frames_array(mode) == false, "Bad mode for get caller 
class");
+      if (get_caller_class(mode) && index == start_index && 
method->caller_sensitive()) {
+        ResourceMark rm(THREAD);
+        THROW_MSG_0(vmSymbols::java_lang_UnsupportedOperationException(),
+          err_msg("StackWalker::getCallerClass called from @CallerSensitive %s 
method",
+                  method->name_and_sig_as_C_string()));
+      }
+

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157464/webrev.03/

Mandy

On Sep 13, 2016, at 4:09 PM, Brent Christian <brent.christ...@oracle.com> wrote:

Hi, Mandy

Is a new JVM_STACKWALK_GET_CALLER_CLASS bit needed in jvm.h?  AFAICT, 
JVM_STACKWALK_FILL_CLASS_REFS_ONLY is already only set for the getCallerClass() 
case.

Could the new get_caller_class() instead check if 
JVM_STACKWALK_FILL_CLASS_REFS_ONLY is set?  (Yes, this would be a third 
function checking the same thing, along with need_method_info() and 
use_frames_array()...)

-Brent
On 09/13/2016 03:18 PM, Mandy Chung wrote:
Hi Daniel,

StackWalker::getCallerClass is a convenient method to find the caller class and 
is specified to skip the hidden frames (that’s the caller we are interested 
in).   Since StackWalker only asks VM to fill in classes, the library can’t 
tell if it’s an anonymous class or not.

Your question prompts me to revise the patch and simply skip the hidden frame 
if this stack walk is to lookup caller class to match the specification.  I 
think this is a better fix:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157464/webrev.02/

We may re-examine this in the future if getCallerClass should get MemberName 
like the walk method such that we can determine if it’s a hidden frame and the 
performance difference.

Mandy

On Sep 13, 2016, at 11:54 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:

Hi Mandy,

This looks good to me.
But I wonder about these 5 lines - isn't this introducing a change
of behavior if the caller is an anonymous class?

149       InstanceKlass* ik = method->method_holder();
150       if (ik->is_anonymous()) {
151         // use the host class as the caller class
152         ik = ik->host_klass();
153       }

What is the reason for returning the host class instead?

best regards,

-- daniel

On 13/09/16 19:24, Mandy Chung wrote:
Webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157464/webrev.01

This revises the proposal posted some time ago [1].

StackWalker::getCallerClass is a convenient method to find the caller class. It 
will return the invoker of the MethodHandle and java.lang.reflect.Method for 
the method calling StackWalker::getCallerClass, as it’s currently specified.

This issue is related to MethodHandle for @CallerSensitive method.  It behaves 
as if the caller is the lookup class and in the current implementation, the 
actual caller class is not the lookup class but a generated class.

One intended usage of StackWalker::getCallerClass is to be called by library 
code acting as an agent that calls @CallerSensitive method on behalf of the 
true caller and typically it will call an appropriate method with the 
appropriate parameter (e.g. ResourceBundle.getBundle(String, ClassLoader).

Given that StackWalker::getCallerClass is not expected to be used by any @CS 
method, this patch proposes to catch and throw an exception if 
StackWalker::getCallerClass is called by a @CS method.  This will allow time to 
revisit this when such need is identified.

thanks
Mandy
[1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-July/042345.html






Reply via email to