From 249dc2fbfccd9a61dd5943d270ef3fc9bc1406c0 Mon Sep 17 00:00:00 2001 From: David Holmes Date: Mon, 5 Dec 2022 01:04:44 -0500 Subject: [PATCH 1/2] 8292674: ReportJNIFatalError should print all java frames --- src/hotspot/share/prims/jni.cpp | 2 +- src/hotspot/share/prims/jniCheck.cpp | 2 +- src/hotspot/share/prims/jniCheck.hpp | 4 +- src/hotspot/share/runtime/javaThread.cpp | 52 +++++++++++++++++++ src/hotspot/share/runtime/javaThread.hpp | 6 +++ ...estPrimitiveArrayCriticalWithBadParam.java | 42 ++++++++++----- 6 files changed, 92 insertions(+), 16 deletions(-) diff --git a/src/hotspot/share/prims/jni.cpp b/src/hotspot/share/prims/jni.cpp index c17dde92c6d3e..814ecdfbf6fed 100644 --- a/src/hotspot/share/prims/jni.cpp +++ b/src/hotspot/share/prims/jni.cpp @@ -626,7 +626,7 @@ JNI_ENTRY(void, jni_FatalError(JNIEnv *env, const char *msg)) HOTSPOT_JNI_FATALERROR_ENTRY(env, (char *) msg); tty->print_cr("FATAL ERROR in native method: %s", msg); - thread->print_stack(); + thread->print_jni_stack(); os::abort(); // Dump core and abort JNI_END diff --git a/src/hotspot/share/prims/jniCheck.cpp b/src/hotspot/share/prims/jniCheck.cpp index 6fbd44b5bccd7..a9dc9bb6545a7 100644 --- a/src/hotspot/share/prims/jniCheck.cpp +++ b/src/hotspot/share/prims/jniCheck.cpp @@ -142,7 +142,7 @@ static const char * fatal_non_utf8_class_name2 = "\""; // When in VM state: static void ReportJNIWarning(JavaThread* thr, const char *msg) { tty->print_cr("WARNING in native method: %s", msg); - thr->print_stack(); + thr->print_jni_stack(); } // When in NATIVE state: diff --git a/src/hotspot/share/prims/jniCheck.hpp b/src/hotspot/share/prims/jniCheck.hpp index 6f3ee18d6b0d9..a41c79441d698 100644 --- a/src/hotspot/share/prims/jniCheck.hpp +++ b/src/hotspot/share/prims/jniCheck.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -35,7 +35,7 @@ extern "C" { // When in VM state: static inline void ReportJNIFatalError(JavaThread* thr, const char *msg) { tty->print_cr("FATAL ERROR in native method: %s", msg); - thr->print_stack(); + thr->print_jni_stack(); os::abort(true); } } diff --git a/src/hotspot/share/runtime/javaThread.cpp b/src/hotspot/share/runtime/javaThread.cpp index 772fd27b81dd5..7d74136f771e5 100644 --- a/src/hotspot/share/runtime/javaThread.cpp +++ b/src/hotspot/share/runtime/javaThread.cpp @@ -1682,6 +1682,16 @@ oop JavaThread::current_park_blocker() { return NULL; } +// Print stack trace for checked JNI warnings and JNI fatal errors. +// This is the external format from above, but selecting the platform +// or vthread as applicable. +void JavaThread::print_jni_stack() { + if (is_vthread_mounted()) { + print_vthread_stack_on(tty); + } else { + print_stack_on(tty); + } +} void JavaThread::print_stack_on(outputStream* st) { if (!has_last_Java_frame()) return; @@ -1715,6 +1725,48 @@ void JavaThread::print_stack_on(outputStream* st) { } } +void JavaThread::print_vthread_stack_on(outputStream* st) { + assert(is_vthread_mounted(), "Caller should have checked this"); + assert(has_last_Java_frame(), "must be"); + + Thread* current_thread = Thread::current(); + ResourceMark rm(current_thread); + HandleMark hm(current_thread); + + RegisterMap reg_map(this, + RegisterMap::UpdateMap::skip, + RegisterMap::ProcessFrames::skip, + RegisterMap::WalkContinuation::include); + ContinuationEntry* cont_entry = last_continuation(); + vframe* start_vf = last_java_vframe(®_map); + int count = 0; + for (vframe* f = start_vf; f != NULL; f = f->sender()) { + // Watch for end of vthread stack + if (Continuation::is_continuation_enterSpecial(f->fr())) { + assert(cont_entry == Continuation::get_continuation_entry_for_entry_frame(this, f->fr()), ""); + if (cont_entry->is_virtual_thread()) { + break; + } + cont_entry = cont_entry->parent(); + } + if (f->is_java_frame()) { + javaVFrame* jvf = javaVFrame::cast(f); + java_lang_Throwable::print_stack_element(st, jvf->method(), jvf->bci()); + + // Print out lock information + if (JavaMonitorsInStackTrace) { + jvf->print_lock_info_on(st, count); + } + } else { + // Ignore non-Java frames + } + + // Bail-out case for too deep stacks if MaxJavaStackTraceDepth > 0 + count++; + if (MaxJavaStackTraceDepth > 0 && MaxJavaStackTraceDepth == count) return; + } +} + #if INCLUDE_JVMTI // Rebind JVMTI thread state from carrier to virtual or from virtual to carrier. JvmtiThreadState* JavaThread::rebind_to_jvmti_thread_state_of(oop thread_oop) { diff --git a/src/hotspot/share/runtime/javaThread.hpp b/src/hotspot/share/runtime/javaThread.hpp index acd6b759cec66..479e32bc7bc64 100644 --- a/src/hotspot/share/runtime/javaThread.hpp +++ b/src/hotspot/share/runtime/javaThread.hpp @@ -955,6 +955,12 @@ class JavaThread: public Thread { // Print stack trace in external format void print_stack_on(outputStream* st); void print_stack() { print_stack_on(tty); } + void print_vthread_stack_on(outputStream* st); + + // Print stack trace for checked JNI warnings and JNI fatal errors. + // This is the external format from above, but selecting the platform + // or vthread as applicable. + void print_jni_stack(); // Print stack traces in various internal formats void trace_stack() PRODUCT_RETURN; diff --git a/test/hotspot/jtreg/runtime/jni/checked/TestPrimitiveArrayCriticalWithBadParam.java b/test/hotspot/jtreg/runtime/jni/checked/TestPrimitiveArrayCriticalWithBadParam.java index c64b21c7d1ba9..36c48a7fd1984 100644 --- a/test/hotspot/jtreg/runtime/jni/checked/TestPrimitiveArrayCriticalWithBadParam.java +++ b/test/hotspot/jtreg/runtime/jni/checked/TestPrimitiveArrayCriticalWithBadParam.java @@ -1,5 +1,6 @@ /* * Copyright (c) 2021, Red Hat, Inc. All rights reserved. + * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -24,9 +25,11 @@ /** * @test TestPrimitiveArrayCriticalWithBadParam - * @bug 8269697 + * @bug 8269697 8292674 * @summary -Xcheck:jni should catch wrong parameter passed to GetPrimitiveArrayCritical + * @comment Tests reporting with regular thread and virtual thread. * @library /test/lib + * @enablePreview * @run main/native TestPrimitiveArrayCriticalWithBadParam */ import java.util.List; @@ -47,39 +50,54 @@ public class TestPrimitiveArrayCriticalWithBadParam { public static void main(String[] args) { if (args.length > 0) { - test(); + test(args[0]); } else { - runTest(); + runTest(false); + runTest(true); } } - private static void runTest() { + private static void runTest(boolean useVThread) { List pbArgs = new ArrayList<>(); pbArgs.add("-XX:-CreateCoredumpOnCrash"); pbArgs.add("-Xcheck:jni"); + pbArgs.add("--enable-preview"); pbArgs.add("-Djava.library.path=" + Utils.TEST_NATIVE_PATH); pbArgs.add(TestPrimitiveArrayCriticalWithBadParam.class.getName()); - pbArgs.add("test"); + pbArgs.add(useVThread ? "vtest" : "test"); try { ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(pbArgs.toArray(new String[0])); OutputAnalyzer analyzer = new OutputAnalyzer(pb.start()); // -Xcheck:jni should warn the bad parameter analyzer.shouldContain("FATAL ERROR in native method: Primitive type array expected but not received for JNI array operation"); + analyzer.shouldContain("TestPrimitiveArrayCriticalWithBadParam.pin"); analyzer.shouldNotHaveExitValue(0); } catch (IOException e) { throw new RuntimeException(e); } } - private static void test() { - Object[] objs = new Object[10]; - for (int i = 0; i < objs.length; i++) { - objs[i] = new MyClass(); + private static void test(String mode) { + Runnable r = () -> { + Object[] objs = new Object[10]; + for (int i = 0; i < objs.length; i++) { + objs[i] = new MyClass(); + } + pin(objs); + System.out.println("Object array pinned"); + unpin(objs); + }; + if (mode.equals("vtest")) { + Thread t = Thread.ofVirtual().start(r); + try { + t.join(); + } catch (InterruptedException ex) { + throw new Error("unexpected", ex); + } + } else { + r.run(); } - pin(objs); - System.out.println("Object array pinned"); - unpin(objs); } public static class MyClass { public Object ref = new Object(); From fa1d7b0c77212d893a8d1a79a4a90d7cbdabd7cb Mon Sep 17 00:00:00 2001 From: David Holmes Date: Wed, 7 Dec 2022 17:46:01 -0500 Subject: [PATCH 2/2] Changed RegisterMap to include process_frame and update, for object monitor examination. Updated stack printing for exception checks. --- src/hotspot/share/prims/jniCheck.cpp | 2 +- src/hotspot/share/runtime/javaThread.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/prims/jniCheck.cpp b/src/hotspot/share/prims/jniCheck.cpp index a9dc9bb6545a7..70187f7818511 100644 --- a/src/hotspot/share/prims/jniCheck.cpp +++ b/src/hotspot/share/prims/jniCheck.cpp @@ -193,7 +193,7 @@ check_pending_exception(JavaThread* thr) { IN_VM( tty->print_cr("WARNING in native method: JNI call made without checking exceptions when required to from %s", thr->get_pending_jni_exception_check()); - thr->print_stack(); + thr->print_jni_stack(); ) thr->clear_pending_jni_exception_check(); // Just complain once } diff --git a/src/hotspot/share/runtime/javaThread.cpp b/src/hotspot/share/runtime/javaThread.cpp index 7d74136f771e5..e4787a35d4e9d 100644 --- a/src/hotspot/share/runtime/javaThread.cpp +++ b/src/hotspot/share/runtime/javaThread.cpp @@ -1734,8 +1734,8 @@ void JavaThread::print_vthread_stack_on(outputStream* st) { HandleMark hm(current_thread); RegisterMap reg_map(this, - RegisterMap::UpdateMap::skip, - RegisterMap::ProcessFrames::skip, + RegisterMap::UpdateMap::include, + RegisterMap::ProcessFrames::include, RegisterMap::WalkContinuation::include); ContinuationEntry* cont_entry = last_continuation(); vframe* start_vf = last_java_vframe(®_map);