Race condition on array length checks

[alex-h-strachan]

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]

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

[sankalp1999]

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]

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

[tniessen]

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

[syg]

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]

@syg thanks for the detective work there.