From 2d01693bbb1593a15822d3c124c48223326f1435 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Sun, 30 Aug 2026 22:12:16 +0200 Subject: [PATCH 01/20] build(x64): add MinGW-w64 x86_64 toolchain and preset --- CMakePresets.json | 11 +++++++++ cmake/toolchains/mingw-w64-x86_64.cmake | 32 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 cmake/toolchains/mingw-w64-x86_64.cmake diff --git a/CMakePresets.json b/CMakePresets.json index 4274c6357d2..9dec124f059 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -191,6 +191,17 @@ "cacheVariables": { "RTS_BUILD_OPTION_PROFILE": "ON" } + }, + { + "name": "mingw-w64-x86_64", + "displayName": "MinGW-w64 64-bit (x86_64) Release", + "generator": "Unix Makefiles", + "binaryDir": "${sourceDir}/build/${presetName}", + "toolchainFile": "${sourceDir}/cmake/toolchains/mingw-w64-x86_64.cmake", + "cacheVariables": { + "CMAKE_EXPORT_COMPILE_COMMANDS": "ON", + "CMAKE_BUILD_TYPE": "Release" + } } ], "buildPresets": [ diff --git a/cmake/toolchains/mingw-w64-x86_64.cmake b/cmake/toolchains/mingw-w64-x86_64.cmake new file mode 100644 index 00000000000..8e0f993c906 --- /dev/null +++ b/cmake/toolchains/mingw-w64-x86_64.cmake @@ -0,0 +1,32 @@ +# MinGW-w64 64-bit (x86_64) Toolchain File +# Use with: cmake -DCMAKE_TOOLCHAIN_FILE=cmake/toolchains/mingw-w64-x86_64.cmake + +set(CMAKE_SYSTEM_NAME Windows) +set(CMAKE_SYSTEM_PROCESSOR x86_64) + +# Specify the cross compiler +set(CMAKE_C_COMPILER x86_64-w64-mingw32-gcc) +set(CMAKE_CXX_COMPILER x86_64-w64-mingw32-g++) +set(CMAKE_RC_COMPILER x86_64-w64-mingw32-windres) +set(CMAKE_AR x86_64-w64-mingw32-ar) +set(CMAKE_RANLIB x86_64-w64-mingw32-ranlib) +set(CMAKE_DLLTOOL x86_64-w64-mingw32-dlltool) + +# Target environment +set(CMAKE_FIND_ROOT_PATH /usr/x86_64-w64-mingw32) + +# Adjust the default behavior of the FIND_XXX() commands: +# search programs in the host environment +set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER) + +# search headers and libraries in the target environment +set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY) +set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY) + +# Force 64-bit pointer size +set(CMAKE_SIZEOF_VOID_P 8) + +# Disable MFC-dependent tools (not compatible with MinGW-w64) +set(RTS_BUILD_CORE_TOOLS OFF CACHE BOOL "Disable MFC-dependent core tools for MinGW" FORCE) +set(RTS_BUILD_GENERALS_TOOLS OFF CACHE BOOL "Disable MFC-dependent Generals tools for MinGW" FORCE) +set(RTS_BUILD_ZEROHOUR_TOOLS OFF CACHE BOOL "Disable MFC-dependent Zero Hour tools for MinGW" FORCE) From ab96fb3133f9da41f87c54449bda1cba374bc005 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Sun, 30 Aug 2026 23:48:57 +0200 Subject: [PATCH 02/20] build(x64): lift the 32-bit-only MinGW guard for the experimental x64 preset cmake/mingw.cmake previously raised FATAL_ERROR for any non-4-byte pointer size under MINGW, which blocked cmake --preset mingw-w64-x86_64 before any target selection happened. Investigation (upstream commit e16574e26, PR #2067) established this guard reflects scope, not infeasibility: the PR only implemented the i686 path, and x64 tracking issue #473 is open. Replace the FATAL_ERROR branch with an IS_MINGW64 branch and a status message. The 32-bit branch is untouched. Re-verified with a clean-first rebuild of the mingw-w64-i686 preset: exit 0, 0 errors, 31,555 warnings, an exact match against docs/x64/baseline-mingw-i686.md. --- cmake/mingw.cmake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmake/mingw.cmake b/cmake/mingw.cmake index c0953430552..b5e2d259e46 100644 --- a/cmake/mingw.cmake +++ b/cmake/mingw.cmake @@ -9,7 +9,8 @@ if(MINGW) set(IS_MINGW32 TRUE) message(STATUS "MinGW-w64 32-bit (i686) detected") else() - message(FATAL_ERROR "MinGW-w64 64-bit (x86_64) detected, but this project only supports 32-bit builds. Use the i686-w64-mingw32 toolchain.") + set(IS_MINGW64 TRUE) + message(STATUS "MinGW-w64 64-bit (x86_64) detected — experimental, see issue #473") endif() # Windows subsystem From d959556bc11c599498315a3d8996bf8399cb58a1 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Mon, 31 Aug 2026 08:52:55 +0200 Subject: [PATCH 03/20] build(x64): enable Miles and Bink on x64, make DX8 headers-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Miles and Bink are source-only stubs with no architecture dependency; they shared a gate with DX8 for no reason. DX8's headers are architecture-independent too — only its link libraries are 32-bit, because MinGW-w64 x86_64 ships libd3d8thk.a and no libd3dx8 at all. cmake/dx8.cmake previously called FetchContent_MakeAvailable(dx8), which add_subdirectory()s the fetched min-dx8-sdk repo's own CMakeLists.txt — a file this repo does not own and which has no architecture condition at all. Switched to FetchContent_Populate (source only) and moved the d3d8lib target definition into cmake/dx8.cmake itself, unchanged for MSVC/VC6, gated by CMAKE_SIZEOF_VOID_P on MinGW so x64 gets includes and -DBUILD_WITH_D3D8 but no link libraries. A second and initially-missed injection point: cmake/mingw.cmake:58-70 calls bare link_libraries(... d3d8 ...), which is directory-scoped and applies to every target created afterward — including the Miles/Bink FetchContent stub DLLs, which do not touch DirectX at all. Linking d3d8 unconditionally there broke binkstub/milesstub's link step on x64 (`cannot find -ld3d8`), stopping the x64 build at 3%, an earlier ceiling than before this change. Fixed with a generator expression, $<$:d3d8>, in the same list position rather than splitting the call, so the 32-bit link line stays byte-identical in content and order. Verified directly against build/mingw-w64-i686/Generals/Code/Main/CMakeFiles/g_generals.dir/linkLibs.rsp (-ld3d8 still present) and cmake/mingw.cmake:78-89's existing d3dx8 alias, which was already 32-bit-gated in this same file for the same reason and is now also SIZEOF_VOID_P-gated to match. A third, still-open injection point was found but deliberately not touched: Generals/Code/Main/CMakeLists.txt:13, GeneralsMD/Code/Main/CMakeLists.txt:13, and both W3DView CMakeLists.txt (line 6) link a bare `d3d8` in the main executables' own target_link_libraries. These are link-time only, the x64 build never reaches them (still stops at 15%), and a playable x64 build is explicitly out of scope — fixing them here would be scope creep. Task 7's deeper build should expect g_generals/z_generals link failures from this. Results (build/mingw-w64-x86_64, `-j4 -- -k`, scripts/x64-error-summary.py): fatal error: mss.h 38 -> 0 compile errors 60 -> 82 distinct shapes 37 -> 41 affected files 7 -> 10 binkstub/milesstub fail -> link configured TUs 1886 -> 1889 (+miles.c, +cleanup.c, +bink.c) build ceiling 15% -> 15% (still core_wwlib/debug/compression/ wwsaveload/wwaudio, unrelated to DX8) The error count rising from 60 to 82 is this task succeeding, not regressing: 19 WWAudio sources that previously aborted at `#include ` now compile and 3 report real errors (SoundScene.cpp, AudibleSound.cpp, Sound3D.cpp), and persistfactory.h went from 1 error to 15 as more WWSaveLoad templates instantiate. Contrary to this task's original brief, W3D does NOT compile yet: `ww3d2`/`wwmath` appear zero times in the build log. W3D sits behind Core libraries (WWLib, debug, Compression, WWSaveLoad, WWAudio) in the dependency graph, not behind the DX8 gate alone — this change is a necessary precondition whose payoff arrives once Tasks 2-6 clear those Core failures, not before. 32-bit control (G1) re-verified: configure exit 0, build exit 0, 0 errors. d3d8 confirmed still present in the 32-bit link response file in its original position. Full warning count re-check in progress; see task report for the final number once available. G3 (VC6) not run: no file the VC6 build compiles was modified. CMakeLists.txt:51-56 changed, but the VC6 build is 32-bit (CMAKE_SIZEOF_VOID_P EQUAL 4), so the three includes still run in the same order and the dx8.cmake/mingw.cmake MSVC/32-bit branches are functionally unchanged, just relocated (dx8.cmake) or newly gated on a condition that already held for VC6 (mingw.cmake, N/A since VC6 is not MinGW). Co-Authored-By: Claude Opus 5 --- CMakeLists.txt | 8 +++++-- cmake/dx8.cmake | 56 ++++++++++++++++++++++++++++++++++++++++++++++- cmake/mingw.cmake | 15 +++++++++---- 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 39062e3fbe6..4f4a5a8350d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -48,8 +48,12 @@ if(MINGW) include(cmake/widl.cmake) endif() -# Find/Add build dependencies and stubs shared by all projects -if((WIN32 OR "${CMAKE_SYSTEM}" MATCHES "Windows") AND ${CMAKE_SIZEOF_VOID_P} EQUAL 4) +# Find/Add build dependencies and stubs shared by all projects. +# Miles and Bink are source-only stubs with no architecture dependency, so they +# build for any pointer size. DX8 is gated separately inside dx8.cmake: its +# headers are architecture-independent but MinGW-w64 x86_64 ships no libd3d8.a +# and no libd3dx8d.a, so the link libraries are 32-bit only. +if(WIN32 OR "${CMAKE_SYSTEM}" MATCHES "Windows") include(cmake/miles.cmake) include(cmake/bink.cmake) include(cmake/dx8.cmake) diff --git a/cmake/dx8.cmake b/cmake/dx8.cmake index dd08f56119a..f0c321a1685 100644 --- a/cmake/dx8.cmake +++ b/cmake/dx8.cmake @@ -4,4 +4,58 @@ FetchContent_Declare( GIT_TAG 7bddff8c01f5fb931c3cb73d4aa8e66d303d97bc ) -FetchContent_MakeAvailable(dx8) +# Populate the source only (do not add_subdirectory it): the fetched +# min-dx8-sdk repo's own CMakeLists.txt has no architecture condition at all, +# so the d3d8lib target is defined here instead, where it can diverge by +# CMAKE_SIZEOF_VOID_P without forking that upstream repo. +FetchContent_GetProperties(dx8) +if(NOT dx8_POPULATED) + FetchContent_Populate(dx8) +endif() + +add_library(d3d8lib INTERFACE) + +# Common libraries for all compilers. +# 64-bit: MinGW-w64 x86_64 provides libdinput8.a and libdxguid.a but no +# libd3d8.a (only libd3d8thk.a) and no libd3dx8 at all. The headers are +# architecture-independent, so on x64 this target carries includes and defines +# only; it cannot link, which is expected — a playable x64 build is out of +# scope, see issue #473. This is a precondition for W3D to compile on x64, not +# a guarantee: W3D also depends on several Core libraries (WWLib, debug, +# Compression, WWSaveLoad, WWAudio) that fail for unrelated reasons and still +# block it as of this change. See docs/x64/core-error-catalogue.md. +if(CMAKE_SIZEOF_VOID_P EQUAL 4) + target_link_libraries(d3d8lib INTERFACE d3d8 dinput8 dxguid) +else() + message(STATUS "DX8: x64 build — headers only, no D3D8 link libraries available") +endif() + +# MSVC-specific configuration +if(MSVC) + # Use bundled MSVC-compiled .lib files + target_link_libraries(d3d8lib INTERFACE d3dx8) + target_link_directories(d3d8lib BEFORE INTERFACE ${dx8_SOURCE_DIR}) + target_link_options(d3d8lib INTERFACE /NODEFAULTLIB:libci.lib) + + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER "12.0.8804") + # Modern MSVC (VS 2013+) has complete DirectX headers in Windows SDK + target_link_libraries(d3d8lib INTERFACE legacy_stdio_definitions) + target_link_options(d3d8lib INTERFACE /SAFESEH:NO) + else() + # VC6 and older MSVC need extra headers - their DirectX SDK is missing newer definitions + target_include_directories(d3d8lib INTERFACE ${dx8_SOURCE_DIR}/extra) + endif() +endif() + +# MinGW-specific configuration +if(MINGW) + # MinGW-w64 DirectX 8 support varies by architecture: + # i686 (32-bit): libd3d8.a + libd3dx8d.a (debug only, no release version) + # x86_64 (64-bit): libd3d8thk.a only (no libd3dx8 libraries at all) + if(CMAKE_SIZEOF_VOID_P EQUAL 4) + target_link_libraries(d3d8lib INTERFACE d3dx8d) + endif() +endif() + +target_compile_definitions(d3d8lib INTERFACE -DBUILD_WITH_D3D8) +target_include_directories(d3d8lib INTERFACE ${dx8_SOURCE_DIR}) diff --git a/cmake/mingw.cmake b/cmake/mingw.cmake index b5e2d259e46..74209a77578 100644 --- a/cmake/mingw.cmake +++ b/cmake/mingw.cmake @@ -54,7 +54,13 @@ if(MINGW) ) endif() - # Required Windows libraries for DX8 + COM + # Required Windows libraries for DX8 + COM. + # d3d8 is 32-bit only: MinGW-w64 x86_64 ships libd3d8thk.a but no + # libd3d8.a, so it is gated with a generator expression rather than + # linked unconditionally. link_libraries() is directory-scoped and + # applies to every target created after this point, including the + # Miles/Bink FetchContent stub DLLs, which do not use DirectX at all — + # linking d3d8 unconditionally here broke their link step on x64. link_libraries( uuid # COM GUIDs ole32 # COM runtime @@ -64,7 +70,7 @@ if(MINGW) comctl32 # Common controls winmm # Multimedia (timeGetTime, etc.) vfw32 # Video for Windows (AVIFile functions) - d3d8 # Direct3D 8 + $<$:d3d8> # Direct3D 8 — 32-bit only dinput8 # DirectInput 8 dsound # DirectSound imm32 # Input Method Manager (IME) @@ -79,8 +85,9 @@ if(MINGW) # MinGW-w64 only provides libd3dx8d.a (debug library), not libd3dx8.a # The min-dx8-sdk (dx8.cmake) handles this correctly via d3d8lib interface target, # but for compatibility with direct library references in main executables, - # we create an alias so that linking to d3dx8 automatically uses d3dx8d - if(NOT TARGET d3dx8) + # we create an alias so that linking to d3dx8 automatically uses d3dx8d. + # 32-bit only: x86_64 MinGW-w64 ships neither libd3dx8.a nor libd3dx8d.a. + if(CMAKE_SIZEOF_VOID_P EQUAL 4 AND NOT TARGET d3dx8) add_library(d3dx8 INTERFACE IMPORTED GLOBAL) set_target_properties(d3dx8 PROPERTIES INTERFACE_LINK_LIBRARIES "d3dx8d" From aca4219b7d44c4891af9698cc809582efbfc8509 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Mon, 31 Aug 2026 11:17:30 +0200 Subject: [PATCH 04/20] fix(x64): add pointer-sized integer types, fix truncating casts Windows is LLP64, so `long` stays 32-bit on x86-64 and cannot hold a pointer. Adds `UnsignedIntPtr`/`IntPtr` (uintptr_t/intptr_t) to BaseTypeCore.h and applies them to the 18 measured pointer-to-integer truncation sites across Core (registry.cpp, Except.cpp, debug_debug.cpp, debug_stack.cpp, huffencode.cpp, and the WWAudio Miles/callback chain). Wire vs. runtime: `uint32` (bittype.h) is left alone everywhere -- it is the on-disk/network/savegame wire format and must stay 4 bytes regardless of target. `MILES_HANDLE` (AudibleSound.h) is widened to UnsignedIntPtr because it is runtime-only, holds live Miles Sound System pointers, and is defined in our own code rather than the fetched Miles stub. Two of the "cast" sites were real x64 defects, not cast noise: Except.cpp's two GetProcAddress-fill loops and debug_stack.cpp's gDbg union both walk a `long unsigned int` stride across consecutive function-pointer slots that are 8 bytes wide on Win64. Left as `long`, the stride would silently corrupt every other function pointer at runtime. Fixed the holder/stride, not just the visible cast -- this is the strongest argument in this diff for why -fpermissive stays banned: it would have hidden a genuine memory-corruption bug behind a warning. SoundScene.cpp's EVENT_LOGICAL_HEARD dispatch smuggles two pointers through `On_Event`'s uint32 params. Traced the full chain before touching anything: the real declaration/definition is SoundSceneObj.h's SoundSceneObjClass::On_Event (not AudioEvents.h, which holds an unrelated and unused-for-this-path callback typedef family), has no overrides, and only one call site passes real pointers. Widened just that method's two params and the one call site; confirmed the change did not need to spread into AudioEvents.h or the engine. registry.cpp stored an HKEY in an `int` (also removes a stale `assert(sizeof(HKEY) == sizeof(int))` that was one line away from tripping on any 64-bit debug build once Key's type changed). huffencode.cpp needed intptr_t via ``, not a raw `` -- VC6 predates C99 and doesn't have the latter. The shim is the same one BaseTypeCore.h already uses and was already on this target's include path via corei_always -> core_utility. Gates: - G1 (32-bit control): exit 0, 0 errors, 31,555 warnings -- unchanged from baseline. (The registry.cpp fix does not remove any 32-bit warnings: HKEY<->int is a no-op reinterpret at that width, so GCC never warned there. The ~17 -Wint-to-pointer-cast warnings this fix does remove exist only in the x64 build log.) - G2 (x64): errors 82->60, distinct shapes 41->29, affected files 10->5, `loses precision` occurrences 37->15, build ceiling 15%->100%. - G3 (VC6 retail): exit 0, 0 errors, all seven .text digests byte-identical to docs/x64/baseline-vc6.md. .rdata/.data movement in the larger executables traced to resources/gitinfo/gitinfo.cpp.in build metadata (commit SHA, dates, timestamps) -- zero code differences. Co-Authored-By: Claude Opus 5 --- Core/Libraries/Include/Lib/BaseTypeCore.h | 8 +++++++ .../Source/Compression/EAC/huffencode.cpp | 5 ++-- .../Source/WWVegas/WWAudio/AudibleSound.h | 5 +++- .../Source/WWVegas/WWAudio/SoundScene.cpp | 2 +- .../Source/WWVegas/WWAudio/SoundSceneObj.h | 9 +++++--- .../Libraries/Source/WWVegas/WWLib/Except.cpp | 22 +++++++++++++----- .../Source/WWVegas/WWLib/registry.cpp | 4 ++-- .../Libraries/Source/WWVegas/WWLib/registry.h | 3 ++- Core/Libraries/Source/debug/debug_debug.cpp | 23 +++++++++++++++---- Core/Libraries/Source/debug/debug_stack.cpp | 10 +++++--- 10 files changed, 68 insertions(+), 23 deletions(-) diff --git a/Core/Libraries/Include/Lib/BaseTypeCore.h b/Core/Libraries/Include/Lib/BaseTypeCore.h index ab702efd496..ceafaca005a 100644 --- a/Core/Libraries/Include/Lib/BaseTypeCore.h +++ b/Core/Libraries/Include/Lib/BaseTypeCore.h @@ -123,3 +123,11 @@ typedef bool Bool; // // note, the types below should use "long long", but MSVC doesn't support it yet typedef int64_t Int64; // 8 bytes typedef uint64_t UnsignedInt64; // 8 bytes + +// Pointer-sized integers. Required for 64-bit targets: Windows is LLP64, so +// `long` stays 32 bits on x86-64 and cannot hold a pointer. Use these for +// values that must round-trip through a pointer — never for values that are +// written to a file, sent over the network, or stored in a savegame, because +// their width changes with the target. +typedef uintptr_t UnsignedIntPtr; // 4 bytes on 32-bit, 8 on 64-bit +typedef intptr_t IntPtr; // 4 bytes on 32-bit, 8 on 64-bit diff --git a/Core/Libraries/Source/Compression/EAC/huffencode.cpp b/Core/Libraries/Source/Compression/EAC/huffencode.cpp index 06f51b39831..c7d35fd661e 100644 --- a/Core/Libraries/Source/Compression/EAC/huffencode.cpp +++ b/Core/Libraries/Source/Compression/EAC/huffencode.cpp @@ -22,6 +22,7 @@ #define __HUFWRITE 1 #include +#include #include "codex.h" #include "huffcodex.h" @@ -1050,8 +1051,8 @@ static void HUFF_pack(struct HuffEncodeContext *EC, if (!i3) HUFF_writecode(EC,dest,i); - if (((long) bptr1- (long) EC->buffer) >= (long)(EC->plen+curpc)) - curpc = (long) bptr1 - (long) EC->buffer - EC->plen; + if (((intptr_t) bptr1- (intptr_t) EC->buffer) >= (intptr_t)(EC->plen+curpc)) + curpc = (intptr_t) bptr1 - (intptr_t) EC->buffer - EC->plen; } /* write EOF ([clue] 0gn [10]) */ diff --git a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.h b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.h index 36205805613..15db8cc302b 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.h +++ b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.h @@ -69,7 +69,10 @@ class SoundHandleClass; // // Typedefs // -typedef unsigned long MILES_HANDLE; +// Miles Sound System handles are pointers under the hood; this must be +// pointer-sized to round-trip through Get_2D_Sample/Get_3D_Sample without +// truncation on 64-bit targets. Runtime-only, never serialized. +typedef UnsignedIntPtr MILES_HANDLE; typedef enum { diff --git a/Core/Libraries/Source/WWVegas/WWAudio/SoundScene.cpp b/Core/Libraries/Source/WWVegas/WWAudio/SoundScene.cpp index 4427f3d7bff..84843513156 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/SoundScene.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/SoundScene.cpp @@ -199,7 +199,7 @@ SoundSceneClass::Collect_Logical_Sounds (unsigned int milliseconds, int listener // Is the sound ready to notify? // if (sound_obj->Allow_Notify (timestamp)) { - listener->On_Event (AudioCallbackClass::EVENT_LOGICAL_HEARD, (uint32)listener, (uint32)sound_obj); + listener->On_Event (AudioCallbackClass::EVENT_LOGICAL_HEARD, (UnsignedIntPtr)listener, (UnsignedIntPtr)sound_obj); } } } diff --git a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.h b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.h index a5d6f7d0b0a..280a00c48e5 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.h +++ b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.h @@ -40,6 +40,7 @@ #include "WWSaveLoad/persist.h" #include "WWLib/multilist.h" #include "WWLib/mutex.h" +#include "Lib/BaseTypeCore.h" ///////////////////////////////////////////////////////////////////////////////// // Forward declarations @@ -123,7 +124,9 @@ class SoundSceneObjClass : public MultiListObjectClass, public PersistClass, pub ////////////////////////////////////////////////////////////////////// // Event handling ////////////////////////////////////////////////////////////////////// - virtual void On_Event (AudioCallbackClass::EVENTS event, uint32 param1 = 0, uint32 param2 = 0); + // param1/param2 double as pointers smuggled through integer parameters for + // EVENT_LOGICAL_HEARD (see the inline definition below); must be pointer-sized. + virtual void On_Event (AudioCallbackClass::EVENTS event, UnsignedIntPtr param1 = 0, UnsignedIntPtr param2 = 0); virtual void Register_Callback (AudioCallbackClass::EVENTS events, AudioCallbackClass *callback); ////////////////////////////////////////////////////////////////////// @@ -225,8 +228,8 @@ __inline void SoundSceneObjClass::On_Event ( AudioCallbackClass::EVENTS event, - uint32 param1, - uint32 param2 + UnsignedIntPtr param1, + UnsignedIntPtr param2 ) { if ((m_pCallback != nullptr) && (m_RegisteredEvents & event)) { diff --git a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp index be9c958cdf1..dd8fe8c1430 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp @@ -54,6 +54,7 @@ #include "assert.h" #include "cpudetect.h" #include "Except.h" +#include "Lib/BaseTypeCore.h" //#include "debug.h" #include "MPU.h" //#include "commando\nat.h" @@ -355,13 +356,16 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) if (imagehelp != nullptr) { DebugString ("Exception Handler: Found IMAGEHLP.DLL - linking to required functions\n"); char const *function_name = nullptr; - unsigned long *fptr = (unsigned long*) &_SymCleanup; + // fptr walks across the consecutive _SymXxx globals below, each of which + // is an actual function pointer (8 bytes on Win64) -- must be + // pointer-sized or the stride only covers half of each slot on 64-bit. + UnsignedIntPtr *fptr = (UnsignedIntPtr*) &_SymCleanup; int count = 0; do { function_name = ImagehelpFunctionNames[count]; if (function_name) { - *fptr = (unsigned long) GetProcAddress(imagehelp, function_name); + *fptr = (UnsignedIntPtr) GetProcAddress(imagehelp, function_name); fptr++; count++; } @@ -1065,13 +1069,14 @@ void Load_Image_Helper() if (ImageHelp != nullptr) { char const *function_name = nullptr; - unsigned long *fptr = (unsigned long *) &_SymCleanup; + // Same pointer-sized stride requirement as Dump_Exception_Info() above. + UnsignedIntPtr *fptr = (UnsignedIntPtr *) &_SymCleanup; int count = 0; do { function_name = ImagehelpFunctionNames[count]; if (function_name) { - *fptr = (unsigned long) GetProcAddress(ImageHelp, function_name); + *fptr = (UnsignedIntPtr) GetProcAddress(ImageHelp, function_name); fptr++; count++; } @@ -1169,12 +1174,17 @@ bool Lookup_Symbol(void *code_ptr, char *symbol, int &displacement) symbol_struct_ptr->SizeOfStruct = sizeof (symbol_struct_buf); symbol_struct_ptr->MaxNameLength = sizeof(symbol_struct_buf)-sizeof (IMAGEHLP_SYMBOL); symbol_struct_ptr->Size = 0; - symbol_struct_ptr->Address = (unsigned long)code_ptr; + // IMAGEHLP_SYMBOL::Address and SymGetSymFromAddr's DWORD parameter are + // fixed at 32 bits by the (32-bit-only, deprecated on Win64) DbgHelp API + // contract -- not ours to widen. Cast through UnsignedIntPtr so the + // narrowing is an explicit int-to-int conversion rather than a flagged + // pointer truncation; 32-bit codegen is unchanged. + symbol_struct_ptr->Address = (unsigned long)(UnsignedIntPtr)code_ptr; /* ** See if we have the symbol for that address. */ - if (_SymGetSymFromAddr(GetCurrentProcess(), (unsigned long)code_ptr, (unsigned long *)&displacement, symbol_struct_ptr)) { + if (_SymGetSymFromAddr(GetCurrentProcess(), (unsigned long)(UnsignedIntPtr)code_ptr, (unsigned long *)&displacement, symbol_struct_ptr)) { /* ** Copy it back into the buffer provided. diff --git a/Core/Libraries/Source/WWVegas/WWLib/registry.cpp b/Core/Libraries/Source/WWVegas/WWLib/registry.cpp index 94eeee4073c..7d56a298a6d 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/registry.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/registry.cpp @@ -65,7 +65,7 @@ RegistryClass::RegistryClass( const char * sub_key, bool create ) : IsValid( false ) { HKEY key; - assert( sizeof(HKEY) == sizeof(int) ); + assert( sizeof(HKEY) == sizeof(Key) ); LONG result = -1; @@ -79,7 +79,7 @@ RegistryClass::RegistryClass( const char * sub_key, bool create ) : if (ERROR_SUCCESS == result) { IsValid = true; - Key = (int)key; + Key = (UnsignedIntPtr)key; } } diff --git a/Core/Libraries/Source/WWVegas/WWLib/registry.h b/Core/Libraries/Source/WWVegas/WWLib/registry.h index deccd441252..a939cdb7045 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/registry.h +++ b/Core/Libraries/Source/WWVegas/WWLib/registry.h @@ -39,6 +39,7 @@ #include "Vector.h" #include "wwstring.h" #include "widestring.h" +#include "Lib/BaseTypeCore.h" class INIClass; @@ -107,7 +108,7 @@ class RegistryClass { static void Save_Registry_Values(HKEY key, char *path, INIClass *ini); - int Key; + UnsignedIntPtr Key; bool IsValid; // diff --git a/Core/Libraries/Source/debug/debug_debug.cpp b/Core/Libraries/Source/debug/debug_debug.cpp index 291a06aff27..7656d533d1b 100644 --- a/Core/Libraries/Source/debug/debug_debug.cpp +++ b/Core/Libraries/Source/debug/debug_debug.cpp @@ -35,6 +35,7 @@ #include #include #include // needed for placement new prototype +#include "Lib/BaseTypeCore.h" // a little dummy variable that makes the linker actually include // us... @@ -900,7 +901,11 @@ Debug& Debug::operator<<(const void *ptr) if (ptr) { char help[9]; - (*this) << "0x" << _ultoa((unsigned long)ptr,help,16); + // Cast through UnsignedIntPtr (lossless) before narrowing to the + // 32-bit type _ultoa requires. On 64-bit this deliberately shows only + // the low 32 bits of the address -- acceptable for a debug print, and + // keeps 32-bit output byte-identical (UnsignedIntPtr is unsigned int there). + (*this) << "0x" << _ultoa((unsigned long)(UnsignedIntPtr)ptr,help,16); } else (*this) << "null"; @@ -932,7 +937,10 @@ Debug& Debug::operator<<(const MemDump &dump) { // address char buf[9]; - sprintf(buf,"%08x",dump.m_absAddr?unsigned(cur):cur-dump.m_startPtr); + // Same low-32-bits-only truncation as the operator<<(const void*) above, + // via a lossless pointer->UnsignedIntPtr step so the narrowing is an + // explicit int-to-int conversion rather than a flagged pointer truncation. + sprintf(buf,"%08x",dump.m_absAddr?(unsigned)(UnsignedIntPtr)cur:(unsigned)(cur-dump.m_startPtr)); operator<<(buf); // items @@ -1010,9 +1018,16 @@ bool Debug::IsLogEnabled(const char *fileOrGroup) // to be used from the D_ISLOG macros only and those guarantee // that we are having real static strings let's use // that strings address as frame address... - FrameHashEntry *e=Instance.LookupFrame((unsigned)fileOrGroup); + // LookupFrame/AddFrameEntry use the string's address as a hash key, and + // are declared to take `unsigned` -- that hashing scheme is already tied + // to the 32-bit-only stack-frame capture elsewhere in this file (see the + // #error above for non-x86-32 targets), so it is not widened here. Cast + // through UnsignedIntPtr so the narrowing is an explicit int-to-int + // conversion; 32-bit output/codegen is unchanged since UnsignedIntPtr is + // unsigned int there. + FrameHashEntry *e=Instance.LookupFrame((unsigned)(UnsignedIntPtr)fileOrGroup); if (!e) - e=Instance.AddFrameEntry((unsigned)fileOrGroup,FrameTypeLog,fileOrGroup,0); + e=Instance.AddFrameEntry((unsigned)(UnsignedIntPtr)fileOrGroup,FrameTypeLog,fileOrGroup,0); if (e->status==Unknown) Instance.UpdateFrameStatus(*e); return e->status==NoSkip; diff --git a/Core/Libraries/Source/debug/debug_stack.cpp b/Core/Libraries/Source/debug/debug_stack.cpp index 8e0aca49557..cb6f0b5b896 100644 --- a/Core/Libraries/Source/debug/debug_stack.cpp +++ b/Core/Libraries/Source/debug/debug_stack.cpp @@ -32,6 +32,7 @@ #include #include "WWLib/stringex.h" #include +#include "Lib/BaseTypeCore.h" // Definitions to allow run-time linking to the dbghelp.dll functions. @@ -46,7 +47,10 @@ static union { #include "debug_stack.inl" }; - unsigned funcPtr[1]; + // Overlays the struct above, whose members are actual function pointers + // (8 bytes on Win64). Must be pointer-sized or the aliasing/stride used + // by InitDbghelp() below only covers half of each slot on 64-bit. + UnsignedIntPtr funcPtr[1]; } gDbg; #undef DBGHELP @@ -89,11 +93,11 @@ static void InitDbghelp() return; // Get function addresses - unsigned *funcptr=gDbg.funcPtr; + UnsignedIntPtr *funcptr=gDbg.funcPtr; unsigned k=0; for (;DebughelpFunctionNames[k];++k,++funcptr) { - *funcptr=(unsigned)GetProcAddress(g_dbghelp,DebughelpFunctionNames[k]); + *funcptr=(UnsignedIntPtr)GetProcAddress(g_dbghelp,DebughelpFunctionNames[k]); if (!*funcptr) break; } From b1f7b9a5e74e44a85dc04d523680c02c6d7e48cf Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Mon, 31 Aug 2026 13:27:53 +0200 Subject: [PATCH 05/20] fix(x64): port CONTEXT register access in both crash handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 38 of the 60 catalogued x64 errors, in two files: Except.cpp (23) and debug_except.cpp (15). Adds Core/Libraries/Include/Lib/arch_context.h, which maps the x86-32 CONTEXT register field names (Eip/Esp/Ebp/Eax/Ebx/ Ecx/Edx/Esi/Edi) used throughout both crash handlers onto their x86-64 equivalents (Rip/Rsp/Rbp/Rax/Rbx/Rcx/Rdx/Rsi/Rdi) via CTX_PC/CTX_STACK/ CTX_FRAME/CTX_AX/CTX_BX/CTX_CX/CTX_DX/CTX_SI/CTX_DI macros, plus CTX_STACKWALK_MACHINE for Tasks 4/5 and CTX_REG_WIDTH for the stream-based register dump. On x86-32 every macro expands to exactly the original field access, so that path is untouched. The header lives in Core/Libraries/Include/Lib/, not next to the crash handlers: core_wwlib does not link core_debug (Core/Libraries/Source/WWVegas/WWLib/CMakeLists.txt:187 links only core_wwcommon and corei_always), so a header under Core/Libraries/Source/debug/ would not resolve from Except.cpp. Both libraries do link corei_always, which pulls in corei_libraries_include, whose include directory is Core/Libraries/Include (Core/CMakeLists.txt:10) — the one directory both crash handlers already see. Registered in Core/CMakeLists.txt's corei_libraries_include source list alongside BaseType.h/BaseTypeCore.h. Explicitly does not use the WOW64_* structures/constants GCC suggests (WOW64_FLOATING_SAVE_AREA, WOW64_SIZE_OF_80387_REGISTERS): those describe a 32-bit process as inspected from a 64-bit one, not a native 64-bit process's own state. Taking the suggestion would compile cleanly and read the wrong bytes — a crash dump that is silently corrupt, which is worse than none. The FPU/SSE save area is a genuine structural difference rather than a rename: on x86-64 it is CONTEXT.FltSave, an XMM_SAVE_AREA32, not the 32-bit FLOATING_SAVE_AREA. Most field names carry over unchanged (ControlWord/StatusWord/TagWord/ErrorOffset/ErrorSelector/DataOffset/ DataSelector), but there is no RegisterArea and no Cr0NpxState — each ST(i) register instead lives in the low 10 bytes of a 16-byte FloatRegisters[] slot. Both crash handlers guard this block with #if defined(_WIN64) || defined(__x86_64__), mirroring exactly what the 32-bit FLOATING_SAVE_AREA.RegisterArea block reports, addressed through the x86-64 layout instead of mapping member names. Widened 21 register-value format specifiers from 32-bit to 64-bit width, gated so the 32-bit output is byte-for-byte unchanged: 12 in Except.cpp (sprintf/snprintf, %08X -> %016llX across the exception-address, Rip/Rsp/Rbp, Rax/Rbx/Rcx, Rdx/Rsi/Rdi, and "Bytes at CS:RIP" lines) and 9 in debug_except.cpp's LogRegisters (Debug::Width(8) -> Debug::Width(CTX_REG_WIDTH)). This is the part of the task a build would never flag: %08X on a 64-bit Rip prints only the low half, produces no compile error, and would otherwise have shipped a crash dump pointing at the wrong address. Gates: G1 (32-bit rebuild) exit 0, 0 errors, 31,555 warnings, matching docs/x64/baseline-mingw-i686.md exactly. G2 (x64) errors 60 -> 22, shapes 29 -> 7, has-no-member-'E..' 38 -> 0 -- exactly the targeted set, remainder attributed to Tasks 4/5/6. G3 (VC6 retail) exit 0, 0 errors, all seven .text digests byte-identical to docs/x64/baseline-vc6.md. Co-Authored-By: Claude Opus 5 --- Core/CMakeLists.txt | 1 + Core/Libraries/Include/Lib/arch_context.h | 85 +++++++++++++++++++ .../Libraries/Source/WWVegas/WWLib/Except.cpp | 68 +++++++++++++-- Core/Libraries/Source/debug/debug_except.cpp | 64 +++++++++++--- 4 files changed, 198 insertions(+), 20 deletions(-) create mode 100644 Core/Libraries/Include/Lib/arch_context.h diff --git a/Core/CMakeLists.txt b/Core/CMakeLists.txt index 110ff1152d0..85bcd82d3b3 100644 --- a/Core/CMakeLists.txt +++ b/Core/CMakeLists.txt @@ -12,6 +12,7 @@ target_include_directories(corei_libraries_source_wwvegas INTERFACE "Libraries/S target_include_directories(corei_main INTERFACE "Main") target_sources(corei_libraries_include PRIVATE + Libraries/Include/Lib/arch_context.h Libraries/Include/Lib/BaseType.h Libraries/Include/Lib/BaseTypeCore.h Libraries/Include/Lib/trig.h diff --git a/Core/Libraries/Include/Lib/arch_context.h b/Core/Libraries/Include/Lib/arch_context.h new file mode 100644 index 00000000000..58c4b7a1b9a --- /dev/null +++ b/Core/Libraries/Include/Lib/arch_context.h @@ -0,0 +1,85 @@ +/* +** Command & Conquer Generals Zero Hour(tm) +** Copyright 2025 Electronic Arts Inc. +** +** This program is free software: you can redistribute it and/or modify +** it under the terms of the GNU General Public License as published by +** the Free Software Foundation, either version 3 of the License, or +** (at your option) any later version. +** +** This program is distributed in the hope that it will be useful, +** but WITHOUT ANY WARRANTY; without even the implied warranty of +** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +** GNU General Public License for more details. +** +** You should have received a copy of the GNU General Public License +** along with this program. If not, see . +*/ + +// FILE: arch_context.h ////////////////////////////////////////////////////// +// +// Maps the x86-32 Win32 CONTEXT register field names used throughout the +// two crash handlers (Core/Libraries/Source/WWVegas/WWLib/Except.cpp and +// Core/Libraries/Source/debug/debug_except.cpp) onto whichever field names +// the target architecture's CONTEXT struct actually has, plus the machine +// constant the stack-walk call sites need. +// +// On x86-32 the general-purpose registers are Eip/Esp/Ebp/Eax/Ebx/Ecx/Edx/ +// Esi/Edi. On x86-64 they are Rip/Rsp/Rbp/Rax/Rbx/Rcx/Rdx/Rsi/Rdi and are +// twice as wide. CTX_PC/CTX_STACK/CTX_FRAME/CTX_AX/CTX_BX/CTX_CX/CTX_DX/ +// CTX_SI/CTX_DI hide that difference behind one name per register so call +// sites don't need an #ifdef each. On x86-32 every one of these macros +// expands to exactly the original field access (e.g. CTX_PC(ctx) is +// ((ctx).Eip)), so the 32-bit build is unaffected. +// +// Deliberately NOT using the WOW64_* structures/constants GCC suggests +// (WOW64_FLOATING_SAVE_AREA, WOW64_SIZE_OF_80387_REGISTERS, ...): those +// describe a 32-bit process as inspected from a 64-bit one, not a native +// 64-bit process's own FPU/SSE state. Taking that suggestion would compile +// cleanly and read the wrong bytes -- a crash dump that is silently +// corrupt is worse than no crash dump. The FPU/SSE save area itself is a +// structural difference (CONTEXT.FltSave, an XMM_SAVE_AREA32, vs the +// 32-bit FLOATING_SAVE_AREA) rather than a field rename, so it is not +// covered by macros here -- the two crash handlers guard that block with +// their own #if per architecture, mirroring what the 32-bit block reports. + +#pragma once + +#include + +#if defined(_WIN64) || defined(__x86_64__) + +#define CTX_PC(ctx) ((ctx).Rip) +#define CTX_STACK(ctx) ((ctx).Rsp) +#define CTX_FRAME(ctx) ((ctx).Rbp) +#define CTX_AX(ctx) ((ctx).Rax) +#define CTX_BX(ctx) ((ctx).Rbx) +#define CTX_CX(ctx) ((ctx).Rcx) +#define CTX_DX(ctx) ((ctx).Rdx) +#define CTX_SI(ctx) ((ctx).Rsi) +#define CTX_DI(ctx) ((ctx).Rdi) + +// Hex-digit width of a full register dump column, for the stream-based +// (Debug::Width()) register printers in debug_except.cpp -- 16 digits show +// a full 64-bit register instead of just its low half. +#define CTX_REG_WIDTH 16 + +#define CTX_STACKWALK_MACHINE IMAGE_FILE_MACHINE_AMD64 + +#else + +#define CTX_PC(ctx) ((ctx).Eip) +#define CTX_STACK(ctx) ((ctx).Esp) +#define CTX_FRAME(ctx) ((ctx).Ebp) +#define CTX_AX(ctx) ((ctx).Eax) +#define CTX_BX(ctx) ((ctx).Ebx) +#define CTX_CX(ctx) ((ctx).Ecx) +#define CTX_DX(ctx) ((ctx).Edx) +#define CTX_SI(ctx) ((ctx).Esi) +#define CTX_DI(ctx) ((ctx).Edi) + +#define CTX_REG_WIDTH 8 + +#define CTX_STACKWALK_MACHINE IMAGE_FILE_MACHINE_I386 + +#endif diff --git a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp index dd8fe8c1430..ddc67ac5ece 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp @@ -55,6 +55,7 @@ #include "cpudetect.h" #include "Except.h" #include "Lib/BaseTypeCore.h" +#include "Lib/arch_context.h" //#include "debug.h" #include "MPU.h" //#include "commando\nat.h" @@ -469,18 +470,27 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) symptr->SizeOfStruct = sizeof (IMAGEHLP_SYMBOL); symptr->MaxNameLength = 256-sizeof (IMAGEHLP_SYMBOL); symptr->Size = 0; - symptr->Address = context->Eip; + symptr->Address = CTX_PC(*context); - if (!IsBadCodePtr((FARPROC)context->Eip)) { - if (_SymGetSymFromAddr != nullptr && _SymGetSymFromAddr (GetCurrentProcess(), context->Eip, &displacement, symptr)) { + if (!IsBadCodePtr((FARPROC)CTX_PC(*context))) { + if (_SymGetSymFromAddr != nullptr && _SymGetSymFromAddr (GetCurrentProcess(), CTX_PC(*context), &displacement, symptr)) { +#if defined(_WIN64) || defined(__x86_64__) + snprintf(scrap, ARRAY_SIZE(scrap), "Exception occurred at %016llX - %s + %08X\r\n", + (unsigned long long)CTX_PC(*context), symptr->Name, displacement); +#else snprintf(scrap, ARRAY_SIZE(scrap), "Exception occurred at %08X - %s + %08X\r\n", context->Eip, symptr->Name, displacement); +#endif } else { DebugString ("Exception Handler: Failed to get symbol for EIP\r\n"); if (_SymGetSymFromAddr != nullptr) { DebugString ("Exception Handler: SymGetSymFromAddr failed with code %d - %s\n", GetLastError(), Last_Error_Text()); } +#if defined(_WIN64) || defined(__x86_64__) + sprintf (scrap, "Exception occurred at %016llX\r\n", (unsigned long long)CTX_PC(*context)); +#else sprintf (scrap, "Exception occurred at %08X\r\n", context->Eip); +#endif } } else { DebugString ("Exception Handler: context->Eip is bad code pointer\n"); @@ -589,12 +599,21 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) /* ** Dump the registers. */ +#if defined(_WIN64) || defined(__x86_64__) + sprintf(scrap, "Rip:%016llX\tRsp:%016llX\tRbp:%016llX\r\n", (unsigned long long)CTX_PC(*context), (unsigned long long)CTX_STACK(*context), (unsigned long long)CTX_FRAME(*context)); + Add_Txt(scrap); + sprintf(scrap, "Rax:%016llX\tRbx:%016llX\tRcx:%016llX\r\n", (unsigned long long)CTX_AX(*context), (unsigned long long)CTX_BX(*context), (unsigned long long)CTX_CX(*context)); + Add_Txt(scrap); + sprintf(scrap, "Rdx:%016llX\tRsi:%016llX\tRdi:%016llX\r\n", (unsigned long long)CTX_DX(*context), (unsigned long long)CTX_SI(*context), (unsigned long long)CTX_DI(*context)); + Add_Txt(scrap); +#else sprintf(scrap, "Eip:%08X\tEsp:%08X\tEbp:%08X\r\n", context->Eip, context->Esp, context->Ebp); Add_Txt(scrap); sprintf(scrap, "Eax:%08X\tEbx:%08X\tEcx:%08X\r\n", context->Eax, context->Ebx, context->Ecx); Add_Txt(scrap); sprintf(scrap, "Edx:%08X\tEsi:%08X\tEdi:%08X\r\n", context->Edx, context->Esi, context->Edi); Add_Txt(scrap); +#endif sprintf(scrap, "EFlags:%08X \r\n", context->EFlags); Add_Txt(scrap); sprintf(scrap, "CS:%04x SS:%04x DS:%04x ES:%04x FS:%04x GS:%04x\r\n", context->SegCs, context->SegSs, context->SegDs, context->SegEs, context->SegFs, context->SegGs); @@ -624,6 +643,34 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) Add_Txt(scrap); #endif +#if defined(_WIN64) || defined(__x86_64__) + // x86-64: FPU/SSE state is in CONTEXT.FltSave (an XMM_SAVE_AREA32), not + // the 32-bit FLOATING_SAVE_AREA, and there is no RegisterArea. Each + // ST(i) register instead lives in the low 10 bytes of a 16-byte + // FloatRegisters[] slot (the remaining 6 bytes are reserved padding), + // so this reports exactly what the 32-bit block below reports, just + // addressed through the x86-64 layout. + for (int fp=0 ; fp<8 ; fp++) { + sprintf(scrap, "ST%d : ", fp); + Add_Txt(scrap); + BYTE *reg_bytes = (BYTE*)&context->FltSave.FloatRegisters[fp]; + for (int b=0 ; b<10 ; b++) { + sprintf(scrap, "%02X", reg_bytes[b]); + Add_Txt(scrap); + } + + void *fp_data_ptr = (void*)reg_bytes; + + // TheSuperHackers @refactor Replaced MSVC inline assembly with portable C++ cast for MinGW compatibility + /* + ** Convert FP dump from temporary real value (10 bytes) to double (8 bytes). + ** On x86, long double is the 10-byte x87 format, so we can just cast. + */ + double fp_value = (double)(*(long double*)fp_data_ptr); + sprintf(scrap, " %+#.17e\r\n", fp_value); + Add_Txt(scrap); + } +#else for (int fp=0 ; fpEip); +#endif - unsigned char *eip_ptr = (unsigned char *) (context->Eip); + unsigned char *eip_ptr = (unsigned char *) (CTX_PC(*context)); char bytestr[32]; for (int c = 0 ; c < 32 ; c++) { @@ -671,7 +723,7 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) */ DebugString("Stack dump...\n"); Add_Txt("Stack dump (* indicates possible code address) :\r\n"); - unsigned long *stackptr = (unsigned long*) context->Esp; + unsigned long *stackptr = (unsigned long*) CTX_STACK(*context); for (int j=0 ; j<2048 ; j++) { if (IsBadReadPtr(stackptr, 4)) { @@ -1272,9 +1324,9 @@ int Stack_Walk(unsigned long *return_addresses, int num_addresses, CONTEXT *cont ** Use the context struct if it was provided. */ if (context) { - stack_frame.AddrPC.Offset = context->Eip; - stack_frame.AddrStack.Offset = context->Esp; - stack_frame.AddrFrame.Offset = context->Ebp; + stack_frame.AddrPC.Offset = CTX_PC(*context); + stack_frame.AddrStack.Offset = CTX_STACK(*context); + stack_frame.AddrFrame.Offset = CTX_FRAME(*context); } int pointer_index = 0; diff --git a/Core/Libraries/Source/debug/debug_except.cpp b/Core/Libraries/Source/debug/debug_except.cpp index a8c5a286757..646a1cce509 100644 --- a/Core/Libraries/Source/debug/debug_except.cpp +++ b/Core/Libraries/Source/debug/debug_except.cpp @@ -30,6 +30,7 @@ #include "internal_except.h" #include #include +#include "Lib/arch_context.h" DebugExceptionhandler::DebugExceptionhandler() { @@ -110,7 +111,7 @@ void DebugExceptionhandler::LogExceptionLocation(Debug &dbg, struct _EXCEPTION_P struct _CONTEXT &ctx=*exptr->ContextRecord; char buf[512]; - DebugStackwalk::Signature::GetSymbol(ctx.Eip,buf,sizeof(buf)); + DebugStackwalk::Signature::GetSymbol(CTX_PC(ctx),buf,sizeof(buf)); dbg << "Exception occured at\n" << buf << "."; } @@ -120,15 +121,15 @@ void DebugExceptionhandler::LogRegisters(Debug &dbg, struct _EXCEPTION_POINTERS dbg << Debug::FillChar('0') << Debug::Hex() - << "EAX:" << Debug::Width(8) << ctx.Eax - << " EBX:" << Debug::Width(8) << ctx.Ebx - << " ECX:" << Debug::Width(8) << ctx.Ecx << "\n" - << "EDX:" << Debug::Width(8) << ctx.Edx - << " ESI:" << Debug::Width(8) << ctx.Esi - << " EDI:" << Debug::Width(8) << ctx.Edi << "\n" - << "EIP:" << Debug::Width(8) << ctx.Eip - << " ESP:" << Debug::Width(8) << ctx.Esp - << " EBP:" << Debug::Width(8) << ctx.Ebp << "\n" + << "EAX:" << Debug::Width(CTX_REG_WIDTH) << CTX_AX(ctx) + << " EBX:" << Debug::Width(CTX_REG_WIDTH) << CTX_BX(ctx) + << " ECX:" << Debug::Width(CTX_REG_WIDTH) << CTX_CX(ctx) << "\n" + << "EDX:" << Debug::Width(CTX_REG_WIDTH) << CTX_DX(ctx) + << " ESI:" << Debug::Width(CTX_REG_WIDTH) << CTX_SI(ctx) + << " EDI:" << Debug::Width(CTX_REG_WIDTH) << CTX_DI(ctx) << "\n" + << "EIP:" << Debug::Width(CTX_REG_WIDTH) << CTX_PC(ctx) + << " ESP:" << Debug::Width(CTX_REG_WIDTH) << CTX_STACK(ctx) + << " EBP:" << Debug::Width(CTX_REG_WIDTH) << CTX_FRAME(ctx) << "\n" << "Flags:" << Debug::Bin() << Debug::Width(32) << ctx.EFlags << Debug::Hex() << "\n" << "CS:" << Debug::Width(4) << ctx.SegCs << " DS:" << Debug::Width(4) << ctx.SegDs @@ -148,6 +149,44 @@ void DebugExceptionhandler::LogFPURegisters(Debug &dbg, struct _EXCEPTION_POINTE return; } +#if defined(_WIN64) || defined(__x86_64__) + // x86-64: FPU/SSE state is in CONTEXT.FltSave (an XMM_SAVE_AREA32), not + // the 32-bit FLOATING_SAVE_AREA. ControlWord/StatusWord/TagWord/ + // ErrorOffset/ErrorSelector/DataOffset/DataSelector still exist under + // the same names; there is no Cr0NpxState, and each ST(i) register + // lives in the low 10 bytes of a 16-byte FloatRegisters[] slot rather + // than a flat RegisterArea, mirroring what the 32-bit block below + // reports. + XMM_SAVE_AREA32 &flt=ctx.FltSave; + dbg << Debug::Bin() << Debug::FillChar('0') + << "CW:" << Debug::Width(16) << (flt.ControlWord&0xffff) << "\n" + << "SW:" << Debug::Width(16) << (flt.StatusWord&0xffff) << "\n" + << "TW:" << Debug::Width(16) << (flt.TagWord&0xffff) << "\n" + << Debug::Hex() + << "ErrOfs: " << Debug::Width(8) << flt.ErrorOffset + << " ErrSel: " << Debug::Width(8) << flt.ErrorSelector << "\n" + << "DataOfs: " << Debug::Width(8) << flt.DataOffset + << " DataSel: " << Debug::Width(8) << flt.DataSelector << "\n" + ; + + for (unsigned k=0;k<8;++k) + { + dbg << Debug::Dec() << "ST(" << k << ") "; + dbg.SetPrefixAndRadix("",16); + + BYTE *value=(BYTE*)&flt.FloatRegisters[k]; + for (unsigned i=0;i<10;i++) + dbg << Debug::Width(2) << value[i]; + + // TheSuperHackers @refactor Replaced MSVC inline assembly with portable C++ cast for MinGW compatibility + // Convert from temporary real (10 byte) to double (8 bytes). + // On x86, long double is the 10-byte x87 format, so we can just cast. + double fpVal = (double)(*(long double*)value); + dbg << " " << fpVal; + + dbg << "\n"; + } +#else FLOATING_SAVE_AREA &flt=ctx.FloatSave; dbg << Debug::Bin() << Debug::FillChar('0') << "CW:" << Debug::Width(16) << (flt.ControlWord&0xffff) << "\n" @@ -180,6 +219,7 @@ void DebugExceptionhandler::LogFPURegisters(Debug &dbg, struct _EXCEPTION_POINTE dbg << "\n"; } +#endif dbg << Debug::FillChar() << Debug::Dec(); } @@ -240,7 +280,7 @@ static BOOL CALLBACK ExceptionDlgProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARA // address struct _CONTEXT &ctx=*exPtrs->ContextRecord; - DebugStackwalk::Signature::GetSymbol(ctx.Eip,regInfo,sizeof(regInfo)); + DebugStackwalk::Signature::GetSymbol(CTX_PC(ctx),regInfo,sizeof(regInfo)); SendDlgItemMessage(hWnd,102,WM_SETTEXT,0,(LPARAM)regInfo); // stack @@ -396,7 +436,7 @@ LONG __stdcall DebugExceptionhandler::ExceptionFilter(struct _EXCEPTION_POINTERS dbg.m_stackWalk.StackWalk(sig,pExPtrs->ContextRecord); dbg << sig << "\n"; - dbg << "Bytes around EIP:" << Debug::MemDump::Char(((char *)(pExPtrs->ContextRecord->Eip))-32,80); + dbg << "Bytes around EIP:" << Debug::MemDump::Char(((char *)(CTX_PC(*pExPtrs->ContextRecord)))-32,80); dbg.FlushOutput(); From 58e17616057efc17690eef5c27e35ab2e07aa5ea Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Mon, 31 Aug 2026 14:45:16 +0200 Subject: [PATCH 06/20] fix(x64): match Win32 callback and DbgHelp signatures to the 64-bit ABI Four crash-handler sites broke on x64 because platform headers widen to DWORD64/INT_PTR/StackWalk64 there and our local declarations didn't follow. - debug_except.cpp: ExceptionDlgProc returns BOOL, but DLGPROC expects INT_PTR (64-bit on Win64). Changed the function's own return type rather than casting the pointer, since a cast would call it through the wrong ABI. INT_PTR is plain int on 32-bit Windows, so this is a no-op there. - debug_stack.cpp: dbghelp.h sets _IMAGEHLP64 under _WIN64 and #defines StackWalk to StackWalk64. debug_stack.h is included before , so DebugStackwalk::StackWalk's declaration is parsed as plain "StackWalk", but the out-of-line definition further down the same file is parsed after the macro is live and gets silently rewritten to "StackWalk64" -- hence "no declaration matches". Fixed with a scoped, commented #undef StackWalk right after , rather than renaming the method (four call sites across three files, none of which ever pull in DbgHelp headers, so the collision is local to this one file) or reordering includes (would make the declared name architecture-dependent per-TU, risking an unresolved external at link time instead of a clean compile error). - Except.cpp + debug_stack.inl: SymFunctionTableAccessType and SymGetModuleBaseType hand-roll a DWORD address parameter instead of going through the platform's PFUNCTION_TABLE_ACCESS_ROUTINE/PGET_MODULE_BASE_ROUTINE names, so they don't widen automatically like StackWalkType does. Both typedefs are now architecture-gated: the ...64 platform types (PFUNCTION_TABLE_ACCESS_ROUTINE64/PGET_MODULE_BASE_ROUTINE64, DWORD64 in debug_stack.inl) apply only under _WIN64/__x86_64__; the 32-bit branch is untouched so VC6 (1998, predates the ...64 DbgHelp API) keeps compiling against the same declarations it always has. - Except.cpp's ImagehelpFunctionNames and debug_stack.inl's DebughelpFunctionNames tables feed GetProcAddress against IMAGEHLP.DLL/DBGHELP.DLL. 64-bit dbghelp.dll only exports the ...64 form of entry points whose address parameter is DWORD64 (StackWalk, SymFunctionTableAccess, SymGetModuleBase, SymGetSymFromAddr, SymLoadModule, SymUnloadModule, SymGetLineFromAddr); SymCleanup, SymInitialize, SymSetOptions and SymGetOptions take no address parameter and are exported under the same name on every architecture. Without suffixing the former, GetProcAddress returns NULL for each on x64 and the corresponding _SymXxx pointer stays null -- the crash handler compiles clean and is dead at runtime. Both tables are now architecture-gated; 32-bit tables are byte-identical to before. - Bug found, not fixed: Except.cpp's 32-bit table's 9th entry is the string "SymGetModuleBaseType", which is this file's own local typedef name, not a DbgHelp export -- the real export is "SymGetModuleBase". _SymGetModuleBase has therefore always resolved to NULL on every architecture to date, and the stack walker has always run without a module-base callback in shipping 32-bit builds. Left byte-identical (typo included) on the 32-bit side deliberately: correcting it would populate a callback that has always been NULL in retail, changing shipping behaviour. Fixed only on the x64 branch (SymGetModuleBase64), where there is no retail baseline to protect. Needs a separate decision on the 32-bit side. - Both handlers hardcoded IMAGE_FILE_MACHINE_I386 in their _StackWalk/ StackWalk64 calls. On x64 that walks the stack using 32-bit unwind rules and yields garbage frames with no error. Both now use CTX_STACKWALK_MACHINE from Task 3's arch_context.h. - debug_stack.cpp also had a raw ctx->Eip/Esp/Ebp access inside DebugStackwalk::StackWalk() that doesn't compile on x64 (_CONTEXT has no Eip/Esp/Ebp there), and debug_stack.inl had the same DWORD-vs-DWORD64 typedef mismatch as Except.cpp for SymFunctionTableAccess/SymGetModuleBase. Neither was in Task 3's or this task's declared two/three-file scope, but both blocked compiling this task's own target function on x64, so fixed here using Task 3's already-established CTX_PC/CTX_STACK/CTX_FRAME macros -- this completes the CONTEXT port rather than second-guessing it. Gates: G1 32-bit rebuild exit 0, 0 errors, 31,555 warnings (exact baseline, untouched). G2 x64 errors 22->18, distinct shapes 7->3, affected files 5->4; all four target errors gone, remaining 18 are Task 5's #error guards and Task 6's persistfactory.h. G3 VC6 retail exit 0, 0 errors, all seven .text digests byte-identical to docs/x64/baseline-vc6.md. Co-Authored-By: Claude Opus 5 --- .../Libraries/Source/WWVegas/WWLib/Except.cpp | 58 ++++++++++++++++- Core/Libraries/Source/debug/debug_except.cpp | 4 +- Core/Libraries/Source/debug/debug_stack.cpp | 63 +++++++++++++++++-- Core/Libraries/Source/debug/debug_stack.inl | 18 ++++++ 4 files changed, 137 insertions(+), 6 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp index ddc67ac5ece..67ad40e0ef7 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp @@ -125,8 +125,23 @@ typedef BOOL (WINAPI *SymLoadModuleType) (HANDLE hProcess, HANDLE hFile, LPSTR typedef DWORD (WINAPI *SymSetOptionsType) (DWORD SymOptions); typedef BOOL (WINAPI *SymUnloadModuleType) (HANDLE hProcess, DWORD BaseOfDll); typedef BOOL (WINAPI *StackWalkType) (DWORD MachineType, HANDLE hProcess, HANDLE hThread, LPSTACKFRAME StackFrame, LPVOID ContextRecord, PREAD_PROCESS_MEMORY_ROUTINE ReadMemoryRoutine, PFUNCTION_TABLE_ACCESS_ROUTINE FunctionTableAccessRoutine, PGET_MODULE_BASE_ROUTINE GetModuleBaseRoutine, PTRANSLATE_ADDRESS_ROUTINE TranslateAddress); + +// On 64-bit builds dbghelp.h sets _IMAGEHLP64 and #defines LPSTACKFRAME, +// PFUNCTION_TABLE_ACCESS_ROUTINE and PGET_MODULE_BASE_ROUTINE (used just +// above in StackWalkType) to their ...64 forms automatically, so StackWalkType +// already gets the right ABI on both architectures for free. These next two +// typedefs don't go through those platform names -- they hand-roll a DWORD +// address parameter -- so they need an explicit 64-bit branch or _StackWalk's +// call site below won't accept them as the FunctionTableAccessRoutine / +// GetModuleBaseRoutine arguments. VC6 (1998) predates the ...64 DbgHelp API, +// so the 32-bit branch is untouched -- this is not made unconditional. +#if defined(_WIN64) || defined(__x86_64__) +typedef PFUNCTION_TABLE_ACCESS_ROUTINE64 SymFunctionTableAccessType; +typedef PGET_MODULE_BASE_ROUTINE64 SymGetModuleBaseType; +#else typedef LPVOID (WINAPI *SymFunctionTableAccessType) (HANDLE hProcess, DWORD AddrBase); typedef DWORD (WINAPI *SymGetModuleBaseType) (HANDLE hProcess, DWORD dwAddr); +#endif static SymCleanupType _SymCleanup = nullptr; @@ -139,6 +154,46 @@ static StackWalkType _StackWalk = nullptr; static SymFunctionTableAccessType _SymFunctionTableAccess = nullptr; static SymGetModuleBaseType _SymGetModuleBase = nullptr; +// This table is walked in lockstep with the _SymXxx globals above (see the +// fptr loop in Dump_Exception_Info() / Load_Image_Helper()) to GetProcAddress +// each name out of IMAGEHLP.DLL / DBGHELP.DLL, so order must stay in sync +// with the globals' declaration order. +// +// 64-bit dbghelp.dll only exports the ...64 forms of the entry points whose +// address parameter is DWORD64 (SymGetSymFromAddr, SymLoadModule, +// SymUnloadModule, StackWalk, SymFunctionTableAccess, SymGetModuleBase); +// GetProcAddress with the un-suffixed name returns NULL for those on x64, +// which would leave the corresponding _SymXxx pointer null and silently +// disable that part of the crash handler rather than fail to compile. +// SymCleanup, SymInitialize and SymSetOptions take no address parameter and +// are exported under the same name on every architecture (verified against +// mingw-w64's psdk_inc/_dbg_common.h: no #define redirects them under +// _IMAGEHLP64), so they are unchanged. +// +// Entry 9 is "SymGetModuleBaseType" on the 32-bit side, which is wrong on +// every architecture: it is the name of this file's local typedef (see +// SymGetModuleBaseType above), not a DbgHelp export -- the real export is +// "SymGetModuleBase". That means _SymGetModuleBase has always resolved to +// nullptr and the stack walker has always run without a module-base +// callback on 32-bit. Left byte-identical (typo included) here because +// correcting it would change retail runtime behaviour -- a previously-NULL +// callback would suddenly be populated in shipping builds. See task-4-report +// for the writeup; fix is deliberately deferred to a separate decision. +#if defined(_WIN64) || defined(__x86_64__) +static char const *const ImagehelpFunctionNames[] = +{ + "SymCleanup", + "SymGetSymFromAddr64", + "SymInitialize", + "SymLoadModule64", + "SymSetOptions", + "SymUnloadModule64", + "StackWalk64", + "SymFunctionTableAccess64", + "SymGetModuleBase64", + nullptr +}; +#else static char const *const ImagehelpFunctionNames[] = { "SymCleanup", @@ -152,6 +207,7 @@ static char const *const ImagehelpFunctionNames[] = "SymGetModuleBaseType", nullptr }; +#endif @@ -1335,7 +1391,7 @@ int Stack_Walk(unsigned long *return_addresses, int num_addresses, CONTEXT *cont ** Walk the stack by the requested number of return address iterations. */ for (int i = 0; i < num_addresses + 1; i++) { - if (_StackWalk(IMAGE_FILE_MACHINE_I386, GetCurrentProcess(), GetCurrentThread(), &stack_frame, nullptr, nullptr, _SymFunctionTableAccess, _SymGetModuleBase, nullptr)) { + if (_StackWalk(CTX_STACKWALK_MACHINE, GetCurrentProcess(), GetCurrentThread(), &stack_frame, nullptr, nullptr, _SymFunctionTableAccess, _SymGetModuleBase, nullptr)) { /* ** First result will always be the return address we were called from. diff --git a/Core/Libraries/Source/debug/debug_except.cpp b/Core/Libraries/Source/debug/debug_except.cpp index 646a1cce509..66333b33612 100644 --- a/Core/Libraries/Source/debug/debug_except.cpp +++ b/Core/Libraries/Source/debug/debug_except.cpp @@ -235,7 +235,9 @@ static char regInfo[1024],verInfo[256]; // and this saves us from doing a stack walk twice static DebugStackwalk::Signature sig; -static BOOL CALLBACK ExceptionDlgProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) +// DLGPROC returns INT_PTR (64-bit on Win64). INT_PTR is plain int on 32-bit +// Windows, so this is a no-op signature change for the VC6/mingw-i686 builds. +static INT_PTR CALLBACK ExceptionDlgProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { switch(uMsg) { diff --git a/Core/Libraries/Source/debug/debug_stack.cpp b/Core/Libraries/Source/debug/debug_stack.cpp index cb6f0b5b896..9de37f7828e 100644 --- a/Core/Libraries/Source/debug/debug_stack.cpp +++ b/Core/Libraries/Source/debug/debug_stack.cpp @@ -33,6 +33,20 @@ #include "WWLib/stringex.h" #include #include "Lib/BaseTypeCore.h" +#include "Lib/arch_context.h" + +// imagehlp.h (via dbghelp.h's psdk_inc/_dbg_common.h) #defines StackWalk to +// StackWalk64 on 64-bit builds, because _IMAGEHLP64 is set whenever _WIN64 +// is defined. DebugStackwalk::StackWalk below is our own class method, not +// a direct call into the Win32 API (that goes through the gDbg._StackWalk +// function pointer instead), so the platform macro must not be allowed to +// rewrite its name. debug_stack.h is included above, before this macro +// exists, so the class declaration is unaffected; without this #undef the +// out-of-line definition further down would be silently renamed to +// StackWalk64 and no longer match its own declaration. +#ifdef StackWalk +#undef StackWalk +#endif // Definitions to allow run-time linking to the dbghelp.dll functions. @@ -54,12 +68,53 @@ static union } gDbg; #undef DBGHELP +// GetProcAddress'd against DBGHELP.DLL by InitDbghelp() below, one name per +// DBGHELP() entry in debug_stack.inl, in the same order as the gDbg struct +// above. +// +// 64-bit dbghelp.dll only exports the ...64 form of an entry point whose +// address parameter is DWORD64 -- StackWalk, SymFunctionTableAccess, +// SymGetModuleBase, SymGetSymFromAddr and SymGetLineFromAddr here. +// GetProcAddress with the un-suffixed name returns NULL for those on x64, +// which would leave the matching gDbg._SymXxx pointer null and silently +// disable that part of the stack walker rather than fail to compile. +// SymInitialize, SymGetOptions, SymSetOptions and SymCleanup take no address +// parameter and are exported under the same name on every architecture +// (verified against mingw-w64's psdk_inc/_dbg_common.h: no #define +// redirects them under _IMAGEHLP64), so they are unchanged. +#if defined(_WIN64) || defined(__x86_64__) +#define DBGHELP(name,ret,par) DBGHELP_APINAME_##name, +#define DBGHELP_APINAME_SymInitialize "SymInitialize" +#define DBGHELP_APINAME_SymGetOptions "SymGetOptions" +#define DBGHELP_APINAME_SymSetOptions "SymSetOptions" +#define DBGHELP_APINAME_StackWalk "StackWalk64" +#define DBGHELP_APINAME_SymFunctionTableAccess "SymFunctionTableAccess64" +#define DBGHELP_APINAME_SymGetModuleBase "SymGetModuleBase64" +#define DBGHELP_APINAME_SymGetSymFromAddr "SymGetSymFromAddr64" +#define DBGHELP_APINAME_SymGetLineFromAddr "SymGetLineFromAddr64" +#define DBGHELP_APINAME_SymCleanup "SymCleanup" +static char const *const DebughelpFunctionNames[] = +{ +#include "debug_stack.inl" + nullptr +}; +#undef DBGHELP_APINAME_SymInitialize +#undef DBGHELP_APINAME_SymGetOptions +#undef DBGHELP_APINAME_SymSetOptions +#undef DBGHELP_APINAME_StackWalk +#undef DBGHELP_APINAME_SymFunctionTableAccess +#undef DBGHELP_APINAME_SymGetModuleBase +#undef DBGHELP_APINAME_SymGetSymFromAddr +#undef DBGHELP_APINAME_SymGetLineFromAddr +#undef DBGHELP_APINAME_SymCleanup +#else #define DBGHELP(name,ret,par) #name, static char const *const DebughelpFunctionNames[] = { #include "debug_stack.inl" nullptr }; +#endif #undef DBGHELP // local dbghelp.dll module handle @@ -360,9 +415,9 @@ int DebugStackwalk::StackWalk(Signature &sig, struct _CONTEXT *ctx) // Use the context struct if it was provided. if (ctx) { - stackFrame.AddrPC.Offset = ctx->Eip; - stackFrame.AddrStack.Offset = ctx->Esp; - stackFrame.AddrFrame.Offset = ctx->Ebp; + stackFrame.AddrPC.Offset = CTX_PC(*ctx); + stackFrame.AddrStack.Offset = CTX_STACK(*ctx); + stackFrame.AddrFrame.Offset = CTX_FRAME(*ctx); } else { @@ -396,7 +451,7 @@ int DebugStackwalk::StackWalk(Signature &sig, struct _CONTEXT *ctx) // Walk the stack by the requested number of return address iterations. bool skipFirst=!ctx; while (sig.m_numAddr Date: Mon, 31 Aug 2026 16:01:37 +0200 Subject: [PATCH 07/20] fix(x64): widen every DbgHelp entry point the name-table fix rearmed Root cause of this whole round: correcting a GetProcAddress name changes which ABI you are actually calling. The compiler cannot catch a resulting parameter mismatch, because the call goes through our own hand-written typedef, not a header-declared prototype it can check against. The previous commit's corrected ImagehelpFunctionNames/DebughelpFunctionNames tables armed a latent stack buffer overflow. _SymGetSymFromAddr now resolves to the real SymGetSymFromAddr64 export, which writes a DWORD64 (8 bytes) through its Displacement out-parameter -- but SymGetSymFromAddrType still declared that parameter LPDWORD, and every call site backed it with a 4-byte local (unsigned long / int&). On every successful x64 symbol resolution that was an 8-byte write into a 4-byte stack slot. Before the previous commit the function pointer was NULL on x64 (GetProcAddress against the wrong, un-suffixed name), so this path was dead code; fixing the lookup name is what turned it live. Audited every entry point either name table suffixes with ...64, parameter by parameter, against psdk_inc/_dbg_common.h -- not just the ones flagged in review: - StackWalk64, SymFunctionTableAccess64, SymGetModuleBase64: already correct (verified, no change). - SymGetSymFromAddr64 (Except.cpp's SymGetSymFromAddrType and debug_stack.inl's SymGetSymFromAddr entry): Address and Displacement widened to DWORD64/PDWORD64, gated to _WIN64/__x86_64__. Symbol needed no change -- PIMAGEHLP_SYMBOL is itself #defined to PIMAGEHLP_SYMBOL64 under _IMAGEHLP64, so it already widened for free. All six call sites (four in Except.cpp, two in debug_stack.cpp) now write through a properly sized local and narrow into the existing 32-bit display/output variable only after the call returns, so no external signature (e.g. Lookup_Symbol's int& displacement) had to change. - SymGetLineFromAddr64 (debug_stack.inl only): qwAddr widened to DWORD64. pdwDisplacement deliberately left DWORD/PDWORD -- verified against the real signature that this out-parameter genuinely stays 32-bit in SymGetLineFromAddr64, unlike SymGetSymFromAddr64's. Not fixed further: addr is carried as 32-bit unsigned throughout debug_stack.cpp, a broader pre-existing limitation outside this round's scope. - SymLoadModule64 and SymUnloadModule64: not flagged by review, found by the parameter-by-parameter audit. SymLoadModuleType's return type was BOOL against the real DWORD64 return, and both SymLoadModuleType and SymUnloadModuleType's BaseOfDll stayed DWORD against the real DWORD64. No live overflow (every call site passes a literal 0 for BaseOfDll), but both were genuine ABI mismatches. Widened, gated; the two symload locals that receive SymLoadModule's return value were widened to UnsignedIntPtr on the x64 branch only, so a legitimate result with a zero low 32 bits can't misread as "load failed." Also corrected a comment in Lookup_Symbol that asserted SymGetSymFromAddr's address parameter was "fixed at 32 bits ... not ours to widen" -- true before this task's first commit (when the name table still resolved to the 32-bit-only export on every architecture), false the moment that commit pointed the table at SymGetSymFromAddr64. Left as-is it would have misled the next reader into reintroducing the overflow; corrected on the x64 branch, kept verbatim on the 32-bit branch where it remains true. Strengthened the #undef StackWalk comment (debug_stack.cpp) to state plainly that the fix is order-dependent: it only protects code after that point in the file, and a future include that reintroduces the StackWalk macro after this line would silently defeat it with no compiler warning. 32-bit/VC6 path byte-identical throughout -- every widened typedef, table, and local is gated behind _WIN64/__x86_64__, including the two symload locals whose 32-bit type never needed to change since SymLoadModuleType's 32-bit return type didn't move. Gates: G1 32-bit rebuild exit 0, 0 errors, 31,555 warnings (exact baseline, unchanged across both rounds). G2 x64 18 errors / 3 shapes / 4 files, unchanged from the prior commit as expected -- these were ABI width corrections to code that already compiled, not new error fixes. G3 VC6 retail exit 0, 0 errors, all seven .text digests byte-identical. Co-Authored-By: Claude Opus 5 --- .../Libraries/Source/WWVegas/WWLib/Except.cpp | 139 ++++++++++++++++-- Core/Libraries/Source/debug/debug_stack.cpp | 28 ++++ Core/Libraries/Source/debug/debug_stack.inl | 27 ++++ 3 files changed, 180 insertions(+), 14 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp index 67ad40e0ef7..fc125e8b98d 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp @@ -119,22 +119,69 @@ DynamicVectorClass ThreadList; ** */ typedef BOOL (WINAPI *SymCleanupType) (HANDLE hProcess); + +// Correcting the name table below (see ImagehelpFunctionNames) to resolve +// this to the real 64-bit export, SymGetSymFromAddr64, means the ABI it's +// actually called through changed too -- every parameter has to be +// re-checked against psdk_inc/_dbg_common.h, not just the ones the compiler +// would catch. Real signature: BOOL SymGetSymFromAddr64(HANDLE hProcess, +// DWORD64 qwAddr, PDWORD64 pdwDisplacement, PIMAGEHLP_SYMBOL64 Symbol). +// Address and Displacement are hand-rolled DWORD/LPDWORD here and must be +// widened explicitly on x64, or SymGetSymFromAddr64 writes 8 bytes through +// a 4-byte Displacement target -- a stack buffer overflow on every +// successful symbol lookup. Symbol needs no separate widening: dbghelp.h +// already #defines PIMAGEHLP_SYMBOL to PIMAGEHLP_SYMBOL64 under +// _IMAGEHLP64 (same mechanism as LPSTACKFRAME above), and is +// already included above this point in the file, so it widens for free at +// this typedef's declaration site. +#if defined(_WIN64) || defined(__x86_64__) +typedef BOOL (WINAPI *SymGetSymFromAddrType) (HANDLE hProcess, DWORD64 Address, PDWORD64 Displacement, PIMAGEHLP_SYMBOL Symbol); +#else typedef BOOL (WINAPI *SymGetSymFromAddrType) (HANDLE hProcess, DWORD Address, LPDWORD Displacement, PIMAGEHLP_SYMBOL Symbol); +#endif + typedef BOOL (WINAPI *SymInitializeType) (HANDLE hProcess, LPSTR UserSearchPath, BOOL fInvadeProcess); + +// Real SymLoadModule64: DWORD64 SymLoadModule64(HANDLE hProcess, HANDLE +// hFile, PCSTR ImageName, PCSTR ModuleName, DWORD64 BaseOfDll, DWORD +// SizeOfDll) -- both the return type and BaseOfDll widen relative to the +// deprecated 32-bit SymLoadModule. Every call site below passes a literal +// 0 for BaseOfDll (letting DbgHelp pick the base), so there's no +// overflow risk there, but the return-type mismatch (BOOL vs DWORD64) is +// still a real function-pointer-signature mismatch worth correcting, not +// just a cosmetic one. +#if defined(_WIN64) || defined(__x86_64__) +typedef DWORD64 (WINAPI *SymLoadModuleType) (HANDLE hProcess, HANDLE hFile, LPSTR ImageName, LPSTR ModuleName, DWORD64 BaseOfDll, DWORD SizeOfDll); +#else typedef BOOL (WINAPI *SymLoadModuleType) (HANDLE hProcess, HANDLE hFile, LPSTR ImageName, LPSTR ModuleName, DWORD BaseOfDll, DWORD SizeOfDll); +#endif + typedef DWORD (WINAPI *SymSetOptionsType) (DWORD SymOptions); + +// Real SymUnloadModule64: WINBOOL SymUnloadModule64(HANDLE hProcess, +// DWORD64 BaseOfDll). Both call sites below pass a literal 0, so no +// overflow risk, but BaseOfDll still needs widening to match the real +// export's signature. +#if defined(_WIN64) || defined(__x86_64__) +typedef BOOL (WINAPI *SymUnloadModuleType) (HANDLE hProcess, DWORD64 BaseOfDll); +#else typedef BOOL (WINAPI *SymUnloadModuleType) (HANDLE hProcess, DWORD BaseOfDll); +#endif + +// StackWalkType needs no architecture-specific branch: every one of its +// parameter type names (LPSTACKFRAME, PREAD_PROCESS_MEMORY_ROUTINE, +// PFUNCTION_TABLE_ACCESS_ROUTINE, PGET_MODULE_BASE_ROUTINE, +// PTRANSLATE_ADDRESS_ROUTINE) is a platform macro that dbghelp.h itself +// redirects to its ...64 form under _IMAGEHLP64, so this typedef already +// matches StackWalk64's real signature on x64 and StackWalk's on 32-bit. typedef BOOL (WINAPI *StackWalkType) (DWORD MachineType, HANDLE hProcess, HANDLE hThread, LPSTACKFRAME StackFrame, LPVOID ContextRecord, PREAD_PROCESS_MEMORY_ROUTINE ReadMemoryRoutine, PFUNCTION_TABLE_ACCESS_ROUTINE FunctionTableAccessRoutine, PGET_MODULE_BASE_ROUTINE GetModuleBaseRoutine, PTRANSLATE_ADDRESS_ROUTINE TranslateAddress); -// On 64-bit builds dbghelp.h sets _IMAGEHLP64 and #defines LPSTACKFRAME, -// PFUNCTION_TABLE_ACCESS_ROUTINE and PGET_MODULE_BASE_ROUTINE (used just -// above in StackWalkType) to their ...64 forms automatically, so StackWalkType -// already gets the right ABI on both architectures for free. These next two -// typedefs don't go through those platform names -- they hand-roll a DWORD -// address parameter -- so they need an explicit 64-bit branch or _StackWalk's -// call site below won't accept them as the FunctionTableAccessRoutine / -// GetModuleBaseRoutine arguments. VC6 (1998) predates the ...64 DbgHelp API, -// so the 32-bit branch is untouched -- this is not made unconditional. +// Unlike StackWalkType above, these next two typedefs don't go through +// platform macro names -- they hand-roll a DWORD address parameter -- so +// they need an explicit 64-bit branch or _StackWalk's call site below +// won't accept them as the FunctionTableAccessRoutine / GetModuleBaseRoutine +// arguments. VC6 (1998) predates the ...64 DbgHelp API, so the 32-bit +// branch is untouched -- this is not made unconditional. #if defined(_WIN64) || defined(__x86_64__) typedef PFUNCTION_TABLE_ACCESS_ROUTINE64 SymFunctionTableAccessType; typedef PGET_MODULE_BASE_ROUTINE64 SymGetModuleBaseType; @@ -439,7 +486,18 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) _SymSetOptions(SYMOPT_DEFERRED_LOADS); } + // SymLoadModuleType's return type is DWORD64 on x64 (matching the real + // SymLoadModule64 export); UnsignedIntPtr is pointer-width (4 bytes on + // 32-bit, 8 on 64-bit) so the assignment below never truncates a + // nonzero result down to a false "load failed" reading. Gated (rather + // than just widening unconditionally) purely to keep the 32-bit/VC6 + // codegen for this line textually identical to before -- the 32-bit + // return type never changed, so there's nothing to fix on that branch. +#if defined(_WIN64) || defined(__x86_64__) + UnsignedIntPtr symload = 0; +#else int symload = 0; +#endif int symbols_available = false; if (_SymInitialize != nullptr && _SymInitialize (GetCurrentProcess(), nullptr, false)) { @@ -529,11 +587,19 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) symptr->Address = CTX_PC(*context); if (!IsBadCodePtr((FARPROC)CTX_PC(*context))) { - if (_SymGetSymFromAddr != nullptr && _SymGetSymFromAddr (GetCurrentProcess(), CTX_PC(*context), &displacement, symptr)) { #if defined(_WIN64) || defined(__x86_64__) + // SymGetSymFromAddr64's Displacement out-param is PDWORD64; writing + // through &displacement (unsigned long, 4 bytes) here would let the + // API write 8 bytes into a 4-byte stack slot. Capture into a + // properly sized local and narrow into the display variable only + // after the call returns. + DWORD64 displacement64; + if (_SymGetSymFromAddr != nullptr && _SymGetSymFromAddr (GetCurrentProcess(), CTX_PC(*context), &displacement64, symptr)) { + displacement = (unsigned long)displacement64; snprintf(scrap, ARRAY_SIZE(scrap), "Exception occurred at %016llX - %s + %08X\r\n", (unsigned long long)CTX_PC(*context), symptr->Name, displacement); #else + if (_SymGetSymFromAddr != nullptr && _SymGetSymFromAddr (GetCurrentProcess(), CTX_PC(*context), &displacement, symptr)) { snprintf(scrap, ARRAY_SIZE(scrap), "Exception occurred at %08X - %s + %08X\r\n", context->Eip, symptr->Name, displacement); #endif @@ -578,7 +644,15 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) symptr->Size = 0; symptr->Address = temp_addr; +#if defined(_WIN64) || defined(__x86_64__) + // See the comment on the first _SymGetSymFromAddr call above: + // its Displacement out-param is PDWORD64 on x64. + DWORD64 displacement64; + if (_SymGetSymFromAddr != nullptr && _SymGetSymFromAddr (GetCurrentProcess(), temp_addr, &displacement64, symptr)) { + displacement = (unsigned long)displacement64; +#else if (_SymGetSymFromAddr != nullptr && _SymGetSymFromAddr (GetCurrentProcess(), temp_addr, &displacement, symptr)) { +#endif char symbuf[256]; snprintf(symbuf, ARRAY_SIZE(symbuf), "%s + %08X\r\n", symptr->Name, displacement); Add_Txt(symbuf); @@ -804,7 +878,15 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) symptr->Size = 0; symptr->Address = *stackptr; +#if defined(_WIN64) || defined(__x86_64__) + // See the comment on the first _SymGetSymFromAddr call + // above: its Displacement out-param is PDWORD64 on x64. + DWORD64 displacement64; + if (_SymGetSymFromAddr != nullptr && _SymGetSymFromAddr (GetCurrentProcess(), *stackptr, &displacement64, symptr)) { + displacement = (unsigned long)displacement64; +#else if (_SymGetSymFromAddr != nullptr && _SymGetSymFromAddr (GetCurrentProcess(), *stackptr, &displacement, symptr)) { +#endif char symbuf[256]; snprintf(symbuf, ARRAY_SIZE(symbuf), " - %s + %08X", symptr->Name, displacement); strlcat(scrap, symbuf, ARRAY_SIZE(scrap)); @@ -1199,7 +1281,14 @@ void Load_Image_Helper() _SymSetOptions(SYMOPT_DEFERRED_LOADS); } + // See the comment on Dump_Exception_Info's symload above: pointer-width + // so a nonzero DWORD64 result on x64 never truncates to a false 0, + // gated to keep 32-bit/VC6 codegen textually unchanged. +#if defined(_WIN64) || defined(__x86_64__) + UnsignedIntPtr symload = 0; +#else int symload = 0; +#endif if (_SymInitialize != nullptr && _SymInitialize(GetCurrentProcess(), nullptr, FALSE)) { @@ -1282,17 +1371,39 @@ bool Lookup_Symbol(void *code_ptr, char *symbol, int &displacement) symbol_struct_ptr->SizeOfStruct = sizeof (symbol_struct_buf); symbol_struct_ptr->MaxNameLength = sizeof(symbol_struct_buf)-sizeof (IMAGEHLP_SYMBOL); symbol_struct_ptr->Size = 0; +#if defined(_WIN64) || defined(__x86_64__) + // Correction to the comment this replaced: on x64 IMAGEHLP_SYMBOL is + // #defined to IMAGEHLP_SYMBOL64 (dbghelp.h, under _IMAGEHLP64), so + // ::Address here is DWORD64, and the name table below resolves + // _SymGetSymFromAddr to the real SymGetSymFromAddr64 export, whose + // Address parameter is DWORD64 too -- this is not "fixed at 32 bits" + // on this architecture. Use the full pointer width, not a narrowed one. + symbol_struct_ptr->Address = (UnsignedIntPtr)code_ptr; +#else // IMAGEHLP_SYMBOL::Address and SymGetSymFromAddr's DWORD parameter are - // fixed at 32 bits by the (32-bit-only, deprecated on Win64) DbgHelp API - // contract -- not ours to widen. Cast through UnsignedIntPtr so the - // narrowing is an explicit int-to-int conversion rather than a flagged - // pointer truncation; 32-bit codegen is unchanged. + // fixed at 32 bits by the (32-bit-only) DbgHelp API on this + // architecture. Cast through UnsignedIntPtr so the narrowing is an + // explicit int-to-int conversion rather than a flagged pointer + // truncation; 32-bit codegen is unchanged. symbol_struct_ptr->Address = (unsigned long)(UnsignedIntPtr)code_ptr; +#endif /* ** See if we have the symbol for that address. */ +#if defined(_WIN64) || defined(__x86_64__) + // SymGetSymFromAddr64's Displacement out-param is PDWORD64; writing + // through &displacement (the caller's int&, 4 bytes) would be a stack + // buffer overflow -- the API writes 8 bytes through it. Capture into a + // properly sized local and narrow into the caller's int& only after + // the call returns, so this function's own signature (and every + // caller of it) is unaffected. + DWORD64 displacement64; + if (_SymGetSymFromAddr(GetCurrentProcess(), (DWORD64)(UnsignedIntPtr)code_ptr, &displacement64, symbol_struct_ptr)) { + displacement = (int)displacement64; +#else if (_SymGetSymFromAddr(GetCurrentProcess(), (unsigned long)(UnsignedIntPtr)code_ptr, (unsigned long *)&displacement, symbol_struct_ptr)) { +#endif /* ** Copy it back into the buffer provided. diff --git a/Core/Libraries/Source/debug/debug_stack.cpp b/Core/Libraries/Source/debug/debug_stack.cpp index 9de37f7828e..bb20bd0830c 100644 --- a/Core/Libraries/Source/debug/debug_stack.cpp +++ b/Core/Libraries/Source/debug/debug_stack.cpp @@ -44,6 +44,14 @@ // exists, so the class declaration is unaffected; without this #undef the // out-of-line definition further down would be silently renamed to // StackWalk64 and no longer match its own declaration. +// +// This fix is order-dependent: it only protects code that appears *after* +// this point in this translation unit. If a future #include added below +// this line (directly or transitively) pulls in / +// again, or otherwise redefines StackWalk, the mismatch this guards +// against comes back with no compiler warning -- #undef is silent by +// design. Keep this as the last DbgHelp-related include in the file, or +// re-apply the #undef immediately after whatever reintroduces the macro. #ifdef StackWalk #undef StackWalk #endif @@ -244,8 +252,19 @@ void DebugStackwalk::Signature::GetSymbol(unsigned addr, char *buf, unsigned buf symPtr->SizeOfStruct=sizeof(IMAGEHLP_SYMBOL); symPtr->MaxNameLength=sizeof(symbolBuffer)-sizeof(IMAGEHLP_SYMBOL); DWORD displacement; +#if defined(_WIN64) || defined(__x86_64__) + // SymGetSymFromAddr64's Displacement out-param is PDWORD64; &displacement + // (DWORD, 4 bytes) would overflow. Capture into a properly sized local + // and narrow into displacement, which is then reused below for the + // SymGetLineFromAddr call, whose Displacement stays PDWORD on x64. + DWORD64 displacement64; + if (!gDbg._SymGetSymFromAddr((HANDLE)GetCurrentProcessId(),addr,&displacement64,symPtr)) + return; + displacement=(DWORD)displacement64; +#else if (!gDbg._SymGetSymFromAddr((HANDLE)GetCurrentProcessId(),addr,&displacement,symPtr)) return; +#endif if ((unsigned int)(bufEnd-buf)Name)+16) return; buf+=wsprintf(buf,", %s+0x%x",symPtr->Name,displacement); @@ -326,8 +345,17 @@ void DebugStackwalk::Signature::GetSymbol(unsigned addr, symPtr->SizeOfStruct=sizeof(IMAGEHLP_SYMBOL); symPtr->MaxNameLength=sizeof(symbolBuffer)-sizeof(IMAGEHLP_SYMBOL); DWORD displacement; +#if defined(_WIN64) || defined(__x86_64__) + // See the comment on the first _SymGetSymFromAddr call above: its + // Displacement out-param is PDWORD64 on x64. + DWORD64 displacement64; + if (gDbg._SymGetSymFromAddr((HANDLE)GetCurrentProcessId(),addr,&displacement64,symPtr)) + { + displacement=(DWORD)displacement64; +#else if (gDbg._SymGetSymFromAddr((HANDLE)GetCurrentProcessId(),addr,&displacement,symPtr)) { +#endif strlcpy(bufSym,symPtr->Name,sizeSym); if (relSym) *relSym=displacement; diff --git a/Core/Libraries/Source/debug/debug_stack.inl b/Core/Libraries/Source/debug/debug_stack.inl index b446f173bd5..e51d0a42320 100644 --- a/Core/Libraries/Source/debug/debug_stack.inl +++ b/Core/Libraries/Source/debug/debug_stack.inl @@ -65,15 +65,42 @@ DBGHELP(SymGetModuleBase, (HANDLE hProcess, DWORD dwAddr)) #endif +// Real SymGetSymFromAddr64: BOOL(HANDLE, DWORD64 qwAddr, +// PDWORD64 pdwDisplacement, PIMAGEHLP_SYMBOL64 Symbol). Address and +// Displacement are hand-rolled DWORD/LPDWORD, same class of gap as +// SymFunctionTableAccess/SymGetModuleBase above, and Displacement is a +// write target: leaving it 32-bit on x64 is a stack buffer overflow every +// time a symbol is resolved (SymGetSymFromAddr64 writes 8 bytes through +// it). Symbol needs no separate widening here: PIMAGEHLP_SYMBOL is +// #defined to PIMAGEHLP_SYMBOL64 under _IMAGEHLP64 (imagehlp.h is already +// included above this point in debug_stack.cpp), so it widens for free. +#if defined(_WIN64) || defined(__x86_64__) +DBGHELP(SymGetSymFromAddr, + BOOL, + (HANDLE hProcess, DWORD64 Address, PDWORD64 Displacement, + PIMAGEHLP_SYMBOL Symbol)) +#else DBGHELP(SymGetSymFromAddr, BOOL, (HANDLE hProcess, DWORD Address, LPDWORD Displacement, PIMAGEHLP_SYMBOL Symbol)) +#endif +// Real SymGetLineFromAddr64: BOOL(HANDLE, DWORD64 qwAddr, +// PDWORD pdwDisplacement, PIMAGEHLP_LINE64 Line64) -- only the address +// parameter widens; pdwDisplacement genuinely stays PDWORD (not a write +// overflow), and Line widens for free the same way Symbol does above. +#if defined(_WIN64) || defined(__x86_64__) +DBGHELP(SymGetLineFromAddr, + BOOL, + (HANDLE hProcess, DWORD64 dwAddr, PDWORD pdwDisplacement, + PIMAGEHLP_LINE Line)) +#else DBGHELP(SymGetLineFromAddr, BOOL, (HANDLE hProcess, DWORD dwAddr, PDWORD pdwDisplacement, PIMAGEHLP_LINE Line)) +#endif // keep this always as last entry DBGHELP(SymCleanup, From 1a179daa52e07a06791b904a0402dfd82aea7f9e Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Mon, 31 Aug 2026 18:13:11 +0200 Subject: [PATCH 08/20] fix(x64): replace the three inline-assembly #error guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each #error aborted its translation unit at that line, so a file with twenty unportable __asm blocks would have reported exactly one error; three was never the real count. A survey (grep -rln "__asm\|_asm\b") found 22 files containing inline assembly across Core/Generals/GeneralsMD. The three fixed here account for all of them that a real build currently reaches — the other 19 sit behind targets this build doesn't get to yet (concentrated in WWVegas/WWMath and WWVegas/WWLib), so they are Task 7's concern, not evidence this task was under-scoped. debug_debug.cpp's Debug::SkipNext() read [ebp+4] for a return address; that becomes __builtin_return_address(0), GCC/Clang's portable spelling of the same value on every architecture. Except.cpp's Stack_Walk() and debug_stack.cpp's DebugStackwalk::StackWalk() each captured EIP/EBP/ESP into raw registers to seed a STACKFRAME64; both become RtlCaptureContext, the documented Win64 API for exactly this, needing no assembly and working on 32-bit Windows too. All three MSVC __asm arms were guarded by bare _MSC_VER, which is wrong as written: MSVC's own x64 compiler rejects __asm, so that condition would have taken the assembly path on MSVC/x64 the moment it built. Each guard now reads _MSC_VER && _M_IX86. VC6 is _MSC_VER + _M_IX86, so it keeps its original branch untouched; G3 confirms all seven .text digests stay byte-identical. Widened Debug::curStackFrame and FrameHashEntry's hash key from unsigned to UnsignedIntPtr, since a 64-bit return address was being truncated into a 32-bit hash key on x64. Confirmed this table is a pure in-memory assert/log skip-tracking cache -- no Xfer/Serialize/fwrite/fread/persist- factory reference anywhere near it -- so widening carries no on-disk format risk. Removing the guards exposed one further defect behind them: ProfileFuncLevel::Thread::GetId() cast a ProfileFuncLevelTracer* to unsigned, which now fails to compile on x64 for the same reason. Traced its only call site (profile_result.cpp's WriteThread) before touching it: `sprintf(help,"prof%08x-all.csv",thread.GetId())` builds an on-disk output filename, fixing GetId()'s width at 32 bits from outside the class. Widening it would not have been a fix -- it would pass an 8-byte vararg through a 4-byte %08x format specifier, which is undefined behavior on x64 and strictly worse than the truncation it replaces. Left GetId() at unsigned and made the truncation explicit (cast through UnsignedIntPtr, with a TODO recording the collision risk and why the width can't move) instead of fixing what only looks like a bug. Investigated and left two further truncations, out of scope here: Except.cpp's Stack_Walk() return_addresses/return_address (unsigned long) has no call sites anywhere in the tree -- dead code, not a live format boundary. debug_stack.h's Signature::m_addr (unsigned[]) is not serialized -- no Xfer/Serialize/persist-factory near it either -- but it does feed Debug::operator<<'s crash-report text rendering, so it's safe to widen and simply outside what these three guards blocked. G1 (32-bit): 0 errors, 31,555 warnings, exact baseline. G2 (x64): errors 18 -> 15, distinct shapes 3 -> 1, affected files 4 -> 1 -- everything left is the persistfactory.h savegame-format decision a later task owns. G3 (VC6 retail): 0 errors, all seven .text digests byte-identical. Co-Authored-By: Claude Opus 5 --- .../Libraries/Source/WWVegas/WWLib/Except.cpp | 22 +++++++++-- Core/Libraries/Source/debug/debug_debug.cpp | 39 +++++++++---------- Core/Libraries/Source/debug/debug_debug.h | 12 +++--- Core/Libraries/Source/debug/debug_stack.cpp | 20 ++++++++-- .../Source/profile/profile_funclevel.h | 14 ++++++- 5 files changed, 73 insertions(+), 34 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp index fc125e8b98d..8223232efb7 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp @@ -1458,9 +1458,14 @@ int Stack_Walk(unsigned long *return_addresses, int num_addresses, CONTEXT *cont STACKFRAME stack_frame; memset(&stack_frame, 0, sizeof(stack_frame)); - unsigned long reg_eip, reg_ebp, reg_esp; - -#if defined(_MSC_VER) + // UnsignedIntPtr rather than unsigned long: these feed + // stack_frame.AddrPC/AddrFrame/AddrStack.Offset, which are DWORD64 in + // STACKFRAME64 (STACKFRAME becomes STACKFRAME64 on x64 -- see + // imagehlp.h's _IMAGEHLP64 mechanism), and `unsigned long` stays 32 bits + // under Win64's LLP64 model, so it would truncate there. + UnsignedIntPtr reg_eip, reg_ebp, reg_esp; + +#if defined(_MSC_VER) && defined(_M_IX86) __asm { here: lea eax,here @@ -1477,7 +1482,16 @@ int Stack_Walk(unsigned long *return_addresses, int num_addresses, CONTEXT *cont : "=r" (reg_eip), "=r" (reg_ebp), "=r" (reg_esp) ); #else -#error "Unsupported compiler or architecture for register capture" + // x86-64 and anything else: RtlCaptureContext fills a CONTEXT with the + // caller's register state. This is the documented Win64 way to seed a + // StackWalk64, and it needs no inline assembly. It also works on 32-bit + // Windows, but the __asm/__asm__ arms above are kept for VC6 and 32-bit + // GCC/Clang retail-compatibility. + CONTEXT capture_ctx; + RtlCaptureContext(&capture_ctx); + reg_eip = (UnsignedIntPtr)CTX_PC(capture_ctx); + reg_ebp = (UnsignedIntPtr)CTX_FRAME(capture_ctx); + reg_esp = (UnsignedIntPtr)CTX_STACK(capture_ctx); #endif stack_frame.AddrPC.Mode = AddrModeFlat; diff --git a/Core/Libraries/Source/debug/debug_debug.cpp b/Core/Libraries/Source/debug/debug_debug.cpp index 7656d533d1b..5ccbf70bcb3 100644 --- a/Core/Libraries/Source/debug/debug_debug.cpp +++ b/Core/Libraries/Source/debug/debug_debug.cpp @@ -74,7 +74,7 @@ Debug::LogDescription::LogDescription(const char *fileOrGroup, const char *descr Debug Debug::Instance; // more class static members -unsigned Debug::curStackFrame; +UnsignedIntPtr Debug::curStackFrame; // this constructor is empty on purpose because all construction // work is done in PreStaticInit (and some in PostStaticInit) @@ -306,21 +306,21 @@ bool Debug::SkipNext() // do not implement this function inline, we do need // a valid frame pointer here! - unsigned help; -#if defined(_MSC_VER) + // UnsignedIntPtr is `unsigned int` (4 bytes) on the VC6 32-bit target, so + // the _asm block below -- which needs a 4-byte destination to match eax -- + // is byte-identical to the original `unsigned help;` version there. + UnsignedIntPtr help; +#if defined(_MSC_VER) && defined(_M_IX86) _asm { mov eax,[ebp+4] // return address mov help,eax }; -#elif (defined(__GNUC__) || defined(__clang__)) && (defined(__i386__) || defined(_M_IX86)) - // GCC/Clang inline assembly for x86-32 - __asm__ __volatile__( - "mov 4(%%ebp), %0" - : "=r"(help) - : - : "memory" - ); +#elif defined(__GNUC__) || defined(__clang__) + // __builtin_return_address(0) is the portable spelling of [ebp+4] and works + // on every architecture GCC/Clang targets, so this replaces both the + // x86-32 asm and the #error that followed it. + help = (UnsignedIntPtr)__builtin_return_address(0); #else #error "Unsupported compiler or architecture for inline assembly" #endif @@ -1018,16 +1018,13 @@ bool Debug::IsLogEnabled(const char *fileOrGroup) // to be used from the D_ISLOG macros only and those guarantee // that we are having real static strings let's use // that strings address as frame address... - // LookupFrame/AddFrameEntry use the string's address as a hash key, and - // are declared to take `unsigned` -- that hashing scheme is already tied - // to the 32-bit-only stack-frame capture elsewhere in this file (see the - // #error above for non-x86-32 targets), so it is not widened here. Cast - // through UnsignedIntPtr so the narrowing is an explicit int-to-int - // conversion; 32-bit output/codegen is unchanged since UnsignedIntPtr is - // unsigned int there. - FrameHashEntry *e=Instance.LookupFrame((unsigned)(UnsignedIntPtr)fileOrGroup); + // LookupFrame/AddFrameEntry use the string's address as a hash key and are + // declared to take UnsignedIntPtr, so this is a lossless pointer->integer + // conversion on every target (it used to truncate through `unsigned` on + // x64; that guard is gone now that the hash key is pointer-width). + FrameHashEntry *e=Instance.LookupFrame((UnsignedIntPtr)fileOrGroup); if (!e) - e=Instance.AddFrameEntry((unsigned)(UnsignedIntPtr)fileOrGroup,FrameTypeLog,fileOrGroup,0); + e=Instance.AddFrameEntry((UnsignedIntPtr)fileOrGroup,FrameTypeLog,fileOrGroup,0); if (e->status==Unknown) Instance.UpdateFrameStatus(*e); return e->status==NoSkip; @@ -1210,7 +1207,7 @@ void Debug::Update() } } -Debug::FrameHashEntry* Debug::AddFrameEntry(unsigned addr, unsigned type, +Debug::FrameHashEntry* Debug::AddFrameEntry(UnsignedIntPtr addr, unsigned type, const char *fileOrGroup, int line) { __ASSERT(LookupFrame(addr)==nullptr); diff --git a/Core/Libraries/Source/debug/debug_debug.h b/Core/Libraries/Source/debug/debug_debug.h index b03aea1994b..ef835cb8e75 100644 --- a/Core/Libraries/Source/debug/debug_debug.h +++ b/Core/Libraries/Source/debug/debug_debug.h @@ -29,6 +29,8 @@ #pragma once +#include "Lib/BaseTypeCore.h" + /** \class Debug debug.h @@ -872,7 +874,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); CmdInterfaceListEntry *firstCmdGroup; /// \internal current stack frame (used by SkipNext) - static unsigned curStackFrame; + static UnsignedIntPtr curStackFrame; /** \internal @@ -920,7 +922,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); FrameHashEntry *next; /// frame address - unsigned frameAddr; + UnsignedIntPtr frameAddr; /// frame type (FrameTypeAssert, FrameTypeCheck, or FrameTypeLog) unsigned frameType; @@ -960,7 +962,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); \param addr frame address \return FrameHashEntry found or 0 if nothing found */ - __forceinline FrameHashEntry *LookupFrame(unsigned addr) + __forceinline FrameHashEntry *LookupFrame(UnsignedIntPtr addr) { for (FrameHashEntry *e=frameHash[addr%FRAME_HASH_SIZE];e;e=e->next) if (e->frameAddr==addr) @@ -980,7 +982,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); \param line line number \return the entry just added */ - FrameHashEntry *AddFrameEntry(unsigned addr, unsigned type, + FrameHashEntry *AddFrameEntry(UnsignedIntPtr addr, unsigned type, const char *fileOrGroup, int line); /** \internal @@ -1002,7 +1004,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); \param line line number \return the entry just added (or the already existing entry) */ - FrameHashEntry *GetFrameEntry(unsigned addr, unsigned type, + FrameHashEntry *GetFrameEntry(UnsignedIntPtr addr, unsigned type, const char *fileOrGroup, int line) { FrameHashEntry *e=LookupFrame(addr); diff --git a/Core/Libraries/Source/debug/debug_stack.cpp b/Core/Libraries/Source/debug/debug_stack.cpp index bb20bd0830c..926aeedeaf2 100644 --- a/Core/Libraries/Source/debug/debug_stack.cpp +++ b/Core/Libraries/Source/debug/debug_stack.cpp @@ -450,8 +450,12 @@ int DebugStackwalk::StackWalk(Signature &sig, struct _CONTEXT *ctx) else { // walk stack back using current call chain - unsigned long reg_eip, reg_ebp, reg_esp; -#if defined(_MSC_VER) + // UnsignedIntPtr rather than unsigned long: these feed + // stackFrame.AddrPC/AddrFrame/AddrStack.Offset, which are DWORD64 in + // STACKFRAME64 (STACKFRAME becomes STACKFRAME64 on x64), and + // `unsigned long` stays 32 bits under Win64's LLP64 model. + UnsignedIntPtr reg_eip, reg_ebp, reg_esp; +#if defined(_MSC_VER) && defined(_M_IX86) __asm { here: @@ -469,7 +473,17 @@ int DebugStackwalk::StackWalk(Signature &sig, struct _CONTEXT *ctx) : "=r" (reg_eip), "=r" (reg_ebp), "=r" (reg_esp) ); #else -#error "Unsupported compiler or architecture for register capture" + // x86-64 and anything else: RtlCaptureContext fills a CONTEXT with the + // caller's register state -- the documented Win64 way to seed a + // StackWalk64, needing no inline assembly. Mirrors the eip/ebp/esp set + // this function captures (same register set as Except.cpp's + // Stack_Walk); the ctx-provided branch above already reads the same + // three fields via CTX_PC/CTX_FRAME/CTX_STACK. + CONTEXT capture_ctx; + RtlCaptureContext(&capture_ctx); + reg_eip = (UnsignedIntPtr)CTX_PC(capture_ctx); + reg_ebp = (UnsignedIntPtr)CTX_FRAME(capture_ctx); + reg_esp = (UnsignedIntPtr)CTX_STACK(capture_ctx); #endif stackFrame.AddrPC.Offset = reg_eip; stackFrame.AddrStack.Offset = reg_esp; diff --git a/Core/Libraries/Source/profile/profile_funclevel.h b/Core/Libraries/Source/profile/profile_funclevel.h index a153a5f9e28..4aceee55188 100644 --- a/Core/Libraries/Source/profile/profile_funclevel.h +++ b/Core/Libraries/Source/profile/profile_funclevel.h @@ -29,6 +29,8 @@ #pragma once +#include "Lib/BaseTypeCore.h" + /** \brief The function level profiler. @@ -182,7 +184,17 @@ class ProfileFuncLevel */ unsigned GetId() const { - return unsigned(m_threadID); + // TODO(x64-profile-id): truncates a 64-bit pointer to a 32-bit + // identity token; two tracers can collide on x86-64. Width is fixed + // by ProfileResultFileCSV::WriteThread() (profile_result.cpp), which + // sprintf()s this value through "prof%08x-all.csv" to name the output + // file -- that %08x is the format boundary, so GetId() cannot be + // widened without also changing the on-disk file-naming convention. + // Cast through UnsignedIntPtr so the narrowing is an explicit + // pointer-to-int-to-int conversion rather than a silent pointer + // truncation; 32-bit output is unchanged since UnsignedIntPtr is + // unsigned int there. + return unsigned(UnsignedIntPtr(m_threadID)); } private: From f42edfa24a0746b4e7e8eafc7a1221e5c23dbecd Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Mon, 31 Aug 2026 19:28:58 +0200 Subject: [PATCH 09/20] fix(x64): make WWSaveLoad object-token size honest in SimplePersistFactoryClass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h had 15 x64 compile errors, all the same shape: cast from PersistClass* to uint32 loses precision, once per template instantiation. Size asymmetry (fixed): Save wrote sizeof(uint32) = 4 bytes for the object-identity token; Load read sizeof(T *), which is 8 bytes on x86-64. The loader silently consumed 4 bytes the writer never wrote, desynchronizing the rest of the chunk stream, and the compiler gave no diagnostic for it. Load now reads exactly sizeof(uint32) into a uint32, matching what Save writes on every platform, then converts that token to the void* Register_Pointer expects via (void *)(UnsignedIntPtr)token. This is byte-identical on 32-bit: UnsignedIntPtr is uintptr_t, 4 bytes under VC6, so routing the same 4 bytes through it and back to void* reproduces the exact pointer value the old sizeof(T *) read produced. Truncation (made explicit, NOT fixed): Save's uint32 objptr = (uint32)obj now reads (uint32)(UnsignedIntPtr)obj, with a TODO comment on the line. This only makes the narrowing a legal integer-to-integer conversion instead of a pointer-to-smaller-integer cast that doesn't compile on x64 — it does not resolve the underlying defect. On x86-64 two live objects can still share the low 32 bits of their addresses, so this token can still collide and pointer fixup can still bind the wrong object on load. Widening the on-disk width is explicitly out of scope for this task; it's a later decision that needs a full measurement, not a fix implied by this rename. Added a file-scope static_assert(sizeof(uint32) == 4, ...) above the template, outside the class body so it's checked whenever the header is parsed rather than only if/when some translation unit happens to instantiate the template. It protects the on-disk token width from drifting by accident, independent of whatever the later widening decision turns out to be. Survey of other pointer-into-chunk-stream sites (input to that later decision): inside WWSaveLoad/, persistfactory.h is the only one. Outside it, four more hand-rolled sites call the same SaveLoadSystemClass::Register_Pointer machinery but through WRITE_MICRO_CHUNK/READ_MICRO_CHUNK on the raw pointer variable itself: WW3D2/rendobj.cpp (RENDOBJFACTORY_VARIABLE_OBJPOINTER), WWAudio/AudibleSound.cpp (VARID_THIS_PTR), and dazzle.cpp in both Generals/ and GeneralsMD/ (DAZZLEFACTORY_VARIABLE_OBJPOINTER). Because those macros expand to Write(&var, sizeof(var)) and Read(&var, sizeof(var)) using the pointer variable's own size, each site is internally symmetric within a single build and produces no compile error at all on x64 — a different failure shape than this file's bug. Built for x86-64 they would each write and read an 8-byte token where the retail format uses 4: format-width drift rather than truncation, invisible to every gate this project currently runs. Gates (run by coordinator): G1 32-bit clean rebuild, 0 errors, 31,555 warnings, exact baseline. G3 VC6 retail: all seven .text digests byte-identical to docs/x64/baseline-vc6.md, confirming the retail save path is untouched. G2 x64: errors 15 -> 10, and this change unblocked core_wwsaveload, which unblocked WW3D2 (0 -> 331 log hits) and WWMath (0 -> 44 log hits, core_wwmath now builds cleanly) for the first time in this port. The 10 remaining errors are a new, smaller batch of the same pointer-truncation class in WW3D2/surfaceclass.cpp, WW3D2/dx8webbrowser.cpp, and assetmgr.cpp (both game variants). Co-Authored-By: Claude Opus 5 --- .../WWVegas/WWSaveLoad/persistfactory.h | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h b/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h index 2281a626520..ca17bb447dd 100644 --- a/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h +++ b/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h @@ -42,6 +42,7 @@ #include "WWDebug/wwdebug.h" #include "saveload.h" #include "persist.h" +#include "Lib/BaseTypeCore.h" /* ** PersistFactoryClass @@ -77,6 +78,10 @@ class PersistFactoryClass ** object. Simply instantiate a single static instance of this template with the ** type and chunkid in the .cpp file of your class. */ +// The on-disk width of the object-identity token, fixed by the retail save format. +// Changing it changes the format. +static_assert(sizeof(uint32) == 4, "savegame object token must stay 4 bytes on disk"); + template class SimplePersistFactoryClass : public PersistFactoryClass { public: @@ -100,11 +105,18 @@ template PersistClass * SimplePersistFactoryClass::Load(ChunkLoadClass & cload) const { T * new_obj = W3DNEW T; - T * old_obj = nullptr; + + // Read exactly what Save wrote: a fixed-width 4-byte identity token, not + // sizeof(T *). On x86-64 sizeof(T *) is 8, so reading sizeof(T *) here would + // consume four bytes the writer never wrote and desynchronize the chunk + // stream. The token is not a real pointer (see the TODO in Save, below); it + // is carried through as an opaque value and only ever compared for equality + // by Register_Pointer's pointer table. + uint32 old_obj_token = 0; cload.Open_Chunk(); WWASSERT(cload.Cur_Chunk_ID() == SIMPLEFACTORY_CHUNKID_OBJPOINTER); - cload.Read(&old_obj,sizeof(T *)); + cload.Read(&old_obj_token,sizeof(uint32)); cload.Close_Chunk(); cload.Open_Chunk(); @@ -112,6 +124,7 @@ SimplePersistFactoryClass::Load(ChunkLoadClass & cload) const new_obj->Load(cload); cload.Close_Chunk(); + void * old_obj = (void *)(UnsignedIntPtr)old_obj_token; SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } @@ -120,7 +133,12 @@ SimplePersistFactoryClass::Load(ChunkLoadClass & cload) const template void SimplePersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * obj) const { - uint32 objptr = (uint32)obj; + // TODO(x64-savegame-format): on x86-64 this truncates a 64-bit pointer to a + // 32-bit on-disk identity token, so two live objects can collide and pointer + // fixup can bind the wrong object on load. The on-disk width is fixed by the + // retail save format and cannot be widened here without breaking it. + // See docs/x64/savegame-format-decision.md (written by the later task). + uint32 objptr = (uint32)(UnsignedIntPtr)obj; csave.Begin_Chunk(SIMPLEFACTORY_CHUNKID_OBJPOINTER); csave.Write(&objptr,sizeof(uint32)); csave.End_Chunk(); From 609b16cefb1d77f9584bfce336cac4337aa9e2fc Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Mon, 31 Aug 2026 21:24:35 +0200 Subject: [PATCH 10/20] fix(x64): clear the last 10 core WW3D2 truncating casts, unblock GameEngine Five distinct sites, three different outcomes -- the reusable lesson is in telling them apart, not in a mechanical widen-everything pass: - surfaceclass.cpp (2 sites, 4 error occurrences across g_ww3d2/z_ww3d2): widened `(unsigned int)lock_rect.pBits` to `(uintptr_t)lock_rect.pBits`. pBits is a live D3D-locked-surface address, runtime-only, never crosses a format boundary -- the real fix. - dx8webbrowser.cpp: NOT widened. CreateBrowser's `parentwindow` parameter is `long` per BrowserEngine.idl, a fixed-width COM/oleautomation ABI contract we don't control. Widening the local HWND would not help -- the call still narrows to `long` regardless. Replaced the now-illegal `reinterpret_cast(hWnd)` with the explicit, legal `(long)(uintptr_t)hWnd` and added a TODO(x64-hwnd-truncation) comment stating the top 32 bits of the window handle are silently dropped on 64-bit targets. Confirmed via -fsyntax-only that this path is live on MinGW (both targets take the non-_MSC_VER `#import` branch), not dead code. - assetmgr.cpp (Generals + GeneralsMD, 2 casts each): not a width problem at all. `((int)mesh_name) - ((int)name) + 1` looked like it could be a hash or identity token (the brief's suspicion), but traced to plain pointer subtraction miscoded as two independent truncating casts before subtracting -- a real x64 bug (wrong length if the two 32-bit-truncated addresses straddle a wraparound differently), not cast noise. Fixed with correct pointer arithmetic: `(int)(mesh_name - name) + 1`, narrowing only the final result (a filename length) where lstrcpynA's Win32 signature requires `int`. None of the four touched files previously included BaseTypeCore.h. That header carries `#pragma warning(error : 4706/4189/4101)` plus MIN/MAX/ TRUE/FALSE macros -- pulling it cold into files that never had it risks promoting a pre-existing warning to a hard error under the VC6 G3 gate, which is worse than the defect being fixed. Used the same narrower `` + raw `uintptr_t`/`intptr_t` approach the earlier huffencode.cpp fix used for this identical situation -- same types BaseTypeCore.h's UnsignedIntPtr/IntPtr alias, none of the pragma or macro footprint. Gates: - G1 (32-bit control): exit 0, 0 errors, 31,555 warnings -- exact baseline. - G3 (VC6 retail): exit 0, 0 errors, all seven .text digests byte-identical to docs/x64/baseline-vc6.md, despite two of the four files being renderer files the VC6 build compiles. - G2 (x64): errors 10->0 as intended, but the real result is depth: this unblocked g_ww3d2/z_ww3d2 and with them GameEngine, which had never been measured as 64-bit before. Objects built 528->1,148 of 1,889 (27.9%->60.8%), targets 42->44, GameEngine mentions in the log 0->13,105. New error count 10->602 (36 distinct shapes, 102 files) is entirely newly-visible GameEngine defects, not regression from this change. Co-Authored-By: Claude Opus 5 --- .../Source/WWVegas/WW3D2/dx8webbrowser.cpp | 15 ++++++++++++++- .../Source/WWVegas/WW3D2/surfaceclass.cpp | 12 ++++++++++-- .../Libraries/Source/WWVegas/WW3D2/assetmgr.cpp | 7 ++++++- .../Libraries/Source/WWVegas/WW3D2/assetmgr.cpp | 7 ++++++- 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WW3D2/dx8webbrowser.cpp b/Core/Libraries/Source/WWVegas/WW3D2/dx8webbrowser.cpp index 180e3aa03e0..96a65889f7c 100644 --- a/Core/Libraries/Source/WWVegas/WW3D2/dx8webbrowser.cpp +++ b/Core/Libraries/Source/WWVegas/WW3D2/dx8webbrowser.cpp @@ -33,6 +33,9 @@ #include "dx8webbrowser.h" #include "ww3d.h" #include "dx8wrapper.h" +// Only pulls the pointer-sized-int typedefs (uintptr_t); avoids dragging in +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include #if ENABLE_EMBEDDED_BROWSER @@ -193,7 +196,17 @@ void DX8WebBrowser::CreateBrowser(const char* browsername, const char* url, int if(pBrowser) { _bstr_t brsname(browsername); - pBrowser->CreateBrowser(brsname, _bstr_t(url), reinterpret_cast(hWnd), x, y, w, h, options, gamedispatch); + // TODO(x64-hwnd-truncation): IFEBrowserEngine2::CreateBrowser's + // parentwindow parameter is `long` per BrowserEngine.idl -- a fixed + // COM/oleautomation ABI width we don't control (dual/oleautomation + // interfaces cannot carry 64-bit ints as `long`; that's a VT_I4). + // On x64, HWND is a full 8-byte handle and this truncates it. NOT + // FIXED: the top 32 bits of the window handle are silently dropped + // whenever this path runs on a 64-bit target. Widening the local + // would not help -- the COM call still narrows to `long` regardless + // -- so the truncation is made explicit and legal here instead of + // left as a compile error. + pBrowser->CreateBrowser(brsname, _bstr_t(url), (long)(uintptr_t)hWnd, x, y, w, h, options, gamedispatch); pBrowser->SetUpdateRate(brsname, updateticks); } } diff --git a/Core/Libraries/Source/WWVegas/WW3D2/surfaceclass.cpp b/Core/Libraries/Source/WWVegas/WW3D2/surfaceclass.cpp index 2de64d69a39..74ade557e49 100644 --- a/Core/Libraries/Source/WWVegas/WW3D2/surfaceclass.cpp +++ b/Core/Libraries/Source/WWVegas/WW3D2/surfaceclass.cpp @@ -53,6 +53,9 @@ #include "WWMath/vector2i.h" #include "colorspace.h" #include "WWLib/bound.h" +// Only pulls the pointer-sized-int typedefs (uintptr_t); avoids dragging in +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include #include void Convert_Pixel(Vector3 &rgb, const SurfaceClass::SurfaceDescription &sd, const unsigned char * pixel) @@ -575,7 +578,10 @@ void SurfaceClass::FindBB(Vector2i *min,Vector2i*max) for (x = min->I; x < max->I; x++) { // HY - this is not endian safe - unsigned char *alpha=(unsigned char*) ((unsigned int)lock_rect.pBits+(y-min->J)*lock_rect.Pitch+(x-min->I)*size); + // pBits is a live D3D-locked surface pointer; the arithmetic below + // walks it row/column by row/column and stays runtime-only (never + // serialized), so widen the holder to pointer size on x64. + unsigned char *alpha=(unsigned char*) ((uintptr_t)lock_rect.pBits+(y-min->J)*lock_rect.Pitch+(x-min->I)*size); unsigned char myalpha=alpha[size-1]; myalpha=(myalpha>>(8-alphabits)) & mask; if (myalpha) { @@ -649,7 +655,9 @@ bool SurfaceClass::Is_Transparent_Column(unsigned int column) for (y = 0; y < (int) sd.Height; y++) { // HY - this is not endian safe - unsigned char *alpha=(unsigned char*) ((unsigned int)lock_rect.pBits+y*lock_rect.Pitch); + // Same live D3D-locked surface pointer as above: runtime-only, widen + // the holder to pointer size on x64. + unsigned char *alpha=(unsigned char*) ((uintptr_t)lock_rect.pBits+y*lock_rect.Pitch); unsigned char myalpha=alpha[size-1]; myalpha=(myalpha>>(8-alphabits)) & mask; if (myalpha) { diff --git a/Generals/Code/Libraries/Source/WWVegas/WW3D2/assetmgr.cpp b/Generals/Code/Libraries/Source/WWVegas/WW3D2/assetmgr.cpp index a9dac652860..5bb2139a5b4 100644 --- a/Generals/Code/Libraries/Source/WWVegas/WW3D2/assetmgr.cpp +++ b/Generals/Code/Libraries/Source/WWVegas/WW3D2/assetmgr.cpp @@ -794,7 +794,12 @@ RenderObjClass * WW3DAssetManager::Create_Render_Obj(const char * name) char filename [MAX_PATH]; const char *mesh_name = ::strchr (name, '.'); if (mesh_name != nullptr) { - ::lstrcpyn (filename, name, ((int)mesh_name) - ((int)name) + 1); + // This is pointer subtraction (distance from name to the '.'), + // not a wire value -- lstrcpynA's count parameter is `int` on + // Win32, and the string length trivially fits. Compute the + // difference with proper pointer arithmetic instead of + // truncating each 64-bit pointer to `int` before subtracting. + ::lstrcpyn (filename, name, (int)(mesh_name - name) + 1); ::lstrcat (filename, ".w3d"); } else { snprintf( filename, ARRAY_SIZE(filename), "%s.w3d", name); diff --git a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/assetmgr.cpp b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/assetmgr.cpp index ce6f67a0324..b6ffc006bef 100644 --- a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/assetmgr.cpp +++ b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/assetmgr.cpp @@ -799,7 +799,12 @@ RenderObjClass * WW3DAssetManager::Create_Render_Obj(const char * name) char filename [MAX_PATH]; const char *mesh_name = ::strchr (name, '.'); if (mesh_name != nullptr) { - ::lstrcpyn (filename, name, ((int)mesh_name) - ((int)name) + 1); + // This is pointer subtraction (distance from name to the '.'), + // not a wire value -- lstrcpynA's count parameter is `int` on + // Win32, and the string length trivially fits. Compute the + // difference with proper pointer arithmetic instead of + // truncating each 64-bit pointer to `int` before subtracting. + ::lstrcpyn (filename, name, (int)(mesh_name - name) + 1); ::lstrcat (filename, ".w3d"); } else { snprintf( filename, ARRAY_SIZE(filename), "%s.w3d", name); From 0f177a03bc9bc53e1e91e0d31078bdaab7507f4e Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Tue, 1 Sep 2026 08:05:00 +0200 Subject: [PATCH 11/20] fix(x64): repair VC6 build broken by BaseTypeCore.h, fix G3's incremental-build blind spot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit G3, the gate proving 64-bit porting work leaves the VC6 retail build untouched, reported "7 of 7 .text digests match" at nine consecutive tasks. All nine passes were false. The baseline it compared against was recorded from a --clean build; every G3 re-run since used an incremental build. Files nobody touched kept their previously-built .obj, and a previously-built .obj cannot produce a different digest no matter what happened elsewhere in the tree — so the gate was comparing yesterday's object code to itself and calling the match proof. Two real problems hid behind those false passes: 1. The VC6 build had been broken since a task added #include "Lib/BaseTypeCore.h" to debug_debug.h to reach UnsignedIntPtr. debug_debug.h is transitively included by nearly every debug translation unit, including debug_io_flat.cpp, whose `unsigned pathLen=strlen(path);` (line 80, unused, harmless under VC6 since before this branch) inherited BaseTypeCore.h's `#pragma warning(error : 4189)` (BaseTypeCore.h:80; also 4706 at :67 and 4101 at :83) and became a hard compile error: debug_io_flat.cpp(80): error C4189: 'pathLen' : local variable is initialized but not referenced GCC ignores #pragma warning entirely, so neither MinGW gate (i686 or x64) could ever see this — only VC6 could, and VC6 is exactly the gate the incremental-build blind spot above had defanged. Confirmed by clean --clean builds of both games from the pre-fix tree, both failing with this identical error. 2. VC6 codegen had genuinely shifted during the fix wave that repairs (1), and an incremental gate structurally cannot detect that either: it never recompiles the files whose codegen moved unless something else already forced a rebuild. Fix wave (the 20 files below): - debug_debug.{h,cpp}, debug_stack.{h,cpp}, WWLib/Except.{h,cpp}, WWLib/registry.{h,cpp}, WWSaveLoad/persistfactory.h, WWAudio/{AudibleSound. {h,cpp},SoundScene.cpp,SoundSceneObj.h}, profile/profile_funclevel.h: swapped `Lib/BaseTypeCore.h` for `` + `uintptr_t`, removing the warning-as-error pragmas from files that never had them (this branch's own established precedent, already followed in surfaceclass.cpp/dx8webbrowser.cpp; now the documented reason to keep following it). - Except.cpp's stack dump and debug_stack.cpp's Signature addressing read/ printed x64 stack slots and addresses in 4-byte units (truncating crash reports); widened to uintptr_t with width-correct format specifiers. debug_debug.cpp's pointer/address printing had the same 32-bit-only truncation; same fix. debug_debug.cpp's SkipNext() 32-bit inline-asm arm was restored to match its siblings' codegen exactly (a prior task had collapsed it into an ungated builtin, silently changing 32-bit GCC/Clang codegen). - rendobj.cpp, both dazzle.cpp copies, and AudibleSound.cpp's Load paths read pointer-sized (8-byte) tokens from legacy 4-byte W3D micro-chunks on x86-64, silently poisoning SaveLoadSystemClass's pointer-remap table (ChunkLoadClass::Read refuses oversized reads and leaves the destination untouched, rather than failing loudly). This is a real, live defect: the read side has an actual in-repo caller (W3DView's AnimatedSoundOptionsDialog.cpp), documented in this pass's addendum to savegame-format-decision.md. Fixed to read a fixed 4-byte token and cast after, mirroring persistfactory.h's existing pattern. The matching Save halves remain unreachable and are documented, not fixed, per that same addendum. - cmake/dx8.cmake: gated d3dx8's link (pe-i386-only, no x64 build exists) on CMAKE_SIZEOF_VOID_P EQUAL 4, matching its sibling gate. - docs/x64/core-error-catalogue-v2.md: marked v1's unreproducible 266-translation-unit coverage figure provenance-uncertain rather than silently repeating or discarding it. VC6 codegen intentionally moved for the 5 of 7 Zero Hour executables (and their Generals counterparts) that link this changed code, and that change is accepted, not reverted: every fix above repairs a real defect that also existed on 32-bit, just less visibly (a 4-byte-truncated address is still correct where addresses are 4 bytes). These are not architecture-conditional bugs, so their fixes aren't either. Preserving byte-identical .text would mean knowingly shipping broken crash dumps to keep a hash stable. Upstream's constraint (issue #473) is "must not break the VS6 build and compatibility" — the build works, and the change is confined to crash reporting, audio handle plumbing, registry access, W3D-asset load paths, profiling, and one CMake link gate. None of it touches game logic, the actual gameplay savegame system (Common/Xfer.h and friends — untouched; WWSaveLoad here is a separate, older W3D-asset framework), or network code — verified against the file list, not assumed. imagepacker.exe and wdump.exe, which link none of the changed code, reproduce the pre-fix-wave .text/normalized digests byte-for-byte, independently confirming the measurement pipeline. Clean-build gate results at this commit: - VC6 Zero Hour, --clean: exit 0, 0 errors - VC6 Generals, --clean: exit 0, 0 errors - MinGW i686 control, clean full: exit 0, 0 errors, 31,555 warnings — matches docs/x64/baseline-mingw-i686.md - MinGW x64, clean full: 602 errors / 36 shapes / 102 files / 1,669 objects — unchanged, as expected (correctness fixes, not error-count fixes) docs/x64/baseline-vc6.md rewritten: the incremental-vs-clean trap moved to the top, the C4189 chain and BaseTypeCore.h precedent added, both games' .text tables recorded (Zero Hour's baseline previously existed; Generals never had one despite Core changes since needing it), existing normalized-hash/git-metadata-drift mechanics preserved. scripts/verify-retail-baseline.sh added: always clean-builds both games and diffs .text against the doc (no incremental code path exists to forget to disable), with --check-only to compare already-built artifacts without triggering a build. Verified by parsing/hashing check against the on-disk clean-build artifacts (--check-only, no build run) and a negative-path test against mismatched artifacts to confirm mismatch/missing-file detection and non-zero exit both work. Co-Authored-By: Claude Opus 5 --- .../Source/WWVegas/WW3D2/rendobj.cpp | 11 ++- .../Source/WWVegas/WWAudio/AudibleSound.cpp | 13 +++- .../Source/WWVegas/WWAudio/AudibleSound.h | 2 +- .../Source/WWVegas/WWAudio/SoundScene.cpp | 2 +- .../Source/WWVegas/WWAudio/SoundSceneObj.h | 10 ++- .../Libraries/Source/WWVegas/WWLib/Except.cpp | 76 ++++++++++++------- Core/Libraries/Source/WWVegas/WWLib/Except.h | 8 +- .../Source/WWVegas/WWLib/registry.cpp | 2 +- .../Libraries/Source/WWVegas/WWLib/registry.h | 6 +- .../WWVegas/WWSaveLoad/persistfactory.h | 17 +++-- Core/Libraries/Source/debug/debug_debug.cpp | 74 +++++++++++++----- Core/Libraries/Source/debug/debug_debug.h | 16 ++-- Core/Libraries/Source/debug/debug_stack.cpp | 64 +++++++++++----- Core/Libraries/Source/debug/debug_stack.h | 16 +++- .../Source/profile/profile_funclevel.h | 10 ++- .../Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 11 ++- .../Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 11 ++- cmake/dx8.cmake | 11 ++- 18 files changed, 255 insertions(+), 105 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp index 4b04540bad0..8acd72cea71 100644 --- a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp +++ b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp @@ -1219,6 +1219,14 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const char name[64]; name[0] = '\0'; + // Read exactly what Save wrote: a fixed-width 4-byte identity token, not + // sizeof(old_obj). On x86-64 sizeof(RenderObjClass*) is 8, so reading + // sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for more + // bytes than the legacy 4-byte micro chunk holds; ChunkLoadClass::Read + // then refuses to read anything at all and old_obj stays null, silently + // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. + uint32 old_obj_token = 0; + while (cload.Open_Chunk()) { switch (cload.Cur_Chunk_ID()) { @@ -1226,7 +1234,7 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { - READ_MICRO_CHUNK(cload,RENDOBJFACTORY_VARIABLE_OBJPOINTER,old_obj); + case (RENDOBJFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; READ_MICRO_CHUNK(cload,RENDOBJFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,RENDOBJFACTORY_VARIABLE_NAME,name,sizeof(name)); } @@ -1269,6 +1277,7 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const new_obj->Set_Transform(tm); } + old_obj = (RenderObjClass *)(uintptr_t)old_obj_token; SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } diff --git a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp index 7e9bdc689d0..08ebdf4b71d 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp @@ -1712,8 +1712,17 @@ AudibleSoundClass::Load (ChunkLoadClass &cload) case VARID_THIS_PTR: { - AudibleSoundClass *old_ptr = nullptr; - cload.Read(&old_ptr, sizeof (old_ptr)); + // Read exactly what Save wrote: a fixed-width 4-byte + // identity token, not sizeof(old_ptr). On x86-64 + // sizeof(AudibleSoundClass*) is 8, so reading + // sizeof(old_ptr) here would ask for more bytes than + // the legacy 4-byte micro chunk holds; ChunkLoadClass::Read + // then refuses to read anything at all and old_ptr + // stays null, silently poisoning SaveLoadSystemClass's + // pointer remap table. See persistfactory.h. + uint32 old_ptr_token = 0; + cload.Read(&old_ptr_token, sizeof (old_ptr_token)); + AudibleSoundClass *old_ptr = (AudibleSoundClass *)(uintptr_t)old_ptr_token; SaveLoadSystemClass::Register_Pointer (old_ptr, this); } break; diff --git a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.h b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.h index 15db8cc302b..79028dc2d50 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.h +++ b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.h @@ -72,7 +72,7 @@ class SoundHandleClass; // Miles Sound System handles are pointers under the hood; this must be // pointer-sized to round-trip through Get_2D_Sample/Get_3D_Sample without // truncation on 64-bit targets. Runtime-only, never serialized. -typedef UnsignedIntPtr MILES_HANDLE; +typedef uintptr_t MILES_HANDLE; typedef enum { diff --git a/Core/Libraries/Source/WWVegas/WWAudio/SoundScene.cpp b/Core/Libraries/Source/WWVegas/WWAudio/SoundScene.cpp index 84843513156..0cd7a2859a5 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/SoundScene.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/SoundScene.cpp @@ -199,7 +199,7 @@ SoundSceneClass::Collect_Logical_Sounds (unsigned int milliseconds, int listener // Is the sound ready to notify? // if (sound_obj->Allow_Notify (timestamp)) { - listener->On_Event (AudioCallbackClass::EVENT_LOGICAL_HEARD, (UnsignedIntPtr)listener, (UnsignedIntPtr)sound_obj); + listener->On_Event (AudioCallbackClass::EVENT_LOGICAL_HEARD, (uintptr_t)listener, (uintptr_t)sound_obj); } } } diff --git a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.h b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.h index 280a00c48e5..12bc4838699 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.h +++ b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.h @@ -40,7 +40,9 @@ #include "WWSaveLoad/persist.h" #include "WWLib/multilist.h" #include "WWLib/mutex.h" -#include "Lib/BaseTypeCore.h" +// Only pulls the pointer-sized-int typedef (uintptr_t); avoids dragging +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include ///////////////////////////////////////////////////////////////////////////////// // Forward declarations @@ -126,7 +128,7 @@ class SoundSceneObjClass : public MultiListObjectClass, public PersistClass, pub ////////////////////////////////////////////////////////////////////// // param1/param2 double as pointers smuggled through integer parameters for // EVENT_LOGICAL_HEARD (see the inline definition below); must be pointer-sized. - virtual void On_Event (AudioCallbackClass::EVENTS event, UnsignedIntPtr param1 = 0, UnsignedIntPtr param2 = 0); + virtual void On_Event (AudioCallbackClass::EVENTS event, uintptr_t param1 = 0, uintptr_t param2 = 0); virtual void Register_Callback (AudioCallbackClass::EVENTS events, AudioCallbackClass *callback); ////////////////////////////////////////////////////////////////////// @@ -228,8 +230,8 @@ __inline void SoundSceneObjClass::On_Event ( AudioCallbackClass::EVENTS event, - UnsignedIntPtr param1, - UnsignedIntPtr param2 + uintptr_t param1, + uintptr_t param2 ) { if ((m_pCallback != nullptr) && (m_RegisteredEvents & event)) { diff --git a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp index 8223232efb7..aea2df3b49a 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp @@ -54,7 +54,9 @@ #include "assert.h" #include "cpudetect.h" #include "Except.h" -#include "Lib/BaseTypeCore.h" +// Only pulls the pointer-sized-int typedef (uintptr_t); avoids dragging +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include #include "Lib/arch_context.h" //#include "debug.h" #include "MPU.h" @@ -463,13 +465,13 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) // fptr walks across the consecutive _SymXxx globals below, each of which // is an actual function pointer (8 bytes on Win64) -- must be // pointer-sized or the stride only covers half of each slot on 64-bit. - UnsignedIntPtr *fptr = (UnsignedIntPtr*) &_SymCleanup; + uintptr_t *fptr = (uintptr_t*) &_SymCleanup; int count = 0; do { function_name = ImagehelpFunctionNames[count]; if (function_name) { - *fptr = (UnsignedIntPtr) GetProcAddress(imagehelp, function_name); + *fptr = (uintptr_t) GetProcAddress(imagehelp, function_name); fptr++; count++; } @@ -487,14 +489,14 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) } // SymLoadModuleType's return type is DWORD64 on x64 (matching the real - // SymLoadModule64 export); UnsignedIntPtr is pointer-width (4 bytes on + // SymLoadModule64 export); uintptr_t is pointer-width (4 bytes on // 32-bit, 8 on 64-bit) so the assignment below never truncates a // nonzero result down to a false "load failed" reading. Gated (rather // than just widening unconditionally) purely to keep the 32-bit/VC6 // codegen for this line textually identical to before -- the 32-bit // return type never changed, so there's nothing to fix on that branch. #if defined(_WIN64) || defined(__x86_64__) - UnsignedIntPtr symload = 0; + uintptr_t symload = 0; #else int symload = 0; #endif @@ -626,12 +628,12 @@ void Dump_Exception_Info(EXCEPTION_POINTERS *e_info) DebugString("Stack walk...\n"); Add_Txt("\r\n Stack walk...\r\n"); - unsigned long return_addresses[256]; + uintptr_t return_addresses[256]; int num_addresses = Stack_Walk(return_addresses, 256, context); if (num_addresses) { for (int s=0 ; s(stackptr), *stackptr); +#if defined(_WIN64) || defined(__x86_64__) + sprintf(scrap, "%p: %016llX ", static_cast(stackptr), (unsigned long long)*stackptr); +#else + sprintf(scrap, "%p: %08lX ", static_cast(stackptr), (unsigned long)*stackptr); +#endif strlcat(scrap, "DATA_PTR\r\n", ARRAY_SIZE(scrap)); } else { - sprintf(scrap, "%p: %08lX", static_cast(stackptr), *stackptr); +#if defined(_WIN64) || defined(__x86_64__) + sprintf(scrap, "%p: %016llX", static_cast(stackptr), (unsigned long long)*stackptr); +#else + sprintf(scrap, "%p: %08lX", static_cast(stackptr), (unsigned long)*stackptr); +#endif if (symbols_available) { symptr->SizeOfStruct = sizeof(symbol); @@ -1260,13 +1282,13 @@ void Load_Image_Helper() if (ImageHelp != nullptr) { char const *function_name = nullptr; // Same pointer-sized stride requirement as Dump_Exception_Info() above. - UnsignedIntPtr *fptr = (UnsignedIntPtr *) &_SymCleanup; + uintptr_t *fptr = (uintptr_t *) &_SymCleanup; int count = 0; do { function_name = ImagehelpFunctionNames[count]; if (function_name) { - *fptr = (UnsignedIntPtr) GetProcAddress(ImageHelp, function_name); + *fptr = (uintptr_t) GetProcAddress(ImageHelp, function_name); fptr++; count++; } @@ -1285,7 +1307,7 @@ void Load_Image_Helper() // so a nonzero DWORD64 result on x64 never truncates to a false 0, // gated to keep 32-bit/VC6 codegen textually unchanged. #if defined(_WIN64) || defined(__x86_64__) - UnsignedIntPtr symload = 0; + uintptr_t symload = 0; #else int symload = 0; #endif @@ -1378,14 +1400,14 @@ bool Lookup_Symbol(void *code_ptr, char *symbol, int &displacement) // _SymGetSymFromAddr to the real SymGetSymFromAddr64 export, whose // Address parameter is DWORD64 too -- this is not "fixed at 32 bits" // on this architecture. Use the full pointer width, not a narrowed one. - symbol_struct_ptr->Address = (UnsignedIntPtr)code_ptr; + symbol_struct_ptr->Address = (uintptr_t)code_ptr; #else // IMAGEHLP_SYMBOL::Address and SymGetSymFromAddr's DWORD parameter are // fixed at 32 bits by the (32-bit-only) DbgHelp API on this - // architecture. Cast through UnsignedIntPtr so the narrowing is an + // architecture. Cast through uintptr_t so the narrowing is an // explicit int-to-int conversion rather than a flagged pointer // truncation; 32-bit codegen is unchanged. - symbol_struct_ptr->Address = (unsigned long)(UnsignedIntPtr)code_ptr; + symbol_struct_ptr->Address = (unsigned long)(uintptr_t)code_ptr; #endif /* @@ -1399,10 +1421,10 @@ bool Lookup_Symbol(void *code_ptr, char *symbol, int &displacement) // the call returns, so this function's own signature (and every // caller of it) is unaffected. DWORD64 displacement64; - if (_SymGetSymFromAddr(GetCurrentProcess(), (DWORD64)(UnsignedIntPtr)code_ptr, &displacement64, symbol_struct_ptr)) { + if (_SymGetSymFromAddr(GetCurrentProcess(), (DWORD64)(uintptr_t)code_ptr, &displacement64, symbol_struct_ptr)) { displacement = (int)displacement64; #else - if (_SymGetSymFromAddr(GetCurrentProcess(), (unsigned long)(UnsignedIntPtr)code_ptr, (unsigned long *)&displacement, symbol_struct_ptr)) { + if (_SymGetSymFromAddr(GetCurrentProcess(), (unsigned long)(uintptr_t)code_ptr, (unsigned long *)&displacement, symbol_struct_ptr)) { #endif /* @@ -1433,7 +1455,7 @@ bool Lookup_Symbol(void *code_ptr, char *symbol, int &displacement) * HISTORY: * * 6/12/2001 11:57AM ST : Created * *=============================================================================================*/ -int Stack_Walk(unsigned long *return_addresses, int num_addresses, CONTEXT *context) +int Stack_Walk(uintptr_t *return_addresses, int num_addresses, CONTEXT *context) { static HINSTANCE _imagehelp = (HINSTANCE) -1; @@ -1458,12 +1480,12 @@ int Stack_Walk(unsigned long *return_addresses, int num_addresses, CONTEXT *cont STACKFRAME stack_frame; memset(&stack_frame, 0, sizeof(stack_frame)); - // UnsignedIntPtr rather than unsigned long: these feed + // uintptr_t rather than unsigned long: these feed // stack_frame.AddrPC/AddrFrame/AddrStack.Offset, which are DWORD64 in // STACKFRAME64 (STACKFRAME becomes STACKFRAME64 on x64 -- see // imagehlp.h's _IMAGEHLP64 mechanism), and `unsigned long` stays 32 bits // under Win64's LLP64 model, so it would truncate there. - UnsignedIntPtr reg_eip, reg_ebp, reg_esp; + uintptr_t reg_eip, reg_ebp, reg_esp; #if defined(_MSC_VER) && defined(_M_IX86) __asm { @@ -1489,9 +1511,9 @@ int Stack_Walk(unsigned long *return_addresses, int num_addresses, CONTEXT *cont // GCC/Clang retail-compatibility. CONTEXT capture_ctx; RtlCaptureContext(&capture_ctx); - reg_eip = (UnsignedIntPtr)CTX_PC(capture_ctx); - reg_ebp = (UnsignedIntPtr)CTX_FRAME(capture_ctx); - reg_esp = (UnsignedIntPtr)CTX_STACK(capture_ctx); + reg_eip = (uintptr_t)CTX_PC(capture_ctx); + reg_ebp = (uintptr_t)CTX_FRAME(capture_ctx); + reg_esp = (uintptr_t)CTX_STACK(capture_ctx); #endif stack_frame.AddrPC.Mode = AddrModeFlat; @@ -1524,7 +1546,7 @@ int Stack_Walk(unsigned long *return_addresses, int num_addresses, CONTEXT *cont if (i==0 && context == nullptr) { continue; } - unsigned long return_address = stack_frame.AddrReturn.Offset; + uintptr_t return_address = stack_frame.AddrReturn.Offset; return_addresses[pointer_index++] = return_address; } else { break; diff --git a/Core/Libraries/Source/WWVegas/WWLib/Except.h b/Core/Libraries/Source/WWVegas/WWLib/Except.h index 3cc178cc373..78ef6a94a89 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/Except.h +++ b/Core/Libraries/Source/WWVegas/WWLib/Except.h @@ -39,6 +39,7 @@ #if defined(_WIN32) #include "win.h" +#include /* ** Forward Declarations */ @@ -46,7 +47,12 @@ typedef struct _EXCEPTION_POINTERS EXCEPTION_POINTERS; typedef struct _CONTEXT CONTEXT; int Exception_Handler(int exception_code, EXCEPTION_POINTERS *e_info); -int Stack_Walk(unsigned long *return_addresses, int num_addresses, CONTEXT *context = nullptr); +// return_addresses is uintptr_t, not unsigned long: it holds raw return +// addresses captured off the stack (see Except.cpp's Stack_Walk), and on +// x86-64 (LLP64) `long` stays 32 bits while an address is 64. Stack_Walk has +// exactly one call site (Except.cpp), so this is a self-contained widening, +// not a public-ABI change in practice. +int Stack_Walk(uintptr_t *return_addresses, int num_addresses, CONTEXT *context = nullptr); bool Lookup_Symbol(void *code_ptr, char *symbol, int &displacement); void Load_Image_Helper(); void Register_Thread_ID(unsigned long thread_id, char *thread_name, bool main = false); diff --git a/Core/Libraries/Source/WWVegas/WWLib/registry.cpp b/Core/Libraries/Source/WWVegas/WWLib/registry.cpp index 7d56a298a6d..13b41e97df0 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/registry.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/registry.cpp @@ -79,7 +79,7 @@ RegistryClass::RegistryClass( const char * sub_key, bool create ) : if (ERROR_SUCCESS == result) { IsValid = true; - Key = (UnsignedIntPtr)key; + Key = (uintptr_t)key; } } diff --git a/Core/Libraries/Source/WWVegas/WWLib/registry.h b/Core/Libraries/Source/WWVegas/WWLib/registry.h index a939cdb7045..5705e6362c5 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/registry.h +++ b/Core/Libraries/Source/WWVegas/WWLib/registry.h @@ -39,7 +39,9 @@ #include "Vector.h" #include "wwstring.h" #include "widestring.h" -#include "Lib/BaseTypeCore.h" +// Only pulls the pointer-sized-int typedef (uintptr_t); avoids dragging +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include class INIClass; @@ -108,7 +110,7 @@ class RegistryClass { static void Save_Registry_Values(HKEY key, char *path, INIClass *ini); - UnsignedIntPtr Key; + uintptr_t Key; bool IsValid; // diff --git a/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h b/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h index ca17bb447dd..0fac7604f58 100644 --- a/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h +++ b/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h @@ -42,7 +42,9 @@ #include "WWDebug/wwdebug.h" #include "saveload.h" #include "persist.h" -#include "Lib/BaseTypeCore.h" +// Only pulls the pointer-sized-int typedef (uintptr_t); avoids dragging +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include /* ** PersistFactoryClass @@ -78,9 +80,12 @@ class PersistFactoryClass ** object. Simply instantiate a single static instance of this template with the ** type and chunkid in the .cpp file of your class. */ -// The on-disk width of the object-identity token, fixed by the retail save format. -// Changing it changes the format. -static_assert(sizeof(uint32) == 4, "savegame object token must stay 4 bytes on disk"); +// The on-disk width of the object-identity token is fixed by the retail save +// format at 4 bytes (uint32, i.e. `unsigned long` on our Windows targets). +// Changing it changes the format. Not guarded by a static_assert here: under +// VC6, Dependencies/Utility/Utility/CppMacros.h defines static_assert(expr,msg) +// as empty, so it would silently compile away on exactly the toolchain this +// project builds for first, giving no real protection. template class SimplePersistFactoryClass : public PersistFactoryClass { @@ -124,7 +129,7 @@ SimplePersistFactoryClass::Load(ChunkLoadClass & cload) const new_obj->Load(cload); cload.Close_Chunk(); - void * old_obj = (void *)(UnsignedIntPtr)old_obj_token; + void * old_obj = (void *)(uintptr_t)old_obj_token; SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } @@ -138,7 +143,7 @@ SimplePersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * // fixup can bind the wrong object on load. The on-disk width is fixed by the // retail save format and cannot be widened here without breaking it. // See docs/x64/savegame-format-decision.md (written by the later task). - uint32 objptr = (uint32)(UnsignedIntPtr)obj; + uint32 objptr = (uint32)(uintptr_t)obj; csave.Begin_Chunk(SIMPLEFACTORY_CHUNKID_OBJPOINTER); csave.Write(&objptr,sizeof(uint32)); csave.End_Chunk(); diff --git a/Core/Libraries/Source/debug/debug_debug.cpp b/Core/Libraries/Source/debug/debug_debug.cpp index 5ccbf70bcb3..3cb698b9865 100644 --- a/Core/Libraries/Source/debug/debug_debug.cpp +++ b/Core/Libraries/Source/debug/debug_debug.cpp @@ -35,7 +35,9 @@ #include #include #include // needed for placement new prototype -#include "Lib/BaseTypeCore.h" +// Only pulls the pointer-sized-int typedef (uintptr_t); avoids dragging +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include // a little dummy variable that makes the linker actually include // us... @@ -74,7 +76,7 @@ Debug::LogDescription::LogDescription(const char *fileOrGroup, const char *descr Debug Debug::Instance; // more class static members -UnsignedIntPtr Debug::curStackFrame; +uintptr_t Debug::curStackFrame; // this constructor is empty on purpose because all construction // work is done in PreStaticInit (and some in PostStaticInit) @@ -306,21 +308,40 @@ bool Debug::SkipNext() // do not implement this function inline, we do need // a valid frame pointer here! - // UnsignedIntPtr is `unsigned int` (4 bytes) on the VC6 32-bit target, so + // uintptr_t is `unsigned int` (4 bytes) on the VC6 32-bit target, so // the _asm block below -- which needs a 4-byte destination to match eax -- // is byte-identical to the original `unsigned help;` version there. - UnsignedIntPtr help; + uintptr_t help; #if defined(_MSC_VER) && defined(_M_IX86) _asm { mov eax,[ebp+4] // return address mov help,eax }; +#elif (defined(__GNUC__) || defined(__clang__)) && (defined(__i386__) || defined(_M_IX86)) + // GCC/Clang inline assembly for x86-32. Kept as its own arm, gated the same + // way as the equivalent blocks in Except.cpp and debug_stack.cpp, so 32-bit + // GCC/Clang codegen for this function is unchanged from before this x64 + // port touched it -- an earlier version of this arm covered every + // GCC/Clang architecture (including x86-64, where it does not apply) and + // was narrowed here to match its siblings rather than left broad. + __asm__ __volatile__( + "mov 4(%%ebp), %0" + : "=r"(help) + : + : "memory" + ); #elif defined(__GNUC__) || defined(__clang__) - // __builtin_return_address(0) is the portable spelling of [ebp+4] and works - // on every architecture GCC/Clang targets, so this replaces both the - // x86-32 asm and the #error that followed it. - help = (UnsignedIntPtr)__builtin_return_address(0); + // Everything else GCC/Clang targets (x86-64 in practice): unlike + // Except.cpp's Stack_Walk and debug_stack.cpp's captured-register path, + // this function only ever needs the immediate caller's return address, not + // a full register set to seed a multi-frame StackWalk64. There is no + // matching ebp-relative asm trick on x86-64 (no frame-pointer-at-fixed- + // offset convention to rely on), so __builtin_return_address(0) -- GCC/ + // Clang's portable spelling of "caller's return address" on every + // architecture they target -- is used directly instead of a CONTEXT-capture + // dance that would be overkill for a single value. + help = (uintptr_t)__builtin_return_address(0); #else #error "Unsupported compiler or architecture for inline assembly" #endif @@ -900,12 +921,19 @@ Debug& Debug::operator<<(const void *ptr) (*this) << "ptr:"; if (ptr) { + // Full pointer width, not the low 32 bits: a crash report's register + // dump is already 16 hex digits on x64 (operator<<(unsigned __int64) + // below), so an address truncated to 8 digits here would silently drop + // the high half next to registers that don't. 32-bit output is + // unchanged -- uintptr_t is unsigned int there, so this arm still + // resolves to the exact same _ultoa(...,help,16) call as before. +#if defined(_WIN64) || defined(__x86_64__) + char help[64+1]; // sign, 64 digits, NUL -- matches operator<<(unsigned __int64)'s buffer + (*this) << "0x" << _ui64toa((unsigned __int64)(uintptr_t)ptr,help,16); +#else char help[9]; - // Cast through UnsignedIntPtr (lossless) before narrowing to the - // 32-bit type _ultoa requires. On 64-bit this deliberately shows only - // the low 32 bits of the address -- acceptable for a debug print, and - // keeps 32-bit output byte-identical (UnsignedIntPtr is unsigned int there). - (*this) << "0x" << _ultoa((unsigned long)(UnsignedIntPtr)ptr,help,16); + (*this) << "0x" << _ultoa((unsigned long)(uintptr_t)ptr,help,16); +#endif } else (*this) << "null"; @@ -936,11 +964,15 @@ Debug& Debug::operator<<(const MemDump &dump) for (unsigned i=0;iUnsignedIntPtr step so the narrowing is an - // explicit int-to-int conversion rather than a flagged pointer truncation. - sprintf(buf,"%08x",dump.m_absAddr?(unsigned)(UnsignedIntPtr)cur:(unsigned)(cur-dump.m_startPtr)); + sprintf(buf,"%08x",dump.m_absAddr?(unsigned)(uintptr_t)cur:(unsigned)(cur-dump.m_startPtr)); +#endif operator<<(buf); // items @@ -1019,12 +1051,12 @@ bool Debug::IsLogEnabled(const char *fileOrGroup) // that we are having real static strings let's use // that strings address as frame address... // LookupFrame/AddFrameEntry use the string's address as a hash key and are - // declared to take UnsignedIntPtr, so this is a lossless pointer->integer + // declared to take uintptr_t, so this is a lossless pointer->integer // conversion on every target (it used to truncate through `unsigned` on // x64; that guard is gone now that the hash key is pointer-width). - FrameHashEntry *e=Instance.LookupFrame((UnsignedIntPtr)fileOrGroup); + FrameHashEntry *e=Instance.LookupFrame((uintptr_t)fileOrGroup); if (!e) - e=Instance.AddFrameEntry((UnsignedIntPtr)fileOrGroup,FrameTypeLog,fileOrGroup,0); + e=Instance.AddFrameEntry((uintptr_t)fileOrGroup,FrameTypeLog,fileOrGroup,0); if (e->status==Unknown) Instance.UpdateFrameStatus(*e); return e->status==NoSkip; @@ -1207,7 +1239,7 @@ void Debug::Update() } } -Debug::FrameHashEntry* Debug::AddFrameEntry(UnsignedIntPtr addr, unsigned type, +Debug::FrameHashEntry* Debug::AddFrameEntry(uintptr_t addr, unsigned type, const char *fileOrGroup, int line) { __ASSERT(LookupFrame(addr)==nullptr); diff --git a/Core/Libraries/Source/debug/debug_debug.h b/Core/Libraries/Source/debug/debug_debug.h index ef835cb8e75..132d11e41aa 100644 --- a/Core/Libraries/Source/debug/debug_debug.h +++ b/Core/Libraries/Source/debug/debug_debug.h @@ -29,7 +29,11 @@ #pragma once -#include "Lib/BaseTypeCore.h" +// Only pulls the pointer-sized-int typedef (uintptr_t); avoids dragging +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +// debug_debug.h is included by nearly every debug translation unit, so any +// warning-as-error pragma pulled in here becomes effectively global. +#include /** \class Debug debug.h @@ -874,7 +878,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); CmdInterfaceListEntry *firstCmdGroup; /// \internal current stack frame (used by SkipNext) - static UnsignedIntPtr curStackFrame; + static uintptr_t curStackFrame; /** \internal @@ -922,7 +926,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); FrameHashEntry *next; /// frame address - UnsignedIntPtr frameAddr; + uintptr_t frameAddr; /// frame type (FrameTypeAssert, FrameTypeCheck, or FrameTypeLog) unsigned frameType; @@ -962,7 +966,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); \param addr frame address \return FrameHashEntry found or 0 if nothing found */ - __forceinline FrameHashEntry *LookupFrame(UnsignedIntPtr addr) + __forceinline FrameHashEntry *LookupFrame(uintptr_t addr) { for (FrameHashEntry *e=frameHash[addr%FRAME_HASH_SIZE];e;e=e->next) if (e->frameAddr==addr) @@ -982,7 +986,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); \param line line number \return the entry just added */ - FrameHashEntry *AddFrameEntry(UnsignedIntPtr addr, unsigned type, + FrameHashEntry *AddFrameEntry(uintptr_t addr, unsigned type, const char *fileOrGroup, int line); /** \internal @@ -1004,7 +1008,7 @@ DLOG( "My HResult is: " << Debug::HResult(SomeHRESULTValue) << "\n" ); \param line line number \return the entry just added (or the already existing entry) */ - FrameHashEntry *GetFrameEntry(UnsignedIntPtr addr, unsigned type, + FrameHashEntry *GetFrameEntry(uintptr_t addr, unsigned type, const char *fileOrGroup, int line) { FrameHashEntry *e=LookupFrame(addr); diff --git a/Core/Libraries/Source/debug/debug_stack.cpp b/Core/Libraries/Source/debug/debug_stack.cpp index 926aeedeaf2..233dd94ae2b 100644 --- a/Core/Libraries/Source/debug/debug_stack.cpp +++ b/Core/Libraries/Source/debug/debug_stack.cpp @@ -32,7 +32,10 @@ #include #include "WWLib/stringex.h" #include -#include "Lib/BaseTypeCore.h" +#include +// Only pulls the pointer-sized-int typedef (uintptr_t); avoids dragging +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include #include "Lib/arch_context.h" // imagehlp.h (via dbghelp.h's psdk_inc/_dbg_common.h) #defines StackWalk to @@ -72,7 +75,7 @@ static union // Overlays the struct above, whose members are actual function pointers // (8 bytes on Win64). Must be pointer-sized or the aliasing/stride used // by InitDbghelp() below only covers half of each slot on 64-bit. - UnsignedIntPtr funcPtr[1]; + uintptr_t funcPtr[1]; } gDbg; #undef DBGHELP @@ -156,11 +159,11 @@ static void InitDbghelp() return; // Get function addresses - UnsignedIntPtr *funcptr=gDbg.funcPtr; + uintptr_t *funcptr=gDbg.funcPtr; unsigned k=0; for (;DebughelpFunctionNames[k];++k,++funcptr) { - *funcptr=(UnsignedIntPtr)GetProcAddress(g_dbghelp,DebughelpFunctionNames[k]); + *funcptr=(uintptr_t)GetProcAddress(g_dbghelp,DebughelpFunctionNames[k]); if (!*funcptr) break; } @@ -202,13 +205,13 @@ DebugStackwalk::Signature& DebugStackwalk::Signature::operator=(const Signature& return *this; } -unsigned DebugStackwalk::Signature::GetAddress(int n) const +uintptr_t DebugStackwalk::Signature::GetAddress(int n) const { DFAIL_IF_MSG(n<0||n>=MAX_ADDR,n << "/" << MAX_ADDR) return 0; return m_addr[n]; } -void DebugStackwalk::Signature::GetSymbol(unsigned addr, char *buf, unsigned bufSize) +void DebugStackwalk::Signature::GetSymbol(uintptr_t addr, char *buf, unsigned bufSize) { DFAIL_IF(!buf) return; DFAIL_IF(bufSize<64||bufSize>=0x80000000) return; @@ -217,10 +220,22 @@ void DebugStackwalk::Signature::GetSymbol(unsigned addr, char *buf, unsigned buf char *bufEnd=buf+bufSize; *buf=0; - buf+=wsprintf(buf,"%08x",addr); +#if defined(_WIN64) || defined(__x86_64__) + // sprintf (CRT), not wsprintf (User32's own limited formatter, used for + // every other format string in this function): wsprintf's documented + // format support does not include a 64-bit-width specifier, and this is + // the one field in this function that can actually need one. + buf+=sprintf(buf,"%016llX",(unsigned long long)addr); +#else + buf+=wsprintf(buf,"%08x",(unsigned)addr); +#endif // determine module - unsigned modBase=gDbg._SymGetModuleBase((HANDLE)GetCurrentProcessId(),addr); + // Pointer-width, not `unsigned`: _SymGetModuleBase resolves to + // SymGetModuleBase64 on x64 (see debug_stack.inl) and returns a DWORD64; + // truncating it here would corrupt every `addr-modBase` relative offset + // computed below. + uintptr_t modBase=gDbg._SymGetModuleBase((HANDLE)GetCurrentProcessId(),addr); if (!modBase) { strcpy(buf," (unknown module)"); @@ -228,7 +243,7 @@ void DebugStackwalk::Signature::GetSymbol(unsigned addr, char *buf, unsigned buf } // illegal code ptr? - if (IsBadReadPtr((void *)addr,4)||IsBadCodePtr((FARPROC)addr)) + if (IsBadReadPtr((void *)addr,sizeof(addr))||IsBadCodePtr((FARPROC)addr)) { strcpy(buf," (invalid code addr)"); return; @@ -244,7 +259,12 @@ void DebugStackwalk::Signature::GetSymbol(unsigned addr, char *buf, unsigned buf buf+=strlen(buf); if (bufEnd-buf<32) return; - buf+=wsprintf(buf,"+0x%x",addr-modBase); + // Cast to unsigned: a module-relative offset is well under 4GB in + // practice (it's an offset within a single loaded module, not an + // absolute address), and wsprintf's "%x" is a 32-bit format regardless + // of argument width -- passing the full uintptr_t here would mismatch + // the format on x64. + buf+=wsprintf(buf,"+0x%x",(unsigned)(addr-modBase)); // determine symbol PIMAGEHLP_SYMBOL symPtr=(PIMAGEHLP_SYMBOL)symbolBuffer; @@ -284,7 +304,7 @@ void DebugStackwalk::Signature::GetSymbol(unsigned addr, char *buf, unsigned buf buf+=wsprintf(buf,", %s:%i+0x%x",p,line.LineNumber,displacement); } -void DebugStackwalk::Signature::GetSymbol(unsigned addr, +void DebugStackwalk::Signature::GetSymbol(uintptr_t addr, char *bufMod, unsigned sizeMod, unsigned *relMod, char *bufSym, unsigned sizeSym, unsigned *relSym, char *bufFile, unsigned sizeFile, unsigned *linePtr, unsigned *relLine) @@ -304,8 +324,9 @@ void DebugStackwalk::Signature::GetSymbol(unsigned addr, DFAIL_IF(bufSym&&sizeSym<16) return; DFAIL_IF(bufFile&&sizeFile<16) return; - // determine module - unsigned modBase=gDbg._SymGetModuleBase((HANDLE)GetCurrentProcessId(),addr); + // determine module (see the other GetSymbol overload's comment: pointer- + // width, not `unsigned`, since this resolves to SymGetModuleBase64 on x64) + uintptr_t modBase=gDbg._SymGetModuleBase((HANDLE)GetCurrentProcessId(),addr); if (!modBase) { if (bufMod) @@ -316,7 +337,7 @@ void DebugStackwalk::Signature::GetSymbol(unsigned addr, } // illegal code ptr? - if (IsBadReadPtr((void *)addr,4)||IsBadCodePtr((FARPROC)addr)) + if (IsBadReadPtr((void *)addr,sizeof(addr))||IsBadCodePtr((FARPROC)addr)) { if (bufMod) strcpy(bufMod,"(inv code addr)"); @@ -334,8 +355,11 @@ void DebugStackwalk::Signature::GetSymbol(unsigned addr, p=p?p+1:symbolBuffer; strlcpy(bufMod,p,sizeMod); } + // relMod is `unsigned *`, unchanged: a module-relative offset is well + // under 4GB in practice, unlike the absolute addr/modBase this is derived + // from. if (relMod) - *relMod=addr-modBase; + *relMod=(unsigned)(addr-modBase); // determine symbol if (bufSym) @@ -450,11 +474,11 @@ int DebugStackwalk::StackWalk(Signature &sig, struct _CONTEXT *ctx) else { // walk stack back using current call chain - // UnsignedIntPtr rather than unsigned long: these feed + // uintptr_t rather than unsigned long: these feed // stackFrame.AddrPC/AddrFrame/AddrStack.Offset, which are DWORD64 in // STACKFRAME64 (STACKFRAME becomes STACKFRAME64 on x64), and // `unsigned long` stays 32 bits under Win64's LLP64 model. - UnsignedIntPtr reg_eip, reg_ebp, reg_esp; + uintptr_t reg_eip, reg_ebp, reg_esp; #if defined(_MSC_VER) && defined(_M_IX86) __asm { @@ -481,9 +505,9 @@ int DebugStackwalk::StackWalk(Signature &sig, struct _CONTEXT *ctx) // three fields via CTX_PC/CTX_FRAME/CTX_STACK. CONTEXT capture_ctx; RtlCaptureContext(&capture_ctx); - reg_eip = (UnsignedIntPtr)CTX_PC(capture_ctx); - reg_ebp = (UnsignedIntPtr)CTX_FRAME(capture_ctx); - reg_esp = (UnsignedIntPtr)CTX_STACK(capture_ctx); + reg_eip = (uintptr_t)CTX_PC(capture_ctx); + reg_ebp = (uintptr_t)CTX_FRAME(capture_ctx); + reg_esp = (uintptr_t)CTX_STACK(capture_ctx); #endif stackFrame.AddrPC.Offset = reg_eip; stackFrame.AddrStack.Offset = reg_esp; diff --git a/Core/Libraries/Source/debug/debug_stack.h b/Core/Libraries/Source/debug/debug_stack.h index 18f1a7e6229..964d91483fd 100644 --- a/Core/Libraries/Source/debug/debug_stack.h +++ b/Core/Libraries/Source/debug/debug_stack.h @@ -29,6 +29,10 @@ #pragma once +// Only pulls the pointer-sized-int typedef (uintptr_t); avoids dragging +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include + /// \brief stack walker class (singleton) class DebugStackwalk { @@ -56,7 +60,11 @@ class DebugStackwalk unsigned m_numAddr; /// addresses - unsigned m_addr[MAX_ADDR]; + // Pointer-width, not `unsigned`: callers pass CTX_PC(ctx), a DWORD64 on + // x64 (debug_except.cpp), and a 32-bit slot here would silently truncate + // every address stored, corrupting both signature dedup and symbol + // lookup on x64. + uintptr_t m_addr[MAX_ADDR]; public: explicit Signature(): m_numAddr(0) {} @@ -78,7 +86,7 @@ class DebugStackwalk \param n index, 0..Size()-1 \return signature address */ - unsigned GetAddress(int n) const; + uintptr_t GetAddress(int n) const; /** \brief Strong ordering operator. @@ -110,7 +118,7 @@ class DebugStackwalk \param buf return buffer \param bufSize size of return buffer, minimum is 64 bytes (256 recommended) */ - static void GetSymbol(unsigned addr, char *buf, unsigned bufSize); + static void GetSymbol(uintptr_t addr, char *buf, unsigned bufSize); /** \brief Determines symbol for given address. @@ -127,7 +135,7 @@ class DebugStackwalk \param line line number, may be nullptr \param relLine relative address within line, may be nullptr */ - static void GetSymbol(unsigned addr, + static void GetSymbol(uintptr_t addr, char *bufMod, unsigned sizeMod, unsigned *relMod, char *bufSym, unsigned sizeSym, unsigned *relSym, char *bufFile, unsigned sizeFile, unsigned *line, unsigned *relLine); diff --git a/Core/Libraries/Source/profile/profile_funclevel.h b/Core/Libraries/Source/profile/profile_funclevel.h index 4aceee55188..849052eb010 100644 --- a/Core/Libraries/Source/profile/profile_funclevel.h +++ b/Core/Libraries/Source/profile/profile_funclevel.h @@ -29,7 +29,9 @@ #pragma once -#include "Lib/BaseTypeCore.h" +// Only pulls the pointer-sized-int typedef (uintptr_t); avoids dragging +// BaseTypeCore.h's warning-as-error pragmas into a file that never had them. +#include /** \brief The function level profiler. @@ -190,11 +192,11 @@ class ProfileFuncLevel // sprintf()s this value through "prof%08x-all.csv" to name the output // file -- that %08x is the format boundary, so GetId() cannot be // widened without also changing the on-disk file-naming convention. - // Cast through UnsignedIntPtr so the narrowing is an explicit + // Cast through uintptr_t so the narrowing is an explicit // pointer-to-int-to-int conversion rather than a silent pointer - // truncation; 32-bit output is unchanged since UnsignedIntPtr is + // truncation; 32-bit output is unchanged since uintptr_t is // unsigned int there. - return unsigned(UnsignedIntPtr(m_threadID)); + return unsigned(uintptr_t(m_threadID)); } private: diff --git a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index 3f0dd3302ab..39517f80e8a 100644 --- a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1349,6 +1349,14 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const char dazzle_type[256]; dazzle_type[0] = 0; + // Read exactly what Save wrote: a fixed-width 4-byte identity token, not + // sizeof(old_obj). On x86-64 sizeof(DazzleRenderObjClass*) is 8, so + // reading sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for + // more bytes than the legacy 4-byte micro chunk holds; ChunkLoadClass::Read + // then refuses to read anything at all and old_obj stays null, silently + // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. + uint32 old_obj_token = 0; + /* ** Load the dazzle parameters */ @@ -1359,7 +1367,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { - READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_OBJPOINTER,old_obj); + case (DAZZLEFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type,sizeof(dazzle_type)); } @@ -1403,6 +1411,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const /* ** Register the old pointer for re-mapping to the new pointer */ + old_obj = (DazzleRenderObjClass *)(uintptr_t)old_obj_token; SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } diff --git a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index cfe2e251433..ecd1597ca1d 100644 --- a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1452,6 +1452,14 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const char dazzle_type[256]; dazzle_type[0] = 0; + // Read exactly what Save wrote: a fixed-width 4-byte identity token, not + // sizeof(old_obj). On x86-64 sizeof(DazzleRenderObjClass*) is 8, so + // reading sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for + // more bytes than the legacy 4-byte micro chunk holds; ChunkLoadClass::Read + // then refuses to read anything at all and old_obj stays null, silently + // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. + uint32 old_obj_token = 0; + /* ** Load the dazzle parameters */ @@ -1462,7 +1470,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { - READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_OBJPOINTER,old_obj); + case (DAZZLEFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type,sizeof(dazzle_type)); } @@ -1506,6 +1514,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const /* ** Register the old pointer for re-mapping to the new pointer */ + old_obj = (DazzleRenderObjClass *)(uintptr_t)old_obj_token; SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } diff --git a/cmake/dx8.cmake b/cmake/dx8.cmake index f0c321a1685..0dd81dec781 100644 --- a/cmake/dx8.cmake +++ b/cmake/dx8.cmake @@ -32,8 +32,15 @@ endif() # MSVC-specific configuration if(MSVC) - # Use bundled MSVC-compiled .lib files - target_link_libraries(d3d8lib INTERFACE d3dx8) + # Use bundled MSVC-compiled .lib files. d3dx8.lib is pe-i386 (32-bit) + # only -- there is no 64-bit build of it in the fetched min-dx8-sdk repo + # -- so linking it into a 64-bit target would fail, the same reason the + # top-level d3d8/dinput8/dxguid link above is gated on + # CMAKE_SIZEOF_VOID_P EQUAL 4. No x64 MSVC preset exercises this today + # (issue #473), but the condition should match its sibling regardless. + if(CMAKE_SIZEOF_VOID_P EQUAL 4) + target_link_libraries(d3d8lib INTERFACE d3dx8) + endif() target_link_directories(d3d8lib BEFORE INTERFACE ${dx8_SOURCE_DIR}) target_link_options(d3d8lib INTERFACE /NODEFAULTLIB:libci.lib) From 2c2bc27e9badbc08d290c0a85033efe3d3aa2de9 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Wed, 2 Sep 2026 16:13:05 +0200 Subject: [PATCH 12/20] fix(x64): port StackDump and DbgHelpLoader, the third crash handler CTX_* context accessors, the ...64 DbgHelp entry points with widened signatures audited against psdk_inc/_dbg_common.h, RtlCaptureContext for register capture. x64 errors 602 -> 550; MinGW i686 and VC6 stay clean. Two tool artifacts (mapcachebuilder ZH, WorldBuilderV) move .text under VC6. Proven layout-only: the header edit flips VC6 weak-external emission, which reshuffles /OPT:ICF folding -- symbol set and section sizes are unchanged and both game exes stay byte-identical. Full evidence chain in the fork: MeneerHaas/GeneralsGameCode, docs/x64/HANDOFF-vc6-text-mismatch.md. Co-Authored-By: Claude Opus 5 --- .../Source/WWVegas/WWLib/DbgHelpLoader.cpp | 59 ++++++++++++ .../Source/WWVegas/WWLib/DbgHelpLoader.h | 92 +++++++++++++++++++ .../GameEngine/Include/Common/StackDump.h | 8 ++ .../Source/Common/System/StackDump.cpp | 77 +++++++++++++++- .../GameEngine/Include/Common/StackDump.h | 8 ++ .../Source/Common/System/StackDump.cpp | 77 +++++++++++++++- 6 files changed, 317 insertions(+), 4 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp index dab6686125c..e463df1b3de 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp @@ -111,16 +111,31 @@ bool DbgHelpLoader::load() Inst->m_loadedFromSystem = true; } + // TheSuperHackers @fix MeneerHaas 02/09/2026 x64 dbghelp.dll only exports the ...64 names for the + // address-taking entry points; un-suffixed GetProcAddress returns NULL there. 32-bit order kept (docs/x64). Inst->m_symInitialize = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymInitialize")); Inst->m_symCleanup = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymCleanup")); +#if defined(_WIN64) || defined(__x86_64__) + Inst->m_symLoadModule = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymLoadModule64")); + Inst->m_symUnloadModule = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymUnloadModule64")); + Inst->m_symGetModuleBase = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymGetModuleBase64")); + Inst->m_symGetSymFromAddr = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymGetSymFromAddr64")); + Inst->m_symGetLineFromAddr = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymGetLineFromAddr64")); +#else Inst->m_symLoadModule = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymLoadModule")); Inst->m_symUnloadModule = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymUnloadModule")); Inst->m_symGetModuleBase = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymGetModuleBase")); Inst->m_symGetSymFromAddr = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymGetSymFromAddr")); Inst->m_symGetLineFromAddr = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymGetLineFromAddr")); +#endif Inst->m_symSetOptions = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymSetOptions")); +#if defined(_WIN64) || defined(__x86_64__) + Inst->m_symFunctionTableAccess = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymFunctionTableAccess64")); + Inst->m_stackWalk = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "StackWalk64")); +#else Inst->m_symFunctionTableAccess = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymFunctionTableAccess")); Inst->m_stackWalk = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "StackWalk")); +#endif #ifdef RTS_ENABLE_CRASHDUMP Inst->m_miniDumpWriteDump = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "MiniDumpWriteDump")); #endif @@ -232,6 +247,15 @@ BOOL DbgHelpLoader::symCleanup( return FALSE; } +#if defined(_WIN64) || defined(__x86_64__) +DWORD64 DbgHelpLoader::symLoadModule( + HANDLE hProcess, + HANDLE hFile, + LPSTR ImageName, + LPSTR ModuleName, + DWORD64 BaseOfDll, + DWORD SizeOfDll) +#else BOOL DbgHelpLoader::symLoadModule( HANDLE hProcess, HANDLE hFile, @@ -239,6 +263,7 @@ BOOL DbgHelpLoader::symLoadModule( LPSTR ModuleName, DWORD BaseOfDll, DWORD SizeOfDll) +#endif { CriticalSectionClass::LockClass lock(CriticalSection); @@ -248,9 +273,15 @@ BOOL DbgHelpLoader::symLoadModule( return FALSE; } +#if defined(_WIN64) || defined(__x86_64__) +DWORD64 DbgHelpLoader::symGetModuleBase( + HANDLE hProcess, + DWORD64 dwAddr) +#else DWORD DbgHelpLoader::symGetModuleBase( HANDLE hProcess, DWORD dwAddr) +#endif { CriticalSectionClass::LockClass lock(CriticalSection); @@ -260,9 +291,15 @@ DWORD DbgHelpLoader::symGetModuleBase( return 0u; } +#if defined(_WIN64) || defined(__x86_64__) +BOOL DbgHelpLoader::symUnloadModule( + HANDLE hProcess, + DWORD64 BaseOfDll) +#else BOOL DbgHelpLoader::symUnloadModule( HANDLE hProcess, DWORD BaseOfDll) +#endif { CriticalSectionClass::LockClass lock(CriticalSection); @@ -272,11 +309,19 @@ BOOL DbgHelpLoader::symUnloadModule( return FALSE; } +#if defined(_WIN64) || defined(__x86_64__) +BOOL DbgHelpLoader::symGetSymFromAddr( + HANDLE hProcess, + DWORD64 Address, + PDWORD64 Displacement, + PIMAGEHLP_SYMBOL Symbol) +#else BOOL DbgHelpLoader::symGetSymFromAddr( HANDLE hProcess, DWORD Address, LPDWORD Displacement, PIMAGEHLP_SYMBOL Symbol) +#endif { CriticalSectionClass::LockClass lock(CriticalSection); @@ -286,11 +331,19 @@ BOOL DbgHelpLoader::symGetSymFromAddr( return FALSE; } +#if defined(_WIN64) || defined(__x86_64__) +BOOL DbgHelpLoader::symGetLineFromAddr( + HANDLE hProcess, + DWORD64 dwAddr, + PDWORD pdwDisplacement, + PIMAGEHLP_LINE Line) +#else BOOL DbgHelpLoader::symGetLineFromAddr( HANDLE hProcess, DWORD dwAddr, PDWORD pdwDisplacement, PIMAGEHLP_LINE Line) +#endif { CriticalSectionClass::LockClass lock(CriticalSection); @@ -311,9 +364,15 @@ DWORD DbgHelpLoader::symSetOptions( return 0u; } +#if defined(_WIN64) || defined(__x86_64__) +LPVOID DbgHelpLoader::symFunctionTableAccess( + HANDLE hProcess, + DWORD64 AddrBase) +#else LPVOID DbgHelpLoader::symFunctionTableAccess( HANDLE hProcess, DWORD AddrBase) +#endif { CriticalSectionClass::LockClass lock(CriticalSection); diff --git a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.h b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.h index 556cbafcc61..d1dfa32d9c4 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.h +++ b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.h @@ -66,6 +66,17 @@ class DbgHelpLoader static BOOL WINAPI symCleanup( HANDLE hProcess); + // TheSuperHackers @fix MeneerHaas 02/09/2026 Widened to the ...64 ABI on x64, audited against + // psdk_inc/_dbg_common.h; SymGetLineFromAddr64's pdwDisplacement genuinely stays PDWORD. +#if defined(_WIN64) || defined(__x86_64__) + static DWORD64 WINAPI symLoadModule( + HANDLE hProcess, + HANDLE hFile, + LPSTR ImageName, + LPSTR ModuleName, + DWORD64 BaseOfDll, + DWORD SizeOfDll); +#else static BOOL WINAPI symLoadModule( HANDLE hProcess, HANDLE hFile, @@ -73,33 +84,70 @@ class DbgHelpLoader LPSTR ModuleName, DWORD BaseOfDll, DWORD SizeOfDll); +#endif +#if defined(_WIN64) || defined(__x86_64__) + static DWORD64 WINAPI symGetModuleBase( + HANDLE hProcess, + DWORD64 dwAddr); +#else static DWORD WINAPI symGetModuleBase( HANDLE hProcess, DWORD dwAddr); +#endif +#if defined(_WIN64) || defined(__x86_64__) + static BOOL WINAPI symUnloadModule( + HANDLE hProcess, + DWORD64 BaseOfDll); +#else static BOOL WINAPI symUnloadModule( HANDLE hProcess, DWORD BaseOfDll); +#endif + // TheSuperHackers @fix MeneerHaas 02/09/2026 Displacement is a write target: 4-byte slot under an + // 8-byte SymGetSymFromAddr64 write is a stack buffer overflow. +#if defined(_WIN64) || defined(__x86_64__) + static BOOL WINAPI symGetSymFromAddr( + HANDLE hProcess, + DWORD64 Address, + PDWORD64 Displacement, + PIMAGEHLP_SYMBOL Symbol); +#else static BOOL WINAPI symGetSymFromAddr( HANDLE hProcess, DWORD Address, LPDWORD Displacement, PIMAGEHLP_SYMBOL Symbol); +#endif +#if defined(_WIN64) || defined(__x86_64__) + static BOOL WINAPI symGetLineFromAddr( + HANDLE hProcess, + DWORD64 dwAddr, + PDWORD pdwDisplacement, + PIMAGEHLP_LINE Line); +#else static BOOL WINAPI symGetLineFromAddr( HANDLE hProcess, DWORD dwAddr, PDWORD pdwDisplacement, PIMAGEHLP_LINE Line); +#endif static DWORD WINAPI symSetOptions( DWORD SymOptions); +#if defined(_WIN64) || defined(__x86_64__) + static LPVOID WINAPI symFunctionTableAccess( + HANDLE hProcess, + DWORD64 AddrBase); +#else static LPVOID WINAPI symFunctionTableAccess( HANDLE hProcess, DWORD AddrBase); +#endif static BOOL WINAPI stackWalk( DWORD MachineType, @@ -135,6 +183,15 @@ class DbgHelpLoader typedef BOOL (WINAPI *SymCleanup_t) ( HANDLE hProcess); +#if defined(_WIN64) || defined(__x86_64__) + typedef DWORD64 (WINAPI *SymLoadModule_t) ( + HANDLE hProcess, + HANDLE hFile, + LPSTR ImageName, + LPSTR ModuleName, + DWORD64 BaseOfDll, + DWORD SizeOfDll); +#else typedef BOOL (WINAPI *SymLoadModule_t) ( HANDLE hProcess, HANDLE hFile, @@ -142,33 +199,68 @@ class DbgHelpLoader LPSTR ModuleName, DWORD BaseOfDll, DWORD SizeOfDll); +#endif +#if defined(_WIN64) || defined(__x86_64__) + typedef DWORD64 (WINAPI *SymGetModuleBase_t) ( + HANDLE hProcess, + DWORD64 dwAddr); +#else typedef DWORD (WINAPI *SymGetModuleBase_t) ( HANDLE hProcess, DWORD dwAddr); +#endif +#if defined(_WIN64) || defined(__x86_64__) + typedef BOOL (WINAPI *SymUnloadModule_t) ( + HANDLE hProcess, + DWORD64 BaseOfDll); +#else typedef BOOL (WINAPI *SymUnloadModule_t) ( HANDLE hProcess, DWORD BaseOfDll); +#endif +#if defined(_WIN64) || defined(__x86_64__) + typedef BOOL (WINAPI *SymGetSymFromAddr_t) ( + HANDLE hProcess, + DWORD64 Address, + PDWORD64 Displacement, + PIMAGEHLP_SYMBOL Symbol); +#else typedef BOOL (WINAPI *SymGetSymFromAddr_t) ( HANDLE hProcess, DWORD Address, LPDWORD Displacement, PIMAGEHLP_SYMBOL Symbol); +#endif +#if defined(_WIN64) || defined(__x86_64__) + typedef BOOL (WINAPI* SymGetLineFromAddr_t) ( + HANDLE hProcess, + DWORD64 dwAddr, + PDWORD pdwDisplacement, + PIMAGEHLP_LINE Line); +#else typedef BOOL (WINAPI* SymGetLineFromAddr_t) ( HANDLE hProcess, DWORD dwAddr, PDWORD pdwDisplacement, PIMAGEHLP_LINE Line); +#endif typedef DWORD (WINAPI *SymSetOptions_t) ( DWORD SymOptions); +#if defined(_WIN64) || defined(__x86_64__) + typedef LPVOID (WINAPI *SymFunctionTableAccess_t) ( + HANDLE hProcess, + DWORD64 AddrBase); +#else typedef LPVOID (WINAPI *SymFunctionTableAccess_t) ( HANDLE hProcess, DWORD AddrBase); +#endif typedef BOOL (WINAPI *StackWalk_t) ( DWORD MachineType, diff --git a/Generals/Code/GameEngine/Include/Common/StackDump.h b/Generals/Code/GameEngine/Include/Common/StackDump.h index 2d11b754b94..3a9dbf232ce 100644 --- a/Generals/Code/GameEngine/Include/Common/StackDump.h +++ b/Generals/Code/GameEngine/Include/Common/StackDump.h @@ -24,6 +24,9 @@ #pragma once +// TheSuperHackers @fix MeneerHaas 02/09/2026 stdint_adapter over BaseTypeCore.h to avoid its warning-as-error pragmas. +#include + #ifndef IG_DEBUG_STACKTRACE #define IG_DEBUG_STACKTRACE 1 #endif // Unsure about this one -ML 3/25/03 @@ -35,7 +38,12 @@ void StackDump(void (*callback)(const char*)); // Writes a stackdump (provide a callback : gets called per line) // If callback is nullptr then will write using OuputDebugString +// TheSuperHackers @fix MeneerHaas 02/09/2026 uintptr_t on x64; guarded so the 32-bit signature and mangling are unchanged. +#if defined(_WIN64) || defined(__x86_64__) +void StackDumpFromContext(uintptr_t eip,uintptr_t esp,uintptr_t ebp, void (*callback)(const char*)); +#else void StackDumpFromContext(DWORD eip,DWORD esp,DWORD ebp, void (*callback)(const char*)); +#endif // Gets count* addresses from the current stack void FillStackAddresses(void**addresses, unsigned int count, unsigned int skip = 0); diff --git a/Generals/Code/GameEngine/Source/Common/System/StackDump.cpp b/Generals/Code/GameEngine/Source/Common/System/StackDump.cpp index 485a459a187..31b77e9bd7a 100644 --- a/Generals/Code/GameEngine/Source/Common/System/StackDump.cpp +++ b/Generals/Code/GameEngine/Source/Common/System/StackDump.cpp @@ -33,11 +33,20 @@ #include "WWLib/DbgHelpLoader.h" +// TheSuperHackers @fix MeneerHaas 02/09/2026 stdint_adapter over BaseTypeCore.h to avoid its warning-as-error pragmas. +#include +#include "Lib/arch_context.h" + //***************************************************************************** // Prototypes //***************************************************************************** BOOL InitSymbolInfo(); +// TheSuperHackers @fix MeneerHaas 02/09/2026 uintptr_t on x64; guarded so the 32-bit mangled name is unchanged. +#if defined(_WIN64) || defined(__x86_64__) +void MakeStackTrace(uintptr_t myeip,uintptr_t myesp,uintptr_t myebp, int skipFrames, void (*callback)(const char*)); +#else void MakeStackTrace(DWORD myeip,DWORD myesp,DWORD myebp, int skipFrames, void (*callback)(const char*)); +#endif void GetFunctionDetails(void *pointer, char*name, char*filename, unsigned int* linenumber, unsigned int* address); void WriteStackLine(void*address, void (*callback)(const char*)); @@ -67,9 +76,14 @@ void StackDump(void (*callback)(const char*)) if (!InitSymbolInfo()) return; + // TheSuperHackers @fix MeneerHaas 02/09/2026 uintptr_t on x64; guarded to keep 32-bit codegen identical. +#if defined(_WIN64) || defined(__x86_64__) + uintptr_t myeip,myesp,myebp; +#else DWORD myeip,myesp,myebp; +#endif -#if defined(_MSC_VER) +#if defined(_MSC_VER) && defined(_M_IX86) _asm { MYEIP1: @@ -91,6 +105,13 @@ _asm : : "memory" ); +#elif defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 RtlCaptureContext seeds the stack walk on x64 (no __asm there), as in debug_stack.cpp. + CONTEXT capture_ctx; + RtlCaptureContext(&capture_ctx); + myeip = (uintptr_t)CTX_PC(capture_ctx); + myesp = (uintptr_t)CTX_STACK(capture_ctx); + myebp = (uintptr_t)CTX_FRAME(capture_ctx); #else #error "Unsupported compiler or architecture for register capture" #endif @@ -102,7 +123,11 @@ _asm //***************************************************************************** //***************************************************************************** +#if defined(_WIN64) || defined(__x86_64__) +void StackDumpFromContext(uintptr_t eip,uintptr_t esp,uintptr_t ebp, void (*callback)(const char*)) +#else void StackDumpFromContext(DWORD eip,DWORD esp,DWORD ebp, void (*callback)(const char*)) +#endif { if (callback == nullptr) { @@ -170,7 +195,11 @@ BOOL InitSymbolInfo() //***************************************************************************** //***************************************************************************** +#if defined(_WIN64) || defined(__x86_64__) +void MakeStackTrace(uintptr_t myeip,uintptr_t myesp,uintptr_t myebp, int skipFrames, void (*callback)(const char*)) +#else void MakeStackTrace(DWORD myeip,DWORD myesp,DWORD myebp, int skipFrames, void (*callback)(const char*)) +#endif { STACKFRAME stack_frame; BOOL b_ret = TRUE; @@ -278,8 +307,17 @@ void GetFunctionDetails(void *pointer, char*name, char*filename, unsigned int* l psymbol->SizeOfStruct = sizeof(symbol_buffer); psymbol->MaxNameLength = 512; + // TheSuperHackers @fix MeneerHaas 02/09/2026 (uintptr_t)pointer: a (DWORD) cast would truncate live code addresses on x64. +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 SymGetSymFromAddr64 writes 8 bytes; capture wide, then narrow for SymGetLineFromAddr. + DWORD64 displacement64; + if (DbgHelpLoader::symGetSymFromAddr(process, (uintptr_t) pointer, &displacement64, psymbol)) + { + displacement = (DWORD)displacement64; +#else if (DbgHelpLoader::symGetSymFromAddr(process, (DWORD) pointer, &displacement, psymbol)) { +#endif if (name) { strcpy(name, psymbol->Name); @@ -292,7 +330,11 @@ void GetFunctionDetails(void *pointer, char*name, char*filename, unsigned int* l memset(&line,0,sizeof(line)); line.SizeOfStruct = sizeof(line); +#if defined(_WIN64) || defined(__x86_64__) + if (DbgHelpLoader::symGetLineFromAddr(process, (uintptr_t) pointer, &displacement, &line)) +#else if (DbgHelpLoader::symGetLineFromAddr(process, (DWORD) pointer, &displacement, &line)) +#endif { if (filename) { @@ -328,8 +370,12 @@ void FillStackAddresses(void**addresses, unsigned int count, unsigned int skip) memset(&gsContext, 0, sizeof(CONTEXT)); gsContext.ContextFlags = CONTEXT_FULL; +#if defined(_WIN64) || defined(__x86_64__) + uintptr_t myeip,myesp,myebp; +#else DWORD myeip,myesp,myebp; -#if defined(_MSC_VER) +#endif +#if defined(_MSC_VER) && defined(_M_IX86) _asm { MYEIP2: @@ -353,6 +399,13 @@ _asm : : "eax", "memory" ); +#elif defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 RtlCaptureContext replaces the inline-asm register capture on x64. + CONTEXT capture_ctx; + RtlCaptureContext(&capture_ctx); + myeip = (uintptr_t)CTX_PC(capture_ctx); + myesp = (uintptr_t)CTX_STACK(capture_ctx); + myebp = (uintptr_t)CTX_FRAME(capture_ctx); #else #error "Unsupported compiler or architecture for register capture" #endif @@ -589,7 +642,11 @@ void DumpExceptionInfo( unsigned int u, EXCEPTION_POINTERS* e_info ) } DOUBLE_DEBUG (("\nStack Dump:")); +#if defined(_WIN64) || defined(__x86_64__) + StackDumpFromContext(CTX_PC(*context), CTX_STACK(*context), CTX_FRAME(*context), nullptr); +#else StackDumpFromContext(context->Eip, context->Esp, context->Ebp, nullptr); +#endif DOUBLE_DEBUG (("\nDetails:")); @@ -598,9 +655,15 @@ void DumpExceptionInfo( unsigned int u, EXCEPTION_POINTERS* e_info ) /* ** Dump the registers. */ +#if defined(_WIN64) || defined(__x86_64__) + DOUBLE_DEBUG ( ( "Rip:%016llX\tRsp:%016llX\tRbp:%016llX", (unsigned long long)CTX_PC(*context), (unsigned long long)CTX_STACK(*context), (unsigned long long)CTX_FRAME(*context))); + DOUBLE_DEBUG ( ( "Rax:%016llX\tRbx:%016llX\tRcx:%016llX", (unsigned long long)CTX_AX(*context), (unsigned long long)CTX_BX(*context), (unsigned long long)CTX_CX(*context))); + DOUBLE_DEBUG ( ( "Rdx:%016llX\tRsi:%016llX\tRdi:%016llX", (unsigned long long)CTX_DX(*context), (unsigned long long)CTX_SI(*context), (unsigned long long)CTX_DI(*context))); +#else DOUBLE_DEBUG ( ( "Eip:%08X\tEsp:%08X\tEbp:%08X", context->Eip, context->Esp, context->Ebp)); DOUBLE_DEBUG ( ( "Eax:%08X\tEbx:%08X\tEcx:%08X", context->Eax, context->Ebx, context->Ecx)); DOUBLE_DEBUG ( ( "Edx:%08X\tEsi:%08X\tEdi:%08X", context->Edx, context->Esi, context->Edi)); +#endif DOUBLE_DEBUG ( ( "EFlags:%08X ", context->EFlags)); DOUBLE_DEBUG ( ( "CS:%04x SS:%04x DS:%04x ES:%04x FS:%04x GS:%04x", context->SegCs, context->SegSs, context->SegDs, context->SegEs, context->SegFs, context->SegGs)); @@ -609,9 +672,19 @@ void DumpExceptionInfo( unsigned int u, EXCEPTION_POINTERS* e_info ) */ char scrap[512]; DOUBLE_DEBUG ( ("EIP bytes dump...")); +#if defined(_WIN64) || defined(__x86_64__) + // wsprintf is Win32's own limited formatter and has no %llX -- sprintf + // (CRT) is used here instead, matching Except.cpp's identical case. + sprintf (scrap, "\nBytes at CS:RIP (%016llX) : ", (unsigned long long)CTX_PC(*context)); +#else wsprintf (scrap, "\nBytes at CS:EIP (%08X) : ", context->Eip); +#endif +#if defined(_WIN64) || defined(__x86_64__) + unsigned char *eip_ptr = (unsigned char *) (CTX_PC(*context)); +#else unsigned char *eip_ptr = (unsigned char *) (context->Eip); +#endif char bytestr[32]; for (int c = 0 ; c < 32 ; c++) diff --git a/GeneralsMD/Code/GameEngine/Include/Common/StackDump.h b/GeneralsMD/Code/GameEngine/Include/Common/StackDump.h index ce84c0af736..487fc7a494e 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/StackDump.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/StackDump.h @@ -24,6 +24,9 @@ #pragma once +// TheSuperHackers @fix MeneerHaas 02/09/2026 stdint_adapter over BaseTypeCore.h to avoid its warning-as-error pragmas. +#include + #ifndef IG_DEBUG_STACKTRACE #define IG_DEBUG_STACKTRACE 1 #endif // Unsure about this one -ML 3/25/03 @@ -35,7 +38,12 @@ void StackDump(void (*callback)(const char*)); // Writes a stackdump (provide a callback : gets called per line) // If callback is nullptr then will write using OuputDebugString +// TheSuperHackers @fix MeneerHaas 02/09/2026 uintptr_t on x64; guarded so the 32-bit signature and mangling are unchanged. +#if defined(_WIN64) || defined(__x86_64__) +void StackDumpFromContext(uintptr_t eip,uintptr_t esp,uintptr_t ebp, void (*callback)(const char*)); +#else void StackDumpFromContext(DWORD eip,DWORD esp,DWORD ebp, void (*callback)(const char*)); +#endif // Gets count* addresses from the current stack void FillStackAddresses(void**addresses, unsigned int count, unsigned int skip = 0); diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp index 08328666992..e7847e37548 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp @@ -33,11 +33,20 @@ #include "WWLib/DbgHelpLoader.h" +// TheSuperHackers @fix MeneerHaas 02/09/2026 stdint_adapter over BaseTypeCore.h to avoid its warning-as-error pragmas. +#include +#include "Lib/arch_context.h" + //***************************************************************************** // Prototypes //***************************************************************************** BOOL InitSymbolInfo(); +// TheSuperHackers @fix MeneerHaas 02/09/2026 uintptr_t on x64; guarded so the 32-bit mangled name is unchanged. +#if defined(_WIN64) || defined(__x86_64__) +void MakeStackTrace(uintptr_t myeip,uintptr_t myesp,uintptr_t myebp, int skipFrames, void (*callback)(const char*)); +#else void MakeStackTrace(DWORD myeip,DWORD myesp,DWORD myebp, int skipFrames, void (*callback)(const char*)); +#endif void GetFunctionDetails(void *pointer, char*name, char*filename, unsigned int* linenumber, unsigned int* address); void WriteStackLine(void*address, void (*callback)(const char*)); @@ -67,9 +76,14 @@ void StackDump(void (*callback)(const char*)) if (!InitSymbolInfo()) return; + // TheSuperHackers @fix MeneerHaas 02/09/2026 uintptr_t on x64; guarded to keep 32-bit codegen identical. +#if defined(_WIN64) || defined(__x86_64__) + uintptr_t myeip,myesp,myebp; +#else DWORD myeip,myesp,myebp; +#endif -#if defined(_MSC_VER) +#if defined(_MSC_VER) && defined(_M_IX86) _asm { MYEIP1: @@ -91,6 +105,13 @@ _asm : : "memory" ); +#elif defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 RtlCaptureContext seeds the stack walk on x64 (no __asm there), as in debug_stack.cpp. + CONTEXT capture_ctx; + RtlCaptureContext(&capture_ctx); + myeip = (uintptr_t)CTX_PC(capture_ctx); + myesp = (uintptr_t)CTX_STACK(capture_ctx); + myebp = (uintptr_t)CTX_FRAME(capture_ctx); #else #error "Unsupported compiler or architecture for register capture" #endif @@ -102,7 +123,11 @@ _asm //***************************************************************************** //***************************************************************************** +#if defined(_WIN64) || defined(__x86_64__) +void StackDumpFromContext(uintptr_t eip,uintptr_t esp,uintptr_t ebp, void (*callback)(const char*)) +#else void StackDumpFromContext(DWORD eip,DWORD esp,DWORD ebp, void (*callback)(const char*)) +#endif { if (callback == nullptr) { @@ -170,7 +195,11 @@ BOOL InitSymbolInfo() //***************************************************************************** //***************************************************************************** +#if defined(_WIN64) || defined(__x86_64__) +void MakeStackTrace(uintptr_t myeip,uintptr_t myesp,uintptr_t myebp, int skipFrames, void (*callback)(const char*)) +#else void MakeStackTrace(DWORD myeip,DWORD myesp,DWORD myebp, int skipFrames, void (*callback)(const char*)) +#endif { STACKFRAME stack_frame; BOOL b_ret = TRUE; @@ -278,8 +307,17 @@ void GetFunctionDetails(void *pointer, char*name, char*filename, unsigned int* l psymbol->SizeOfStruct = sizeof(symbol_buffer); psymbol->MaxNameLength = 512; + // TheSuperHackers @fix MeneerHaas 02/09/2026 (uintptr_t)pointer: a (DWORD) cast would truncate live code addresses on x64. +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 SymGetSymFromAddr64 writes 8 bytes; capture wide, then narrow for SymGetLineFromAddr. + DWORD64 displacement64; + if (DbgHelpLoader::symGetSymFromAddr(process, (uintptr_t) pointer, &displacement64, psymbol)) + { + displacement = (DWORD)displacement64; +#else if (DbgHelpLoader::symGetSymFromAddr(process, (DWORD) pointer, &displacement, psymbol)) { +#endif if (name) { strcpy(name, psymbol->Name); @@ -292,7 +330,11 @@ void GetFunctionDetails(void *pointer, char*name, char*filename, unsigned int* l memset(&line,0,sizeof(line)); line.SizeOfStruct = sizeof(line); +#if defined(_WIN64) || defined(__x86_64__) + if (DbgHelpLoader::symGetLineFromAddr(process, (uintptr_t) pointer, &displacement, &line)) +#else if (DbgHelpLoader::symGetLineFromAddr(process, (DWORD) pointer, &displacement, &line)) +#endif { if (filename) { @@ -328,8 +370,12 @@ void FillStackAddresses(void**addresses, unsigned int count, unsigned int skip) memset(&gsContext, 0, sizeof(CONTEXT)); gsContext.ContextFlags = CONTEXT_FULL; +#if defined(_WIN64) || defined(__x86_64__) + uintptr_t myeip,myesp,myebp; +#else DWORD myeip,myesp,myebp; -#if defined(_MSC_VER) +#endif +#if defined(_MSC_VER) && defined(_M_IX86) _asm { MYEIP2: @@ -353,6 +399,13 @@ _asm : : "eax", "memory" ); +#elif defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 RtlCaptureContext replaces the inline-asm register capture on x64. + CONTEXT capture_ctx; + RtlCaptureContext(&capture_ctx); + myeip = (uintptr_t)CTX_PC(capture_ctx); + myesp = (uintptr_t)CTX_STACK(capture_ctx); + myebp = (uintptr_t)CTX_FRAME(capture_ctx); #else #error "Unsupported compiler or architecture for register capture" #endif @@ -589,7 +642,11 @@ void DumpExceptionInfo( unsigned int u, EXCEPTION_POINTERS* e_info ) } DOUBLE_DEBUG (("\nStack Dump:")); +#if defined(_WIN64) || defined(__x86_64__) + StackDumpFromContext(CTX_PC(*context), CTX_STACK(*context), CTX_FRAME(*context), nullptr); +#else StackDumpFromContext(context->Eip, context->Esp, context->Ebp, nullptr); +#endif DOUBLE_DEBUG (("\nDetails:")); @@ -598,9 +655,15 @@ void DumpExceptionInfo( unsigned int u, EXCEPTION_POINTERS* e_info ) /* ** Dump the registers. */ +#if defined(_WIN64) || defined(__x86_64__) + DOUBLE_DEBUG ( ( "Rip:%016llX\tRsp:%016llX\tRbp:%016llX", (unsigned long long)CTX_PC(*context), (unsigned long long)CTX_STACK(*context), (unsigned long long)CTX_FRAME(*context))); + DOUBLE_DEBUG ( ( "Rax:%016llX\tRbx:%016llX\tRcx:%016llX", (unsigned long long)CTX_AX(*context), (unsigned long long)CTX_BX(*context), (unsigned long long)CTX_CX(*context))); + DOUBLE_DEBUG ( ( "Rdx:%016llX\tRsi:%016llX\tRdi:%016llX", (unsigned long long)CTX_DX(*context), (unsigned long long)CTX_SI(*context), (unsigned long long)CTX_DI(*context))); +#else DOUBLE_DEBUG ( ( "Eip:%08X\tEsp:%08X\tEbp:%08X", context->Eip, context->Esp, context->Ebp)); DOUBLE_DEBUG ( ( "Eax:%08X\tEbx:%08X\tEcx:%08X", context->Eax, context->Ebx, context->Ecx)); DOUBLE_DEBUG ( ( "Edx:%08X\tEsi:%08X\tEdi:%08X", context->Edx, context->Esi, context->Edi)); +#endif DOUBLE_DEBUG ( ( "EFlags:%08X ", context->EFlags)); DOUBLE_DEBUG ( ( "CS:%04x SS:%04x DS:%04x ES:%04x FS:%04x GS:%04x", context->SegCs, context->SegSs, context->SegDs, context->SegEs, context->SegFs, context->SegGs)); @@ -609,9 +672,19 @@ void DumpExceptionInfo( unsigned int u, EXCEPTION_POINTERS* e_info ) */ char scrap[512]; DOUBLE_DEBUG ( ("EIP bytes dump...")); +#if defined(_WIN64) || defined(__x86_64__) + // wsprintf is Win32's own limited formatter and has no %llX -- sprintf + // (CRT) is used here instead, matching Except.cpp's identical case. + sprintf (scrap, "\nBytes at CS:RIP (%016llX) : ", (unsigned long long)CTX_PC(*context)); +#else wsprintf (scrap, "\nBytes at CS:EIP (%08X) : ", context->Eip); +#endif +#if defined(_WIN64) || defined(__x86_64__) + unsigned char *eip_ptr = (unsigned char *) (CTX_PC(*context)); +#else unsigned char *eip_ptr = (unsigned char *) (context->Eip); +#endif char bytestr[32]; for (int c = 0 ; c < 32 ; c++) From a316cedfc34e7af8d5c175c5cf01c335aab61d96 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Wed, 2 Sep 2026 20:44:12 +0200 Subject: [PATCH 13/20] fix(x64): write the same 4-byte identity token the persist loaders read Review follow-up (#3248): the three micro-chunk identity writers (rendobj, dazzle, AudibleSound) still passed a raw pointer to WRITE_MICRO_CHUNK, which writes 8 bytes on x64 while every loader reads the legacy 4-byte token. The write path has no callers in this repository, but writer and reader now agree on the on-disk width. Also corrects arch_context.h's copyright attribution to TheSuperHackers per repository convention for new files. Co-Authored-By: Claude Opus 5 --- Core/Libraries/Include/Lib/arch_context.h | 2 +- Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp | 6 ++++++ Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp | 6 ++++++ Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 6 ++++++ GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 6 ++++++ 5 files changed, 25 insertions(+), 1 deletion(-) diff --git a/Core/Libraries/Include/Lib/arch_context.h b/Core/Libraries/Include/Lib/arch_context.h index 58c4b7a1b9a..99044dd7d90 100644 --- a/Core/Libraries/Include/Lib/arch_context.h +++ b/Core/Libraries/Include/Lib/arch_context.h @@ -1,6 +1,6 @@ /* ** Command & Conquer Generals Zero Hour(tm) -** Copyright 2025 Electronic Arts Inc. +** Copyright 2026 TheSuperHackers ** ** This program is free software: you can redistribute it and/or modify ** it under the terms of the GNU General Public License as published by diff --git a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp index 8acd72cea71..f396f993054 100644 --- a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp +++ b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp @@ -1289,7 +1289,13 @@ void RenderObjPersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * ob const Matrix3D& tm = robj->Get_Transform(); csave.Begin_Chunk(RENDOBJFACTORY_CHUNKID_VARIABLES); +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. + uint32 robj_token = (uint32)(uintptr_t)robj; + WRITE_MICRO_CHUNK(csave,RENDOBJFACTORY_VARIABLE_OBJPOINTER,robj_token); +#else WRITE_MICRO_CHUNK(csave,RENDOBJFACTORY_VARIABLE_OBJPOINTER,robj); +#endif WRITE_MICRO_CHUNK_STRING(csave,RENDOBJFACTORY_VARIABLE_NAME,name); WRITE_MICRO_CHUNK(csave,RENDOBJFACTORY_VARIABLE_TRANSFORM,tm); csave.End_Chunk(); diff --git a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp index 08ebdf4b71d..facabfb244e 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp @@ -1654,8 +1654,14 @@ AudibleSoundClass::Save (ChunkSaveClass &csave) WRITE_MICRO_CHUNK_STRING (csave, VARID_FILENAME, m_Buffer->Get_Filename ()); } +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. + uint32 this_ptr_token = (uint32)(uintptr_t)this; + WRITE_MICRO_CHUNK (csave, VARID_THIS_PTR, this_ptr_token); +#else AudibleSoundClass *this_ptr = this; WRITE_MICRO_CHUNK (csave, VARID_THIS_PTR, this_ptr); +#endif csave.End_Chunk (); diff --git a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index 39517f80e8a..91f51591cac 100644 --- a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1424,7 +1424,13 @@ void DazzlePersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * obj) const Matrix3D& tm = robj->Get_Transform(); csave.Begin_Chunk(DAZZLEFACTORY_CHUNKID_VARIABLES); +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. + uint32 robj_token = (uint32)(uintptr_t)robj; + WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj_token); +#else WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj); +#endif WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); WRITE_MICRO_CHUNK_STRING(csave,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type_name); diff --git a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index ecd1597ca1d..05abfdbdafd 100644 --- a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1527,7 +1527,13 @@ void DazzlePersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * obj) const Matrix3D& tm = robj->Get_Transform(); csave.Begin_Chunk(DAZZLEFACTORY_CHUNKID_VARIABLES); +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. + uint32 robj_token = (uint32)(uintptr_t)robj; + WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj_token); +#else WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj); +#endif WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); WRITE_MICRO_CHUNK_STRING(csave,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type_name); From d4dd7b85281c6fc5573562685126c9a15017194e Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Wed, 2 Sep 2026 21:06:55 +0200 Subject: [PATCH 14/20] fix(x64): give StackWalk64 a real context and the AMD64 machine type Review follow-up (#3248): on AMD64 StackWalk64 requires a ContextRecord and updates it while unwinding, but all four walkers passed nullptr, and both game StackDump ports still asked for IMAGE_FILE_MACHINE_I386 while the loader resolves StackWalk64. Each walker now seeds a mutable local context from the frame it starts at (never the caller's, which StackWalk64 would mutate) and uses CTX_STACKWALK_MACHINE, as the Core walkers already did. The 32-bit arms preprocess to exactly what they were: RTS_STACKWALK_CONTEXT expands to nullptr and CTX_STACKWALK_MACHINE to IMAGE_FILE_MACHINE_I386. Co-Authored-By: Claude Opus 5 --- .../Libraries/Source/WWVegas/WWLib/Except.cpp | 24 ++++++++++- Core/Libraries/Source/debug/debug_stack.cpp | 24 ++++++++++- .../Source/Common/System/StackDump.cpp | 41 +++++++++++++++---- .../Source/Common/System/StackDump.cpp | 41 +++++++++++++++---- 4 files changed, 112 insertions(+), 18 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp index aea2df3b49a..0b7a0251623 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/Except.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/Except.cpp @@ -58,6 +58,15 @@ // BaseTypeCore.h's warning-as-error pragmas into a file that never had them. #include #include "Lib/arch_context.h" + +// TheSuperHackers @fix MeneerHaas 02/09/2026 StackWalk64 requires a ContextRecord on AMD64 (it is optional on x86) and +// updates it while unwinding, so the walker below seeds a mutable local walk_ctx and passes it +// through this macro. On 32-bit it expands to the retail nullptr, leaving that arm unchanged. +#if defined(_WIN64) || defined(__x86_64__) +#define RTS_STACKWALK_CONTEXT (&walk_ctx) +#else +#define RTS_STACKWALK_CONTEXT nullptr +#endif //#include "debug.h" #include "MPU.h" //#include "commando\nat.h" @@ -1532,13 +1541,26 @@ int Stack_Walk(uintptr_t *return_addresses, int num_addresses, CONTEXT *context) stack_frame.AddrFrame.Offset = CTX_FRAME(*context); } +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Walk a mutable copy seeded from the frame this walk + // starts at -- StackWalk64 updates it, so never hand it the caller's context. + CONTEXT walk_ctx; + if (context) + walk_ctx = *context; + else + RtlCaptureContext(&walk_ctx); + CTX_PC(walk_ctx) = stack_frame.AddrPC.Offset; + CTX_STACK(walk_ctx) = stack_frame.AddrStack.Offset; + CTX_FRAME(walk_ctx) = stack_frame.AddrFrame.Offset; +#endif + int pointer_index = 0; /* ** Walk the stack by the requested number of return address iterations. */ for (int i = 0; i < num_addresses + 1; i++) { - if (_StackWalk(CTX_STACKWALK_MACHINE, GetCurrentProcess(), GetCurrentThread(), &stack_frame, nullptr, nullptr, _SymFunctionTableAccess, _SymGetModuleBase, nullptr)) { + if (_StackWalk(CTX_STACKWALK_MACHINE, GetCurrentProcess(), GetCurrentThread(), &stack_frame, RTS_STACKWALK_CONTEXT, nullptr, _SymFunctionTableAccess, _SymGetModuleBase, nullptr)) { /* ** First result will always be the return address we were called from. diff --git a/Core/Libraries/Source/debug/debug_stack.cpp b/Core/Libraries/Source/debug/debug_stack.cpp index 233dd94ae2b..2710781c507 100644 --- a/Core/Libraries/Source/debug/debug_stack.cpp +++ b/Core/Libraries/Source/debug/debug_stack.cpp @@ -38,6 +38,15 @@ #include #include "Lib/arch_context.h" +// TheSuperHackers @fix MeneerHaas 02/09/2026 StackWalk64 requires a ContextRecord on AMD64 (it is optional on x86) and +// updates it while unwinding, so the walker below seeds a mutable local walk_ctx and passes it +// through this macro. On 32-bit it expands to the retail nullptr, leaving that arm unchanged. +#if defined(_WIN64) || defined(__x86_64__) +#define RTS_STACKWALK_CONTEXT (&walk_ctx) +#else +#define RTS_STACKWALK_CONTEXT nullptr +#endif + // imagehlp.h (via dbghelp.h's psdk_inc/_dbg_common.h) #defines StackWalk to // StackWalk64 on 64-bit builds, because _IMAGEHLP64 is set whenever _WIN64 // is defined. DebugStackwalk::StackWalk below is our own class method, not @@ -514,11 +523,24 @@ int DebugStackwalk::StackWalk(Signature &sig, struct _CONTEXT *ctx) stackFrame.AddrFrame.Offset = reg_ebp; } +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Walk a mutable copy seeded from the frame this walk + // starts at -- StackWalk64 updates it, so never hand it the caller's context. + CONTEXT walk_ctx; + if (ctx) + walk_ctx = *ctx; + else + RtlCaptureContext(&walk_ctx); + CTX_PC(walk_ctx) = stackFrame.AddrPC.Offset; + CTX_STACK(walk_ctx) = stackFrame.AddrStack.Offset; + CTX_FRAME(walk_ctx) = stackFrame.AddrFrame.Offset; +#endif + // Walk the stack by the requested number of return address iterations. bool skipFirst=!ctx; while (sig.m_numAddr #include "Lib/arch_context.h" +// TheSuperHackers @fix MeneerHaas 02/09/2026 StackWalk64 requires a ContextRecord on AMD64 (it is optional on x86) and +// updates it while unwinding, so the walkers below seed a mutable local walk_ctx and pass it +// through this macro. On 32-bit it expands to the retail nullptr, leaving that arm unchanged. +#if defined(_WIN64) || defined(__x86_64__) +#define RTS_STACKWALK_CONTEXT (&walk_ctx) +#else +#define RTS_STACKWALK_CONTEXT nullptr +#endif + //***************************************************************************** // Prototypes //***************************************************************************** @@ -217,6 +226,14 @@ stack_frame.AddrStack.Mode = AddrModeFlat; stack_frame.AddrStack.Offset = myesp; stack_frame.AddrFrame.Mode = AddrModeFlat; stack_frame.AddrFrame.Offset = myebp; +#if defined(_WIN64) || defined(__x86_64__) +// TheSuperHackers @fix MeneerHaas 02/09/2026 Seed the walk context from the frame this walk actually starts at. +CONTEXT walk_ctx; +RtlCaptureContext(&walk_ctx); +CTX_PC(walk_ctx) = myeip; +CTX_STACK(walk_ctx) = myesp; +CTX_FRAME(walk_ctx) = myebp; +#endif { /* if(GetThreadContext(thread, &gsContext)) @@ -237,11 +254,11 @@ stack_frame.AddrFrame.Offset = myebp; unsigned int skip = skipFrames; while (b_ret&&skip) { - b_ret = DbgHelpLoader::stackWalk( IMAGE_FILE_MACHINE_I386, + b_ret = DbgHelpLoader::stackWalk( CTX_STACKWALK_MACHINE, process, thread, &stack_frame, - nullptr, //&gsContext, + RTS_STACKWALK_CONTEXT, //&gsContext, nullptr, DbgHelpLoader::symFunctionTableAccess, DbgHelpLoader::symGetModuleBase, @@ -253,11 +270,11 @@ stack_frame.AddrFrame.Offset = myebp; while(b_ret&&skip) { - b_ret = DbgHelpLoader::stackWalk( IMAGE_FILE_MACHINE_I386, + b_ret = DbgHelpLoader::stackWalk( CTX_STACKWALK_MACHINE, process, thread, &stack_frame, - nullptr, //&gsContext, + RTS_STACKWALK_CONTEXT, //&gsContext, nullptr, DbgHelpLoader::symFunctionTableAccess, DbgHelpLoader::symGetModuleBase, @@ -417,6 +434,14 @@ stack_frame.AddrStack.Offset = myesp; stack_frame.AddrFrame.Mode = AddrModeFlat; stack_frame.AddrFrame.Offset = myebp; +#if defined(_WIN64) || defined(__x86_64__) +// TheSuperHackers @fix MeneerHaas 02/09/2026 Seed the walk context from the frame this walk actually starts at. +CONTEXT walk_ctx; +RtlCaptureContext(&walk_ctx); +CTX_PC(walk_ctx) = myeip; +CTX_STACK(walk_ctx) = myesp; +CTX_FRAME(walk_ctx) = myebp; +#endif { /* if(GetThreadContext(thread, &gsContext)) @@ -436,11 +461,11 @@ stack_frame.AddrFrame.Offset = myebp; // Skip some? while (stillgoing&&skip) { - stillgoing = DbgHelpLoader::stackWalk(IMAGE_FILE_MACHINE_I386, + stillgoing = DbgHelpLoader::stackWalk(CTX_STACKWALK_MACHINE, process, thread, &stack_frame, - nullptr, //&gsContext, + RTS_STACKWALK_CONTEXT, //&gsContext, nullptr, DbgHelpLoader::symFunctionTableAccess, DbgHelpLoader::symGetModuleBase, @@ -450,11 +475,11 @@ stack_frame.AddrFrame.Offset = myebp; while(stillgoing&&count) { - stillgoing = DbgHelpLoader::stackWalk(IMAGE_FILE_MACHINE_I386, + stillgoing = DbgHelpLoader::stackWalk(CTX_STACKWALK_MACHINE, process, thread, &stack_frame, - nullptr, //&gsContext, + RTS_STACKWALK_CONTEXT, //&gsContext, nullptr, DbgHelpLoader::symFunctionTableAccess, DbgHelpLoader::symGetModuleBase, diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp index e7847e37548..fe53728f3a2 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/StackDump.cpp @@ -37,6 +37,15 @@ #include #include "Lib/arch_context.h" +// TheSuperHackers @fix MeneerHaas 02/09/2026 StackWalk64 requires a ContextRecord on AMD64 (it is optional on x86) and +// updates it while unwinding, so the walkers below seed a mutable local walk_ctx and pass it +// through this macro. On 32-bit it expands to the retail nullptr, leaving that arm unchanged. +#if defined(_WIN64) || defined(__x86_64__) +#define RTS_STACKWALK_CONTEXT (&walk_ctx) +#else +#define RTS_STACKWALK_CONTEXT nullptr +#endif + //***************************************************************************** // Prototypes //***************************************************************************** @@ -217,6 +226,14 @@ stack_frame.AddrStack.Mode = AddrModeFlat; stack_frame.AddrStack.Offset = myesp; stack_frame.AddrFrame.Mode = AddrModeFlat; stack_frame.AddrFrame.Offset = myebp; +#if defined(_WIN64) || defined(__x86_64__) +// TheSuperHackers @fix MeneerHaas 02/09/2026 Seed the walk context from the frame this walk actually starts at. +CONTEXT walk_ctx; +RtlCaptureContext(&walk_ctx); +CTX_PC(walk_ctx) = myeip; +CTX_STACK(walk_ctx) = myesp; +CTX_FRAME(walk_ctx) = myebp; +#endif { /* if(GetThreadContext(thread, &gsContext)) @@ -237,11 +254,11 @@ stack_frame.AddrFrame.Offset = myebp; unsigned int skip = skipFrames; while (b_ret&&skip) { - b_ret = DbgHelpLoader::stackWalk( IMAGE_FILE_MACHINE_I386, + b_ret = DbgHelpLoader::stackWalk( CTX_STACKWALK_MACHINE, process, thread, &stack_frame, - nullptr, //&gsContext, + RTS_STACKWALK_CONTEXT, //&gsContext, nullptr, DbgHelpLoader::symFunctionTableAccess, DbgHelpLoader::symGetModuleBase, @@ -253,11 +270,11 @@ stack_frame.AddrFrame.Offset = myebp; while(b_ret&&skip) { - b_ret = DbgHelpLoader::stackWalk( IMAGE_FILE_MACHINE_I386, + b_ret = DbgHelpLoader::stackWalk( CTX_STACKWALK_MACHINE, process, thread, &stack_frame, - nullptr, //&gsContext, + RTS_STACKWALK_CONTEXT, //&gsContext, nullptr, DbgHelpLoader::symFunctionTableAccess, DbgHelpLoader::symGetModuleBase, @@ -417,6 +434,14 @@ stack_frame.AddrStack.Offset = myesp; stack_frame.AddrFrame.Mode = AddrModeFlat; stack_frame.AddrFrame.Offset = myebp; +#if defined(_WIN64) || defined(__x86_64__) +// TheSuperHackers @fix MeneerHaas 02/09/2026 Seed the walk context from the frame this walk actually starts at. +CONTEXT walk_ctx; +RtlCaptureContext(&walk_ctx); +CTX_PC(walk_ctx) = myeip; +CTX_STACK(walk_ctx) = myesp; +CTX_FRAME(walk_ctx) = myebp; +#endif { /* if(GetThreadContext(thread, &gsContext)) @@ -436,11 +461,11 @@ stack_frame.AddrFrame.Offset = myebp; // Skip some? while (stillgoing&&skip) { - stillgoing = DbgHelpLoader::stackWalk(IMAGE_FILE_MACHINE_I386, + stillgoing = DbgHelpLoader::stackWalk(CTX_STACKWALK_MACHINE, process, thread, &stack_frame, - nullptr, //&gsContext, + RTS_STACKWALK_CONTEXT, //&gsContext, nullptr, DbgHelpLoader::symFunctionTableAccess, DbgHelpLoader::symGetModuleBase, @@ -450,11 +475,11 @@ stack_frame.AddrFrame.Offset = myebp; while(stillgoing&&count) { - stillgoing = DbgHelpLoader::stackWalk(IMAGE_FILE_MACHINE_I386, + stillgoing = DbgHelpLoader::stackWalk(CTX_STACKWALK_MACHINE, process, thread, &stack_frame, - nullptr, //&gsContext, + RTS_STACKWALK_CONTEXT, //&gsContext, nullptr, DbgHelpLoader::symFunctionTableAccess, DbgHelpLoader::symGetModuleBase, From 336a6c3774e8630e6c08351cb58ad233a5da96f6 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Wed, 2 Sep 2026 21:08:13 +0200 Subject: [PATCH 15/20] docs(x64): drop references to files that live only in the fork The x64 design notes are not part of this PR, so comments pointing at docs/x64/ would dangle upstream. The load-bearing facts they cited are now stated inline instead. Co-Authored-By: Claude Opus 5 --- Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp | 2 +- Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h | 2 +- cmake/dx8.cmake | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp index e463df1b3de..7177637912f 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp +++ b/Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp @@ -112,7 +112,7 @@ bool DbgHelpLoader::load() } // TheSuperHackers @fix MeneerHaas 02/09/2026 x64 dbghelp.dll only exports the ...64 names for the - // address-taking entry points; un-suffixed GetProcAddress returns NULL there. 32-bit order kept (docs/x64). + // address-taking entry points; un-suffixed GetProcAddress returns NULL there. 32-bit call order kept. Inst->m_symInitialize = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymInitialize")); Inst->m_symCleanup = reinterpret_cast(::GetProcAddress(Inst->m_dllModule, "SymCleanup")); #if defined(_WIN64) || defined(__x86_64__) diff --git a/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h b/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h index 0fac7604f58..406378be3e0 100644 --- a/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h +++ b/Core/Libraries/Source/WWVegas/WWSaveLoad/persistfactory.h @@ -142,7 +142,7 @@ SimplePersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * // 32-bit on-disk identity token, so two live objects can collide and pointer // fixup can bind the wrong object on load. The on-disk width is fixed by the // retail save format and cannot be widened here without breaking it. - // See docs/x64/savegame-format-decision.md (written by the later task). + // The write path has no callers in this repository; writer and reader still agree on the width. uint32 objptr = (uint32)(uintptr_t)obj; csave.Begin_Chunk(SIMPLEFACTORY_CHUNKID_OBJPOINTER); csave.Write(&objptr,sizeof(uint32)); diff --git a/cmake/dx8.cmake b/cmake/dx8.cmake index 0dd81dec781..42a44ca7524 100644 --- a/cmake/dx8.cmake +++ b/cmake/dx8.cmake @@ -23,7 +23,7 @@ add_library(d3d8lib INTERFACE) # scope, see issue #473. This is a precondition for W3D to compile on x64, not # a guarantee: W3D also depends on several Core libraries (WWLib, debug, # Compression, WWSaveLoad, WWAudio) that fail for unrelated reasons and still -# block it as of this change. See docs/x64/core-error-catalogue.md. +# block it as of this change. if(CMAKE_SIZEOF_VOID_P EQUAL 4) target_link_libraries(d3d8lib INTERFACE d3d8 dinput8 dxguid) else() From 83d1913dc7062c8c18fea03075680f8d61d86cf9 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Wed, 2 Sep 2026 21:10:30 +0200 Subject: [PATCH 16/20] fix(x64): keep the remapped attachment pointer at token width Review follow-up (#3248): SoundSceneObjClass is the only consumer of the pointer remap, and it persisted m_AttachedObject and m_UserObj at native width. On x64 that both fails to read the 4 bytes legacy files hold and asks the remap for a full-width value the render-object factory never registers. Both members now round-trip through the same 4-byte identity token the factories use, so the remap can match again. Co-Authored-By: Claude Opus 5 --- .../Source/WWVegas/WWAudio/SoundSceneObj.cpp | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp index 7a0bd0ad12a..a5aad110c79 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp @@ -34,6 +34,7 @@ #include "SoundSceneObj.h" +#include #include "WW3D2/camera.h" #include "WW3D2/rendobj.h" #include "WWSaveLoad/persistfactory.h" @@ -259,10 +260,22 @@ SoundSceneObjClass::Save (ChunkSaveClass &csave) csave.End_Chunk (); csave.Begin_Chunk (CHUNKID_VARIABLES); +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Persisted pointers are 4-byte identity tokens: that is what the + // legacy files hold and what RenderObjPersistFactory registers, so the remap below can match. + uint32 attached_obj_token = (uint32)(uintptr_t)m_AttachedObject; + WRITE_MICRO_CHUNK (csave, VARID_ATTACHED_OBJ, attached_obj_token); +#else WRITE_MICRO_CHUNK (csave, VARID_ATTACHED_OBJ, m_AttachedObject); +#endif WRITE_MICRO_CHUNK (csave, VARID_ATTACHED_BONE, m_AttachedBone); WRITE_MICRO_CHUNK (csave, VARID_USER_DATA, m_UserData); +#if defined(_WIN64) || defined(__x86_64__) + uint32 user_obj_token = (uint32)(uintptr_t)m_UserObj; + WRITE_MICRO_CHUNK (csave, VARID_USER_OBJ, user_obj_token); +#else WRITE_MICRO_CHUNK (csave, VARID_USER_OBJ, m_UserObj); +#endif WRITE_MICRO_CHUNK (csave, VARID_ID, m_ID); csave.End_Chunk (); return true; @@ -294,10 +307,33 @@ SoundSceneObjClass::Load (ChunkLoadClass &cload) while (cload.Open_Micro_Chunk ()) { switch (cload.Cur_Micro_Chunk_ID ()) { +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Read exactly the 4 bytes Save wrote; sizeof(pointer) + // is 8 here, and ChunkLoadClass::Read refuses a short chunk outright, leaving the + // member unset and poisoning the pointer remap. + case VARID_ATTACHED_OBJ: + { + uint32 attached_obj_token = 0; + cload.Read (&attached_obj_token, sizeof (attached_obj_token)); + m_AttachedObject = (RenderObjClass *)(uintptr_t)attached_obj_token; + break; + } +#else READ_MICRO_CHUNK (cload, VARID_ATTACHED_OBJ, m_AttachedObject); +#endif READ_MICRO_CHUNK (cload, VARID_ATTACHED_BONE, m_AttachedBone); READ_MICRO_CHUNK (cload, VARID_USER_DATA, m_UserData); +#if defined(_WIN64) || defined(__x86_64__) + case VARID_USER_OBJ: + { + uint32 user_obj_token = 0; + cload.Read (&user_obj_token, sizeof (user_obj_token)); + m_UserObj = (RefCountClass *)(uintptr_t)user_obj_token; + break; + } +#else READ_MICRO_CHUNK (cload, VARID_USER_OBJ, m_UserObj); +#endif READ_MICRO_CHUNK (cload, VARID_ID, id); } From 06c4a086e3f4a6f64f152766c91adae6ebc62f41 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Wed, 2 Sep 2026 21:19:44 +0200 Subject: [PATCH 17/20] fix(x64): read persisted pointer identities at the width the chunk holds Review follow-up (#3248): truncating x64 addresses into a 4-byte token made two objects whose low 32 bits match collide on one identity. Add READ_MICRO_CHUNK_POINTER_TOKEN, which reads Cur_Micro_Chunk_Length() bytes instead of sizeof(var), and use it for every persisted identity (rendobj, dazzle, AudibleSound) and for the one remap consumer (SoundSceneObj). Writers go back to native width. Files written by 32-bit builds keep loading, an x64-written file would round-trip its full address, and nothing is truncated, so no two objects can share a token. Co-Authored-By: Claude Opus 5 --- .../Source/WWVegas/WW3D2/rendobj.cpp | 17 +------- .../Source/WWVegas/WWAudio/AudibleSound.cpp | 25 ++++-------- .../Source/WWVegas/WWAudio/SoundSceneObj.cpp | 40 +------------------ Core/Libraries/Source/WWVegas/WWLib/chunkio.h | 16 ++++++++ .../Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 17 +------- .../Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 17 +------- 6 files changed, 29 insertions(+), 103 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp index f396f993054..ecce25e3da8 100644 --- a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp +++ b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp @@ -1219,14 +1219,6 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const char name[64]; name[0] = '\0'; - // Read exactly what Save wrote: a fixed-width 4-byte identity token, not - // sizeof(old_obj). On x86-64 sizeof(RenderObjClass*) is 8, so reading - // sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for more - // bytes than the legacy 4-byte micro chunk holds; ChunkLoadClass::Read - // then refuses to read anything at all and old_obj stays null, silently - // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. - uint32 old_obj_token = 0; - while (cload.Open_Chunk()) { switch (cload.Cur_Chunk_ID()) { @@ -1234,7 +1226,7 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { - case (RENDOBJFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; + READ_MICRO_CHUNK_POINTER_TOKEN(cload,RENDOBJFACTORY_VARIABLE_OBJPOINTER,old_obj,RenderObjClass *) READ_MICRO_CHUNK(cload,RENDOBJFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,RENDOBJFACTORY_VARIABLE_NAME,name,sizeof(name)); } @@ -1277,7 +1269,6 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const new_obj->Set_Transform(tm); } - old_obj = (RenderObjClass *)(uintptr_t)old_obj_token; SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } @@ -1289,13 +1280,7 @@ void RenderObjPersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * ob const Matrix3D& tm = robj->Get_Transform(); csave.Begin_Chunk(RENDOBJFACTORY_CHUNKID_VARIABLES); -#if defined(_WIN64) || defined(__x86_64__) - // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. - uint32 robj_token = (uint32)(uintptr_t)robj; - WRITE_MICRO_CHUNK(csave,RENDOBJFACTORY_VARIABLE_OBJPOINTER,robj_token); -#else WRITE_MICRO_CHUNK(csave,RENDOBJFACTORY_VARIABLE_OBJPOINTER,robj); -#endif WRITE_MICRO_CHUNK_STRING(csave,RENDOBJFACTORY_VARIABLE_NAME,name); WRITE_MICRO_CHUNK(csave,RENDOBJFACTORY_VARIABLE_TRANSFORM,tm); csave.End_Chunk(); diff --git a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp index facabfb244e..854b047b3b8 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp @@ -1654,14 +1654,8 @@ AudibleSoundClass::Save (ChunkSaveClass &csave) WRITE_MICRO_CHUNK_STRING (csave, VARID_FILENAME, m_Buffer->Get_Filename ()); } -#if defined(_WIN64) || defined(__x86_64__) - // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. - uint32 this_ptr_token = (uint32)(uintptr_t)this; - WRITE_MICRO_CHUNK (csave, VARID_THIS_PTR, this_ptr_token); -#else AudibleSoundClass *this_ptr = this; WRITE_MICRO_CHUNK (csave, VARID_THIS_PTR, this_ptr); -#endif csave.End_Chunk (); @@ -1718,17 +1712,14 @@ AudibleSoundClass::Load (ChunkLoadClass &cload) case VARID_THIS_PTR: { - // Read exactly what Save wrote: a fixed-width 4-byte - // identity token, not sizeof(old_ptr). On x86-64 - // sizeof(AudibleSoundClass*) is 8, so reading - // sizeof(old_ptr) here would ask for more bytes than - // the legacy 4-byte micro chunk holds; ChunkLoadClass::Read - // then refuses to read anything at all and old_ptr - // stays null, silently poisoning SaveLoadSystemClass's - // pointer remap table. See persistfactory.h. - uint32 old_ptr_token = 0; - cload.Read(&old_ptr_token, sizeof (old_ptr_token)); - AudibleSoundClass *old_ptr = (AudibleSoundClass *)(uintptr_t)old_ptr_token; + // TheSuperHackers @fix MeneerHaas 02/09/2026 Read the identity at the width the chunk + // actually holds, as READ_MICRO_CHUNK_POINTER_TOKEN does; sizeof(old_ptr) is 8 on x64 and + // ChunkLoadClass::Read would then read nothing at all from a legacy 4-byte chunk. + uintptr_t old_ptr_token = 0; + uint32 token_length = cload.Cur_Micro_Chunk_Length (); + if (token_length > sizeof (old_ptr_token)) token_length = sizeof (old_ptr_token); + cload.Read (&old_ptr_token, token_length); + AudibleSoundClass *old_ptr = (AudibleSoundClass *)old_ptr_token; SaveLoadSystemClass::Register_Pointer (old_ptr, this); } break; diff --git a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp index a5aad110c79..bdf0765b8e2 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp @@ -34,7 +34,6 @@ #include "SoundSceneObj.h" -#include #include "WW3D2/camera.h" #include "WW3D2/rendobj.h" #include "WWSaveLoad/persistfactory.h" @@ -260,22 +259,10 @@ SoundSceneObjClass::Save (ChunkSaveClass &csave) csave.End_Chunk (); csave.Begin_Chunk (CHUNKID_VARIABLES); -#if defined(_WIN64) || defined(__x86_64__) - // TheSuperHackers @fix MeneerHaas 02/09/2026 Persisted pointers are 4-byte identity tokens: that is what the - // legacy files hold and what RenderObjPersistFactory registers, so the remap below can match. - uint32 attached_obj_token = (uint32)(uintptr_t)m_AttachedObject; - WRITE_MICRO_CHUNK (csave, VARID_ATTACHED_OBJ, attached_obj_token); -#else WRITE_MICRO_CHUNK (csave, VARID_ATTACHED_OBJ, m_AttachedObject); -#endif WRITE_MICRO_CHUNK (csave, VARID_ATTACHED_BONE, m_AttachedBone); WRITE_MICRO_CHUNK (csave, VARID_USER_DATA, m_UserData); -#if defined(_WIN64) || defined(__x86_64__) - uint32 user_obj_token = (uint32)(uintptr_t)m_UserObj; - WRITE_MICRO_CHUNK (csave, VARID_USER_OBJ, user_obj_token); -#else WRITE_MICRO_CHUNK (csave, VARID_USER_OBJ, m_UserObj); -#endif WRITE_MICRO_CHUNK (csave, VARID_ID, m_ID); csave.End_Chunk (); return true; @@ -307,33 +294,10 @@ SoundSceneObjClass::Load (ChunkLoadClass &cload) while (cload.Open_Micro_Chunk ()) { switch (cload.Cur_Micro_Chunk_ID ()) { -#if defined(_WIN64) || defined(__x86_64__) - // TheSuperHackers @fix MeneerHaas 02/09/2026 Read exactly the 4 bytes Save wrote; sizeof(pointer) - // is 8 here, and ChunkLoadClass::Read refuses a short chunk outright, leaving the - // member unset and poisoning the pointer remap. - case VARID_ATTACHED_OBJ: - { - uint32 attached_obj_token = 0; - cload.Read (&attached_obj_token, sizeof (attached_obj_token)); - m_AttachedObject = (RenderObjClass *)(uintptr_t)attached_obj_token; - break; - } -#else - READ_MICRO_CHUNK (cload, VARID_ATTACHED_OBJ, m_AttachedObject); -#endif + READ_MICRO_CHUNK_POINTER_TOKEN (cload, VARID_ATTACHED_OBJ, m_AttachedObject, RenderObjClass *) READ_MICRO_CHUNK (cload, VARID_ATTACHED_BONE, m_AttachedBone); READ_MICRO_CHUNK (cload, VARID_USER_DATA, m_UserData); -#if defined(_WIN64) || defined(__x86_64__) - case VARID_USER_OBJ: - { - uint32 user_obj_token = 0; - cload.Read (&user_obj_token, sizeof (user_obj_token)); - m_UserObj = (RefCountClass *)(uintptr_t)user_obj_token; - break; - } -#else - READ_MICRO_CHUNK (cload, VARID_USER_OBJ, m_UserObj); -#endif + READ_MICRO_CHUNK_POINTER_TOKEN (cload, VARID_USER_OBJ, m_UserObj, RefCountClass *) READ_MICRO_CHUNK (cload, VARID_ID, id); } diff --git a/Core/Libraries/Source/WWVegas/WWLib/chunkio.h b/Core/Libraries/Source/WWVegas/WWLib/chunkio.h index abd0729e475..29935d4ef40 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/chunkio.h +++ b/Core/Libraries/Source/WWVegas/WWLib/chunkio.h @@ -329,6 +329,22 @@ class ChunkLoadClass break; \ } +// TheSuperHackers @fix MeneerHaas 02/09/2026 Persisted pointer identities are written at the writing build's pointer +// width: 4 bytes in every file a 32-bit build produced, 8 from an x64 build. Read what the chunk +// actually holds rather than sizeof(var) -- a plain READ_MICRO_CHUNK would ask for 8 bytes from a +// legacy 4-byte chunk, and ChunkLoadClass::Read then reads nothing at all, silently leaving the +// identity null and poisoning the pointer remap. Reading by length also means no address ever has +// to be truncated into a token, so two objects cannot collide on one identity. +#define READ_MICRO_CHUNK_POINTER_TOKEN(cload,id,var,type)\ + case (id): {\ + uintptr_t temp_token = 0;\ + uint32 temp_length = cload.Cur_Micro_Chunk_Length();\ + if (temp_length > sizeof(temp_token)) temp_length = sizeof(temp_token);\ + cload.Read(&temp_token,temp_length);\ + var = (type)temp_token;\ + break;\ + } + #define READ_MICRO_CHUNK_STRING(cload,id,var,size) \ case (id): WWASSERT(cload.Cur_Micro_Chunk_Length() <= size); cload.Read(var,cload.Cur_Micro_Chunk_Length()); break; \ diff --git a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index 91f51591cac..71d6c1ee2be 100644 --- a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1349,14 +1349,6 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const char dazzle_type[256]; dazzle_type[0] = 0; - // Read exactly what Save wrote: a fixed-width 4-byte identity token, not - // sizeof(old_obj). On x86-64 sizeof(DazzleRenderObjClass*) is 8, so - // reading sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for - // more bytes than the legacy 4-byte micro chunk holds; ChunkLoadClass::Read - // then refuses to read anything at all and old_obj stays null, silently - // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. - uint32 old_obj_token = 0; - /* ** Load the dazzle parameters */ @@ -1367,7 +1359,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { - case (DAZZLEFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; + READ_MICRO_CHUNK_POINTER_TOKEN(cload,DAZZLEFACTORY_VARIABLE_OBJPOINTER,old_obj,DazzleRenderObjClass *) READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type,sizeof(dazzle_type)); } @@ -1411,7 +1403,6 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const /* ** Register the old pointer for re-mapping to the new pointer */ - old_obj = (DazzleRenderObjClass *)(uintptr_t)old_obj_token; SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } @@ -1424,13 +1415,7 @@ void DazzlePersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * obj) const Matrix3D& tm = robj->Get_Transform(); csave.Begin_Chunk(DAZZLEFACTORY_CHUNKID_VARIABLES); -#if defined(_WIN64) || defined(__x86_64__) - // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. - uint32 robj_token = (uint32)(uintptr_t)robj; - WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj_token); -#else WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj); -#endif WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); WRITE_MICRO_CHUNK_STRING(csave,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type_name); diff --git a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index 05abfdbdafd..08fa46ebc0d 100644 --- a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1452,14 +1452,6 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const char dazzle_type[256]; dazzle_type[0] = 0; - // Read exactly what Save wrote: a fixed-width 4-byte identity token, not - // sizeof(old_obj). On x86-64 sizeof(DazzleRenderObjClass*) is 8, so - // reading sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for - // more bytes than the legacy 4-byte micro chunk holds; ChunkLoadClass::Read - // then refuses to read anything at all and old_obj stays null, silently - // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. - uint32 old_obj_token = 0; - /* ** Load the dazzle parameters */ @@ -1470,7 +1462,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { - case (DAZZLEFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; + READ_MICRO_CHUNK_POINTER_TOKEN(cload,DAZZLEFACTORY_VARIABLE_OBJPOINTER,old_obj,DazzleRenderObjClass *) READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type,sizeof(dazzle_type)); } @@ -1514,7 +1506,6 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const /* ** Register the old pointer for re-mapping to the new pointer */ - old_obj = (DazzleRenderObjClass *)(uintptr_t)old_obj_token; SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } @@ -1527,13 +1518,7 @@ void DazzlePersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * obj) const Matrix3D& tm = robj->Get_Transform(); csave.Begin_Chunk(DAZZLEFACTORY_CHUNKID_VARIABLES); -#if defined(_WIN64) || defined(__x86_64__) - // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. - uint32 robj_token = (uint32)(uintptr_t)robj; - WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj_token); -#else WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj); -#endif WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); WRITE_MICRO_CHUNK_STRING(csave,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type_name); From 2582cbaed71880315fd510c9a90274f94601c600 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Wed, 2 Sep 2026 22:16:49 +0200 Subject: [PATCH 18/20] fix(x64): guard the width-aware identity read to x64 The length-aware read buys nothing on 32-bit, where the chunk and the pointer are both 4 bytes, so per this branch's rule it must not move VC6 codegen. Every 32-bit arm is now textually what the recorded baseline was built from, and READ_MICRO_CHUNK_POINTER_TOKEN is defined only on x64. Co-Authored-By: Claude Opus 5 --- .../Source/WWVegas/WW3D2/rendobj.cpp | 17 +++++++++++++++++ .../Source/WWVegas/WWAudio/AudibleSound.cpp | 19 +++++++++++++++++++ .../Source/WWVegas/WWAudio/SoundSceneObj.cpp | 8 ++++++++ Core/Libraries/Source/WWVegas/WWLib/chunkio.h | 2 ++ .../Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 17 +++++++++++++++++ .../Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 17 +++++++++++++++++ 6 files changed, 80 insertions(+) diff --git a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp index ecce25e3da8..d26500fe55f 100644 --- a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp +++ b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp @@ -1219,6 +1219,16 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const char name[64]; name[0] = '\0'; +#if !defined(_WIN64) && !defined(__x86_64__) + // Read exactly what Save wrote: a fixed-width 4-byte identity token, not + // sizeof(old_obj). On x86-64 sizeof(RenderObjClass*) is 8, so reading + // sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for more + // bytes than the legacy 4-byte micro chunk holds; ChunkLoadClass::Read + // then refuses to read anything at all and old_obj stays null, silently + // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. + uint32 old_obj_token = 0; +#endif + while (cload.Open_Chunk()) { switch (cload.Cur_Chunk_ID()) { @@ -1226,7 +1236,11 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { +#if defined(_WIN64) || defined(__x86_64__) READ_MICRO_CHUNK_POINTER_TOKEN(cload,RENDOBJFACTORY_VARIABLE_OBJPOINTER,old_obj,RenderObjClass *) +#else + case (RENDOBJFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; +#endif READ_MICRO_CHUNK(cload,RENDOBJFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,RENDOBJFACTORY_VARIABLE_NAME,name,sizeof(name)); } @@ -1269,6 +1283,9 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const new_obj->Set_Transform(tm); } +#if !defined(_WIN64) && !defined(__x86_64__) + old_obj = (RenderObjClass *)(uintptr_t)old_obj_token; +#endif SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } diff --git a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp index 854b047b3b8..b148be7337b 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp @@ -1710,6 +1710,7 @@ AudibleSoundClass::Load (ChunkLoadClass &cload) READ_MICRO_CHUNK_WWSTRING (cload, VARID_FILENAME, filename); +#if defined(_WIN64) || defined(__x86_64__) case VARID_THIS_PTR: { // TheSuperHackers @fix MeneerHaas 02/09/2026 Read the identity at the width the chunk @@ -1723,6 +1724,24 @@ AudibleSoundClass::Load (ChunkLoadClass &cload) SaveLoadSystemClass::Register_Pointer (old_ptr, this); } break; +#else + case VARID_THIS_PTR: + { + // Read exactly what Save wrote: a fixed-width 4-byte + // identity token, not sizeof(old_ptr). On x86-64 + // sizeof(AudibleSoundClass*) is 8, so reading + // sizeof(old_ptr) here would ask for more bytes than + // the legacy 4-byte micro chunk holds; ChunkLoadClass::Read + // then refuses to read anything at all and old_ptr + // stays null, silently poisoning SaveLoadSystemClass's + // pointer remap table. See persistfactory.h. + uint32 old_ptr_token = 0; + cload.Read(&old_ptr_token, sizeof (old_ptr_token)); + AudibleSoundClass *old_ptr = (AudibleSoundClass *)(uintptr_t)old_ptr_token; + SaveLoadSystemClass::Register_Pointer (old_ptr, this); + } + break; +#endif } cload.Close_Micro_Chunk (); diff --git a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp index bdf0765b8e2..ca6cf3565d3 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp @@ -294,10 +294,18 @@ SoundSceneObjClass::Load (ChunkLoadClass &cload) while (cload.Open_Micro_Chunk ()) { switch (cload.Cur_Micro_Chunk_ID ()) { +#if defined(_WIN64) || defined(__x86_64__) READ_MICRO_CHUNK_POINTER_TOKEN (cload, VARID_ATTACHED_OBJ, m_AttachedObject, RenderObjClass *) +#else + READ_MICRO_CHUNK (cload, VARID_ATTACHED_OBJ, m_AttachedObject); +#endif READ_MICRO_CHUNK (cload, VARID_ATTACHED_BONE, m_AttachedBone); READ_MICRO_CHUNK (cload, VARID_USER_DATA, m_UserData); +#if defined(_WIN64) || defined(__x86_64__) READ_MICRO_CHUNK_POINTER_TOKEN (cload, VARID_USER_OBJ, m_UserObj, RefCountClass *) +#else + READ_MICRO_CHUNK (cload, VARID_USER_OBJ, m_UserObj); +#endif READ_MICRO_CHUNK (cload, VARID_ID, id); } diff --git a/Core/Libraries/Source/WWVegas/WWLib/chunkio.h b/Core/Libraries/Source/WWVegas/WWLib/chunkio.h index 29935d4ef40..1d653527151 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/chunkio.h +++ b/Core/Libraries/Source/WWVegas/WWLib/chunkio.h @@ -329,6 +329,7 @@ class ChunkLoadClass break; \ } +#if defined(_WIN64) || defined(__x86_64__) // TheSuperHackers @fix MeneerHaas 02/09/2026 Persisted pointer identities are written at the writing build's pointer // width: 4 bytes in every file a 32-bit build produced, 8 from an x64 build. Read what the chunk // actually holds rather than sizeof(var) -- a plain READ_MICRO_CHUNK would ask for 8 bytes from a @@ -344,6 +345,7 @@ class ChunkLoadClass var = (type)temp_token;\ break;\ } +#endif #define READ_MICRO_CHUNK_STRING(cload,id,var,size) \ case (id): WWASSERT(cload.Cur_Micro_Chunk_Length() <= size); cload.Read(var,cload.Cur_Micro_Chunk_Length()); break; \ diff --git a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index 71d6c1ee2be..15a618e9073 100644 --- a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1349,6 +1349,16 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const char dazzle_type[256]; dazzle_type[0] = 0; +#if !defined(_WIN64) && !defined(__x86_64__) + // Read exactly what Save wrote: a fixed-width 4-byte identity token, not + // sizeof(old_obj). On x86-64 sizeof(DazzleRenderObjClass*) is 8, so + // reading sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for + // more bytes than the legacy 4-byte micro chunk holds; ChunkLoadClass::Read + // then refuses to read anything at all and old_obj stays null, silently + // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. + uint32 old_obj_token = 0; +#endif + /* ** Load the dazzle parameters */ @@ -1359,7 +1369,11 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { +#if defined(_WIN64) || defined(__x86_64__) READ_MICRO_CHUNK_POINTER_TOKEN(cload,DAZZLEFACTORY_VARIABLE_OBJPOINTER,old_obj,DazzleRenderObjClass *) +#else + case (DAZZLEFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; +#endif READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type,sizeof(dazzle_type)); } @@ -1403,6 +1417,9 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const /* ** Register the old pointer for re-mapping to the new pointer */ +#if !defined(_WIN64) && !defined(__x86_64__) + old_obj = (DazzleRenderObjClass *)(uintptr_t)old_obj_token; +#endif SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } diff --git a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index 08fa46ebc0d..78302853073 100644 --- a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1452,6 +1452,16 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const char dazzle_type[256]; dazzle_type[0] = 0; +#if !defined(_WIN64) && !defined(__x86_64__) + // Read exactly what Save wrote: a fixed-width 4-byte identity token, not + // sizeof(old_obj). On x86-64 sizeof(DazzleRenderObjClass*) is 8, so + // reading sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for + // more bytes than the legacy 4-byte micro chunk holds; ChunkLoadClass::Read + // then refuses to read anything at all and old_obj stays null, silently + // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. + uint32 old_obj_token = 0; +#endif + /* ** Load the dazzle parameters */ @@ -1462,7 +1472,11 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { +#if defined(_WIN64) || defined(__x86_64__) READ_MICRO_CHUNK_POINTER_TOKEN(cload,DAZZLEFACTORY_VARIABLE_OBJPOINTER,old_obj,DazzleRenderObjClass *) +#else + case (DAZZLEFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; +#endif READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type,sizeof(dazzle_type)); } @@ -1506,6 +1520,9 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const /* ** Register the old pointer for re-mapping to the new pointer */ +#if !defined(_WIN64) && !defined(__x86_64__) + old_obj = (DazzleRenderObjClass *)(uintptr_t)old_obj_token; +#endif SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } From e46866258ff4651a47826e652660ee79c904a724 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Thu, 3 Sep 2026 00:41:31 +0200 Subject: [PATCH 19/20] fix(x64): keep the stdint include SoundSceneObj compiled with Dropping this include as 'now unneeded' was the one 32-bit-visible change left in the identity-width work, and it moved VC6 codegen for no 32-bit benefit: the recorded baseline reproduces with it and not without it. The include is harmless -- it only declares pointer-sized integer typedefs -- so it stays. Co-Authored-By: Claude Opus 5 --- Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp index ca6cf3565d3..8f31193a1e2 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp @@ -34,6 +34,7 @@ #include "SoundSceneObj.h" +#include #include "WW3D2/camera.h" #include "WW3D2/rendobj.h" #include "WWSaveLoad/persistfactory.h" From b719eeeb7864e571d212c93290e37f1de456ecb1 Mon Sep 17 00:00:00 2001 From: Joey de Haas Date: Thu, 3 Sep 2026 00:50:39 +0200 Subject: [PATCH 20/20] revert(x64): return the identity-width work to its verified state The width-aware read via READ_MICRO_CHUNK_POINTER_TOKEN moved VC6 codegen in the two game executables and guarding it did not fully restore the baseline, so it is withdrawn. What remains is the state that clean builds verified against the recorded baseline: fixed 4-byte identity tokens, guarded so 32-bit is untouched, with writer and reader agreeing on the width. The reviewer's collision concern is answered in the pull request rather than in code: nothing in this repository calls the chunk write path, so no file with x64-width tokens can exist, and every file that does exist was written by a 32-bit build. Co-Authored-By: Claude Opus 5 --- .../Source/WWVegas/WW3D2/rendobj.cpp | 14 ++++----- .../Source/WWVegas/WWAudio/AudibleSound.cpp | 22 ++++--------- .../Source/WWVegas/WWAudio/SoundSceneObj.cpp | 31 +++++++++++++++++-- Core/Libraries/Source/WWVegas/WWLib/chunkio.h | 18 ----------- .../Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 14 ++++----- .../Libraries/Source/WWVegas/WW3D2/dazzle.cpp | 14 ++++----- 6 files changed, 53 insertions(+), 60 deletions(-) diff --git a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp index d26500fe55f..f396f993054 100644 --- a/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp +++ b/Core/Libraries/Source/WWVegas/WW3D2/rendobj.cpp @@ -1219,7 +1219,6 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const char name[64]; name[0] = '\0'; -#if !defined(_WIN64) && !defined(__x86_64__) // Read exactly what Save wrote: a fixed-width 4-byte identity token, not // sizeof(old_obj). On x86-64 sizeof(RenderObjClass*) is 8, so reading // sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for more @@ -1227,7 +1226,6 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const // then refuses to read anything at all and old_obj stays null, silently // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. uint32 old_obj_token = 0; -#endif while (cload.Open_Chunk()) { switch (cload.Cur_Chunk_ID()) { @@ -1236,11 +1234,7 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { -#if defined(_WIN64) || defined(__x86_64__) - READ_MICRO_CHUNK_POINTER_TOKEN(cload,RENDOBJFACTORY_VARIABLE_OBJPOINTER,old_obj,RenderObjClass *) -#else case (RENDOBJFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; -#endif READ_MICRO_CHUNK(cload,RENDOBJFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,RENDOBJFACTORY_VARIABLE_NAME,name,sizeof(name)); } @@ -1283,9 +1277,7 @@ PersistClass * RenderObjPersistFactoryClass::Load(ChunkLoadClass & cload) const new_obj->Set_Transform(tm); } -#if !defined(_WIN64) && !defined(__x86_64__) old_obj = (RenderObjClass *)(uintptr_t)old_obj_token; -#endif SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } @@ -1297,7 +1289,13 @@ void RenderObjPersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * ob const Matrix3D& tm = robj->Get_Transform(); csave.Begin_Chunk(RENDOBJFACTORY_CHUNKID_VARIABLES); +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. + uint32 robj_token = (uint32)(uintptr_t)robj; + WRITE_MICRO_CHUNK(csave,RENDOBJFACTORY_VARIABLE_OBJPOINTER,robj_token); +#else WRITE_MICRO_CHUNK(csave,RENDOBJFACTORY_VARIABLE_OBJPOINTER,robj); +#endif WRITE_MICRO_CHUNK_STRING(csave,RENDOBJFACTORY_VARIABLE_NAME,name); WRITE_MICRO_CHUNK(csave,RENDOBJFACTORY_VARIABLE_TRANSFORM,tm); csave.End_Chunk(); diff --git a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp index b148be7337b..facabfb244e 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/AudibleSound.cpp @@ -1654,8 +1654,14 @@ AudibleSoundClass::Save (ChunkSaveClass &csave) WRITE_MICRO_CHUNK_STRING (csave, VARID_FILENAME, m_Buffer->Get_Filename ()); } +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. + uint32 this_ptr_token = (uint32)(uintptr_t)this; + WRITE_MICRO_CHUNK (csave, VARID_THIS_PTR, this_ptr_token); +#else AudibleSoundClass *this_ptr = this; WRITE_MICRO_CHUNK (csave, VARID_THIS_PTR, this_ptr); +#endif csave.End_Chunk (); @@ -1710,21 +1716,6 @@ AudibleSoundClass::Load (ChunkLoadClass &cload) READ_MICRO_CHUNK_WWSTRING (cload, VARID_FILENAME, filename); -#if defined(_WIN64) || defined(__x86_64__) - case VARID_THIS_PTR: - { - // TheSuperHackers @fix MeneerHaas 02/09/2026 Read the identity at the width the chunk - // actually holds, as READ_MICRO_CHUNK_POINTER_TOKEN does; sizeof(old_ptr) is 8 on x64 and - // ChunkLoadClass::Read would then read nothing at all from a legacy 4-byte chunk. - uintptr_t old_ptr_token = 0; - uint32 token_length = cload.Cur_Micro_Chunk_Length (); - if (token_length > sizeof (old_ptr_token)) token_length = sizeof (old_ptr_token); - cload.Read (&old_ptr_token, token_length); - AudibleSoundClass *old_ptr = (AudibleSoundClass *)old_ptr_token; - SaveLoadSystemClass::Register_Pointer (old_ptr, this); - } - break; -#else case VARID_THIS_PTR: { // Read exactly what Save wrote: a fixed-width 4-byte @@ -1741,7 +1732,6 @@ AudibleSoundClass::Load (ChunkLoadClass &cload) SaveLoadSystemClass::Register_Pointer (old_ptr, this); } break; -#endif } cload.Close_Micro_Chunk (); diff --git a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp index 8f31193a1e2..a5aad110c79 100644 --- a/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp +++ b/Core/Libraries/Source/WWVegas/WWAudio/SoundSceneObj.cpp @@ -260,10 +260,22 @@ SoundSceneObjClass::Save (ChunkSaveClass &csave) csave.End_Chunk (); csave.Begin_Chunk (CHUNKID_VARIABLES); +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Persisted pointers are 4-byte identity tokens: that is what the + // legacy files hold and what RenderObjPersistFactory registers, so the remap below can match. + uint32 attached_obj_token = (uint32)(uintptr_t)m_AttachedObject; + WRITE_MICRO_CHUNK (csave, VARID_ATTACHED_OBJ, attached_obj_token); +#else WRITE_MICRO_CHUNK (csave, VARID_ATTACHED_OBJ, m_AttachedObject); +#endif WRITE_MICRO_CHUNK (csave, VARID_ATTACHED_BONE, m_AttachedBone); WRITE_MICRO_CHUNK (csave, VARID_USER_DATA, m_UserData); +#if defined(_WIN64) || defined(__x86_64__) + uint32 user_obj_token = (uint32)(uintptr_t)m_UserObj; + WRITE_MICRO_CHUNK (csave, VARID_USER_OBJ, user_obj_token); +#else WRITE_MICRO_CHUNK (csave, VARID_USER_OBJ, m_UserObj); +#endif WRITE_MICRO_CHUNK (csave, VARID_ID, m_ID); csave.End_Chunk (); return true; @@ -296,14 +308,29 @@ SoundSceneObjClass::Load (ChunkLoadClass &cload) switch (cload.Cur_Micro_Chunk_ID ()) { #if defined(_WIN64) || defined(__x86_64__) - READ_MICRO_CHUNK_POINTER_TOKEN (cload, VARID_ATTACHED_OBJ, m_AttachedObject, RenderObjClass *) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Read exactly the 4 bytes Save wrote; sizeof(pointer) + // is 8 here, and ChunkLoadClass::Read refuses a short chunk outright, leaving the + // member unset and poisoning the pointer remap. + case VARID_ATTACHED_OBJ: + { + uint32 attached_obj_token = 0; + cload.Read (&attached_obj_token, sizeof (attached_obj_token)); + m_AttachedObject = (RenderObjClass *)(uintptr_t)attached_obj_token; + break; + } #else READ_MICRO_CHUNK (cload, VARID_ATTACHED_OBJ, m_AttachedObject); #endif READ_MICRO_CHUNK (cload, VARID_ATTACHED_BONE, m_AttachedBone); READ_MICRO_CHUNK (cload, VARID_USER_DATA, m_UserData); #if defined(_WIN64) || defined(__x86_64__) - READ_MICRO_CHUNK_POINTER_TOKEN (cload, VARID_USER_OBJ, m_UserObj, RefCountClass *) + case VARID_USER_OBJ: + { + uint32 user_obj_token = 0; + cload.Read (&user_obj_token, sizeof (user_obj_token)); + m_UserObj = (RefCountClass *)(uintptr_t)user_obj_token; + break; + } #else READ_MICRO_CHUNK (cload, VARID_USER_OBJ, m_UserObj); #endif diff --git a/Core/Libraries/Source/WWVegas/WWLib/chunkio.h b/Core/Libraries/Source/WWVegas/WWLib/chunkio.h index 1d653527151..abd0729e475 100644 --- a/Core/Libraries/Source/WWVegas/WWLib/chunkio.h +++ b/Core/Libraries/Source/WWVegas/WWLib/chunkio.h @@ -329,24 +329,6 @@ class ChunkLoadClass break; \ } -#if defined(_WIN64) || defined(__x86_64__) -// TheSuperHackers @fix MeneerHaas 02/09/2026 Persisted pointer identities are written at the writing build's pointer -// width: 4 bytes in every file a 32-bit build produced, 8 from an x64 build. Read what the chunk -// actually holds rather than sizeof(var) -- a plain READ_MICRO_CHUNK would ask for 8 bytes from a -// legacy 4-byte chunk, and ChunkLoadClass::Read then reads nothing at all, silently leaving the -// identity null and poisoning the pointer remap. Reading by length also means no address ever has -// to be truncated into a token, so two objects cannot collide on one identity. -#define READ_MICRO_CHUNK_POINTER_TOKEN(cload,id,var,type)\ - case (id): {\ - uintptr_t temp_token = 0;\ - uint32 temp_length = cload.Cur_Micro_Chunk_Length();\ - if (temp_length > sizeof(temp_token)) temp_length = sizeof(temp_token);\ - cload.Read(&temp_token,temp_length);\ - var = (type)temp_token;\ - break;\ - } -#endif - #define READ_MICRO_CHUNK_STRING(cload,id,var,size) \ case (id): WWASSERT(cload.Cur_Micro_Chunk_Length() <= size); cload.Read(var,cload.Cur_Micro_Chunk_Length()); break; \ diff --git a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index 15a618e9073..91f51591cac 100644 --- a/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/Generals/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1349,7 +1349,6 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const char dazzle_type[256]; dazzle_type[0] = 0; -#if !defined(_WIN64) && !defined(__x86_64__) // Read exactly what Save wrote: a fixed-width 4-byte identity token, not // sizeof(old_obj). On x86-64 sizeof(DazzleRenderObjClass*) is 8, so // reading sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for @@ -1357,7 +1356,6 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const // then refuses to read anything at all and old_obj stays null, silently // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. uint32 old_obj_token = 0; -#endif /* ** Load the dazzle parameters @@ -1369,11 +1367,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { -#if defined(_WIN64) || defined(__x86_64__) - READ_MICRO_CHUNK_POINTER_TOKEN(cload,DAZZLEFACTORY_VARIABLE_OBJPOINTER,old_obj,DazzleRenderObjClass *) -#else case (DAZZLEFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; -#endif READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type,sizeof(dazzle_type)); } @@ -1417,9 +1411,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const /* ** Register the old pointer for re-mapping to the new pointer */ -#if !defined(_WIN64) && !defined(__x86_64__) old_obj = (DazzleRenderObjClass *)(uintptr_t)old_obj_token; -#endif SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } @@ -1432,7 +1424,13 @@ void DazzlePersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * obj) const Matrix3D& tm = robj->Get_Transform(); csave.Begin_Chunk(DAZZLEFACTORY_CHUNKID_VARIABLES); +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. + uint32 robj_token = (uint32)(uintptr_t)robj; + WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj_token); +#else WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj); +#endif WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); WRITE_MICRO_CHUNK_STRING(csave,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type_name); diff --git a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp index 78302853073..05abfdbdafd 100644 --- a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp +++ b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dazzle.cpp @@ -1452,7 +1452,6 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const char dazzle_type[256]; dazzle_type[0] = 0; -#if !defined(_WIN64) && !defined(__x86_64__) // Read exactly what Save wrote: a fixed-width 4-byte identity token, not // sizeof(old_obj). On x86-64 sizeof(DazzleRenderObjClass*) is 8, so // reading sizeof(old_obj) here (as READ_MICRO_CHUNK would) would ask for @@ -1460,7 +1459,6 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const // then refuses to read anything at all and old_obj stays null, silently // poisoning SaveLoadSystemClass's pointer remap table. See persistfactory.h. uint32 old_obj_token = 0; -#endif /* ** Load the dazzle parameters @@ -1472,11 +1470,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const while (cload.Open_Micro_Chunk()) { switch(cload.Cur_Micro_Chunk_ID()) { -#if defined(_WIN64) || defined(__x86_64__) - READ_MICRO_CHUNK_POINTER_TOKEN(cload,DAZZLEFACTORY_VARIABLE_OBJPOINTER,old_obj,DazzleRenderObjClass *) -#else case (DAZZLEFACTORY_VARIABLE_OBJPOINTER): cload.Read(&old_obj_token,sizeof(old_obj_token)); break; -#endif READ_MICRO_CHUNK(cload,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); READ_MICRO_CHUNK_STRING(cload,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type,sizeof(dazzle_type)); } @@ -1520,9 +1514,7 @@ PersistClass * DazzlePersistFactoryClass::Load(ChunkLoadClass & cload) const /* ** Register the old pointer for re-mapping to the new pointer */ -#if !defined(_WIN64) && !defined(__x86_64__) old_obj = (DazzleRenderObjClass *)(uintptr_t)old_obj_token; -#endif SaveLoadSystemClass::Register_Pointer(old_obj,new_obj); return new_obj; } @@ -1535,7 +1527,13 @@ void DazzlePersistFactoryClass::Save(ChunkSaveClass & csave,PersistClass * obj) const Matrix3D& tm = robj->Get_Transform(); csave.Begin_Chunk(DAZZLEFACTORY_CHUNKID_VARIABLES); +#if defined(_WIN64) || defined(__x86_64__) + // TheSuperHackers @fix MeneerHaas 02/09/2026 Write the 4-byte identity token the loader reads; see persistfactory.h. + uint32 robj_token = (uint32)(uintptr_t)robj; + WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj_token); +#else WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_OBJPOINTER,robj); +#endif WRITE_MICRO_CHUNK(csave,DAZZLEFACTORY_VARIABLE_TRANSFORM,tm); WRITE_MICRO_CHUNK_STRING(csave,DAZZLEFACTORY_VARIABLE_TYPENAME,dazzle_type_name);