[Windows] Fix loading debug symbol files over 2GB.

[bruvzg]

No description provided.

[hpvb]

The changes look super straightforward and correct.

[akien-mga]

Looks good to me! I'll give it a try locally, also rebased on top of #100274.

Could you propose that fix upstream? They seem to be active integrating patches nowadays.

[Repiteo]

If this fixes the 2GB debug symbol limit, shouldn't we also remove the current separate_debug_symbols workaround?

 platform/windows/SCsub                        | 2 +-
 platform/windows/platform_windows_builders.py | 8 +++-----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/platform/windows/SCsub b/platform/windows/SCsub
index 1ddefb9c33..4d4d69cbe9 100644
--- a/platform/windows/SCsub
+++ b/platform/windows/SCsub
@@ -125,7 +125,7 @@ if env["d3d12"]:
         )
 
 if not os.getenv("VCINSTALLDIR"):
-    if env["debug_symbols"]:
+    if env["debug_symbols"] and env["separate_debug_symbols"]:
         env.AddPostAction(prog, env.Run(platform_windows_builders.make_debug_mingw))
         if env["windows_subsystem"] == "gui":
             env.AddPostAction(prog_wrap, env.Run(platform_windows_builders.make_debug_mingw))
diff --git a/platform/windows/platform_windows_builders.py b/platform/windows/platform_windows_builders.py
index 2020e68748..1bd9ab3070 100644
--- a/platform/windows/platform_windows_builders.py
+++ b/platform/windows/platform_windows_builders.py
@@ -5,8 +5,6 @@ import os
 
 def make_debug_mingw(target, source, env):
     dst = str(target[0])
-    # Force separate debug symbols if executable size is larger than 1.9 GB.
-    if env["separate_debug_symbols"] or os.stat(dst).st_size >= 2040109465:
-        os.system("{0} --only-keep-debug {1} {1}.debugsymbols".format(env["OBJCOPY"], dst))
-        os.system("{0} --strip-debug --strip-unneeded {1}".format(env["STRIP"], dst))
-        os.system("{0} --add-gnu-debuglink={1}.debugsymbols {1}".format(env["OBJCOPY"], dst))
+    os.system("{0} --only-keep-debug {1} {1}.debugsymbols".format(env["OBJCOPY"], dst))
+    os.system("{0} --strip-debug --strip-unneeded {1}".format(env["STRIP"], dst))
+    os.system("{0} --add-gnu-debuglink={1}.debugsymbols {1}".format(env["OBJCOPY"], dst))

[bruvzg]

If this fixes the 2GB debug symbol limit, shouldn't we also remove the current separate_debug_symbols workaround?

No, it fixes loading separate symbols over 2GB, Windows still can't load exe over 1.9 GB and there's no workaround for it (it's file format limitation), so embedded symbols still won't work.

[akien-mga]

I'll give it a try locally, also rebased on top of #100274.

Tested that locally, it seems to work well. All those binaries have a working backtrace:

-rwxr-xr-x. 1 root root     190936 Dec 11 16:20 bin/godot.windows.editor.x86_64.dwarf4.console.exe
-rwxr-xr-x. 1 root root      75546 Dec 11 16:20 bin/godot.windows.editor.x86_64.dwarf4.console.exe.debugsymbols
-rwxr-xr-x. 1 root root  129045771 Dec 11 16:20 bin/godot.windows.editor.x86_64.dwarf4.exe
-rwxr-xr-x. 1 root root 2306190149 Dec 11 16:20 bin/godot.windows.editor.x86_64.dwarf4.exe.debugsymbols
-rwxr-xr-x. 1 root root     190936 Dec 11 16:16 bin/godot.windows.editor.x86_64.dwarf5.console.exe
-rwxr-xr-x. 1 root root      75020 Dec 11 16:16 bin/godot.windows.editor.x86_64.dwarf5.console.exe.debugsymbols
-rwxr-xr-x. 1 root root  129045771 Dec 11 16:16 bin/godot.windows.editor.x86_64.dwarf5.exe
-rwxr-xr-x. 1 root root 1896596107 Dec 11 16:16 bin/godot.windows.editor.x86_64.dwarf5.exe.debugsymbols

I tried too with LTO, and those binaries have a working backtrace but without any Godot specific symbols:

[1] __C_specific_handler (dlls/ntdll/unwind.c:2260)
[2] call_seh_handlers (dlls/ntdll/signal_x86_64.c:211)
[3] dispatch_exception (dlls/ntdll/exception.c:252)
[4] __tmainCRTStartup (../crt/crtexe.c:272)
[5] WinMainCRTStartup (../crt/crtexe.c:162)
[6] BaseThreadInitThunk (dlls/kernel32/thread.c:61)

Not sure what's the state of GCC LTO and debugging, so this is probably not related to this change. This might be a problem for our goal of providing debuginfo for official builds though, so maybe something to look at next.

[akien-mga]

Thanks!