diff --git a/src/hotspot/cpu/aarch64/frame_aarch64.hpp b/src/hotspot/cpu/aarch64/frame_aarch64.hpp index 3d1f588359f..e58d66d5cf5 100644 --- a/src/hotspot/cpu/aarch64/frame_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/frame_aarch64.hpp @@ -165,7 +165,7 @@ frame(intptr_t* sp, intptr_t* unextended_sp, intptr_t* fp, address pc); - frame(intptr_t* sp, intptr_t* unextended_sp, intptr_t* fp, address pc, CodeBlob* cb); + frame(intptr_t* sp, intptr_t* unextended_sp, intptr_t* fp, address pc, CodeBlob* cb, bool allow_cb_null = false); // used for fast frame construction by continuations frame(intptr_t* sp, intptr_t* unextended_sp, intptr_t* fp, address pc, CodeBlob* cb, const ImmutableOopMap* oop_map, bool on_heap); diff --git a/src/hotspot/cpu/aarch64/frame_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/frame_aarch64.inline.hpp index 12a4b5b4e19..f5b46a53519 100644 --- a/src/hotspot/cpu/aarch64/frame_aarch64.inline.hpp +++ b/src/hotspot/cpu/aarch64/frame_aarch64.inline.hpp @@ -91,7 +91,7 @@ inline frame::frame(intptr_t* sp, intptr_t* fp, address pc) { init(sp, fp, pc); } -inline frame::frame(intptr_t* sp, intptr_t* unextended_sp, intptr_t* fp, address pc, CodeBlob* cb) { +inline frame::frame(intptr_t* sp, intptr_t* unextended_sp, intptr_t* fp, address pc, CodeBlob* cb, bool allow_cb_null) { assert(pauth_ptr_is_raw(pc), "cannot be signed"); intptr_t a = intptr_t(sp); intptr_t b = intptr_t(fp); @@ -102,7 +102,7 @@ inline frame::frame(intptr_t* sp, intptr_t* unextended_sp, intptr_t* fp, address assert(pc != nullptr, "no pc?"); _cb = cb; _oop_map = nullptr; - assert(_cb != nullptr, "pc: " INTPTR_FORMAT, p2i(pc)); + assert(_cb != nullptr || allow_cb_null, "pc: " INTPTR_FORMAT, p2i(pc)); _on_heap = false; DEBUG_ONLY(_frame_index = -1;) diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index ff84024088e..928833d2889 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -177,7 +177,7 @@ frame os::fetch_compiled_frame_from_context(const void* ucVoid) { // JVM compiled with -fno-omit-frame-pointer, so RFP is saved on the stack. frame os::get_sender_for_C_frame(frame* fr) { - return frame(fr->link(), fr->link(), fr->sender_pc()); + return frame(fr->sender_sp(), fr->link(), fr->sender_pc()); } NOINLINE frame os::current_frame() { diff --git a/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp b/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp index 70581166cf1..86721c94797 100644 --- a/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp +++ b/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp @@ -152,8 +152,23 @@ frame os::fetch_compiled_frame_from_context(const void* ucVoid) { // By default, gcc always saves frame pointer rfp on this stack. This // may get turned off by -fomit-frame-pointer. +// The "Procedure Call Standard for the Arm 64-bit Architecture" doesn't +// specify a location for the frame record within a stack frame (6.4.6). +// GCC currently chooses to save it at the top of the frame (lowest address). +// This means that using fr->sender_sp() to set the caller's frame _unextended_sp, +// as we do in x86, is wrong. Using fr->link() instead only makes sense for +// native frames. Setting a correct value for _unextended_sp is important +// if this value is later used to get that frame's caller. This will happen +// if we end up calling frame::sender_for_compiled_frame(), which will be the +// case if the _pc is associated with a CodeBlob that has a _frame_size > 0 +// (nmethod, runtime stub, safepoint stub, etc). frame os::get_sender_for_C_frame(frame* fr) { - return frame(fr->link(), fr->link(), fr->sender_pc()); + address pc = fr->sender_pc(); + CodeBlob* cb = CodeCache::find_blob(pc); + bool use_codeblob = cb != nullptr && cb->frame_size() > 0; + assert(!use_codeblob || !Interpreter::contains(pc), "should not be an interpreter frame"); + intptr_t* sender_sp = use_codeblob ? (fr->link() + frame::metadata_words - cb->frame_size()) : fr->link(); + return frame(sender_sp, sender_sp, fr->link(), pc, cb, true /* allow_cb_null */); } NOINLINE frame os::current_frame() { diff --git a/src/hotspot/share/utilities/vmError.cpp b/src/hotspot/share/utilities/vmError.cpp index 355483b4efe..df360335cc9 100644 --- a/src/hotspot/share/utilities/vmError.cpp +++ b/src/hotspot/share/utilities/vmError.cpp @@ -431,7 +431,7 @@ static frame next_frame(frame fr, Thread* t) { if (!t->is_in_full_stack((address)(fr.real_fp() + 1))) { return invalid; } - if (fr.is_java_frame() || fr.is_native_frame() || fr.is_runtime_frame()) { + if (fr.is_interpreted_frame() || (fr.cb() != nullptr && fr.cb()->frame_size() > 0)) { RegisterMap map(JavaThread::cast(t), RegisterMap::UpdateMap::skip, RegisterMap::ProcessFrames::include, diff --git a/test/hotspot/jtreg/runtime/ErrorHandling/StackWalkNativeToJava.java b/test/hotspot/jtreg/runtime/ErrorHandling/StackWalkNativeToJava.java new file mode 100644 index 00000000000..0c9c846c8b6 --- /dev/null +++ b/test/hotspot/jtreg/runtime/ErrorHandling/StackWalkNativeToJava.java @@ -0,0 +1,134 @@ +/* + * Copyright (c) 2023, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; +import jdk.test.lib.Utils; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/* + * @test StackWalkNativeToJava + * @bug 8316309 + * @summary Check that walking the stack works fine when going from C++ frame to Java frame. + * @requires os.arch=="amd64" | os.arch=="x86_64" | os.arch=="aarch64" + * @requires os.family != "windows" + * @requires vm.flagless + * @library /test/lib + * @run driver StackWalkNativeToJava + */ + +public class StackWalkNativeToJava { + + public static void main(String[] args) throws Exception { + // Check stack walking works fine when sender of C++ frame + // is a Java native method. + testStackWalkNativeToJavaNative("-Xint"); + testStackWalkNativeToJavaNative("-Xcomp", "-XX:CompileCommand=dontinline,StackWalkNativeToJava$TestNativeToJavaNative::*"); + + // Check stack walking works fine when sender of C++ frame + // is a runtime stub or interpreted Java method (VM call from Java). + testStackWalkNativeToJava("-Xint"); + testStackWalkNativeToJava("-Xcomp", "-XX:TieredStopAtLevel=3", + "-XX:CompileCommand=dontinline,StackWalkNativeToJava$TestNativeToJava::*"); + } + + public static void testStackWalkNativeToJavaNative(String... extraFlags) throws Exception { + List commands = new ArrayList<>(); + commands.add("-Xbootclasspath/a:."); + commands.add("-XX:-CreateCoredumpOnCrash"); + commands.add("-XX:+UnlockDiagnosticVMOptions"); + commands.add("-XX:AbortVMOnException=java.lang.IllegalMonitorStateException"); + commands.add("-XX:+ErrorFileToStdout"); + commands.addAll(Arrays.asList(extraFlags)); + commands.add("StackWalkNativeToJava$TestNativeToJavaNative"); + + ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(commands); + OutputAnalyzer output = new OutputAnalyzer(pb.start()); + output.shouldNotContain("java.lang.RuntimeException: Reached statement after obj.wait()"); + output.shouldNotContain("[error occurred during error reporting (printing native stack"); + String[] res = output.getOutput().split("StackWalkNativeToJava\\$TestNativeToJavaNative\\.callNativeMethod\\(\\)V"); + assertTrue(res.length - 1 == 2, res.length - 1); + output.shouldNotHaveExitValue(0); + } + + public static class TestNativeToJavaNative { + public static void main(String[] args) throws Exception { + TestNativeToJavaNative test = new TestNativeToJavaNative(); + test.callNativeMethod(); + } + + public void callNativeMethod() throws Exception { + Object obj = new Object(); + // Trigger a fatal exit due to IllegalMonitorStateException during + // a call to the VM from a Java native method. + obj.wait(); + throw new RuntimeException("Reached statement after obj.wait()"); + } + } + + public static void testStackWalkNativeToJava(String... extraFlags) throws Exception { + List commands = new ArrayList<>(); + commands.add("-Xbootclasspath/a:."); + commands.add("-XX:-CreateCoredumpOnCrash"); + commands.add("-XX:+UnlockDiagnosticVMOptions"); + commands.add("-XX:DiagnoseSyncOnValueBasedClasses=1"); + commands.add("-XX:+ErrorFileToStdout"); + commands.addAll(Arrays.asList(extraFlags)); + commands.add("StackWalkNativeToJava$TestNativeToJava"); + + ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(commands); + OutputAnalyzer output = new OutputAnalyzer(pb.start()); + output.shouldNotContain("java.lang.RuntimeException: Reached statement after synchronized"); + output.shouldNotContain("[error occurred during error reporting (printing native stack"); + String[] res = output.getOutput().split("StackWalkNativeToJava\\$TestNativeToJava\\.callVMMethod\\(\\)V"); + assertTrue(res.length - 1 == 2, res.length - 1); + output.shouldNotHaveExitValue(0); + } + + public static class TestNativeToJava { + static Integer counter = 0; + + public static void main(String[] args) throws Exception { + TestNativeToJava test = new TestNativeToJava(); + test.callVMMethod(); + } + + public void callVMMethod() throws Exception { + // Trigger a fatal exit for trying to synchronize on a value based class + // during a call to the VM from a Java method. + synchronized (counter) { + counter++; + } + throw new RuntimeException("Reached statement after synchronized"); + } + } + + private static void assertTrue(boolean condition, int count) { + if (!condition) { + throw new RuntimeException("Count error: count was " + count); + } + } +} \ No newline at end of file