Skip to content

Fix backtraces on Windows/GNU #37359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions src/libbacktrace/fileline.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,78 @@ POSSIBILITY OF SUCH DAMAGE. */
#include <fcntl.h>
#include <stdlib.h>

#define USE_WIN_EXECNAME (defined(_WIN32_WINNT) && (_WIN32_WINNT >= 0x0600))
#if USE_WIN_EXECNAME
#include <windows.h>
#endif /* USE_WIN_EXECNAME */

#include "backtrace.h"
#include "internal.h"

#ifndef HAVE_GETEXECNAME
#define getexecname() NULL
#endif

/* Targeted fix for Rust backtraces on Windows/GNU.
We can't set the executable name once during during creation
of backtrace state for security reasons, see issue #21889 for
details. So we recalculate it each time the file is accessed.
We also use QueryFullProcessImageName instead of APIs like
GetModuleFileName because it's correctly updated when the
executable is renamed. We alse validate that the opened file
is indeed our executable by using NtAreMappedFilesTheSame. */
#if USE_WIN_EXECNAME

#undef getexecname
static const char *getexecname(void) {
/* Accesses to backtrace functionality from Rust are serialized,
so having a single static unsyncronized buffer is enough. */
static char buf[MAX_PATH];
/* The returned name is later passed to `open`, so it needs to
be encoded in the current locale, so we use `QueryFullProcessImageNameA`.
As a result paths not representable in the current locale are not supported. */
Copy link
Member

Choose a reason for hiding this comment

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

This is super unfortunate.
Yet another reason to avoid MinGW and friends on Windows.

DWORD buf_size = MAX_PATH;
HANDLE process_handle = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, GetCurrentProcessId());
return QueryFullProcessImageNameA(process_handle, 0, buf, &buf_size) ? buf : NULL;
}

static int validate_descriptor(int descriptor) {
FARPROC NtAreMappedFilesTheSame = GetProcAddress(GetModuleHandleW(L"ntdll.dll"),
"NtAreMappedFilesTheSame");
if (!NtAreMappedFilesTheSame) {
return 0;
}
HANDLE file_handle = (HANDLE) _get_osfhandle(descriptor);
if (file_handle == INVALID_HANDLE_VALUE) {
return 0;
}
/* Map the opened file into memory */
HANDLE file_mapping = CreateFileMappingW(file_handle, NULL, PAGE_READONLY | SEC_IMAGE,
0, 0, NULL);
if (!file_mapping) {
return 0;
}
LPVOID mapped_view = MapViewOfFile(file_mapping, FILE_MAP_READ, 0, 0, 0);
if (!mapped_view) {
CloseHandle(file_mapping);
return 0;
}
/* Now "compare" memory at which the opened file is mapped (mapped_view) with memory
at which the current executable is mapped (returned by GetModuleHandleW) */
NTSTATUS status = NtAreMappedFilesTheSame(GetModuleHandleW(NULL), mapped_view);
UnmapViewOfFile(mapped_view);
CloseHandle(file_mapping);
return status == 0;
}

#else /* USE_WIN_EXECNAME */

static int validate_descriptor(int descriptor) {
return 1;
}

#endif /* USE_WIN_EXECNAME */

/* Initialize the fileline information from the executable. Returns 1
on success, 0 on failure. */

Expand Down Expand Up @@ -113,6 +178,11 @@ fileline_initialize (struct backtrace_state *state,
called_error_callback = 1;
break;
}
if (!validate_descriptor(descriptor)) {
close(descriptor);
descriptor = -1;
break;
}
if (descriptor >= 0)
break;
}
Expand Down
1 change: 0 additions & 1 deletion src/test/run-pass/backtrace-debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ macro_rules! dump_and_die {
target_os = "ios",
target_os = "android",
all(target_os = "linux", target_arch = "arm"),
target_os = "windows",
target_os = "freebsd",
target_os = "dragonfly",
target_os = "bitrig",
Expand Down
4 changes: 0 additions & 4 deletions src/test/run-pass/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@ fn runtest(me: &str) {
}

fn main() {
if cfg!(windows) && cfg!(target_env = "gnu") {
return
}

let args: Vec<String> = env::args().collect();
if args.len() >= 2 && args[1] == "fail" {
foo();
Expand Down