From 9f753bfb82e5a1045bc8674659d04c7f15cc4d33 Mon Sep 17 00:00:00 2001 From: Doug Simon Date: Fri, 8 Oct 2021 08:06:53 +0000 Subject: [PATCH 1/3] 8272586: emit abstract machine code in hs-err logs Reviewed-by: kvn, dholmes --- src/hotspot/share/code/nmethod.cpp | 8 +- src/hotspot/share/compiler/compileTask.cpp | 8 +- src/hotspot/share/compiler/compileTask.hpp | 6 +- src/hotspot/share/compiler/disassembler.cpp | 2 +- src/hotspot/share/utilities/vmError.cpp | 166 ++++++++++++----- src/hotspot/share/utilities/vmError.hpp | 4 + .../MachCodeFramesInErrorFile.java | 171 ++++++++++++++++++ 7 files changed, 315 insertions(+), 50 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp index e2b27e5f4f0..5b3125e8789 100644 --- a/src/hotspot/share/code/nmethod.cpp +++ b/src/hotspot/share/code/nmethod.cpp @@ -67,6 +67,7 @@ #include "runtime/sharedRuntime.hpp" #include "runtime/signature.hpp" #include "runtime/sweeper.hpp" +#include "runtime/threadWXSetters.inline.hpp" #include "runtime/vmThread.hpp" #include "utilities/align.hpp" #include "utilities/copy.hpp" @@ -923,7 +924,7 @@ void nmethod::print_on(outputStream* st, const char* msg) const { CompileTask::print(st, this, msg, /*short_form:*/ true); st->print_cr(" (" INTPTR_FORMAT ")", p2i(this)); } else { - CompileTask::print(st, this, msg, /*short_form:*/ false); + CompileTask::print(st, this, msg, /*short_form:*/ false, /* cr */ true, /* timestamp */ false); } } } @@ -2518,7 +2519,7 @@ void nmethod::print(outputStream* st) const { st->print("(n/a) "); } - print_on(tty, NULL); + print_on(st, NULL); if (WizardMode) { st->print("((nmethod*) " INTPTR_FORMAT ") ", p2i(this)); @@ -2869,6 +2870,9 @@ void nmethod::decode2(outputStream* ost) const { AbstractDisassembler::show_block_comment()); #endif + // Decoding an nmethod can write to a PcDescCache (see PcDescCache::add_pc_desc) + MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXWrite, Thread::current());) + st->cr(); this->print(st); st->cr(); diff --git a/src/hotspot/share/compiler/compileTask.cpp b/src/hotspot/share/compiler/compileTask.cpp index d610d8bdcf8..b78806346c5 100644 --- a/src/hotspot/share/compiler/compileTask.cpp +++ b/src/hotspot/share/compiler/compileTask.cpp @@ -236,11 +236,13 @@ void CompileTask::print_tty() { // CompileTask::print_impl void CompileTask::print_impl(outputStream* st, Method* method, int compile_id, int comp_level, bool is_osr_method, int osr_bci, bool is_blocking, - const char* msg, bool short_form, bool cr, + const char* msg, bool short_form, bool cr, bool timestamp, jlong time_queued, jlong time_started) { if (!short_form) { - // Print current time - st->print("%7d ", (int)tty->time_stamp().milliseconds()); + if (timestamp) { + // Print current time + st->print("%7d ", (int)tty->time_stamp().milliseconds()); + } if (Verbose && time_queued != 0) { // Print time in queue and time being processed by compiler thread jlong now = os::elapsed_counter(); diff --git a/src/hotspot/share/compiler/compileTask.hpp b/src/hotspot/share/compiler/compileTask.hpp index 03beb350b1b..90638ae9f0e 100644 --- a/src/hotspot/share/compiler/compileTask.hpp +++ b/src/hotspot/share/compiler/compileTask.hpp @@ -187,16 +187,16 @@ class CompileTask : public CHeapObj { private: static void print_impl(outputStream* st, Method* method, int compile_id, int comp_level, bool is_osr_method = false, int osr_bci = -1, bool is_blocking = false, - const char* msg = NULL, bool short_form = false, bool cr = true, + const char* msg = NULL, bool short_form = false, bool cr = true, bool timestamp = true, jlong time_queued = 0, jlong time_started = 0); public: void print(outputStream* st = tty, const char* msg = NULL, bool short_form = false, bool cr = true); void print_ul(const char* msg = NULL); - static void print(outputStream* st, const nmethod* nm, const char* msg = NULL, bool short_form = false, bool cr = true) { + static void print(outputStream* st, const nmethod* nm, const char* msg = NULL, bool short_form = false, bool cr = true, bool timestamp = true) { print_impl(st, nm->method(), nm->compile_id(), nm->comp_level(), nm->is_osr_method(), nm->is_osr_method() ? nm->osr_entry_bci() : -1, /*is_blocking*/ false, - msg, short_form, cr); + msg, short_form, cr, timestamp); } static void print_ul(const nmethod* nm, const char* msg = NULL); diff --git a/src/hotspot/share/compiler/disassembler.cpp b/src/hotspot/share/compiler/disassembler.cpp index bd9272a8e6c..4cc2f8781c3 100644 --- a/src/hotspot/share/compiler/disassembler.cpp +++ b/src/hotspot/share/compiler/disassembler.cpp @@ -887,7 +887,7 @@ void Disassembler::decode(CodeBlob* cb, outputStream* st) { if (cb->is_nmethod()) { // If we have an nmethod at hand, // call the specialized decoder directly. - decode((nmethod*)cb, st); + ((nmethod*)cb)->decode2(st); return; } diff --git a/src/hotspot/share/utilities/vmError.cpp b/src/hotspot/share/utilities/vmError.cpp index a63dfce5c81..1ad3897b302 100644 --- a/src/hotspot/share/utilities/vmError.cpp +++ b/src/hotspot/share/utilities/vmError.cpp @@ -87,6 +87,7 @@ Thread* VMError::_thread; address VMError::_pc; void* VMError::_siginfo; void* VMError::_context; +bool VMError::_print_native_stack_used = false; const char* VMError::_filename; int VMError::_lineno; size_t VMError::_size; @@ -241,6 +242,107 @@ void VMError::print_stack_trace(outputStream* st, JavaThread* jt, #endif // ZERO } +/** + * Adds `value` to `list` iff it's not already present and there is sufficient + * capacity (i.e. length(list) < `list_capacity`). The length of the list + * is the index of the first nullptr entry. + * + * @ return true if the value was added, false otherwise + */ +static bool add_if_absent(address value, address* list, int list_capacity) { + for (int i = 0; i < list_capacity; i++) { + if (list[i] == value) { + return false; + } + if (list[i] == nullptr) { + list[i] = value; + if (i + 1 < list_capacity) { + list[i + 1] = nullptr; + } + return true; + } + } + return false; +} + +/** + * Prints the VM generated code unit, if any, containing `pc` if it has not already + * been printed. If the code unit is an InterpreterCodelet or StubCodeDesc, it is + * only printed if `is_crash_pc` is true. + * + * @param printed array of code units that have already been printed (delimited by NULL entry) + * @param printed_capacity the capacity of `printed` + * @return true if the code unit was printed, false otherwise + */ +static bool print_code(outputStream* st, Thread* thread, address pc, bool is_crash_pc, + address* printed, int printed_capacity) { + if (Interpreter::contains(pc)) { + if (is_crash_pc) { + // The interpreter CodeBlob is very large so try to print the codelet instead. + InterpreterCodelet* codelet = Interpreter::codelet_containing(pc); + if (codelet != nullptr) { + if (add_if_absent((address) codelet, printed, printed_capacity)) { + codelet->print_on(st); + Disassembler::decode(codelet->code_begin(), codelet->code_end(), st); + return true; + } + } + } + } else { + StubCodeDesc* desc = StubCodeDesc::desc_for(pc); + if (desc != nullptr) { + if (is_crash_pc) { + if (add_if_absent((address) desc, printed, printed_capacity)) { + desc->print_on(st); + Disassembler::decode(desc->begin(), desc->end(), st); + return true; + } + } + } else if (thread != nullptr) { + CodeBlob* cb = CodeCache::find_blob(pc); + if (cb != nullptr && add_if_absent((address) cb, printed, printed_capacity)) { + // Disassembling nmethod will incur resource memory allocation, + // only do so when thread is valid. + ResourceMark rm(thread); + Disassembler::decode(cb, st); + st->cr(); + return true; + } + } + } + return false; +} + +/** + * Gets the caller frame of `fr`. + * + * @returns an invalid frame (i.e. fr.pc() === 0) if the caller cannot be obtained + */ +static frame next_frame(frame fr, Thread* t) { + // Compiled code may use EBP register on x86 so it looks like + // non-walkable C frame. Use frame.sender() for java frames. + frame invalid; + if (t != nullptr && t->is_Java_thread()) { + // Catch very first native frame by using stack address. + // For JavaThread stack_base and stack_size should be set. + 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()) { + RegisterMap map(t->as_Java_thread(), false); // No update + return fr.sender(&map); + } else { + // is_first_C_frame() does only simple checks for frame pointer, + // it will pass if java compiled code has a pointer in EBP. + if (os::is_first_C_frame(&fr)) return invalid; + return os::get_sender_for_C_frame(&fr); + } + } else { + if (os::is_first_C_frame(&fr)) return invalid; + return os::get_sender_for_C_frame(&fr); + } +} + void VMError::print_native_stack(outputStream* st, frame fr, Thread* t, char* buf, int buf_size) { // see if it's a valid frame @@ -258,26 +360,9 @@ void VMError::print_native_stack(outputStream* st, frame fr, Thread* t, char* bu } } st->cr(); - // Compiled code may use EBP register on x86 so it looks like - // non-walkable C frame. Use frame.sender() for java frames. - if (t && t->is_Java_thread()) { - // Catch very first native frame by using stack address. - // For JavaThread stack_base and stack_size should be set. - if (!t->is_in_full_stack((address)(fr.real_fp() + 1))) { - break; - } - if (fr.is_java_frame() || fr.is_native_frame() || fr.is_runtime_frame()) { - RegisterMap map(t->as_Java_thread(), false); // No update - fr = fr.sender(&map); - } else { - // is_first_C_frame() does only simple checks for frame pointer, - // it will pass if java compiled code has a pointer in EBP. - if (os::is_first_C_frame(&fr)) break; - fr = os::get_sender_for_C_frame(&fr); - } - } else { - if (os::is_first_C_frame(&fr)) break; - fr = os::get_sender_for_C_frame(&fr); + fr = next_frame(fr, t); + if (fr.pc() == nullptr) { + break; } } @@ -747,6 +832,7 @@ void VMError::report(outputStream* st, bool _verbose) { : os::current_frame(); print_native_stack(st, fr, _thread, buf, sizeof(buf)); + _print_native_stack_used = true; } } @@ -821,30 +907,28 @@ void VMError::report(outputStream* st, bool _verbose) { st->cr(); } - STEP("printing code blob if possible") + STEP("printing code blobs if possible") if (_verbose && _context) { - CodeBlob* cb = CodeCache::find_blob(_pc); - if (cb != NULL) { - if (Interpreter::contains(_pc)) { - // The interpreter CodeBlob is very large so try to print the codelet instead. - InterpreterCodelet* codelet = Interpreter::codelet_containing(_pc); - if (codelet != NULL) { - codelet->print_on(st); - Disassembler::decode(codelet->code_begin(), codelet->code_end(), st); - } - } else { - StubCodeDesc* desc = StubCodeDesc::desc_for(_pc); - if (desc != NULL) { - desc->print_on(st); - Disassembler::decode(desc->begin(), desc->end(), st); - } else if (_thread != NULL) { - // Disassembling nmethod will incur resource memory allocation, - // only do so when thread is valid. - ResourceMark rm(_thread); - Disassembler::decode(cb, st); - st->cr(); + if (!_print_native_stack_used) { + // Only try print code of the crashing frame since + // we cannot walk the native stack using next_frame. + const int printed_capacity = 1; + address printed_singleton = nullptr; + address* printed = &printed_singleton; + print_code(st, _thread, _pc, true, printed, 1); + } else { + // Print up to the first 5 unique code units on the stack + const int printed_capacity = 5; + address printed[printed_capacity]; + printed[0] = nullptr; // length(list) == index of first nullptr + + frame fr = os::fetch_frame_from_context(_context); + for (int count = 0; count < printed_capacity && fr.pc() != nullptr; ) { + if (print_code(st, _thread, fr.pc(), fr.pc() == _pc, printed, printed_capacity)) { + count++; } + fr = next_frame(fr, _thread); } } } diff --git a/src/hotspot/share/utilities/vmError.hpp b/src/hotspot/share/utilities/vmError.hpp index 4311830d552..6a18ff9375d 100644 --- a/src/hotspot/share/utilities/vmError.hpp +++ b/src/hotspot/share/utilities/vmError.hpp @@ -51,6 +51,10 @@ class VMError : public AllStatic { static void* _context; // ContextRecord on Windows, // ucontext_t on Solaris/Linux + // records if VMError::print_native_stack was used to + // print the native stack instead of os::platform_print_native_stack + static bool _print_native_stack_used; + // additional info for VM internal errors static const char* _filename; static int _lineno; diff --git a/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java b/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java new file mode 100644 index 00000000000..e187deb1a5c --- /dev/null +++ b/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java @@ -0,0 +1,171 @@ +/* + * Copyright (c) 2021, 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. + */ + +/* + * @test + * @bug 8272586 + * @requires vm.compiler2.enabled + * @summary Test that abstract machine code is dumped for the top frames in a hs-err log + * @library /test/lib + * @modules java.base/jdk.internal.misc + * java.compiler + * java.management + * jdk.internal.jvmstat/sun.jvmstat.monitor + * @run driver MachCodeFramesInErrorFile + */ + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.ArrayList; +import java.util.stream.Stream; +import java.util.stream.Collectors; +import java.util.regex.Pattern; +import java.util.regex.Matcher; + +import jdk.test.lib.process.ProcessTools; +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.Asserts; + +import jdk.internal.misc.Unsafe; + +public class MachCodeFramesInErrorFile { + private static class Crasher { + // Need to make unsafe a compile-time constant so that + // C2 intrinsifies the call to Unsafe.getLong in method3. + private static final Unsafe unsafe = Unsafe.getUnsafe(); + public static void main(String[] args) { + method1(10); + } + + static void method1(long address) { + System.out.println("in method1"); + method2(address); + } + static void method2(long address) { + System.out.println("in method2"); + method3(address); + } + static void method3(long address) { + System.out.println("in method3 and writing 42 to 0x" + Long.toHexString(address)); + unsafe.putLong(address, 42); + } + } + + /** + * Runs Crasher and tries to force compile the methods in Crasher. The inner + * most method crashes the VM with Unsafe. The resulting hs-err log is + * expected to have a min number of MachCode sections. + */ + public static void main(String[] args) throws Exception { + ProcessBuilder pb = ProcessTools.createJavaProcessBuilder( + "-Xmx64m", "--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED", + "-XX:-CreateCoredumpOnCrash", + "-Xcomp", + "-XX:-TieredCompilation", // ensure C2 compiles Crasher.method3 + "-XX:CompileCommand=compileonly,MachCodeFramesInErrorFile$Crasher.m*", + "-XX:CompileCommand=dontinline,MachCodeFramesInErrorFile$Crasher.m*", + Crasher.class.getName()); + OutputAnalyzer output = new OutputAnalyzer(pb.start()); + + // Extract hs_err_pid file. + String hs_err_file = output.firstMatch("# *(\\S*hs_err_pid\\d+\\.log)", 1); + if (hs_err_file == null) { + throw new RuntimeException("Did not find hs_err_pid file in output.\n" + + "stderr:\n" + output.getStderr() + "\n" + + "stdout:\n" + output.getStdout()); + } + Path hsErrPath = Paths.get(hs_err_file); + if (!Files.exists(hsErrPath)) { + throw new RuntimeException("hs_err_pid file missing at " + hsErrPath + ".\n"); + } + String hsErr = Files.readString(hsErrPath); + if (!crashedInCrasherMethod(hsErr)) { + return; + } + List nativeFrames = extractNativeFrames(hsErr); + int compiledJavaFrames = (int) nativeFrames.stream().filter(f -> f.startsWith("J ")).count(); + + Matcher matcher = Pattern.compile("\\[MachCode\\]\\s*\\[Verified Entry Point\\]\\s* # \\{method\\} \\{[^}]*\\} '([^']+)' '([^']+)' in '([^']+)'", Pattern.DOTALL).matcher(hsErr); + List machCodeHeaders = matcher.results().map(mr -> String.format("'%s' '%s' in '%s'", mr.group(1), mr.group(2), mr.group(3))).collect(Collectors.toList()); + String message = "Mach code headers: " + machCodeHeaders + + "\n\nExtracted MachCode:\n" + extractMachCode(hsErr) + + "\n\nExtracted native frames:\n" + String.join("\n", nativeFrames); + int minExpectedMachCodeSections = Math.max(1, compiledJavaFrames); + Asserts.assertTrue(machCodeHeaders.size() >= minExpectedMachCodeSections, message); + } + + /** + * Checks whether the crashing frame is in {@code Crasher.method3}. + */ + private static boolean crashedInCrasherMethod(String hsErr) { + boolean checkProblematicFrame = false; + for (String line : hsErr.split(System.lineSeparator())) { + if (line.startsWith("# Problematic frame:")) { + checkProblematicFrame = true; + } else if (checkProblematicFrame) { + String crasherMethod = Crasher.class.getSimpleName() + ".method3"; + if (!line.contains(crasherMethod)) { + // There's any number of things that can subvert the attempt + // to crash in the expected method. + System.out.println("Crashed in method other than " + crasherMethod + "\n\n" + line + "\n\nSkipping rest of test."); + return false; + } + return true; + } + } + throw new RuntimeException("\"# Problematic frame:\" line missing in hs_err_pid file:\n" + hsErr); + } + + /** + * Gets the lines in {@code hsErr} below the line starting with "Native frames:" up to the next blank line. + */ + private static List extractNativeFrames(String hsErr) { + List res = new ArrayList<>(); + boolean inNativeFrames = false; + for (String line : hsErr.split(System.lineSeparator())) { + if (line.startsWith("Native frames: ")) { + inNativeFrames = true; + } else if (inNativeFrames) { + if (line.trim().isEmpty()) { + return res; + } + res.add(line); + } + } + throw new RuntimeException("\"Native frames:\" line missing in hs_err_pid file:\n" + hsErr); + } + + private static String extractMachCode(String hsErr) { + int start = hsErr.indexOf("[MachCode]"); + if (start != -1) { + int end = hsErr.lastIndexOf("[/MachCode]"); + if (end != -1) { + return hsErr.substring(start, end + "[/MachCode]".length()); + } + return hsErr.substring(start); + } + return null; + } +} From 8c460432c2f4fdf8b0cf539d05ffbe5ecff8625a Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Tue, 12 Oct 2021 06:22:13 +0000 Subject: [PATCH 2/3] 8275031: runtime/ErrorHandling/MachCodeFramesInErrorFile.java fails when hsdis is present Reviewed-by: dholmes, dnsimon --- src/hotspot/share/code/nmethod.cpp | 3 +++ .../runtime/ErrorHandling/MachCodeFramesInErrorFile.java | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp index 5b3125e8789..ce98d9ad72d 100644 --- a/src/hotspot/share/code/nmethod.cpp +++ b/src/hotspot/share/code/nmethod.cpp @@ -2882,7 +2882,10 @@ void nmethod::decode2(outputStream* ost) const { //---< Print real disassembly >--- //---------------------------------- if (! use_compressed_format) { + st->print_cr("[Disassembly]"); Disassembler::decode(const_cast(this), st); + st->bol(); + st->print_cr("[/Disassembly]"); return; } #endif diff --git a/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java b/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java index e187deb1a5c..99a1e5918d2 100644 --- a/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java +++ b/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java @@ -107,6 +107,12 @@ public static void main(String[] args) throws Exception { List nativeFrames = extractNativeFrames(hsErr); int compiledJavaFrames = (int) nativeFrames.stream().filter(f -> f.startsWith("J ")).count(); + Matcher matcherDisasm = Pattern.compile("\\[Disassembly\\].*\\[/Disassembly\\]", Pattern.DOTALL).matcher(hsErr); + if (matcherDisasm.find()) { + // Real disassembly is present, no MachCode is expected. + return; + } + Matcher matcher = Pattern.compile("\\[MachCode\\]\\s*\\[Verified Entry Point\\]\\s* # \\{method\\} \\{[^}]*\\} '([^']+)' '([^']+)' in '([^']+)'", Pattern.DOTALL).matcher(hsErr); List machCodeHeaders = matcher.results().map(mr -> String.format("'%s' '%s' in '%s'", mr.group(1), mr.group(2), mr.group(3))).collect(Collectors.toList()); String message = "Mach code headers: " + machCodeHeaders + From c89bb0c74c6a3e5456e7f4601f96ba98ebf86fb1 Mon Sep 17 00:00:00 2001 From: Doug Simon Date: Tue, 12 Oct 2021 09:49:50 +0000 Subject: [PATCH 3/3] 8274986: max code printed in hs-err logs should be configurable Reviewed-by: never, dholmes --- src/hotspot/share/code/codeBlob.cpp | 7 +- .../share/runtime/flags/jvmFlagLimit.cpp | 1 + src/hotspot/share/runtime/globals.hpp | 4 + src/hotspot/share/utilities/vmError.cpp | 58 ++++--- src/hotspot/share/utilities/vmError.hpp | 3 + .../MachCodeFramesInErrorFile.java | 152 +++++++++--------- 6 files changed, 133 insertions(+), 92 deletions(-) diff --git a/src/hotspot/share/code/codeBlob.cpp b/src/hotspot/share/code/codeBlob.cpp index 94e26584952..ef0ec837a73 100644 --- a/src/hotspot/share/code/codeBlob.cpp +++ b/src/hotspot/share/code/codeBlob.cpp @@ -653,7 +653,12 @@ void CodeBlob::dump_for_addr(address addr, outputStream* st, bool verbose) const nm->method()->print_value_on(st); } st->cr(); - nm->print_nmethod(verbose); + if (verbose && st == tty) { + // verbose is only ever true when called from findpc in debug.cpp + nm->print_nmethod(true); + } else { + nm->print(st); + } return; } st->print_cr(INTPTR_FORMAT " is at code_begin+%d in ", p2i(addr), (int)(addr - code_begin())); diff --git a/src/hotspot/share/runtime/flags/jvmFlagLimit.cpp b/src/hotspot/share/runtime/flags/jvmFlagLimit.cpp index 5b81444eb84..bcc1565e165 100644 --- a/src/hotspot/share/runtime/flags/jvmFlagLimit.cpp +++ b/src/hotspot/share/runtime/flags/jvmFlagLimit.cpp @@ -34,6 +34,7 @@ #include "gc/shared/referenceProcessor.hpp" #include "oops/markWord.hpp" #include "runtime/task.hpp" +#include "utilities/vmError.hpp" //---------------------------------------------------------------------- // Build flagLimitTable[] diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index 8779036b440..191ec81bd0a 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -1371,6 +1371,10 @@ const intx ObjectAlignmentInBytes = 8; develop(intx, StackPrintLimit, 100, \ "number of stack frames to print in VM-level stack dump") \ \ + product(int, ErrorLogPrintCodeLimit, 3, DIAGNOSTIC, \ + "max number of compiled code units to print in error log") \ + range(0, VMError::max_error_log_print_code) \ + \ notproduct(intx, MaxElementPrintSize, 256, \ "maximum number of elements to print") \ \ diff --git a/src/hotspot/share/utilities/vmError.cpp b/src/hotspot/share/utilities/vmError.cpp index 1ad3897b302..7b72f57f60e 100644 --- a/src/hotspot/share/utilities/vmError.cpp +++ b/src/hotspot/share/utilities/vmError.cpp @@ -245,7 +245,8 @@ void VMError::print_stack_trace(outputStream* st, JavaThread* jt, /** * Adds `value` to `list` iff it's not already present and there is sufficient * capacity (i.e. length(list) < `list_capacity`). The length of the list - * is the index of the first nullptr entry. + * is the index of the first nullptr entry or `list_capacity` if there are + * no nullptr entries. * * @ return true if the value was added, false otherwise */ @@ -910,25 +911,44 @@ void VMError::report(outputStream* st, bool _verbose) { STEP("printing code blobs if possible") if (_verbose && _context) { - if (!_print_native_stack_used) { - // Only try print code of the crashing frame since - // we cannot walk the native stack using next_frame. - const int printed_capacity = 1; - address printed_singleton = nullptr; - address* printed = &printed_singleton; - print_code(st, _thread, _pc, true, printed, 1); - } else { - // Print up to the first 5 unique code units on the stack - const int printed_capacity = 5; - address printed[printed_capacity]; - printed[0] = nullptr; // length(list) == index of first nullptr - - frame fr = os::fetch_frame_from_context(_context); - for (int count = 0; count < printed_capacity && fr.pc() != nullptr; ) { - if (print_code(st, _thread, fr.pc(), fr.pc() == _pc, printed, printed_capacity)) { - count++; + const int printed_capacity = max_error_log_print_code; + address printed[printed_capacity]; + printed[0] = nullptr; + int printed_len = 0; + // Even though ErrorLogPrintCodeLimit is ranged checked + // during argument parsing, there's no way to prevent it + // subsequently (i.e., after parsing) being set to a + // value outside the range. + int limit = MIN2(ErrorLogPrintCodeLimit, printed_capacity); + if (limit > 0) { + // Scan the native stack + if (!_print_native_stack_used) { + // Only try to print code of the crashing frame since + // the native stack cannot be walked with next_frame. + if (print_code(st, _thread, _pc, true, printed, printed_capacity)) { + printed_len++; + } + } else { + frame fr = os::fetch_frame_from_context(_context); + while (printed_len < limit && fr.pc() != nullptr) { + if (print_code(st, _thread, fr.pc(), fr.pc() == _pc, printed, printed_capacity)) { + printed_len++; + } + fr = next_frame(fr, _thread); + } + } + + // Scan the Java stack + if (_thread != nullptr && _thread->is_Java_thread()) { + JavaThread* jt = _thread->as_Java_thread(); + if (jt->has_last_Java_frame()) { + for (StackFrameStream sfs(jt, true /* update */, true /* process_frames */); printed_len < limit && !sfs.is_done(); sfs.next()) { + address pc = sfs.current()->pc(); + if (print_code(st, _thread, pc, pc == _pc, printed, printed_capacity)) { + printed_len++; + } + } } - fr = next_frame(fr, _thread); } } } diff --git a/src/hotspot/share/utilities/vmError.hpp b/src/hotspot/share/utilities/vmError.hpp index 6a18ff9375d..736e354677c 100644 --- a/src/hotspot/share/utilities/vmError.hpp +++ b/src/hotspot/share/utilities/vmError.hpp @@ -185,6 +185,9 @@ class VMError : public AllStatic { // which is not NULL and contains bits in every word. static const intptr_t segfault_address = LP64_ONLY(0xABC0000000000ABCULL) NOT_LP64(0x00000ABC); + // Max value for the ErrorLogPrintCodeLimit flag. + static const int max_error_log_print_code = 10; + // Needed when printing signal handlers. NOT_WINDOWS(static const void* crash_handler_address;) diff --git a/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java b/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java index 99a1e5918d2..0095c835d2d 100644 --- a/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java +++ b/test/hotspot/jtreg/runtime/ErrorHandling/MachCodeFramesInErrorFile.java @@ -34,13 +34,17 @@ * @run driver MachCodeFramesInErrorFile */ +import java.lang.annotation.Annotation; +import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.List; import java.util.ArrayList; -import java.util.stream.Stream; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; import java.util.regex.Pattern; import java.util.regex.Matcher; @@ -51,42 +55,73 @@ import jdk.internal.misc.Unsafe; public class MachCodeFramesInErrorFile { + private static class Crasher { - // Need to make unsafe a compile-time constant so that - // C2 intrinsifies the call to Unsafe.getLong in method3. + // Make Crasher.unsafe a compile-time constant so that + // C2 intrinsifies calls to Unsafe intrinsics. private static final Unsafe unsafe = Unsafe.getUnsafe(); - public static void main(String[] args) { - method1(10); + + public static void main(String[] args) throws Exception { + if (args[0].equals("crashInJava")) { + // This test relies on Unsafe.putLong(Object, long, long) being intrinsified + if (!Stream.of(Unsafe.class.getDeclaredMethod("putLong", Object.class, long.class, long.class).getAnnotations()). + anyMatch(a -> a.annotationType().getName().equals("jdk.internal.vm.annotation.IntrinsicCandidate"))) { + throw new RuntimeException("Unsafe.putLong(Object, long, long) is not an intrinsic"); + } + crashInJava1(10); + } else { + assert args[0].equals("crashInVM"); + crashInNative1(10); + } } - static void method1(long address) { - System.out.println("in method1"); - method2(address); + static void crashInJava1(long address) { + System.out.println("in crashInJava1"); + crashInJava2(address); } - static void method2(long address) { - System.out.println("in method2"); - method3(address); + static void crashInJava2(long address) { + System.out.println("in crashInJava2"); + crashInJava3(address); } - static void method3(long address) { - System.out.println("in method3 and writing 42 to 0x" + Long.toHexString(address)); - unsafe.putLong(address, 42); + static void crashInJava3(long address) { + unsafe.putLong(null, address, 42); + System.out.println("wrote value to 0x" + Long.toHexString(address)); } + + static void crashInNative1(long address) { + System.out.println("in crashInNative1"); + crashInNative2(address); + } + static void crashInNative2(long address) { + System.out.println("in crashInNative2"); + crashInNative3(address); + } + static void crashInNative3(long address) { + System.out.println("read value " + unsafe.getLong(address) + " from 0x" + Long.toHexString(address)); + } + } + + public static void main(String[] args) throws Exception { + run(true); + run(false); } /** - * Runs Crasher and tries to force compile the methods in Crasher. The inner + * Runs Crasher in Xcomp mode. The inner * most method crashes the VM with Unsafe. The resulting hs-err log is * expected to have a min number of MachCode sections. */ - public static void main(String[] args) throws Exception { + private static void run(boolean crashInJava) throws Exception { ProcessBuilder pb = ProcessTools.createJavaProcessBuilder( "-Xmx64m", "--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED", "-XX:-CreateCoredumpOnCrash", "-Xcomp", - "-XX:-TieredCompilation", // ensure C2 compiles Crasher.method3 - "-XX:CompileCommand=compileonly,MachCodeFramesInErrorFile$Crasher.m*", - "-XX:CompileCommand=dontinline,MachCodeFramesInErrorFile$Crasher.m*", - Crasher.class.getName()); + "-XX:-TieredCompilation", + "-XX:CompileCommand=compileonly,MachCodeFramesInErrorFile$Crasher.crashIn*", + "-XX:CompileCommand=dontinline,MachCodeFramesInErrorFile$Crasher.crashIn*", + "-XX:CompileCommand=dontinline,*/Unsafe.getLong", // ensures VM call when crashInJava == false + Crasher.class.getName(), + crashInJava ? "crashInJava" : "crashInVM"); OutputAnalyzer output = new OutputAnalyzer(pb.start()); // Extract hs_err_pid file. @@ -101,11 +136,17 @@ public static void main(String[] args) throws Exception { throw new RuntimeException("hs_err_pid file missing at " + hsErrPath + ".\n"); } String hsErr = Files.readString(hsErrPath); - if (!crashedInCrasherMethod(hsErr)) { - return; + if (System.getenv("DEBUG") != null) { + System.err.println(hsErr); } - List nativeFrames = extractNativeFrames(hsErr); - int compiledJavaFrames = (int) nativeFrames.stream().filter(f -> f.startsWith("J ")).count(); + Set frames = new HashSet<>(); + extractFrames(hsErr, frames, true); + if (!crashInJava) { + // A crash in native will have Java frames in the hs-err log + // as there is a Java frame anchor on the stack. + extractFrames(hsErr, frames, false); + } + int compiledJavaFrames = (int) frames.stream().filter(f -> f.startsWith("J ")).count(); Matcher matcherDisasm = Pattern.compile("\\[Disassembly\\].*\\[/Disassembly\\]", Pattern.DOTALL).matcher(hsErr); if (matcherDisasm.find()) { @@ -115,63 +156,30 @@ public static void main(String[] args) throws Exception { Matcher matcher = Pattern.compile("\\[MachCode\\]\\s*\\[Verified Entry Point\\]\\s* # \\{method\\} \\{[^}]*\\} '([^']+)' '([^']+)' in '([^']+)'", Pattern.DOTALL).matcher(hsErr); List machCodeHeaders = matcher.results().map(mr -> String.format("'%s' '%s' in '%s'", mr.group(1), mr.group(2), mr.group(3))).collect(Collectors.toList()); - String message = "Mach code headers: " + machCodeHeaders + - "\n\nExtracted MachCode:\n" + extractMachCode(hsErr) + - "\n\nExtracted native frames:\n" + String.join("\n", nativeFrames); int minExpectedMachCodeSections = Math.max(1, compiledJavaFrames); - Asserts.assertTrue(machCodeHeaders.size() >= minExpectedMachCodeSections, message); - } - - /** - * Checks whether the crashing frame is in {@code Crasher.method3}. - */ - private static boolean crashedInCrasherMethod(String hsErr) { - boolean checkProblematicFrame = false; - for (String line : hsErr.split(System.lineSeparator())) { - if (line.startsWith("# Problematic frame:")) { - checkProblematicFrame = true; - } else if (checkProblematicFrame) { - String crasherMethod = Crasher.class.getSimpleName() + ".method3"; - if (!line.contains(crasherMethod)) { - // There's any number of things that can subvert the attempt - // to crash in the expected method. - System.out.println("Crashed in method other than " + crasherMethod + "\n\n" + line + "\n\nSkipping rest of test."); - return false; - } - return true; - } + if (machCodeHeaders.size() < minExpectedMachCodeSections) { + Asserts.fail(machCodeHeaders.size() + " < " + minExpectedMachCodeSections); } - throw new RuntimeException("\"# Problematic frame:\" line missing in hs_err_pid file:\n" + hsErr); } /** - * Gets the lines in {@code hsErr} below the line starting with "Native frames:" up to the next blank line. + * Extracts the lines in {@code hsErr} below the line starting with + * "Native frames:" or "Java frames:" up to the next blank line + * and adds them to {@code frames}. */ - private static List extractNativeFrames(String hsErr) { - List res = new ArrayList<>(); - boolean inNativeFrames = false; + private static void extractFrames(String hsErr, Set frames, boolean nativeStack) { + String marker = (nativeStack ? "Native" : "Java") + " frames: "; + boolean seenMarker = false; for (String line : hsErr.split(System.lineSeparator())) { - if (line.startsWith("Native frames: ")) { - inNativeFrames = true; - } else if (inNativeFrames) { + if (line.startsWith(marker)) { + seenMarker = true; + } else if (seenMarker) { if (line.trim().isEmpty()) { - return res; + return; } - res.add(line); - } - } - throw new RuntimeException("\"Native frames:\" line missing in hs_err_pid file:\n" + hsErr); - } - - private static String extractMachCode(String hsErr) { - int start = hsErr.indexOf("[MachCode]"); - if (start != -1) { - int end = hsErr.lastIndexOf("[/MachCode]"); - if (end != -1) { - return hsErr.substring(start, end + "[/MachCode]".length()); + frames.add(line); } - return hsErr.substring(start); } - return null; + throw new RuntimeException("\"" + marker + "\" line missing in hs_err_pid file:\n" + hsErr); } }