fix: crash reporting on Windows on Arm #19391

richard-townsend-arm · 2019-07-23T16:43:12Z

Description of Change

This PR contains two back-ports from Chromium 77:

Together, these allow electron to generate crash reports in all circumstances. Previously, if native C++ code (in, say, atom or a node module) was called from Javascript and a fatal exception occurred (e.g. a null pointer dereference), Windows would not be able to unwind the stack correctly and find an exception handler. It would then kill the electron process without leaving behind any indication of what caused the crash. This V8 change, alongside the crashpad capture context changes, mean that field crash reports can be collected for Windows on Arm applications.

CC: @jkleinsc

Checklist

PR description included and stakeholders cc'd
npm test passes
PR title follows semantic commit guidelines
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: no-notes

richard-townsend-arm · 2019-07-23T17:40:23Z

linux-arm64-testing seems to have failed due to a corrupt sscache stream (should be unrelated to the change).

MarshallOfSound · 2019-08-01T20:27:14Z

I've reconfigured this PR to only apply these two patches when targetting arm64. @jkleinsc can you update our CI to set the apply_win_arm64_patches variable for arm builds?

jkleinsc · 2019-08-01T20:33:31Z

@MarshallOfSound build here: https://ci.appveyor.com/project/electron-bot/electron-ldhmv/builds/26411913

jkleinsc · 2019-08-02T13:20:09Z

WOA build with latest commits here: https://ci.appveyor.com/project/electron-bot/electron-ldhmv/builds/26428202

richard-townsend-arm · 2019-08-08T19:03:36Z

CI flaked with this (unrelated) test

not ok 747 session module ses.protocol handles requests from partition
  Error: Timeout of 30000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (C:\projects\src\electron\spec\api-session-spec.js)
      at Test.Runnable._timeoutError (C:\projects\src\electron\spec\node_modules\mocha\lib\runnable.js:440:10)
      at C:\projects\src\electron\spec\node_modules\mocha\lib\runnable.js:251:24
ok 748 session module ses.setProxy(options) allows configuring proxy settings
ok 749 session module ses.setProxy(options) allows configuring proxy settings (callback)

@jkleinsc: could you kick off a build for Windows on Arm?

jkleinsc · 2019-08-08T19:52:36Z

@richard-townsend-arm WOA build here: https://ci.appveyor.com/project/electron-bot/electron-ldhmv/builds/26570556

richard-townsend-arm · 2019-08-08T20:22:16Z

NB @jkleinsc: there should be two additional patches applied after the rest of them. Is --custom-var=apply_win_arm64_patches=True or similar being passed via %GCLIENT_EXTRA_ARGS%?

richard-townsend-arm · 2019-08-08T20:48:37Z

The TARGET_ARCH variable also seems to be unspecified on that previous build.

richard-townsend-arm · 2019-08-08T20:55:54Z

The electron/script/zip_manifests/dist_zip.win.arm64.manifest file should look quite similar to the x64 version, but a few extra DLLs are required (see this file generated from GN's runtime deps). I'll try to get another patch out tomorrow that adds those.

richard-townsend-arm · 2019-08-15T10:51:00Z

So, if everything's building right, together with #19366 and the optional #19383, we should get a test run like the following (i.e. about 2-4 unit test failures).

electron-6-0-x-integration-output.log

One other thing that occurred to me is that because dugite doesn't know how to install git prebuilts on Windows on Arm, you have to install the x86 version of Git onto the test machine and set LOCAL_GIT_DIRECTORY=C:\Program Files (x86)\Git\ in the environment. desktop/dugite#376 should fix this in the long run.

jkleinsc · 2019-08-26T13:56:56Z

@richard-townsend-arm can you rebase this with the latest from 6-0-x? I just merged in the changes so that we can properly build WOA.

richard-townsend-arm · 2019-08-26T14:18:31Z

Rebasing now...

@ThomsonTan

This commit backports [1] (originally written by @ThomsonTan) into V8 7.6. With this patch in place, calls made from JS into the atom core which crash electron.exe will now generate crash dumps on Windows on Arm rather than silently dying. [1] https://chromium-review.googlesource.com/c/v8/v8/+/1701133/11

@kaadam

Backport of [1] (originally written by @kaadam) to Chromium 76's crashpad. This lets you see the register values within the crash dump. [1] https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1632749

richard-townsend-arm · 2019-08-26T14:58:15Z

Cool, so I've confirmed that it builds for WoA, unsure whether it works yet because all my WoA devices are offline, I'm OoO and unable to go kick them.

richard-townsend-arm · 2019-08-26T15:28:50Z

win-ia32-testing-pr has flaked due to an unrelated issue:

not ok 750 session module ses.protocol handles requests from partition
  Error: Timeout of 30000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (C:\projects\src\electron\spec\api-session-spec.js)
      at Test.Runnable._timeoutError (C:\projects\src\electron\spec\node_modules\mocha\lib\runnable.js:440:10)
      at C:\projects\src\electron\spec\node_modules\mocha\lib\runnable.js:251:24
ok 751 session module ses.setProxy(options) allows configuring proxy settings
ok 752 session module ses.setProxy(options) allows configuring proxy settings (callback)

jkleinsc · 2019-08-26T16:08:19Z

WOA build running here: https://ci.appveyor.com/project/electron-bot/electron-ldhmv/builds/26965410

jkleinsc

LGTM

release-clerk · 2019-08-27T14:43:30Z

Release Notes Persisted

no-notes

richard-townsend-arm requested a review from a team as a code owner July 23, 2019 16:43

electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jul 23, 2019

electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jul 24, 2019

MarshallOfSound force-pushed the woa-crashreporting branch from d0a57a4 to 0b62702 Compare August 1, 2019 20:26

richard-townsend-arm changed the title ~~fix: crash reporting on Windows on Arm [WIP] [DO NOT MERGE]~~ fix: crash reporting on Windows on Arm Aug 8, 2019

jkleinsc added the 6-0-x label Aug 15, 2019

jkleinsc mentioned this pull request Aug 15, 2019

build: start building Windows on Arm builds #19780

Merged

3 tasks

richard-townsend-arm and others added 8 commits August 26, 2019 15:23

feat: backport crashpad cpu capture context

01ad734

Backport of [1] (originally written by @kaadam) to Chromium 76's crashpad. This lets you see the register values within the crash dump. [1] https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1632749

build: make win on arm patches only apply for win on arm builds

a2b8d73

Update .patches

c312814

Update .patches

02cfc0c

Update DEPS

2a3dac2

fix: update the patch to the as-landed version

54fdc5d

fix: remove accidental revert of crashpad_pid_check.patch

3a1b8e7

richard-townsend-arm force-pushed the woa-crashreporting branch from 3605ce3 to 3a1b8e7 Compare August 26, 2019 14:32

jkleinsc approved these changes Aug 27, 2019

View reviewed changes

jkleinsc merged commit bb63189 into electron:6-0-x Aug 27, 2019

richard-townsend-arm deleted the woa-crashreporting branch August 27, 2019 15:36

sofianguy added this to Fixed in 6.0.5 in 6.1.x Aug 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: crash reporting on Windows on Arm #19391

fix: crash reporting on Windows on Arm #19391

richard-townsend-arm commented Jul 23, 2019 •

edited

richard-townsend-arm commented Jul 23, 2019

MarshallOfSound commented Aug 1, 2019

jkleinsc commented Aug 1, 2019

jkleinsc commented Aug 2, 2019

richard-townsend-arm commented Aug 8, 2019 •

edited

jkleinsc commented Aug 8, 2019

richard-townsend-arm commented Aug 8, 2019

richard-townsend-arm commented Aug 8, 2019

richard-townsend-arm commented Aug 8, 2019 •

edited

richard-townsend-arm commented Aug 15, 2019

jkleinsc commented Aug 26, 2019

richard-townsend-arm commented Aug 26, 2019

richard-townsend-arm commented Aug 26, 2019

richard-townsend-arm commented Aug 26, 2019 •

edited

jkleinsc commented Aug 26, 2019

jkleinsc left a comment

release-clerk bot commented Aug 27, 2019

fix: crash reporting on Windows on Arm #19391

fix: crash reporting on Windows on Arm #19391

Conversation

richard-townsend-arm commented Jul 23, 2019 • edited

Description of Change

Checklist

Release Notes

richard-townsend-arm commented Jul 23, 2019

MarshallOfSound commented Aug 1, 2019

jkleinsc commented Aug 1, 2019

jkleinsc commented Aug 2, 2019

richard-townsend-arm commented Aug 8, 2019 • edited

jkleinsc commented Aug 8, 2019

richard-townsend-arm commented Aug 8, 2019

richard-townsend-arm commented Aug 8, 2019

richard-townsend-arm commented Aug 8, 2019 • edited

richard-townsend-arm commented Aug 15, 2019

jkleinsc commented Aug 26, 2019

richard-townsend-arm commented Aug 26, 2019

richard-townsend-arm commented Aug 26, 2019

richard-townsend-arm commented Aug 26, 2019 • edited

jkleinsc commented Aug 26, 2019

jkleinsc left a comment

Choose a reason for hiding this comment

release-clerk bot commented Aug 27, 2019

richard-townsend-arm commented Jul 23, 2019 •

edited

richard-townsend-arm commented Aug 8, 2019 •

edited

richard-townsend-arm commented Aug 8, 2019 •

edited

richard-townsend-arm commented Aug 26, 2019 •

edited