[Export] Linux, Windows: Support exporting with debug symbols - #114924
AThousandShips wants to merge 1 commit into
Conversation
| @@ -26,6 +26,9 @@ | |||
| <member name="debug/export_console_wrapper" type="int" setter="" getter=""> | |||
| If [code]true[/code], a console wrapper is exported alongside the main executable, which allows running the project with enabled console output. | |||
| </member> | |||
| <member name="debug/export_debug_symbols" type="int" setter="" getter=""> | |||
| If [code]true[/code], a debug symbol file ([code].debugsymbols[/code]) is exported alongside the main executable. | |||
There was a problem hiding this comment.
Might want to add a note to this (and the console wrapper one for Windows) about where it is from and how to make it, but not sure
There was a problem hiding this comment.
It probably should be mentioned in the https://docs.godotengine.org/en/latest/engine_details/development/compiling/index.html, since other export template relates stuff is there.
There was a problem hiding this comment.
Agreed, will make a PR for that when this has progressed further, there's a section there about debug symbols and separate ones
There was a problem hiding this comment.
If absolutely needed, you could then... [url] tag to this specific section in the docs
There was a problem hiding this comment.
Ultimately most of the details are here, and will add some general details in the manual, but don't think a link here is necessary as most users won't be compiling their own builds and if they do they should know where to look
It is the same, only difference is that |
|
Then I might leave that to a follow-up so someone with macOS can test and develop it specifically |
07c6b43 to
b0ac291
Compare
b0ac291 to
c167d93
Compare
c167d93 to
9c12f3f
Compare
59e4f6c to
6337c44
Compare
| if (err == OK && copy_debug_symbols) { | ||
| const String debug_symbols_path = template_path + ".debugsymbols"; | ||
| if (FileAccess::exists(debug_symbols_path)) { | ||
| err = da->copy(debug_symbols_path, p_path + ".debugsymbols"); | ||
| } | ||
| if (err == OK && copy_wrapper) { | ||
| for (int i = 0; wrapper_extensions[i]; ++i) { | ||
| const String wrapper_path = template_path.get_basename() + wrapper_extensions[i] + ".debugsymbols"; | ||
| if (FileAccess::exists(wrapper_path)) { | ||
| err = da->copy(wrapper_path, p_path.get_basename() + ".console.exe.debugsymbols"); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Windows export templates compiled with MSVC use a .pdb extension, so I would also check for that (after .debugsymbols).
This is not needed for official export templates as they use MinGW, but it can be needed for custom export templates.
There was a problem hiding this comment.
Is that fetched automatically like the .debugsymbols file? Is it an alternative to the debug symbols or a replacement?
There was a problem hiding this comment.
We have this rename check for the .debugsymbols file to ensure renames work, not sure if that works out of the box with the .pdb file, I haven't built with msvc lately so can't test currently but can set up and test
godot/platform/windows/crash_handler_windows_signal.cpp
Lines 271 to 273 in c12e519
Would this need a different check for that case?
There was a problem hiding this comment.
Is it an alternative to the debug symbols or a replacement?
It's an alternative; you can't have both .debugsymbols and .pdb for a single build.
Note that MSVC debug symbols are always separate as .pdb, while MinGW debug symbols can be embedded into the .exe.
Would this need a different check for that case?
Yes, as this check is only intended to find separate MinGW debug symbols.
There is another issue we'll probably need to tackle with MSVC debug symbols: the executable finds the PDB location with a path defined in the .exe, which is written as absolute in Godot. This means that if you export the project to any location that isn't the same path as the location the project was built, the debug symbols won't be recognized automatically.
@bruvzg Any ideas?
There was a problem hiding this comment.
Then I think we can leave that as a follow-up including code to integrate that into the system (official builds are made with gcc so will only be relevant for custom builds with msvc)
There was a problem hiding this comment.
Official templates are not using MSVC, so I think it's OK to skip it for now.
I think path can be set to relative using /PDBALTPATH:, but it still will need change when exporting.
See WindowsUtils::copy_and_rename_pdb which is doing the PDB renaming for GDExtension dlls, something similar probably can be done for the export.
db34f75 to
49451e7
Compare
There was a problem hiding this comment.
Tested locally with custom export templates, it works as expected. Code looks good to me.
However, this will not work with official export templates until we start distributing those .debugsymbols files for Windows and Linux binaries. If we merge this, we should make sure we start doing so in the same minor release the PR is merged in, so that the feature can work out of the box. This will significantly increase the size of Windows/Linux export template downloads, but now that platforms can be selected individually, it should be acceptable.
In a future PR, when we make debug symbols available for the editor, we could add a button somewhere in the editor to download debug symbols for it (and place them next to the editor binary). This will greatly help in troubleshooting crashes in bug reports, and should hopefully make https://github.com/Calinou/godot-debug-builds no longer needed 🙂
|
I think Akien has been working on making symbols available for official builds but haven't kept up with the details |
|
Integrated these changes and some fixes and works with custom templates (manually selected) will test with templates placed in the official folder as well and push the changes soon |
b626b03 to
34f181f
Compare
|
There! Validated the improvements and it all works well now! Will do a pass over the documentation to add the details about Haven't tested with gdb directly yet but the dwarf link works correctly with this PR so it should work as long as it already works in |
|
Awesome! So how should I name the |
|
So for the new naming should simply be the same as the executable plus One thing to consider for the official templates with debug symbols for the editor is that currently we link it to a file based on the export name, which should be This won't really matter for export templates as we fix them with this PR, but the editor might not work correctly if we distribute debug symbols with it, so we'd need to fix those files similarly to this script Will add notes in the manual side on this for what is required for the crash handler and gdb respectively for different scenarios when we've settled on this PR, but in short: The crash handler and gdb will work if a
The crash handler additionally works if the debug symbols match the executable plus just (Will go for lunch and will check in after) For ease of access, for linux:
And Windows:
|
34f181f to
82c23db
Compare
| } | ||
| } | ||
| } | ||
| if (err == OK && copy_debug_symbols) { |
There was a problem hiding this comment.
This section along with the wrapper is getting a bit cluttered but I think it'd be something to clean up in a follow-up PR, properly moving the console wrapper code into the windows platform code to make it easier to maintain
| return OK; | ||
| } | ||
|
|
||
| bool EditorExportPlatformPC::_copy_debugsymbols(Ref<DirAccess> &p_da, const String &p_path, const String &p_symbols_path, Error &r_err) { |
There was a problem hiding this comment.
This could be made into a virtual method but as there's no difference currently I've left it a local method, as it got a bit cluttered with the different options, some of these paths should probably be collapsed and cleaned up
|
An additional step for the build process in the build server to ensure the debug symbols will work when copied directly is to do: strip --remove-section=".gnu_debuglink" linux_${TARGET}.${ARCH}
objcopy --add-gnu-debuglink=linux_${TARGET}.${ARCH}.debugsymbols linux_${TARGET}.${ARCH}As part of the export rename process, as well as for the editor with the same appropriate name, as well as for Windows This requires the build tools for the respective platform though so a bit of an additional step, but it'll make sure that especially the editor will link correctly when distributed with symbols, could be done as part of the build step as well but would require moving the renaming step into that instead of the finishing up step It's not critical for the templates but would be required for the editor symbols to work if they aren't left as the original build output name Edit: for the linux builds this requires the specific buildroot strip/objcopy files like in the build process itself, using the relevant SDK paths, and for Windows it requires the build environment from the same, including using the llvm-root ones for arm builds Not critical but making these adjustments for the resulting files means that you will be able to simply copy the debug symbols from the distribution directly into the exported project without renaming which is useful For the editor it is necessary for those to work correctly though |
|
Ran into a crash when testing this so will do some testing on that Edit: Seems to be unrelated to these changes, will see if I can replicate it more clearly later and report it if it hasn't been reported already |
|
So an update: This won't really matter currently as we adjust it with this export feature, but I'm looking into how to fetch that and use it as a backup for the name if missed, shouldn't be too difficult to just load the section, but it'd help Longer term I'm also considering looking at leveraging the gdb lookup system to fetch the debug symbols, as it looks in a few different locations, and potentially leveraging the But if we are happy with the way this works now I will update the class reference to reflect where it looks for the debug symbols, and start writing the details for the manual, outlining the (current) conditions for finding the relevant handlers etc. |
|
Actually made significant progress on integrating the This would make a lot more parity with the gdb system and match what is expected for debugging on the Linux side Will work on the documentation side for this tomorrow or on Wednesday Edit: PR open #122764 |
82c23db to
50892f4
Compare
|
Amended and adjusted the class reference, should be good to go now, I will create a documentation PR for this as well as #122764 as soon as I am able but as we don't really have any details about the crash handler currently I'd say it'd be better to make one adjustment to fit both Edit: opened a very bare bones manual PR for this: |
|
To reiterate the scope of follow-up changes:
Longer term goals:
More broadly and not directly related:
|
50892f4 to
f643d7f
Compare
f643d7f to
56de93f
Compare
Copies debug symbols (`.debugsymbols`) from the template path, similar to the console wrapper for Windows. Co-authored-by: Pāvels Nadtočajevs <7645683+bruvzg@users.noreply.github.com>
56de93f to
1468db6
Compare
6 participants
Copies debug symbols (
.debugsymbols) from the template path, similar to the console wrapper for Windows.Not sure how applicable this is for macOS, I can see
.dSYMbeing created based on the build scripts but I don't have a macOS setup to check or a build environment. If a similar setup is possible for macOS I can expand it.Ran into this as I was setting up builds with separate symbols and realized there was no simple way to bundle these without manually copying and changing the name, so implemented this for my own use.
This will also help with: