fix: dump correct breakpad symbols on macOS #19042
Conversation
|
(Draft state because I haven't yet integrated this into CI / release. But I've tested the GN targets and they work 👍) |
|
This is ready for review now. I've refactored how symbol generation works to avoid using the not-very-well-maintained |
|
Test release builds running here: |
| @@ -937,6 +937,76 @@ if (is_mac) { | |||
| "@executable_path/../Frameworks", | |||
| ] | |||
| } | |||
|
|
|||
| if (enable_dsyms) { | |||
There was a problem hiding this comment.
This never appears to be set? Unless I'm missing a thing?
There was a problem hiding this comment.
it's set in Chromium's GN files.
It refactors all platforms, but only fixes macOS. Windows (and Linux I think? haven't checked) were already working |
|
@nornagon Should we backport this ? Seems like this must go all the way back to 4 ? |
|
@deepak1556 probably, yeah. |
There was a problem hiding this comment.
Looks like Windows is failing on:
loadDataForPdb and loadDataFromExe failed for -i
Open failed
See https://ci.appveyor.com/project/electron-bot/electron-ia32-release/builds/25699648
And
C:/Python27/python.exe ../../electron/build/dump_syms.py ./dump_syms.exe electron.exe breakpad_symbols gen/electron/electron_app_symbols.stamp
CoCreateInstance CLSID_DiaSource {E6756135-1E65-4D17-8576-610761398C3C} failed (msdia*.dll unregistered?)
Open failed
Traceback (most recent call last):
File "../../electron/build/dump_syms.py", line 49, in <module>
See https://ci.appveyor.com/project/electron-bot/electron-x64-release/builds/25699649
Linux release builds are failing with a missing breakpad_symbols.zip, eg see: https://circleci.com/gh/electron/electron/249756
script/release/uploaders/upload.py
Outdated
| @@ -68,7 +68,7 @@ def main(): | |||
| upload_electron(release, electron_zip, args) | |||
| if get_target_arch() != 'mips64el': | |||
| symbols_zip = os.path.join(OUT_DIR, SYMBOLS_NAME) | |||
| shutil.copy2(os.path.join(OUT_DIR, 'symbols.zip'), symbols_zip) | |||
| shutil.copy2(os.path.join(OUT_DIR, 'breakpad_symbols.zip'), symbols_zip) | |||
There was a problem hiding this comment.
Where does this file actually get created?
There was a problem hiding this comment.
oops, i started down a path of getting gn to make the zip file too but then half-undid it. i'll revert this to symbols.zip, which is what zip-symbols.py creates.
|
Release builds for testing: |
|
Release builds here: |
|
Release builds: |
|
Release builds: |
|
The backport process for this PR has been manually initiated, |
|
/trop run backport-to 6-0-x |
|
The backport process for this PR has been manually initiated, |
|
I was unable to backport this PR to "5-0-x" cleanly; |
|
I was unable to backport this PR to "6-0-x" cleanly; |
|
A maintainer has manually backported this PR to "6-0-x", please check out #19155 |
|
A maintainer has manually backported this PR to "5-0-x", please check out #19157 |
|
A maintainer has manually backported this PR to "4-2-x", please check out #19158 |
|
|
||
| extract_symbols("swiftshader_gles_symbols") { | ||
| binary = | ||
| "$root_out_dir/swiftshader/libGLESv2$_target_shared_library_suffix" |
There was a problem hiding this comment.
@nornagon The symbols for libGLESv2.dll are not present in Sentry even though the other Electron symbols are there. See: https://sentry.io/share/issue/d07971e24b74434f9d3bbfa520b47e0a/
Do you know why that may be? Are we failing to package libGLESv2.dll.pdb on Windows?
Description of Change
The breakpad symbols we're currently generating on macOS only include exported symbols, which makes them approximately useless for symbolication. This mimics how Chromium generates breakpad symbols.
Checklist
npm testpassesRelease Notes
Notes: Fixed macOS breakpad symbol files to include non-public symbols.