RFR: 8255233: InterpreterRuntime::at_unwind should be a JRT_LEAF [v2]

[David Holmes]

On Mon, 26 Oct 2020 08:38:50 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> InterpreterRuntime::at_unwind is called at the very beginning of remove_activation(), to notify concurrent stack processing that a frame is about to be unwound. It is currently a JRT_ENTRY, because it needs a last_Java_frame to see what frame is about to get unwound.
>> 
>> However, there are special return paths used by JVMTI pop frame, that checks if the caller frame is deoptimized, then calls a special path that removes the top activation, assuming that does not enter the deopt handler. The new JRT_ENTRY makes that reasoning invalid.
>> 
>> Therefore, we need this to be a JRT_LEAF, that sets a last Java frame, to make everyone happy. This patch performs that change.
>> 
>> I have run tier 1-5 testing, and manually tested:
>> 
>> while true; do make test JTREG="RETAIN=all" TEST=open/test/hotspot/jtreg/vmTestbase/nsk/jvmti/ForceEarlyReturn/ForceEarlyReturn002 TEST_OPTS_JAVA_OPTIONS="-XX:+UseZGC -Xmx2g -XX:ZCollectionInterval=0.0001 -XX:ZFragmentationLimit=0.01 -XX:+VerifyOops -XX:+ZVerifyViews" ; done
>> 
>> Before the fix it crashes ~1/15 runs with a bad oop. After the fix, it doesn't crash. I have run it more times than my tmux buffer fits (for a day), and it does not fail any more with this fix.
>> 
>> Unfortunately, my testing on AArch64 has been stalled for a day, so I have sent out this PR without the testing of those bits being finished. I won't push until I get the results back, of course. But I am expecting that to be fine, as there is nothing special going on there and it compiles. Will post a comment when the complete results have arrived.
>
> Erik Österlund has updated the pull request incrementally with one additional commit since the last revision:
> 
>   address cast

Seems okay.

One minor requested change below.

Thanks,
David

src/hotspot/share/interpreter/interpreterRuntime.cpp line 1177:

> 1175: JRT_LEAF(void, InterpreterRuntime::at_unwind(JavaThread* thread))
> 1176:   // JRT_END does an implicit safepoint check, hence we are guaranteed to block
> 1177:   // if this is called during a safepoint

The comments are no longer valid. The implicit safepoint check came from the use of ThreadInVMfromJava as part of the JRT_ENTRY.

Also it is far from obvious that  StackWatermarkSet::before_unwind meets all the requirements of a JRT_LEAF method. Please assure me it is. :)

-------------

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/828