Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Renderer crashes on ArrayBuffer->GetBackingStore() when Buffer is too large #31554

Closed
3 tasks done
robinchrist opened this issue Oct 22, 2021 · 7 comments
Closed
3 tasks done
Labels
14-x-y bug 🪲 has-repro-repo Issue can be reproduced by cloning a git repo platform/linux stale

Comments

@robinchrist
Copy link

robinchrist commented Oct 22, 2021

Preflight Checklist

Electron Version

14.1.1

What operating system are you using?

Ubuntu

Operating System Version

Kubuntu 20.04

What arch are you using?

x64

Last Known Working Electron version

N/A

Expected Behavior

ArrayBuffer->GetBackingStore() should not crash the renderer for any buffer size

Actual Behavior

Consider the following snippet in a native addon:

NAN_METHOD(CalculateSync) {
    Nan::HandleScope scope;
    size_t param = 245760;
    v8::Local<v8::ArrayBuffer> fieldDataBuffer = v8::ArrayBuffer::New(v8::Isolate::GetCurrent(), param * sizeof(float));
    v8::Local<v8::Float32Array> fieldDataArr = v8::Float32Array::New(fieldDataBuffer, 0, param);
    
    float* ptr = (float*)(fieldDataBuffer->GetBackingStore()->Data());

    ptr[0] = 1.0;

    info.GetReturnValue().Set(fieldDataArr);
  }

For Electron 9.0.0, if you call this from the JS world for param = 245760 everything is fine.
When you change param to param = 245761 or any bigger value, the renderer process crashes with the following stack trace:

bt
* thread #1, name = 'electron', stop reason = signal SIGILL: illegal instruction operand
  * frame #0: 0x000055d4a39df572 electron`v8::base::OS::Abort() at platform-posix.cc:457:5
    frame #1: 0x000055d4a39d9ad5 electron`::V8_Fatal() at logging.cc:167:3
    frame #2: 0x000055d4a39d96a5 electron`v8::base::(anonymous namespace)::DefaultDcheckHandler(char const*, int, char const*) at logging.cc:57:3
    frame #3: 0x000055d49fc07b20 electron`::Unregister() at backing-store.cc:662:5
    frame #4: 0x000055d49fc074a3 electron`::~BackingStore() at backing-store.cc:166:3
    frame #5: 0x000055d49f81dec7 electron`::__on_zero_shared() [inlined] operator() at memory:2378:5
    frame #6: 0x000055d49f81deba electron`::__on_zero_shared() at memory:3551:5
    frame #7: 0x00007f8a82b50fd5 addon.node`std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release(this=0x0000197ae65b07c0) at shared_ptr_base.h:155:6
    frame #8: 0x00007f8a82b50f8a addon.node`std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count(this=0x00007ffc5baea030) at shared_ptr_base.h:730:11
frame #9: 0x00007f8a82b50f59 addon.node`std::__shared_ptr<v8::BackingStore, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr(this=0x00007ffc5baea028) at shared_ptr_base.h:1169:31
    frame #10: 0x00007f8a82b50e25 addon.node`std::shared_ptr<v8::BackingStore>::~shared_ptr(this=0x00007ffc5baea028) at shared_ptr.h:103:11
    frame #11: 0x00007f8a82b50c83 addon.node`CVO::node::CalculateSync(info=0x00007ffc5baea0c8) at sync.cc:25:18
    frame #12: 0x00007f8a82b4fba9 addon.node`Nan::imp::FunctionCallbackWrapper(info=0x00007ffc5baea450) at nan_callbacks_12_inl.h:176:3
    frame #13: 0x000055d49f8878d7 electron`::Call() at api-arguments-inl.h:158:3
    frame #14: 0x000055d49f886094 electron`::HandleApiCallHelper<false>() at builtins-api.cc:111:36
    frame #15: 0x000055d49f884227 electron`::Builtin_Impl_HandleApiCall() at builtins-api.cc:141:5
    frame #16: 0x000055d49f883d0a electron`::Builtin_HandleApiCall() at builtins-api.cc:129:1
    frame #17: 0x000055d4a07e349f electron`Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit + 63
    frame #18: 0x000055d4a05c5bb5 electron`Builtins_InterpreterEntryTrampoline + 213
    frame #19: 0x000055d4a05bc65a electron`Builtins_JSEntryTrampoline + 90
    frame #20: 0x000055d4a05bc438 electron`Builtins_JSEntry + 120
    frame #21: 0x000055d49f9b1f4c electron`::Invoke() [inlined] Call at simulator.h:142:12
    frame #22: 0x000055d49f9b1f41 electron`::Invoke() at execution.cc:367:33
    frame #23: 0x000055d49f9b0d68 electron`::Call() at execution.cc:461:10
    frame #24: 0x000055d49f95ab80 electron`::Global() at debug-evaluate.cc:54:32
    frame #25: 0x000055d49f811286 electron`::EvaluateGlobal() at api.cc:10119:7
    frame #26: 0x000055d4a026ae09 electron`::evaluate() at v8-runtime-agent-impl.cc:288:24
    frame #27: 0x000055d4a02206b3 electron`::evaluate() at Runtime.cpp:2084:16
    frame #28: 0x000055d4a0201b28 electron`::__call_impl<std::__1::__function::__default_alloc_func<(lambda at gen/v8/src/inspector/protocol/Console.cpp:235:10), void (const v8_crdtp::Dispatchable &)>>() [inlined] operator() at Console.cpp:241:5
    frame #29: 0x000055d4a0201ab8 electron`::__call_impl<std::__1::__function::__default_alloc_func<(lambda at gen/v8/src/inspector/protocol/Console.cpp:235:10), void (const v8_crdtp::Dispatchable &)>>() [inlined] __invoke<(lambda at gen/v8/src/inspector/protocol/Console.cpp:235:10) &, const v8_crdtp::Dispatchable &> at type_traits:3529:1
    frame #30: 0x000055d4a0201ab8 electron`::__call_impl<std::__1::__function::__default_alloc_func<(lambda at gen/v8/src/inspector/protocol/Console.cpp:235:10), void (const v8_crdtp::Dispatchable &)>>() [inlined] __call<(lambda at gen/v8/src/inspector/protocol/Console.cpp:235:10) &, const v8_crdtp::Dispatchable &> at __functional_base:348:9
    frame #31: 0x000055d4a0201ab8 electron`::__call_impl<std::__1::__function::__default_alloc_func<(lambda at gen/v8/src/inspector/protocol/Console.cpp:235:10), void (const v8_crdtp::Dispatchable &)>>() [inlined] operator() at functional:1590:12
    frame #32: 0x000055d4a0201ab8 electron`::__call_impl<std::__1::__function::__default_alloc_func<(lambda at gen/v8/src/inspector/protocol/Console.cpp:235:10), void (const v8_crdtp::Dispatchable &)>>() at functional:2071:16
    frame #33: 0x000055d4a0293d83 electron`::Run() [inlined] operator() at functional:2203:16
    frame #34: 0x000055d4a0293d7d electron`::Run() [inlined] operator() at functional:2473:12
    frame #35: 0x000055d4a0293d7d electron`::Run() at dispatch.cc:501:3
    frame #36: 0x000055d4a0263ca2 electron`::dispatchProtocolMessage() at v8-inspector-session-impl.cc:378:39
    frame #37: 0x000055d4a470abb8 electron`::DispatchProtocolCommandImpl() at devtools_session.cc:223:18
    frame #38: 0x000055d49f6ae459 electron`::Accept() at devtools_agent.mojom-blink.cc:835:13
    frame #39: 0x000055d4a21124af electron`::HandleValidatedMessage() at interface_endpoint_client.cc:554:54
    frame #40: 0x000055d4a2118fcb electron`::Accept() at message_dispatcher.cc:41:19
    frame #41: 0x000055d4a2113c95 electron`::HandleIncomingMessage() at interface_endpoint_client.cc:356:22
    frame #42: 0x000055d4a2444f1f electron`::AcceptOnProxyThread() at ipc_mojo_bootstrap.cc:933:24
    frame #43: 0x000055d4a244193c electron`::RunOnce() [inlined] Invoke<void (IPC::(anonymous namespace)::ChannelAssociatedGroupController::*)(mojo::Message), scoped_refptr<IPC::(anonymous namespace)::ChannelAssociatedGroupController>, mojo::Message> at bind_internal.h:489:12
    frame #44: 0x000055d4a2441913 electron`::RunOnce() [inlined] MakeItSo<void (IPC::(anonymous namespace)::ChannelAssociatedGroupController::*)(mojo::Message), scoped_refptr<IPC::(anonymous namespace)::ChannelAssociatedGroupController>, mojo::Message> at bind_internal.h:623:12
    frame #45: 0x000055d4a2441913 electron`::RunOnce() [inlined] RunImpl<void (IPC::(anonymous namespace)::ChannelAssociatedGroupController::*)(mojo::Message), std::__1::tuple<scoped_refptr<IPC::(anonymous namespace)::ChannelAssociatedGroupController>, mojo::Message>, 0, 1> at bind_internal.h:696:12
    frame #46: 0x000055d4a24418c9 electron`::RunOnce() at bind_internal.h:665:12
    frame #47: 0x000055d4a1dd3842 electron`::RunTask() [inlined] Run at callback.h:98:12
    frame #48: 0x000055d4a1dd37e2 electron`::RunTask() at task_annotator.cc:142:33
    frame #49: 0x000055d4a1df0124 electron`::DoWorkImpl() at thread_controller_with_message_pump_impl.cc:324:23
    frame #50: 0x000055d4a1defdf2 electron`::DoWork() at thread_controller_with_message_pump_impl.cc:248:7
    frame #51: 0x000055d4a1d94064 electron`::Run() at message_pump_default.cc:39:55
    frame #52: 0x000055d4a1df08d7 electron`::Run() at thread_controller_with_message_pump_impl.cc:429:12
    frame #53: 0x000055d4a1db7d22 electron`::Run() at run_loop.cc:124:14
    frame #54: 0x000055d4a6340d65 electron`::RendererMain() at renderer_main.cc:226:16
    frame #55: 0x000055d4a0d5e9cb electron`::Run() at content_main_runner_impl.cc:882:10
    frame #56: 0x000055d4a3846df6 electron`::Main() at main.cc:454:29
    frame #57: 0x000055d49f622871 electron`content::ContentMain(content::ContentMainParams const&) at content_main.cc:19:10
    frame #58: 0x000055d49e51480f electron`main at electron_main.cc:231:10
    frame #59: 0x00007f8a87a8b0b3 libc.so.6`__libc_start_main + 243
    frame #60: 0x000055d49e51452a electron`_start + 42

i.e. param <= 245760 works, but param >= 245761 crashes the renderer.

For Electron 14, the magic number is 229376 (works for param <= 229376, crashes for param >= 229377)

I have tested the following numbers for Electron 9.0.0:

9 works
134217728 crash
67108868 crash
33554438 crash
100000 works
16827219 crash
8463609 crash
528975 crash
264487 crash
182243 works
223365 works
202804 works
243926 works
254206 crash
249066 crash
246496 crash
245211 works
245853 crash
245532 works
245692 works
245772 crash
245732 works
245752 works
245762 crash
245757 works

245760 works
245761 crash

For Electron 14.1.1.:

100000 works
245760 crashes
172880 works
209320 works
227540 works
236650 crashes
232095 crashes
229817 crashes
228678 works
229247 works
229532 crashes
229389 crashes
229318 works
229353 works
229371 works
229380 crashes
229375 works
229377 crashes
229376 works

If you remove the Float32Array and only return the Buffer, everything seems to work fine.

I don't have a stack trace for Electron 14.1.1 at hand... Debug version is still compiling

Repro Repos:
https://github.com/robinchrist/electron-native-arraybuffer-crash-addon
https://github.com/robinchrist/electron-native-arraybuffer-crash-electron

Repro guide:

git clone git@github.com:robinchrist/electron-native-arraybuffer-crash-addon.git
git clone git@github.com:robinchrist/electron-native-arraybuffer-crash-electron.git

cd electron-native-arraybuffer-crash-addon
git checkout electron-14.1.1
npm ci
npm run testpackage

cd ../electron-native-arraybuffer-crash-electron
npm install ../electron-native-arraybuffer-crash-addon/corelib-test-0.0.1-electron1411.tgz
npm i
npm start

in devtools:

addon = window.require('corelib-test')
addon.NATIVE.calculateSync()

-> Renderer crash

change parameter to size_t param = 229376

cd ../electron-native-arraybuffer-crash-addon
npm run testpackage
cd ../electron-native-arraybuffer-crash-electron
npm install ../electron-native-arraybuffer-crash-addon/corelib-test-0.0.1-electron1411.tgz
npm start

in devtools:

addon = window.require('corelib-test')
addon.NATIVE.calculateSync()

-> WORKS

addon.NATIVE.calculateSync()
Float32Array(229376) [1, 0, NaN, NaN, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, …]

OS: Kubuntu 20.04
Compiler:

Ubuntu clang version 12.0.1-++20211011094644+fed41342a82f-1~exp1~20211011215105.3
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Testcase Gist URL

No response

Additional Information

No response

@robinchrist
Copy link
Author

robinchrist commented Oct 25, 2021

Update:
This also happens when building the module with node-gyp (so it's not a cmake-ts problem again / build settings related problem)

i.e.
sync.cc

NAN_METHOD(CalculateSync) {
    Nan::HandleScope scope;
    int32_t len = Nan::To<int32_t>(info[0]).FromJust();
    v8::Local<v8::ArrayBuffer> fieldDataBuffer = v8::ArrayBuffer::New(v8::Isolate::GetCurrent(), len * sizeof(float));
    v8::Local<v8::Float32Array> fieldDataArr = v8::Float32Array::New(fieldDataBuffer, 0, len);

    float* ptr = (float*)(fieldDataBuffer->GetBackingStore()->Data());

    ptr[0] = 1.0;

    info.GetReturnValue().Set(fieldDataArr);
  }

binding.gyp:

{
  "targets": [
    {
      "target_name": "addon",
      "sources": [ "src/addon.cc", "src/sync.cc"],
      "include_dirs": [
          "./node_modules/nan"
      ],
    }
  ]
}

HOME=~/.electron-gyp npx node-gyp rebuild --target=14.1.1 --arch=x64 --dist-url=https://electronjs.org/headers

var addon = window.require('your/absolute/path/addon.node')

addon.calculateSync(229376)

Float32Array(229376) [1, 0, NaN, NaN, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, …]

addon.calculateSync(229377)

DevTools was disconnected from the page

Happens on both g++ 9.3.0 and clang++ 12.0.1-++20211011094644+fed41342a82f-1~exp1~20211011215105.3

@robinchrist
Copy link
Author

robinchrist commented Oct 25, 2021

Update:
Only in the release version does the renderer crash depend on the array size
For the debug version, the renderer crash happens regardless of the array size

The bug does not seem to occur with a standard node.js

@robinchrist
Copy link
Author

Related?

nodejs/node#32463

@robinchrist
Copy link
Author

Current status:

sync.cc

 NAN_METHOD(CalculateSync) {

    Nan::HandleScope scope;
    int32_t len = Nan::To<int32_t>(info[0]).FromJust();
    v8::Local<v8::ArrayBuffer> fieldDataBuffer = v8::ArrayBuffer::New(v8::Isolate::GetCurrent(), len * sizeof(float));

    float* ptr = (float*)(fieldDataBuffer->GetBackingStore()->Data());

    ptr[0] = 1.0;

    info.GetReturnValue().Set(fieldDataBuffer);
  }

build with node-gyp
Electron 9.0.0 debug: Renderer crash for all parameters (Fatal error in ../../v8/src/objects/backing-store.cc, line 662 Debug check failed: !result->second.lock().)
Electron 9.0.0 release: Renderer crash for >= 245761
Electron 14.1.1 release: Renderer crash for >= 212523
Electron 15.2.0 release: Renderer crash for >=215613
Electron 16.0.0-alpha8 release: Renderer crash for >=201111

@robinchrist robinchrist changed the title [Bug]: Renderer crashes on ArrayBuffer->GetBackingStore() when Buffer is too large and TypedArray references it [Bug]: Renderer crashes on ArrayBuffer->GetBackingStore() when Buffer is too large Oct 25, 2021
@robinchrist
Copy link
Author

CC @miniak

@ckerr ckerr added 14-x-y platform/linux has-repro-gist Issue can be reproduced with code at https://gist.github.com/ has-repro-repo Issue can be reproduced by cloning a git repo and removed has-repro-gist Issue can be reproduced with code at https://gist.github.com/ labels Nov 10, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2022

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Oct 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2022

This issue has been closed due to inactivity, and will not be monitored. If this is a bug and you can reproduce this issue on a supported version of Electron please open a new issue and include instructions for reproducing the issue.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
14-x-y bug 🪲 has-repro-repo Issue can be reproduced by cloning a git repo platform/linux stale
Projects
None yet
Development

No branches or pull requests

2 participants