Fixed MSVC support
This commit is contained in:
1 parent
9248925b77
commit
b4e77ed401
8 files changed
+128
-28
No files matched your search
@@ -12,3 +12,16 @@ if(NOT WIN32)
|
|||||||
# (nonexistent, for a static lib) link step.
|
# (nonexistent, for a static lib) link step.
|
||||||
target_link_libraries(hotreload_lib PUBLIC ${CMAKE_DL_LIBS})
|
target_link_libraries(hotreload_lib PUBLIC ${CMAKE_DL_LIBS})
|
||||||
endif()
|
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()
|
||||||
+26
-11
@@ -283,17 +283,32 @@ export-table reader in `SymbolTable.cpp`.
|
|||||||
`_ZN9CppWidget4tickEi` lookup string was derived from a native GCC
|
`_ZN9CppWidget4tickEi` lookup string was derived from a native GCC
|
||||||
build but also worked unmodified against this MinGW/Wine build and
|
build but also worked unmodified against this MinGW/Wine build and
|
||||||
the separate native Clang build).
|
the separate native Clang build).
|
||||||
- **MSVC itself was not run anywhere in this process** -- there is no
|
- **MSVC was never run directly in the environment that built this
|
||||||
Windows machine or MSVC install available in the environment this
|
library** -- there is no Windows machine or MSVC install available
|
||||||
was built in. The MSVC-specific code (`/hotpatch`,
|
there. It has, since, been built and live-tested on real MSVC by
|
||||||
`IMAGE_EXPORT_DIRECTORY` parsing, `VirtualProtect`,
|
someone using this library, surfacing four real fixes now folded
|
||||||
`FlushInstructionCache`) is written directly against Microsoft's
|
in: `windows.h` must be included before `dbghelp.h` (the reverse
|
||||||
documented structures and behavior, and the equivalent PE-reading
|
order fails to compile -- `dbghelp.h` assumes types like `HANDLE`
|
||||||
code was compiled clean against real `<windows.h>` headers via
|
are already defined rather than including their own headers),
|
||||||
MinGW, but treat the MSVC path as unverified until you've built it
|
`NOMINMAX` needs defining before `windows.h` in every file that
|
||||||
on an actual Windows box. If it doesn't work, `SymbolTable.cpp`'s
|
includes it (`SymbolTable.cpp` concretely needs this -- without it,
|
||||||
PE branch and `cmake/HotReload.cmake`'s `/hotpatch` branch are the
|
windows.h's own `min`/`max` macros substitute into that file's
|
||||||
two places to look first.
|
`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 `<windows.h>` 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
|
- **32-bit ARM was not implemented at all**, deliberately: it's the
|
||||||
one architecture in this list where getting it wrong is easy and
|
one architecture in this list where getting it wrong is easy and
|
||||||
getting it verified is hard. Its instructions are either 4 bytes
|
getting it verified is hard. Its instructions are either 4 bytes
|
||||||
|
|||||||
@@ -3,6 +3,13 @@
|
|||||||
#include <cstdint>
|
#include <cstdint>
|
||||||
|
|
||||||
#if defined(_WIN32)
|
#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 <windows.h>
|
#include <windows.h>
|
||||||
#else
|
#else
|
||||||
#include <sys/mman.h>
|
#include <sys/mman.h>
|
||||||
|
|||||||
@@ -3,6 +3,12 @@
|
|||||||
#include <string>
|
#include <string>
|
||||||
|
|
||||||
#if defined(_WIN32)
|
#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 <windows.h>
|
#include <windows.h>
|
||||||
#else
|
#else
|
||||||
#include <dlfcn.h>
|
#include <dlfcn.h>
|
||||||
|
|||||||
@@ -35,6 +35,16 @@ namespace hot_reload::detail
|
|||||||
// headers just gets added directly to loadedBase.
|
// 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 <windows.h>
|
#include <windows.h>
|
||||||
|
|
||||||
namespace
|
namespace
|
||||||
|
|||||||
@@ -29,8 +29,27 @@
|
|||||||
// beyond just this function.
|
// beyond just this function.
|
||||||
#include <cxxabi.h>
|
#include <cxxabi.h>
|
||||||
#elif defined(_MSC_VER)
|
#elif defined(_MSC_VER)
|
||||||
#include <dbghelp.h>
|
// 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 <windows.h>
|
#include <windows.h>
|
||||||
|
#include <dbghelp.h>
|
||||||
|
// 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
|
#endif
|
||||||
|
|
||||||
extern "C"
|
extern "C"
|
||||||
@@ -61,10 +80,12 @@ extern "C"
|
|||||||
// DbgHelp) -- but DbgHelp as a whole is documented as not
|
// DbgHelp) -- but DbgHelp as a whole is documented as not
|
||||||
// thread-safe, so serialize calls to this function
|
// thread-safe, so serialize calls to this function
|
||||||
// yourself if you call it from more than one thread.
|
// yourself if you call it from more than one thread.
|
||||||
// Written against Microsoft's documented behavior; unlike
|
// Exercised on a real Windows build (see README.md's
|
||||||
// most of this library, this specific function was not
|
// toolchain verification notes) -- getting the
|
||||||
// exercised on real Windows -- see README.md's toolchain
|
// windows.h/dbghelp.h include order and NOMINMAX right
|
||||||
// verification notes.
|
// (see the #include block above) and linking dbghelp
|
||||||
|
// explicitly (source/HotReload/CMakeLists.txt) were both
|
||||||
|
// needed to get here.
|
||||||
char buffer[2048];
|
char buffer[2048];
|
||||||
if (UnDecorateSymbolName(name, buffer, sizeof(buffer), UNDNAME_COMPLETE) != 0)
|
if (UnDecorateSymbolName(name, buffer, sizeof(buffer), UNDNAME_COMPLETE) != 0)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -147,25 +147,51 @@ int main(int argc, char** argv)
|
|||||||
auto currentWriteTime = std::filesystem::last_write_time(pluginPath, currentTimeStatus);
|
auto currentWriteTime = std::filesystem::last_write_time(pluginPath, currentTimeStatus);
|
||||||
if (!currentTimeStatus && currentWriteTime != lastWriteTime)
|
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());
|
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)
|
if (status == HOT_RELOAD_OK)
|
||||||
{
|
{
|
||||||
lastWriteTime = currentWriteTime;
|
lastWriteTime = currentWriteTime;
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
std::printf("-- reload failed: %s -- still running the previous version; fix and save again --\n",
|
std::printf(
|
||||||
hot_reload_last_error(ctx));
|
"-- 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");
|
std::printf("\n");
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -42,11 +42,13 @@
|
|||||||
#include "hot_reload/hot_reload.h"
|
#include "hot_reload/hot_reload.h"
|
||||||
|
|
||||||
#include <cstdio>
|
#include <cstdio>
|
||||||
|
#include <iostream>
|
||||||
|
|
||||||
extern "C" HOT_RELOAD_EXPORT int live_tick(int frame)
|
extern "C" HOT_RELOAD_EXPORT int live_tick(int frame)
|
||||||
{
|
{
|
||||||
int multiplier = 2; // <- try changing this
|
int multiplier = 2; // <- try changing this
|
||||||
int value = frame * multiplier;
|
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;
|
return value;
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user