Skip to content

Commit

Permalink
8309613: [Windows] hs_err files sometimes miss information about the …
Browse files Browse the repository at this point in the history
…code containing the error

Reviewed-by: dholmes, stuefe
  • Loading branch information
TheRealMDoerr committed Jun 14, 2023
1 parent 63fe413 commit bd79db3
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 15 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/os/aix/os_aix.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class os::Aix {
// Returns true if ok, false if error.
static bool get_meminfo(meminfo_t* pmi);

static bool platform_print_native_stack(outputStream* st, const void* context, char *buf, int buf_size);
static bool platform_print_native_stack(outputStream* st, const void* context, char *buf, int buf_size, address& lastpc);
static void* resolve_function_descriptor(void* p);
};

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/os/windows/os_windows.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class os::win32 {
static void print_uptime_info(outputStream* st);

static bool platform_print_native_stack(outputStream* st, const void* context,
char *buf, int buf_size);
char *buf, int buf_size, address& lastpc);

static bool register_code_area(char *low, char *high);

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ int os::extra_bang_size_in_bytes() {
return 0;
}

bool os::Aix::platform_print_native_stack(outputStream* st, const void* context, char *buf, int buf_size) {
bool os::Aix::platform_print_native_stack(outputStream* st, const void* context, char *buf, int buf_size, address& lastpc) {
AixNativeCallstack::print_callstack_for_context(st, (const ucontext_t*)context, true, buf, (size_t) buf_size);
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/os_cpu/aix_ppc/os_aix_ppc.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@

#define HAVE_PLATFORM_PRINT_NATIVE_STACK 1
inline bool os::platform_print_native_stack(outputStream* st, const void* context,
char *buf, int buf_size) {
return os::Aix::platform_print_native_stack(st, context, buf, buf_size);
char *buf, int buf_size, address& lastpc) {
return os::Aix::platform_print_native_stack(st, context, buf, buf_size, lastpc);
}

#define HAVE_FUNCTION_DESCRIPTORS 1
Expand Down
9 changes: 5 additions & 4 deletions src/hotspot/os_cpu/windows_x86/os_windows_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ bool os::win32::register_code_area(char *low, char *high) {
* loop in vmError.cpp. We need to roll our own loop.
*/
bool os::win32::platform_print_native_stack(outputStream* st, const void* context,
char *buf, int buf_size)
char *buf, int buf_size, address& lastpc)
{
CONTEXT ctx;
if (context != nullptr) {
Expand All @@ -245,14 +245,14 @@ bool os::win32::platform_print_native_stack(outputStream* st, const void* contex
stk.AddrPC.Mode = AddrModeFlat;

int count = 0;
address lastpc = 0;
address lastpc_internal = 0;
while (count++ < StackPrintLimit) {
intptr_t* sp = (intptr_t*)stk.AddrStack.Offset;
intptr_t* fp = (intptr_t*)stk.AddrFrame.Offset; // NOT necessarily the same as ctx.Rbp!
address pc = (address)stk.AddrPC.Offset;

if (pc != nullptr) {
if (count == 2 && lastpc == pc) {
if (count == 2 && lastpc_internal == pc) {
// Skip it -- StackWalk64() may return the same PC
// (but different SP) on the first try.
} else {
Expand All @@ -268,12 +268,13 @@ bool os::win32::platform_print_native_stack(outputStream* st, const void* contex
}
st->cr();
}
lastpc = pc;
lastpc_internal = pc;
}

PVOID p = WindowsDbgHelp::symFunctionTableAccess64(GetCurrentProcess(), stk.AddrPC.Offset);
if (!p) {
// StackWalk64() can't handle this PC. Calling StackWalk64 again may cause crash.
lastpc = lastpc_internal;
break;
}

Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/os_cpu/windows_x86/os_windows_x86.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
#ifdef AMD64
#define HAVE_PLATFORM_PRINT_NATIVE_STACK 1
inline bool os::platform_print_native_stack(outputStream* st, const void* context,
char *buf, int buf_size) {
return os::win32::platform_print_native_stack(st, context, buf, buf_size);
char *buf, int buf_size, address& lastpc) {
return os::win32::platform_print_native_stack(st, context, buf, buf_size, lastpc);
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/os.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,7 @@ class os: AllStatic {

public:
inline static bool platform_print_native_stack(outputStream* st, const void* context,
char *buf, int buf_size);
char *buf, int buf_size, address& lastpc);

// debugging support (mostly used by debug.cpp but also fatal error handler)
static bool find(address pc, outputStream* st = tty); // OS specific function to make sense out of an address
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/os.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

#ifndef HAVE_PLATFORM_PRINT_NATIVE_STACK
inline bool os::platform_print_native_stack(outputStream* st, const void* context,
char *buf, int buf_size) {
char *buf, int buf_size, address& lastpc) {
return false;
}
#endif
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/utilities/debug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,8 @@ extern "C" JNIEXPORT void pns(void* sp, void* fp, void* pc) { // print native st
extern "C" JNIEXPORT void pns2() { // print native stack
Command c("pns2");
static char buf[O_BUFLEN];
if (os::platform_print_native_stack(tty, nullptr, buf, sizeof(buf))) {
address lastpc = nullptr;
if (os::platform_print_native_stack(tty, nullptr, buf, sizeof(buf), lastpc)) {
// We have printed the native stack in platform-specific code,
// so nothing else to do in this case.
} else {
Expand Down
41 changes: 40 additions & 1 deletion src/hotspot/share/utilities/vmError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,27 @@ static bool print_code(outputStream* st, Thread* thread, address pc, bool is_cra
return false;
}

// Like above, but only try to figure out a short name. Return nullptr if not found.
static const char* find_code_name(address pc) {
if (Interpreter::contains(pc)) {
InterpreterCodelet* codelet = Interpreter::codelet_containing(pc);
if (codelet != nullptr) {
return codelet->description();
}
} else {
StubCodeDesc* desc = StubCodeDesc::desc_for(pc);
if (desc != nullptr) {
return desc->name();
} else {
CodeBlob* cb = CodeCache::find_blob(pc);
if (cb != nullptr) {
return cb->name();
}
}
}
return nullptr;
}

/**
* Gets the caller frame of `fr`.
*
Expand Down Expand Up @@ -675,6 +696,10 @@ void VMError::report(outputStream* st, bool _verbose) {
// don't allocate large buffer on stack
static char buf[O_BUFLEN];

// Native stack trace may get stuck. We try to handle the last pc if it
// belongs to VM generated code.
address lastpc = nullptr;

BEGIN

STEP("printing fatal error message")
Expand Down Expand Up @@ -963,9 +988,16 @@ void VMError::report(outputStream* st, bool _verbose) {
st->cr();

STEP_IF("printing native stack (with source info)", _verbose)
if (os::platform_print_native_stack(st, _context, buf, sizeof(buf))) {
if (os::platform_print_native_stack(st, _context, buf, sizeof(buf), lastpc)) {
// We have printed the native stack in platform-specific code
// Windows/x64 needs special handling.
// Stack walking may get stuck. Try to find the calling code.
if (lastpc != nullptr) {
const char* name = find_code_name(lastpc);
if (name != nullptr) {
st->print_cr("The last pc belongs to %s (printed below).", name);
}
}
} else {
frame fr = _context ? os::fetch_frame_from_context(_context)
: os::current_frame();
Expand Down Expand Up @@ -1070,6 +1102,13 @@ void VMError::report(outputStream* st, bool _verbose) {
// value outside the range.
int limit = MIN2(ErrorLogPrintCodeLimit, printed_capacity);
if (limit > 0) {
// Check if a pc was found by native stack trace above.
if (lastpc != nullptr) {
if (print_code(st, _thread, lastpc, true, printed, printed_capacity)) {
printed_len++;
}
}

// Scan the native stack
if (!_print_native_stack_used) {
// Only try to print code of the crashing frame since
Expand Down

3 comments on commit bd79db3

@TheRealMDoerr
Copy link
Contributor Author

Choose a reason for hiding this comment

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

/backport jdk21

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@openjdk
Copy link

@openjdk openjdk bot commented on bd79db3 Jun 14, 2023

Choose a reason for hiding this comment

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

@TheRealMDoerr the backport was successfully created on the branch TheRealMDoerr-backport-bd79db39 in my personal fork of openjdk/jdk21. To create a pull request with this backport targeting openjdk/jdk21:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit bd79db39 from the openjdk/jdk repository.

The commit being backported was authored by Martin Doerr on 14 Jun 2023 and was reviewed by David Holmes and Thomas Stuefe.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21:

$ git fetch https://github.com/openjdk-bots/jdk21.git TheRealMDoerr-backport-bd79db39:TheRealMDoerr-backport-bd79db39
$ git checkout TheRealMDoerr-backport-bd79db39
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21.git TheRealMDoerr-backport-bd79db39

Please sign in to comment.