Use Windows symbol APIs in the unwind tester.
This should make things a bit easier to debug.
Update-Note: Test binaries on Windows now link to dbghelp.
Bug: 259
Change-Id: I9da1fc89d429080c5250238e4341445922b1dd8e
Reviewed-on: https://boringssl-review.googlesource.com/c/34868
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/abi_self_test.cc b/crypto/abi_self_test.cc
index 0ea7b32..1fcf6bc 100644
--- a/crypto/abi_self_test.cc
+++ b/crypto/abi_self_test.cc
@@ -48,9 +48,9 @@
#if defined(OPENSSL_X86_64)
if (abi_test::UnwindTestsEnabled()) {
EXPECT_NONFATAL_FAILURE(CHECK_ABI_SEH(abi_test_bad_unwind_wrong_register),
- "was not recovered unwinding");
+ "was not recovered");
EXPECT_NONFATAL_FAILURE(CHECK_ABI_SEH(abi_test_bad_unwind_temporary),
- "was not recovered unwinding");
+ "was not recovered");
CHECK_ABI_NO_UNWIND(abi_test_bad_unwind_wrong_register);
CHECK_ABI_NO_UNWIND(abi_test_bad_unwind_temporary);
diff --git a/crypto/test/CMakeLists.txt b/crypto/test/CMakeLists.txt
index d2e4cdf..b968fd7 100644
--- a/crypto/test/CMakeLists.txt
+++ b/crypto/test/CMakeLists.txt
@@ -15,6 +15,9 @@
target_include_directories(test_support_lib PRIVATE ${LIBUNWIND_INCLUDE_DIRS})
target_link_libraries(test_support_lib ${LIBUNWIND_LDFLAGS})
endif()
+if(WIN32)
+ target_link_libraries(test_support_lib dbghelp)
+endif()
add_dependencies(test_support_lib global_target)
add_library(
diff --git a/crypto/test/abi_test.cc b/crypto/test/abi_test.cc
index 39dd2be..ed6a2f8 100644
--- a/crypto/test/abi_test.cc
+++ b/crypto/test/abi_test.cc
@@ -43,6 +43,7 @@
#define SUPPORTS_UNWIND_TEST
OPENSSL_MSVC_PRAGMA(warning(push, 3))
#include <windows.h>
+#include <dbghelp.h>
OPENSSL_MSVC_PRAGMA(warning(pop))
#endif
#endif // X86_64 && SUPPORTS_ABI_TEST
@@ -253,6 +254,8 @@
starting_ip_ = ctx_.Rip;
}
+ crypto_word_t starting_ip() const { return starting_ip_; }
+
// Step unwinds the cursor by one frame. On success, it returns whether there
// were more frames to unwind.
UnwindStatusOr<bool> Step() {
@@ -313,12 +316,8 @@
}
// ToString returns a human-readable representation of the address the cursor
- // started at, using debug information if available.
+ // started at.
const char *ToString() {
- // TODO(davidben): Use SymFromAddr here. See base/debug/stack_trace_win.cc
- // in Chromium for an example. It probably should be called outside the
- // exception handler, which means we need to stash the address in
- // |g_unwind_errors| to defer it.
StrCatSignalSafe(starting_ip_buf_, "0x", WordToHex(starting_ip_).data());
return starting_ip_buf_;
}
@@ -483,14 +482,30 @@
// Errors are saved in a signal handler. We use a static buffer to avoid
// allocation.
static size_t g_num_unwind_errors = 0;
-static char g_unwind_errors[kMaxUnwindErrors][512];
+
+struct UnwindError {
+#if defined(OPENSSL_WINDOWS)
+ crypto_word_t ip;
+#endif
+ char str[512];
+};
+
+static UnwindError g_unwind_errors[kMaxUnwindErrors];
template <typename... Args>
-static void AddUnwindError(Args... args) {
+static void AddUnwindError(UnwindCursor *cursor, Args... args) {
if (g_num_unwind_errors >= kMaxUnwindErrors) {
return;
}
- StrCatSignalSafe(g_unwind_errors[g_num_unwind_errors], args...);
+#if defined(OPENSSL_WINDOWS)
+ // Windows symbol functions should not be called when handling an
+ // exception. Stash the instruction pointer, to be symbolized later.
+ g_unwind_errors[g_num_unwind_errors].ip = cursor->starting_ip();
+ StrCatSignalSafe(g_unwind_errors[g_num_unwind_errors].str, args...);
+#else
+ StrCatSignalSafe(g_unwind_errors[g_num_unwind_errors].str,
+ "unwinding at ", cursor->ToString(), ": ", args...);
+#endif
g_num_unwind_errors++;
}
@@ -533,57 +548,51 @@
} else if (IsAncestorStackFrame(sp, g_trampoline_sp)) {
// This should never happen. We went past |g_trampoline_sp| without
// stopping at |kStopAddress|.
- AddUnwindError("stack frame is before caller at ",
- cursor->ToString());
+ AddUnwindError(cursor, "stack frame is before caller");
g_in_trampoline = false;
} else if (g_num_unwind_errors < kMaxUnwindErrors) {
for (;;) {
UnwindStatusOr<bool> step_ret = cursor->Step();
if (!step_ret.ok()) {
- AddUnwindError("error unwinding from ", cursor->ToString(), ": ",
- step_ret.Error());
+ AddUnwindError(cursor, "error unwinding: ", step_ret.Error());
break;
}
// |Step| returns whether there was a frame to unwind.
if (!step_ret.ValueOrDie()) {
- AddUnwindError("could not unwind to starting frame from ",
- cursor->ToString());
+ AddUnwindError(cursor, "could not unwind to starting frame");
break;
}
UnwindStatusOr<crypto_word_t> cur_sp = cursor->GetSP();
if (!cur_sp.ok()) {
- AddUnwindError("error recovering stack pointer unwinding from ",
- cursor->ToString(), ": ", cur_sp.Error());
+ AddUnwindError(cursor,
+ "error recovering stack pointer: ", cur_sp.Error());
break;
}
if (IsAncestorStackFrame(cur_sp.ValueOrDie(), g_trampoline_sp)) {
- AddUnwindError("unwound past starting frame from ",
- cursor->ToString());
+ AddUnwindError(cursor, "unwound past starting frame");
break;
}
if (cur_sp.ValueOrDie() == g_trampoline_sp) {
// We found the parent frame. Check the return address.
UnwindStatusOr<crypto_word_t> cur_ip = cursor->GetIP();
if (!cur_ip.ok()) {
- AddUnwindError("error recovering return address unwinding from ",
- cursor->ToString(), ": ", cur_ip.Error());
+ AddUnwindError(cursor,
+ "error recovering return address: ", cur_ip.Error());
} else if (cur_ip.ValueOrDie() != kReturnAddress) {
- AddUnwindError("wrong return address unwinding from ",
- cursor->ToString());
+ AddUnwindError(cursor, "wrong return address");
}
// Check the remaining registers.
UnwindStatusOr<CallerState> state = cursor->GetCallerState();
if (!state.ok()) {
- AddUnwindError("error recovering registers unwinding from ",
- cursor->ToString(), ": ", state.Error());
+ AddUnwindError(cursor,
+ "error recovering registers: ", state.Error());
} else {
- ForEachMismatch(
- state.ValueOrDie(), g_trampoline_state, [&](const char *reg) {
- AddUnwindError(reg, " was not recovered unwinding from ",
- cursor->ToString());
- });
+ ForEachMismatch(state.ValueOrDie(), g_trampoline_state,
+ [&](const char *reg) {
+ AddUnwindError(cursor, reg, " was not recovered");
+ });
}
break;
}
@@ -594,7 +603,29 @@
static void ReadUnwindResult(Result *out) {
for (size_t i = 0; i < g_num_unwind_errors; i++) {
- out->errors.emplace_back(g_unwind_errors[i]);
+#if defined(OPENSSL_WINDOWS)
+ const crypto_word_t ip = g_unwind_errors[i].ip;
+ char buf[256];
+ DWORD64 displacement;
+ struct {
+ SYMBOL_INFO info;
+ char name_buf[128];
+ } symbol;
+ memset(&symbol, 0, sizeof(symbol));
+ symbol.info.SizeOfStruct = sizeof(symbol.info);
+ symbol.info.MaxNameLen = sizeof(symbol.name_buf);
+ if (SymFromAddr(GetCurrentProcess(), ip, &displacement, &symbol.info)) {
+ snprintf(buf, sizeof(buf), "unwinding at %s+%llu (0x%s): %s",
+ symbol.info.Name, displacement, WordToHex(ip).data(),
+ g_unwind_errors[i].str);
+ } else {
+ snprintf(buf, sizeof(buf), "unwinding at 0x%s: %s",
+ WordToHex(ip).data(), g_unwind_errors[i].str);
+ }
+ out->errors.emplace_back(buf);
+#else
+ out->errors.emplace_back(g_unwind_errors[i].str);
+#endif
}
if (g_num_unwind_errors == kMaxUnwindErrors) {
out->errors.emplace_back("(additional errors omitted)");
@@ -630,6 +661,11 @@
g_main_thread = GetCurrentThreadId();
+ SymSetOptions(SYMOPT_DEFERRED_LOADS);
+ if (!SymInitialize(GetCurrentProcess(), nullptr, TRUE)) {
+ fprintf(stderr, "Could not initialize symbols.\n");
+ }
+
if (AddVectoredExceptionHandler(0, ExceptionHandler) == nullptr) {
fprintf(stderr, "Error installing exception handler.\n");
abort();
