fix(win): make symbolication and modulefinder independent of the system ANSI code page.

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+**Fixes**:
+
+- Windows: Make symbolication and the modulefinder independent of the system ANSI code page. ([#1389](https://github.com/getsentry/sentry-native/pull/1389))
+
 ## 0.11.1
 
 **Features**:

src/modulefinder/sentry_modulefinder_windows.c

@@ -6,10 +6,9 @@
 
 #ifndef SENTRY_PLATFORM_XBOX
 #    include <dbghelp.h>
-#else
-#    include <Psapi.h>
 #endif
-#include <tlhelp32.h>
+#include <Psapi.h>
+#include <TlHelp32.h>
 
 static bool g_initialized = false;
 static sentry_mutex_t g_mutex = SENTRY__MUTEX_INIT;
@@ -92,6 +91,23 @@ extract_pdb_info(uintptr_t module_addr, sentry_value_t module)
     }
 }
 
+static void
+log_library_load_error(const wchar_t *module_filename_w)
+{
+    const DWORD ec = GetLastError();
+    LPWSTR msg_w = NULL;
+    FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM
+            | FORMAT_MESSAGE_IGNORE_INSERTS,
+        NULL, ec, 0, (LPWSTR)&msg_w, 0, NULL);
+    char *msg = sentry__string_from_wstr(msg_w);
+    char *module_filename = sentry__string_from_wstr(module_filename_w);
+    SENTRY_ERRORF("LoadLibraryExW failed (%lu): %s (\"%s\")\n", ec,
+        msg ? msg : "(no message)", module_filename);
+    sentry_free(module_filename);
+    sentry_free(msg);
+    LocalFree(msg_w);
+}
+
 static void
 load_modules(void)
 {
@@ -104,11 +120,25 @@ load_modules(void)
 
     if (Module32FirstW(snapshot, &module)) {
         do {
-            HMODULE handle = LoadLibraryExW(
-                module.szExePath, NULL, LOAD_LIBRARY_AS_DATAFILE);
+            wchar_t module_filename_w[MAX_PATH];
+            HMODULE module_handle = NULL;
+            if (GetModuleFileNameExW(GetCurrentProcess(), module.hModule,
+                    module_filename_w, MAX_PATH)) {
+                module_handle = LoadLibraryExW(
+                    module_filename_w, NULL, LOAD_LIBRARY_AS_DATAFILE);
+            } else {
+                char *module_name = sentry__string_from_wstr(module.szModule);
+                SENTRY_ERRORF(
+                    "Failed to get module filename for %s", module_name);
+                sentry_free(module_name);
+                continue;
+            }
+            if (!module_handle) {
+                log_library_load_error(module_filename_w);
+                continue;
+            }
             MEMORY_BASIC_INFORMATION vmem_info = { 0 };
-            if (handle
-                && sizeof(vmem_info)
+            if (sizeof(vmem_info)
                     == VirtualQuery(
                         module.modBaseAddr, &vmem_info, sizeof(vmem_info))
                 && vmem_info.State == MEM_COMMIT) {
@@ -120,11 +150,11 @@ load_modules(void)
                 sentry_value_set_by_key(rv, "image_size",
                     sentry_value_new_int32((int32_t)module.modBaseSize));
                 sentry_value_set_by_key(rv, "code_file",
-                    sentry__value_new_string_from_wstr(module.szExePath));
+                    sentry__value_new_string_from_wstr(module_filename_w));
                 extract_pdb_info((uintptr_t)module.modBaseAddr, rv);
                 sentry_value_append(g_modules, rv);
             }
-            FreeLibrary(handle);
+            FreeLibrary(module_handle);
         } while (Module32NextW(snapshot, &module));
     }
 

src/symbolizer/sentry_symbolizer_windows.c

@@ -1,13 +1,12 @@
 #include "sentry_boot.h"
+#include "sentry_string.h"
 
 #include "sentry_symbolizer.h"
 #include "sentry_windows_dbghelp.h"
 
 #include <dbghelp.h>
 #include <malloc.h>
 
-#define MAX_SYM 1024
-
 bool
 sentry__symbolize(
     void *addr, void (*func)(const sentry_frame_info_t *, void *), void *data)
@@ -17,29 +16,41 @@ sentry__symbolize(
     (void)func;
     (void)addr;
 #else
+    if (!addr || !func) {
+        return false;
+    }
     HANDLE proc = sentry__init_dbghelp();
+    size_t symbol_info_size
+        = sizeof(SYMBOL_INFOW) + MAX_SYM_NAME * sizeof(WCHAR);
+    SYMBOL_INFOW *symbol_info = _alloca(symbol_info_size);
+    memset(symbol_info, 0, symbol_info_size);
+    symbol_info->MaxNameLen = MAX_SYM_NAME;
+    symbol_info->SizeOfStruct = sizeof(SYMBOL_INFOW);
+
+    if (!SymFromAddrW(proc, (uintptr_t)addr, NULL, symbol_info)) {
+        return false;
+    }
 
-    SYMBOL_INFO *sym = (SYMBOL_INFO *)_alloca(sizeof(SYMBOL_INFO) + MAX_SYM);
-    memset(sym, 0, sizeof(SYMBOL_INFO) + MAX_SYM);
-    sym->MaxNameLen = MAX_SYM;
-    sym->SizeOfStruct = sizeof(SYMBOL_INFO);
-
-    if (!SymFromAddr(proc, (DWORD64)addr, 0, sym)) {
+    WCHAR mod_path_w[MAX_PATH];
+    const DWORD n = GetModuleFileNameW(
+        (HMODULE)(uintptr_t)symbol_info->ModBase, mod_path_w, MAX_PATH);
+    if (n == 0 || n >= MAX_PATH) {
         return false;
     }
 
-    char mod_name[MAX_PATH];
-    GetModuleFileNameA(
-        (HMODULE)(size_t)sym->ModBase, mod_name, sizeof(mod_name));
+    char *mod_path = sentry__string_from_wstr(mod_path_w);
+    char *symbol_name = sentry__string_from_wstr(symbol_info->Name);
 
-    sentry_frame_info_t frame_info;
-    memset(&frame_info, 0, sizeof(sentry_frame_info_t));
-    frame_info.load_addr = (void *)(size_t)sym->ModBase;
+    sentry_frame_info_t frame_info = { 0 };
+    frame_info.load_addr = (void *)(uintptr_t)symbol_info->ModBase;
     frame_info.instruction_addr = addr;
-    frame_info.symbol_addr = (void *)(size_t)sym->Address;
-    frame_info.symbol = sym->Name;
-    frame_info.object_name = mod_name;
+    frame_info.symbol_addr = (void *)(uintptr_t)symbol_info->Address;
+    frame_info.symbol = symbol_name;
+    frame_info.object_name = mod_path;
     func(&frame_info, data);
+
+    sentry_free(mod_path);
+    sentry_free(symbol_name);
 #endif // SENTRY_PLATFORM_XBOX
 
     return true;

tests/assertions.py

@@ -6,6 +6,7 @@
 import sys
 from dataclasses import dataclass
 from datetime import datetime, UTC
+from pathlib import Path
 
 import msgpack
 
@@ -167,7 +168,9 @@ def assert_event_meta(
         )
 
 
-def assert_stacktrace(envelope, inside_exception=False, check_size=True):
+def assert_stacktrace(
+    envelope, inside_exception=False, check_size=True, check_package=False
+):
     event = envelope.get_event()
 
     parent = event["exception"] if inside_exception else event["threads"]
@@ -182,6 +185,17 @@ def assert_stacktrace(envelope, inside_exception=False, check_size=True):
             for frame in frames
         )
 
+    if check_package:
+        for frame in frames:
+            frame_package = frame.get("package")
+            if frame_package is not None:
+                frame_package_path = Path(frame_package)
+                # only assert on absolute paths, since letting pathlib resolve relative paths is cheating
+                if frame_package_path.is_absolute():
+                    assert (
+                        frame_package_path.is_file()
+                    ), f"package is not a valid file path: '{frame_package}'"
+
 
 def assert_breadcrumb_inner(breadcrumbs, message="debug crumb"):
     expected = {

tests/test_integration_stdout.py

@@ -1,7 +1,9 @@
 import os
+import shutil
 import subprocess
 import sys
 import time
+from pathlib import Path
 
 import pytest
 
@@ -18,6 +20,7 @@
     assert_no_before_send,
     assert_crash_timestamp,
     assert_breakpad_crash,
+    assert_exception,
 )
 from .conditions import has_breakpad, has_files
 
@@ -46,6 +49,59 @@ def test_capture_stdout(cmake):
     assert_event(envelope)
 
 
+def copy_except(src: Path, dst: Path, exceptions: list[str] = None) -> None:
+    """
+    Recursively copy everything from src to dst, except for entries whose
+    names are in `exceptions`.
+    """
+    exceptions = set(exceptions or [])
+
+    dst.mkdir(parents=True, exist_ok=True)
+
+    for entry in src.iterdir():
+        if entry.name in exceptions:
+            continue
+
+        dest = dst / entry.name
+        if entry.is_dir():
+            shutil.copytree(entry, dest, symlinks=True)
+        else:
+            shutil.copy2(entry, dest)
+
+
+@pytest.mark.skipif(not has_files, reason="test needs a local filesystem")
+def test_capture_exception_from_utf8_path_stdout(cmake):
+    """
+    This test verifies that we can handle symbolication from an utf-8 path.
+    """
+    tmp_path = cmake(
+        ["sentry_example"],
+        {
+            "SENTRY_BACKEND": "none",
+            "SENTRY_TRANSPORT": "none",
+        },
+    )
+    # create a cyrillic subdirectory in tmp_path and copy tmp_path into it
+    cwd = tmp_path / "кириллица-тест"
+    cwd.mkdir()
+    copy_except(tmp_path, cwd, exceptions=["кириллица-тест"])
+
+    output = check_output(
+        cwd,
+        "sentry_example",
+        ["stdout", "capture-exception", "add-stacktrace"],
+    )
+    envelope = Envelope.deserialize(output)
+
+    assert_meta(envelope)
+    assert_breadcrumb(envelope)
+    assert_stacktrace(envelope, inside_exception=True, check_package=True)
+    assert_exception(envelope)
+
+    # delete the cyrillic directory, but only after we asserted on stack frame packages being files
+    shutil.rmtree(cwd)
+
+
 def test_dynamic_sdk_name_override(cmake):
     tmp_path = cmake(
         ["sentry_example"],