Review Request: JDK-8157464: StackWalker.getCallerClass() should not filter out non-invoker frames

[Daniel Fuchs]

On 13/07/16 23:35, Mandy Chung wrote:
> I considered that and it’s possible and that would simplify the VM logic would be a good thing (do the filtering in the library as much as possible).
>
> This would need to fill an array of MemberName rather than Class object.  MemberName would need to provide a way to determine if it’s a hidden method (that I have a patch).
>
> This would need performance measurement and possible further tuning on the batch size to minimize the number of up calls that I think better to separate as a follow up issue.

Hi Mandy,

I'm not sure we are talking of the same thing.
This was what I wanted to suggest:

    int frames_decoded = 0;
    for (; !stream.at_end(); stream.next()) {
      Method* method = stream.method();
      int bci = stream.bci();

      if (method == NULL) continue;
+
+    if (StackWalk::get_caller_class(mode)) {
+      // JVM_GetCallerClass may return a hidden method
+      // StackWalker::getCallerClass should return the same caller as 
JVM_GetCallerClass
+      // even the jdk implementation does not use it for performance reason
+      if (method->is_ignored_by_security_stack_walk()) continue;
+    } else {
+      // filter hidden frames
      if (!ShowHiddenFrames && StackWalk::skip_hidden_frames(mode)) {
        if (method->is_hidden()) {
          if (TraceStackWalk) {
            tty->print("  hidden method: "); method->print_short_name();
            tty->print("\n");
          }
          continue;
        }
      }
+    }

      int index = end_index++;
      if (TraceStackWalk) {
        tty->print("  %d: frame method: ", index); 
method->print_short_name();
        tty->print_cr(" bci=%d", bci);

@@ -139,10 +150,14 @@
        Handle stackFrame(frames_array->obj_at(index));
        fill_stackframe(stackFrame, method, bci);
      } else {
        assert (use_frames_array(mode) == false, "Bad mode for get 
caller class");
        frames_array->obj_at_put(index, 
method->method_holder()->java_mirror());
+      // nothing special to do here since we already checked
+      // StackWalk::get_caller_class(mode) above
      }
      if (++frames_decoded >= max_nframes)  break;
    }
    return frames_decoded;
  }



>
> Mandy
>
>> On Jul 13, 2016, at 10:55 PM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
>>
>> Hi Mandy,
>>
>> I wonder whether intermediate frames should be skipped always, whether
>> the method is @CS or not. Indeed StackWalker::getCallerClass() is
>> intented to be called from methods that are not @CS.
>>
>> If so the code in stackwalk.cpp could probably be simplified to simply
>> look at method->is_ignored_by_security_stack_walk() when the
>> JVM_STACKWALK_GET_CALLER_CLASS is set, and the @CS flag
>> could be ignored.
>>
>> (I'm comparing with what I see in jvm.cpp:JVM_GetCallerClass)
>>
>> what do you think?
>>
>> best regards,
>>
>> -- daniel
>>
>> On 13/07/16 12:01, Mandy Chung wrote:
>>> Webrev:
>>>  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157464/webrev.00/index.html
>>>
>>> StackWalker::getCallerClass() is specified to return the invoker of the MethodHandle and java.lang.reflect.Method for the method calling StackWalker::getCallerClass().
>>>
>>> StackWalker::getCallerClass() is not used by any @CallerSensitive method.  Instead 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).
>>>
>>> MethodHandle for @CS method behaves as if the caller is the lookup class.  The actual caller class may not be the lookup class which is left as implementation details.  This patch adjusts the stack walker to return the same caller as jdk.internal.reflect.Reflection::getCallerClass.
>>>
>>> Mandy
>>>
>>>
>>>
>>
>