Skip to content

Commit

Permalink
8316309: AArch64: VMError::print_native_stack() crashes on Java nativ…
Browse files Browse the repository at this point in the history
…e method frame

Backport-of: 2d38495b61ec4a8144fe187b5b11883add3dfd49
  • Loading branch information
GoeLin committed Dec 28, 2023
1 parent 98c94fd commit a607f49
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/cpu/aarch64/frame_aarch64.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/cpu/aarch64/frame_aarch64.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,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);
Expand All @@ -103,7 +103,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;)

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
17 changes: 16 additions & 1 deletion src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/utilities/vmError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,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,
Expand Down
134 changes: 134 additions & 0 deletions test/hotspot/jtreg/runtime/ErrorHandling/StackWalkNativeToJava.java
Original file line number Diff line number Diff line change
@@ -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<String> 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<String> 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);
}
}
}

1 comment on commit a607f49

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.