On Wed, 5 Jul 2023 17:25:24 GMT, Volker Simonis <simo...@openjdk.org> wrote:
>> As the included jtreg test demonstrates, `StackWalker.getCallerClass()` can >> throw an `UnsupportedOperationException` if called reflectively. Currently >> this only happens if we invoke `StackWalker.getCallerClass()` recursively >> reflectively, but this issue will become more prominent once we fix >> [JDK-8285447](https://bugs.openjdk.org/browse/JDK-8285447). The gory details >> follow below: >> >> The protocol between the Java API and the JVM for >> `StackWalker.getCallerClass()/walk()` is as follows: >> - On the Java side, `StackWalker` calls into `StackStreamFactory` for the >> real work. >> - For `StackWalker.getCallerClass()` `StackStreamFactory` basically creates >> a `Class[]` which will be passed down and filled in the JVM. For >> `StackWalker.walk()` it will normally be a `StackFrameInfo[]` (or a >> `LiveStackFrameInfo[]` if the internal `ExtendedOption.LOCALS_AND_OPERANDS` >> option was used). >> - The default size of this arrays is currently >> `StackStreamFactory.SMALL_BATCH` which is 8 (but see >> [JDK-8285447](https://bugs.openjdk.org/browse/JDK-8285447)). >> - `StackStreamFactory` than calls `AbstractStackWalker.callStackWalk()` >> which is a natively implemented in the VM by `JVM_CallStackWalk()`. >> - `JVM_CallStackWalk()` calls `StackWalk::walk()` which calls >> `StackWalk::fetchFirstBatch()` which calls `StackWalk::fill_in_frames()` >> which walks the stack and fills in the available class/stackframe slots in >> the passed in array until the array is full or there are no more stack >> frames, >> - Once `StackWalk::fill_in_frames()` returns, >> `StackWalk::fetchFirstBatch()` calls back to Java by invoking >> `AbstractStackWalker::doStackWalk()` to consume the result. >> - `AbstractStackWalker::doStackWalk()` calls `consumeFrames()` (which is >> overridden depending on whether we initially called `getCallerClass()` or >> `walk()`) which consumes the frames until it either finishes (e.g. finds the >> caller class) or until there are no more frames. >> - In the latter case `consumeFrames()` will call into the the VM again by >> calling `AbstractStackWalker.fetchStackFrames()` to fetch additional frames >> from the stack. >> - `AbstractStackWalker.fetchStackFrames()` is implemented by >> `JVM_MoreStackWalk()` which calls `StackWalk::fetchNextBatch()` which calls >> `StackWalk::fill_in_frames()` (the same method that already fetched the >> initial batch of frames). >> >> Following is a stacktrace of what I've explained so far: >> >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x1... > > Volker Simonis has updated the pull request incrementally with one additional > commit since the last revision: > > Rename new parameter according to the HS coding conventions As discussed before, the test for a `@CallerSensitive` method must be applied to the first non-reflective, non-methodhandle frame below `StackWalker::getCallerClass()`, independent from the batch in which it appears. Because we can not predict which batch this will be, I've removed the previous solution which added a special flag to distinguish the first from the follow up batches. Instead I've added a new flag `_needs_caller_sensitive_check` to the `BaseFrameStream` class because every invocation of `StackWalker::getCallerClass()` will always use a single instance of `BaseFrameStream`. Currently the check for `@CallerSensitive` methods can only be done in the VM because for `StackWalker::getCallerClass()` we only pass the `Class` of each stack frame back to Java but not the corresponding `Method`. On the other hand, we currently skip reflective and methodhandle frames in Java (i.e. with `isReflectionFrame()` and `isMethodHandleFrame()`). In order to avoid the need to pass the stack frame methods back to Java I've therefore implemented (and combined) the the check for reflective and methodhandle frames in the VM (see `is_reflection_or_methodhandle_frame()`). Because `isMethodHandleFrame()` was only used in `ClassBuffer::consumeFrames()` it can be removed. I think we can also skip the remaining test for reflective frames in `AbstractStackWalker::peekFrame()` for the `getCallerClass()` case (because that check is now already done in the VM) but I'll leave that for [JDK-8285447](https://bugs.openjdk.org/browse/JDK-8285447). I've also moved the `ReflectiveGetCallerClassTest.java` test under `java/lang/StackWalker/CallerSensitiveMethod` such that I can use the `java/util/CSM.java` class which gets injected into the base module for the new `@CallerSensitive` test. Finally I realized that some `StackWalker` tests fail if we run with `-javaoption:-XX:+ShowHiddenFrames`. Ideally these tests should be fixed to make them agnostic to `ShowHiddenFrames` but for now I've added a `@requires vm.opt.ShowHiddenFrames == false` tag to exclude them if ran with `-XX:+ShowHiddenFrames`. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14773#issuecomment-1631065562
