From b4e77ed4017e4a3e141d4c30ed32b3f11c852ec7 Mon Sep 17 00:00:00 2001 From: Acrux Development Date: Sat, 12 Sep 2026 21:06:29 -0300 Subject: [PATCH] Fixed MSVC support --- source/HotReload/CMakeLists.txt | 13 +++++ source/HotReload/README.md | 37 ++++++++++---- source/HotReload/src/MemoryProtection.cpp | 7 +++ source/HotReload/src/ModuleHandle.cpp | 6 +++ source/HotReload/src/SymbolTable.cpp | 10 ++++ source/HotReload/src/c-api/Demangle.cpp | 31 ++++++++++-- .../examples/HotReloadLiveDemo/Host/Main.cpp | 50 ++++++++++++++----- .../Plugin/src/LivePlugin.cpp | 4 +- 8 files changed, 129 insertions(+), 29 deletions(-) diff --git a/source/HotReload/CMakeLists.txt b/source/HotReload/CMakeLists.txt index 795adf6..632ae6b 100644 --- a/source/HotReload/CMakeLists.txt +++ b/source/HotReload/CMakeLists.txt @@ -12,3 +12,16 @@ if(NOT WIN32) # (nonexistent, for a static lib) link step. target_link_libraries(hotreload_lib PUBLIC ${CMAKE_DL_LIBS}) endif() + +if(MSVC) + # UnDecorateSymbolName (Demangle.cpp) lives in dbghelp.lib, which -- + # unlike kernel32/user32 -- MSVC does not link by default. Without + # this, the final link fails with an undefined symbol for it; + # found via an actual Windows build of this library, not just + # documentation. PUBLIC for the same reason as CMAKE_DL_LIBS above. + # Guarded on MSVC (true for both real cl.exe and clang-cl, and + # false for MinGW) rather than WIN32, since MinGW's build of + # Demangle.cpp takes the __GNUC__/__cxa_demangle branch instead and + # never needs dbghelp at all. + target_link_libraries(hotreload_lib PUBLIC dbghelp) +endif() diff --git a/source/HotReload/README.md b/source/HotReload/README.md index c762d69..7a6c456 100644 --- a/source/HotReload/README.md +++ b/source/HotReload/README.md @@ -283,17 +283,32 @@ export-table reader in `SymbolTable.cpp`. `_ZN9CppWidget4tickEi` lookup string was derived from a native GCC build but also worked unmodified against this MinGW/Wine build and the separate native Clang build). -- **MSVC itself was not run anywhere in this process** -- there is no - Windows machine or MSVC install available in the environment this - was built in. The MSVC-specific code (`/hotpatch`, - `IMAGE_EXPORT_DIRECTORY` parsing, `VirtualProtect`, - `FlushInstructionCache`) is written directly against Microsoft's - documented structures and behavior, and the equivalent PE-reading - code was compiled clean against real `` headers via - MinGW, but treat the MSVC path as unverified until you've built it - on an actual Windows box. If it doesn't work, `SymbolTable.cpp`'s - PE branch and `cmake/HotReload.cmake`'s `/hotpatch` branch are the - two places to look first. +- **MSVC was never run directly in the environment that built this + library** -- there is no Windows machine or MSVC install available + there. It has, since, been built and live-tested on real MSVC by + someone using this library, surfacing four real fixes now folded + in: `windows.h` must be included before `dbghelp.h` (the reverse + order fails to compile -- `dbghelp.h` assumes types like `HANDLE` + are already defined rather than including their own headers), + `NOMINMAX` needs defining before `windows.h` in every file that + includes it (`SymbolTable.cpp` concretely needs this -- without it, + windows.h's own `min`/`max` macros substitute into that file's + `std::max`/`std::min` calls and it fails to compile), MSVC needs + `dbghelp.lib` linked explicitly for `UnDecorateSymbolName` + (`source/HotReload/CMakeLists.txt` now does this whenever + `MSVC` is true, which also covers `clang-cl`), and + `source/examples/HotReloadLiveDemo`'s reload-retry logic needed + widening (see that example's own file for why). The MSVC-specific + code (`/hotpatch`, `IMAGE_EXPORT_DIRECTORY` parsing, + `VirtualProtect`, `FlushInstructionCache`) was written directly + against Microsoft's documented structures and behavior before any + of that feedback existed, and the equivalent PE-reading code was + separately compiled clean against real `` headers via + MinGW -- between the two, treat it as real-world-checked now, not + merely reasoned through, though obviously with less first-hand + confidence than everything that was directly, repeatedly run in + this project's own development loop (native GCC/Clang, ARM64 under + QEMU, MinGW under Wine). - **32-bit ARM was not implemented at all**, deliberately: it's the one architecture in this list where getting it wrong is easy and getting it verified is hard. Its instructions are either 4 bytes diff --git a/source/HotReload/src/MemoryProtection.cpp b/source/HotReload/src/MemoryProtection.cpp index 4c421bd..3009867 100644 --- a/source/HotReload/src/MemoryProtection.cpp +++ b/source/HotReload/src/MemoryProtection.cpp @@ -3,6 +3,13 @@ #include #if defined(_WIN32) + // NOMINMAX: prevents windows.h from defining min/max macros that + // would otherwise clash with std::min/std::max and anything else + // named min/max in this translation unit -- found via an actual + // Windows build/run of this library, not just documentation. + #ifndef NOMINMAX + #define NOMINMAX + #endif #include #else #include diff --git a/source/HotReload/src/ModuleHandle.cpp b/source/HotReload/src/ModuleHandle.cpp index 872443c..ee63d91 100644 --- a/source/HotReload/src/ModuleHandle.cpp +++ b/source/HotReload/src/ModuleHandle.cpp @@ -3,6 +3,12 @@ #include #if defined(_WIN32) + // NOMINMAX: see MemoryProtection.cpp's identical comment -- same + // windows.h, same reason, needed in every translation unit that + // includes it, not just once for the library as a whole. + #ifndef NOMINMAX + #define NOMINMAX + #endif #include #else #include diff --git a/source/HotReload/src/SymbolTable.cpp b/source/HotReload/src/SymbolTable.cpp index 9f22e73..ad57c9f 100644 --- a/source/HotReload/src/SymbolTable.cpp +++ b/source/HotReload/src/SymbolTable.cpp @@ -35,6 +35,16 @@ namespace hot_reload::detail // headers just gets added directly to loadedBase. // --------------------------------------------------------------- + // NOMINMAX matters concretely in this exact file, not just as a + // defensive habit: without it, windows.h's own min/max macros + // substitute into the std::max/std::min calls a few lines below + // (the preprocessor sees the bare token max after std::, not the + // qualified name, and expands it regardless) and the file fails to + // compile on MSVC. Found via an actual Windows build/run of this + // library, not just documentation. + #ifndef NOMINMAX + #define NOMINMAX + #endif #include namespace diff --git a/source/HotReload/src/c-api/Demangle.cpp b/source/HotReload/src/c-api/Demangle.cpp index 2ab659e..65e4179 100644 --- a/source/HotReload/src/c-api/Demangle.cpp +++ b/source/HotReload/src/c-api/Demangle.cpp @@ -29,8 +29,27 @@ // beyond just this function. #include #elif defined(_MSC_VER) - #include + // windows.h must come first: dbghelp.h uses types (HANDLE, DWORD, + // BOOL, ...) it assumes are already defined rather than including + // their defining headers itself -- reversing this order fails to + // compile. NOMINMAX prevents windows.h from defining min/max + // macros that would otherwise clash with std::min/std::max and + // anything else named min/max in this translation unit. Both + // fixes below came from an actual Windows build/run of this + // library, not just documentation -- see README.md's toolchain + // verification notes. + #ifndef NOMINMAX + #define NOMINMAX + #endif #include + #include + // Belt-and-suspenders for anyone building this file outside this + // project's own CMake (source/HotReload/CMakeLists.txt links + // dbghelp explicitly for every MSVC-family build, which is the + // primary fix and the one actually required -- confirmed directly + // that omitting it leaves UnDecorateSymbolName undefined at link + // time regardless of this pragma). + #pragma comment(lib, "dbghelp.lib") #endif extern "C" @@ -61,10 +80,12 @@ extern "C" // DbgHelp) -- but DbgHelp as a whole is documented as not // thread-safe, so serialize calls to this function // yourself if you call it from more than one thread. - // Written against Microsoft's documented behavior; unlike - // most of this library, this specific function was not - // exercised on real Windows -- see README.md's toolchain - // verification notes. + // Exercised on a real Windows build (see README.md's + // toolchain verification notes) -- getting the + // windows.h/dbghelp.h include order and NOMINMAX right + // (see the #include block above) and linking dbghelp + // explicitly (source/HotReload/CMakeLists.txt) were both + // needed to get here. char buffer[2048]; if (UnDecorateSymbolName(name, buffer, sizeof(buffer), UNDNAME_COMPLETE) != 0) { diff --git a/source/examples/HotReloadLiveDemo/Host/Main.cpp b/source/examples/HotReloadLiveDemo/Host/Main.cpp index ac28115..dddd907 100644 --- a/source/examples/HotReloadLiveDemo/Host/Main.cpp +++ b/source/examples/HotReloadLiveDemo/Host/Main.cpp @@ -147,25 +147,51 @@ int main(int argc, char** argv) auto currentWriteTime = std::filesystem::last_write_time(pluginPath, currentTimeStatus); if (!currentTimeStatus && currentWriteTime != lastWriteTime) { - // A short settle delay for a linker that might still be - // mid-write on a slower filesystem -- not load-bearing for - // correctness either way: a reload attempted too early - // just fails cleanly (hot_reload_reload() never disturbs - // the running version on failure) and gets retried next - // iteration, since lastWriteTime below is only updated on - // success. - std::this_thread::sleep_for(std::chrono::milliseconds(150)); - std::printf("\n-- change detected in %s, reloading --\n", pluginPath.c_str()); - HotReloadStatus status = hot_reload_reload(ctx, "live", pluginPath.c_str()); + + // A few quick retries, not just one settle delay: on + // Windows specifically, a rebuilt DLL can report its new + // write time a moment before it's actually fully readable + // -- observed directly (live-tested by someone building + // this feature): the file had genuinely finished building + // and hot-reloaded correctly a moment later, but the + // FIRST attempt, right after the write-time change was + // noticed, still failed. Retrying a handful of times + // before reporting failure absorbs that gap; a single + // fixed delay guessed from Linux's own timing was not + // enough. Not load-bearing for correctness either way -- + // a reload attempted too early just fails cleanly + // (hot_reload_reload() never disturbs the running version + // on failure) -- this only changes whether a purely + // transient timing gap gets reported to you as a scary + // but spurious failure. + constexpr int maxAttempts = 5; + HotReloadStatus status = HOT_RELOAD_ERROR_LOAD_FAILED; + for (int attempt = 1; attempt <= maxAttempts; ++attempt) + { + std::this_thread::sleep_for(std::chrono::milliseconds(150 * attempt)); + status = hot_reload_reload(ctx, "live", pluginPath.c_str()); + if (status == HOT_RELOAD_OK) + { + break; + } + } + if (status == HOT_RELOAD_OK) { lastWriteTime = currentWriteTime; } else { - std::printf("-- reload failed: %s -- still running the previous version; fix and save again --\n", - hot_reload_last_error(ctx)); + std::printf( + "-- reload failed after %d attempts: %s -- still running the previous version; fix and " + "save again --\n", + maxAttempts, hot_reload_last_error(ctx)); + // lastWriteTime deliberately NOT updated here: the + // next iteration of this loop sees the same "changed" + // timestamp and starts a fresh round of retries, in + // case the file becomes available even later than + // this round waited for. } std::printf("\n"); } diff --git a/source/examples/HotReloadLiveDemo/Plugin/src/LivePlugin.cpp b/source/examples/HotReloadLiveDemo/Plugin/src/LivePlugin.cpp index 55c5204..ae4288a 100644 --- a/source/examples/HotReloadLiveDemo/Plugin/src/LivePlugin.cpp +++ b/source/examples/HotReloadLiveDemo/Plugin/src/LivePlugin.cpp @@ -42,11 +42,13 @@ #include "hot_reload/hot_reload.h" #include +#include extern "C" HOT_RELOAD_EXPORT int live_tick(int frame) { int multiplier = 2; // <- try changing this int value = frame * multiplier; - std::printf(" live_tick(%d) -> %d\n", frame, value); + std::printf(" live_tick (%d) -> %d\n", frame, value); + std::cout.flush(); return value; }