nativeImage.createFromBuffer crashes on Windows on ARM

[jkleinsc]

Preflight Checklist

• I have read the Contributing Guidelines for this project.
• I agree to follow the Code of Conduct that this project adheres to.
• I have searched the issue tracker for an issue that matches the one I want to file, without success.

Issue Details

This issue was discovered while working on #25018

• Electron Version:
  • 11.0.0-nightly.20200723
• Operating System:
  • Windows on ARM
• Last Known Working Electron version:
  • 11.0.0-nightly.20200721.

Expected Behavior

Calling nativeImage.createFromBuffer with an invalid image or a bitmap image should not crash Electron on Windows on ARM

Actual Behavior

Calling nativeImage.createFromBuffer with an invalid image or a bitmap image crashes Electron on Windows on ARM

To Reproduce

The following example of calling nativeImage.createFromBuffer with a bitmap will crash:

const imageA = nativeImage.createFromPath(path.join(__dirname, 'fixtures', 'assets', 'logo.png'));
const imageC = nativeImage.createFromBuffer(imageA.toBitmap(), { ...imageA.getSize(), scaleFactor: 2.0 });

The following example will also crash when the file passed into nativeImage.createFromPath isn't an image:

const image = nativeImage.createFromPath('/Users/somebody/readme.txt')

webContents.startDrag will also cause this crash if the icon passed in isn't an image:

const w = new BrowserWindow({ show: false });
w.webContents.startDrag({ file: __filename, icon: __filename });

Additional Information

Running test cases through windbg reveals the following stack traces:

nativeImage.createFromBuffer(imageA.toBitmap()....)

webContents.startDrag

Assessment

In both cases a longjmp is failing. Apparently Windows thinks it is invalid for the process, as far as I can understand from this: https://docs.microsoft.com/en-us/windows/win32/devnotes/guardchecklongjumptarget
Here's where setjmp gets called: https://source.chromium.org/chromium/chromium/src/+/master:ui/gfx/codec/jpeg_codec.cc;l=179
and here's where longjmp gets called: https://source.chromium.org/chromium/chromium/src/+/master:ui/gfx/codec/jpeg_codec.cc;l=43

This issue was introduced in the chromium roll: #24575

In that chromium roll libjpeg_turbo was updated: https://chromium.googlesource.com/chromium/deps/libjpeg_turbo/+log/0241a1304fd183ee24fbdfe6891f18fdedea38f9..9d4f8005bc6c888e66b00fd00188531ee9bd3344?n=10000&pretty=fuller (edited)
That update includes an update to TurboJPEG 2.0.5
which includes this change: libjpeg-turbo/libjpeg-turbo@ae87a95 which may be the root of the issue.

[nornagon]

Probably a good idea to file this against libjpeg-turbo: https://github.com/libjpeg-turbo/libjpeg-turbo/issues/new/choose

[jkleinsc]

@nornagon I think it may actually be an implementation issue in https://source.chromium.org/chromium/chromium/src/+/master:ui/gfx/codec/jpeg_codec.cc. Eg see https://github.com/libjpeg-turbo/libjpeg-turbo/blob/master/example.txt#L311

[kaadam]

Hi, @jkleinsc unfortunately I couldn't reproduce this issue on my WoA device Asus NovaGo (winver: Windows 10 Pro, OS Build 19041.450, MSVC 16.7.2). I have checked with Electron v11-0.0-nightly.20200825/20200723 ( Node v12.18.3). It could be I missed something.
Could you share your gn args and your Windows environment where these tests are running on?

Many thanks,
Adam

[jkleinsc]

@kadam our CI Windows environment is a Lenovo Yoga C630 running Windows 10 Home 10.0.19041 Build 19041/10.0.19041.423. As far as GN args are concerned, they are here:
https://github.com/electron/electron/blob/master/build/args/testing.gn - for our testing builds
and
https://github.com/electron/electron/blob/master/build/args/release.gn for our release builds.

We also have a Microsoft Surface Pro X in CI which is running on Windows 10 Enterprise v 10.0.18362.752. I can verify if it also fails there.

[jkleinsc]

@kaadam also tested on the Surface Pro X running on Windows 10 Enterprise v 10.0.18362.752 and I can reproduce the issue there as well.

[kaadam]

@jkleinsc
I managed to reproduce this issue, and it seems it is not related to jpeg_codec implementation. Maybe the problem is how llvm is handling longjmp call for arm64-win. I got the this error:
"Unhandled exception at 0x00007FFC1B783838 (ntdll.dll) in electron.exe: RangeChecks instrumentation code detected an out of range array access." Despite we compile it with /guard:cf,nochecks [1].
I could reproduce it with this small example [2]; compiled/linked it as Chromium does. I got the same crash.

This CL [3] causes the issue, so earlier in the build config '/guard:cf/nolongjmp' was set, which allows the users to link in object
files that call setjmp that weren't compiled with '/guard:cf', but it leaved the longjmp unprotected (there was no check).

I would say we should use this ldflag option only for arm64 as a workaround to avoid this crash until it is fixed in llvm.

[1] https://source.chromium.org/chromium/chromium/src/+/master:build/config/win/BUILD.gn;l=100;bpv=1;bpt=0
[2]

#include <setjmp.h>
#include <stdio.h>
jmp_buf buf;
void g() {
  printf("before longjmp\n");
  fflush(stdout);
  longjmp(buf, 1);
}
void f() {
  if (setjmp(buf)) {
    printf("setjmp returned non-zero\n");
    return;
  }
  g();
}
int main() {
  f();
  printf("hello world\n");
}

[3] https://chromium-review.googlesource.com/c/chromium/src/+/2298284/13/build/config/win/BUILD.gn

[richard-townsend-arm]

LLVM/LLD issue opened: https://bugs.llvm.org/show_bug.cgi?id=47463

[jkleinsc]

@kaadam and @richard-townsend-arm thanks for your help on this. I'll put together a PR to use the proper ldflag option only for arm64.

[jkleinsc]

Also, just to close the loop on this, https://chromium-review.googlesource.com/c/chromium/src/+/2298284/13/build/config/win/BUILD.gn was part of the chromium roll here: #24575 where we first saw this issue.

[nornagon]

@jkleinsc @richard-townsend-arm perhaps shoot a quick email to wfh@chromium.org with a link to the llvm bug?

[richard-townsend-arm]

Done ✔️ I don't imagine there'll be a problem passing /guard:cf,nolongjmp temporarily until the toolchain's fixed.

[richard-townsend-arm]

@jkleinsc : if you implement the workaround, could you reference this bug? https://bugs.chromium.org/p/chromium/issues/detail?id=1126549

Otherwise I'll get started on it tomorrow.

[jkleinsc]

@richard-townsend-arm can you verify that bug link? When I click on it, the issue listed (Issue 112654: Crash on clicking App launcher thrice) doesn't appear to relate to this issue.

[jkleinsc]

Ah think I found it: https://crbug.com/1126549

[richard-townsend-arm]

Ooops, apologies (edited with the fix).

[jkleinsc]

Pull request to use /guard:cf,nolongjmp is here: #25396.