RFR: 8292674: ReportJNIFatalError should print all java frames

[David Holmes]

On Wed, 7 Dec 2022 20:01:34 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> When JNI reports errors and warnings and produces a stack trace, we need to see the exact Java code that was being executed. The existing `JavaThread::print_stack` that is used is only for platform/carrier threads and doesn't show any mounted virtual thread. So we introduce a new `JavaThread::print_virtual_stack_on` method to print the mounted virtual thread's stack, and we add `JavaThread::print_jni_stack` as a utility for JNI to call that dispatches to the appropriate function based on the thread type.
>> 
>> The existing test `hotspot/jtreg/runtime/jni/checked/TestPrimitiveArrayCriticalWithBadParam.java` was modified to test regular and virtual threads, and to expect to see the specific stack frame of the Java native method.
>> 
>> Additional manual testing of the JNI `FatalError` method was done by direct fault injection in modified JDK code.
>> 
>> Sanity testing: Oracle tiers 1-4 (to include our -Xcheck:jni run in tier4)
>> 
>> Thanks,
>> David
>
> Looks good to me. Some comments below.
> 
> Thanks,
> Patricio

Thanks for looking at this @pchilano . I've made changes and will re-test before updating the PR. I'm retargetting this to JDK 21 now.

> src/hotspot/share/prims/jniCheck.cpp line 145:
> 
>> 143: static void ReportJNIWarning(JavaThread* thr, const char *msg) {
>> 144:   tty->print_cr("WARNING in native method: %s", msg);
>> 145:   thr->print_jni_stack();
> 
> There is another print_stack() call in check_pending_exception() that wasn't changed. Was that intended?

That was an oversight. I've changed it now too.

> src/hotspot/share/runtime/javaThread.cpp line 1758:
> 
>> 1756:       // Print out lock information
>> 1757:       if (JavaMonitorsInStackTrace) {
>> 1758:         jvf->print_lock_info_on(st, count);
> 
> I see this method looks for objects that are locked in this frame and prints their klass. At least for interpretedVFrame we get the oops from the BasicObjectLocks in the stack so that would already require to set _process_frames. Don't understand completely how the compiled case works but would certainly need that too (and should probably also set _update_map). I see that's actually how JavaThread::print_stack_on() defines them.

Okay so if I want to process monitors I need to change the register map to match the one in `print_stack_on` with regards to update and process frame. Re-testing now.

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

PR: https://git.openjdk.org/jdk/pull/11503