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

Race condition on array length checks #47928

Open
alex-h-strachan opened this issue May 9, 2023 · 10 comments
Open

Race condition on array length checks #47928

alex-h-strachan opened this issue May 9, 2023 · 10 comments
Labels
v8 engine Issues and PRs related to the V8 dependency. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch.

Comments

@alex-h-strachan
Copy link

alex-h-strachan commented May 9, 2023

Version

v18.16.0

Platform

Darwin Alexs-MacBook-Pro-2.local 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar 6 20:59:28 PST 2023; root:xnu-8796.101.5~3/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

node -e "const x = []; for(i = 0; i < 112813859; i++){ x[i] = false };"

will result in a core dump

however:

node -e "const x = []; for(i = 0; i < 112813858; i++){ x[i] = false }; for(i = 0; i < 112813859; i++){ x[i] = false };"

correctly produces RangeError: Invalid array length

It seems initializing a large but legal array causes the checker to prime itself and guard against the next, illegal call.

Core dump log from the first command:

#
# Fatal error in , line 0
# Fatal JavaScript invalid size error 169220804
#
#
#
#FailureMessage Object: 0x16f3893f8
 1: 0x100b894c4 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
 2: 0x101ac5ee8 V8_Fatal(char const*, ...) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
 3: 0x100dfa908 v8::internal::FactoryBase<v8::internal::Factory>::NewFixedArrayWithFiller(v8::internal::Handle<v8::internal::Map>, int, v8::internal::Handle<v8::internal::Oddball>, v8::internal::AllocationType) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
 4: 0x100f86dec v8::internal::(anonymous namespace)::ElementsAccessorBase<v8::internal::(anonymous namespace)::FastPackedObjectElementsAccessor, v8::internal::(anonymous namespace)::ElementsKindTraits<(v8::internal::ElementsKind)2>>::GrowCapacity(v8::internal::Handle<v8::internal::JSObject>, unsigned int) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
 5: 0x101186cd4 v8::internal::Runtime_GrowArrayElements(int, unsigned long*, v8::internal::Isolate*) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
 6: 0x1014dd04c Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
 7: 0x105ed385c
 8: 0x1014664d0 Builtins_JSEntryTrampoline [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
 9: 0x101466164 Builtins_JSEntry [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
10: 0x100dab85c v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
11: 0x100daba30 v8::internal::Execution::CallScript(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
12: 0x100c870f8 v8::Script::Run(v8::Local<v8::Context>, v8::Local<v8::Data>) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
13: 0x100b1e510 node::contextify::ContextifyScript::EvalMachine(v8::Local<v8::Context>, node::Environment*, long long, bool, bool, bool, std::__1::shared_ptr<v8::MicrotaskQueue>, v8::FunctionCallbackInfo<v8::Value> const&) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
14: 0x100b1dea0 node::contextify::ContextifyScript::RunInContext(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
15: 0x100cefa1c v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
16: 0x100cef518 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
17: 0x100ceed44 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
18: 0x1014dd18c Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
19: 0x101468198 Builtins_InterpreterEntryTrampoline [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
20: 0x101468198 Builtins_InterpreterEntryTrampoline [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
21: 0x101468198 Builtins_InterpreterEntryTrampoline [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
22: 0x101468198 Builtins_InterpreterEntryTrampoline [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
23: 0x101468198 Builtins_InterpreterEntryTrampoline [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
24: 0x101468198 Builtins_InterpreterEntryTrampoline [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
25: 0x1014664d0 Builtins_JSEntryTrampoline [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
26: 0x101466164 Builtins_JSEntry [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
27: 0x100dab85c v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
28: 0x100daad90 v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
29: 0x100c9b124 v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
30: 0x100b96290 node::Realm::ExecuteBootstrapper(char const*, std::__1::vector<v8::Local<v8::Value>, std::__1::allocator<v8::Local<v8::Value>>>*) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
31: 0x100af32a8 node::StartExecution(node::Environment*, char const*) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
32: 0x100af31f0 node::StartExecution(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
33: 0x100a7c698 node::LoadEnvironment(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
34: 0x100b668bc node::NodeMainInstance::Run() [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
35: 0x100af6028 node::LoadSnapshotDataAndRun(node::SnapshotData const**, node::InitializationResult const*) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
36: 0x100af62b4 node::Start(int, char**) [/Users/alexstrachan/.nvm/versions/node/v18.16.0/bin/node]
37: 0x182e3bf28 start [/usr/lib/dyld]
zsh: trace trap  node -e "const x = []; for(i = 0; i < 112813859; i++){ x[i] = false };"

How often does it reproduce? Is there a required condition?

100% reproducible, even adjusting --max_old_space_size

What is the expected behavior? Why is that the expected behavior?

I expect the userland RangeError: Invalid array length to be raised any time an illegal array is constructed rather than a core dump.

I certainly don't expect a different behavior depending on if an almost-too-large array was constructed immediately before.

What do you see instead?

Core dump process explosion rather than an error.

Additional information

No response

@mscdex
Copy link
Contributor

mscdex commented May 9, 2023

Can you reproduce it with node v20.x or the main branch?

@sankalp1999
Copy link
Contributor

sankalp1999 commented May 9, 2023

Platform: Darwin [redacted username]-macOS 21.6.0 Darwin Kernel Version 21.6.0: Sat Jun 18 17:07:22 PDT 2022; root:xnu-8020.140.41~1/RELEASE_ARM64_T6000 arm64

Node version: v21.0.0-pre (main branch)

Core dump message below. Able to reproduce.

out/Release/node -e "const x = []; for(i = 0; i < 112813859; i++){ x[i] = false };"


#
# Fatal error in , line 0
# Fatal JavaScript invalid size error 169220804 (see crbug.com/1201626)
#
#
#
#FailureMessage Object: 0x16bdf16e8
 1: 0x1041627a0 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() [[redacted username]node/out/Release/node]
 2: 0x10520b950 V8_Fatal(char const*, ...) [[redacted username]node/out/Release/node]
 3: 0x104429720 v8::internal::FactoryBase<v8::internal::Factory>::NewFixedArrayWithFiller(v8::internal::Handle<v8::internal::Map>, int, v8::internal::Handle<v8::internal::Oddball>, v8::internal::AllocationType) [[redacted username]node/out/Release/node]
 4: 0x1045d2328 v8::internal::(anonymous namespace)::ElementsAccessorBase<v8::internal::(anonymous namespace)::FastPackedObjectElementsAccessor, v8::internal::(anonymous namespace)::ElementsKindTraits<(v8::internal::ElementsKind)2> >::GrowCapacity(v8::internal::Handle<v8::internal::JSObject>, unsigned int) [[redacted username]node/out/Release/node]
 5: 0x104823ad8 v8::internal::Runtime_GrowArrayElements(int, unsigned long*, v8::internal::Isolate*) [[redacted username]node/out/Release/node]
 6: 0x104b80c44 Builtins_CEntry_Return1_ArgvOnStack_NoBuiltinExit [[redacted username]node/out/Release/node]
 7: 0x109f21bfc
 8: 0x104af650c Builtins_JSEntryTrampoline [[redacted username]node/out/Release/node]
 9: 0x104af61f4 Builtins_JSEntry [[redacted username]node/out/Release/node]
10: 0x1043cf818 v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [[redacted username]node/out/Release/node]
11: 0x1043cfe2c v8::internal::Execution::CallScript(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>) [[redacted username]node/out/Release/node]
12: 0x10428c330 v8::Script::Run(v8::Local<v8::Context>, v8::Local<v8::Data>) [[redacted username]node/out/Release/node]
13: 0x1040f4764 node::contextify::ContextifyScript::EvalMachine(v8::Local<v8::Context>, node::Environment*, long long, bool, bool, bool, std::__1::shared_ptr<v8::MicrotaskQueue>, v8::FunctionCallbackInfo<v8::Value> const&) [[redacted username]node/out/Release/node]
14: 0x1040f40b4 node::contextify::ContextifyScript::RunInContext(v8::FunctionCallbackInfo<v8::Value> const&) [[redacted username]node/out/Release/node]
15: 0x1042f4150 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, unsigned long*, int) [[redacted username]node/out/Release/node]
16: 0x1042f39a4 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [[redacted username]node/out/Release/node]
17: 0x104b80b24 Builtins_CEntry_Return1_ArgvOnStack_BuiltinExit [[redacted username]node/out/Release/node]
18: 0x104af83e4 Builtins_InterpreterEntryTrampoline [[redacted username]node/out/Release/node]
19: 0x104af83e4 Builtins_InterpreterEntryTrampoline [[redacted username]node/out/Release/node]
20: 0x104af83e4 Builtins_InterpreterEntryTrampoline [[redacted username]node/out/Release/node]
21: 0x104af83e4 Builtins_InterpreterEntryTrampoline [[redacted username]node/out/Release/node]
22: 0x104af83e4 Builtins_InterpreterEntryTrampoline [[redacted username]node/out/Release/node]
23: 0x104af83e4 Builtins_InterpreterEntryTrampoline [[redacted username]node/out/Release/node]
24: 0x104af83e4 Builtins_InterpreterEntryTrampoline [[redacted username]node/out/Release/node]
25: 0x104af650c Builtins_JSEntryTrampoline [[redacted username]node/out/Release/node]
26: 0x104af61f4 Builtins_JSEntry [[redacted username]node/out/Release/node]
27: 0x1043cf818 v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [[redacted username]node/out/Release/node]
28: 0x1043cf094 v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) [[redacted username]node/out/Release/node]
29: 0x1042a0548 v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) [[redacted username]node/out/Release/node]
30: 0x1040e4ba4 node::builtins::BuiltinLoader::CompileAndCall(v8::Local<v8::Context>, char const*, node::Realm*) [[redacted username]node/out/Release/node]
31: 0x10416f5fc node::Realm::ExecuteBootstrapper(char const*) [[redacted username]node/out/Release/node]
32: 0x1040c971c node::StartExecution(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [[redacted username]node/out/Release/node]
33: 0x10403a2b8 node::LoadEnvironment(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [[redacted username]node/out/Release/node]
34: 0x10413e2d8 node::NodeMainInstance::Run() [[redacted username]node/out/Release/node]
35: 0x1040cbb3c node::LoadSnapshotDataAndRun(node::SnapshotData const**, node::InitializationResultImpl const*) [[redacted username]node/out/Release/node]
36: 0x1040cbe40 node::Start(int, char**) [[redacted username]node/out/Release/node]
37: 0x109e4508c
[1]    50658 trace trap  out/Release/node -e

@sankalp1999
Copy link
Contributor

sankalp1999 commented May 9, 2023

same result with --max-old-space-size=4096 and 8192.

@tniessen tniessen added the v8 engine Issues and PRs related to the V8 dependency. label May 9, 2023
@tniessen
Copy link
Member

tniessen commented May 9, 2023

I can reproduce the issue. @alex-h-strachan what makes you think this is due to a race condition, i.e., concurrent operations?

@syg
Copy link
Contributor

syg commented May 9, 2023

I took a look. It's an unfortunate issue of error behavior divergence due to optimized code.

tl;dr This is https://bugs.chromium.org/p/chromium/issues/detail?id=1201626, in particular this TODO

In the first snippet, const x = []; for(i = 0; i < 112813859; i++){ x[i] = false };, this is a hot loop so it tiers up to the optimizing JIT. The optimizing JIT compiles in a GrowFastElements opcode for the property assignment. Currently in optimized code, throwing exceptions from code generated by GrowFastElements isn't supported. Specifically, the optimized code doesn't keep a NativeContext around for that opcode. Without a NativeContext, the code can't find the right RangeError constructor to use, and FATALs on an invalid array length.

In the second snippet, const x = []; for(i = 0; i < 112813858; i++){ x[i] = false }; for(i = 0; i < 112813859; i++){ x[i] = false };, this is also a hot loop and also tiers up to the optimizing JIT. The first loop is compiled the same as the first snippet. The difference is that every call to GrowFastElements succeeds. AFAICT the second loop is compiled differently, because for i from 0 to 112813857, the second loop doesn't actually need to grow the array, and so compiles the second loop as an in-bounds write. When i becomes 112813858, the write becomes out-of-bounds and we actually deopt. This deopt tiers down to a level that does keep a NativeContext reference around, and so throws the RangeError.

I'll cc in compiler folks in chromium:1201626.

@alex-h-strachan
Copy link
Author

@syg thanks for the detective work there.

@RedYetiDev
Copy link
Member

RedYetiDev commented Aug 8, 2024

$ node -e "const x = []; for(i = 0; i < 112813859; i++){ x[i] = false };"
[eval]:1
const x = []; for(i = 0; i < 112813859; i++){ x[i] = false };
                                                   ^

RangeError: Invalid array length
    at [eval]:1:52
    at runScriptInThisContext (node:internal/vm:209:10)
    at node:internal/process/execution:118:14
    at [eval]-wrapper:6:24
    at runScript (node:internal/process/execution:101:62)
    at evalScript (node:internal/process/execution:136:3)
    at node:internal/main/eval_string:55:3

Node.js v22.6.0

This appears to no longer cause a core dump. I've optimistically closed this issue, but feel free to reopen if you disagree.

@kapilgorve
Copy link

I encountered this issue on v20.

C:\Users\kapil>node -v
v20.5.1

C:\Users\kapil>node -e "const x = []; for(i = 0; i < 112813859; i++){ x[i] = false };"


#
# Fatal error in , line 0
# Fatal JavaScript invalid size error 169220804 (see crbug.com/1201626)
#
#
#
#FailureMessage Object: 0000001F62BFD440
 1: 00007FF6BA29BCDF node_api_throw_syntax_error+203199
 2: 00007FF6BA1A2C2F node::TriggerNodeReport+72815
 3: 00007FF6BB069442 V8_Fatal+162
 4: 00007FF6BAAFD525 v8::Platform::SystemClockTimeMillis+856133
 5: 00007FF6BA97E5A3 v8::base::Thread::StartSynchronously+1456403
 6: 00007FF6BA99CF43 v8::Message::GetIsolate+15459
 7: 00007FF6BA7BEDE3 v8::CodeEvent::GetFunctionName+181699
 8: 00007FF65ACDAAFA

@RedYetiDev RedYetiDev added the v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. label Aug 26, 2024
@RedYetiDev RedYetiDev reopened this Aug 26, 2024
@RedYetiDev
Copy link
Member

I haven't confirmed the reproduction, but I've reopened this issue with the appropriate labels.

@duvallj
Copy link

duvallj commented Sep 9, 2024

Confirmed reproduction on Darwin Kernel Version 23.6.0: Mon Jul 29 21:13:00 PDT 2024; root:xnu-10063.141.2~1/RELEASE_X86_64 x86_64 running Node v20.16.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch.
Projects
None yet
Development

No branches or pull requests

8 participants