Skip to content

Commit 96fde58

Browse files
legendecastargos
legendecas
and
targos
committed
deps: V8: backport c24df23717b6
Original commit message: [execution] Respect isolate stack limit in GetCentralStackView A stack limit can be set for each v8::Isolate. The limit size can be greater than the one specified with --stack-size. `Heap::CollectGarbage` should not crash due to a `CHECK` on `Isolate::IsOnCentralStack()` with an isolate stack limit. Refs: #57114 Fixed: 400996806 Bug: 42202153 Change-Id: I80d0826fcd6a64261b8d745f8f47aa096bc83fb8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6329659 Commit-Queue: Chengzhong Wu <[email protected]> Reviewed-by: Jakob Kummerow <[email protected]> Cr-Commit-Position: refs/heads/main@{#99228} Refs: v8/v8@c24df23 Co-authored-by: Michaël Zasso <[email protected]>
1 parent f82ad83 commit 96fde58

6 files changed

Lines changed: 93 additions & 6 deletions

File tree

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838

3939
# Reset this number to 0 on major V8 upgrades.
4040
# Increment by one for each non-official patch applied to deps/v8.
41-
'v8_embedder_string': '-node.10',
41+
'v8_embedder_string': '-node.11',
4242

4343
##### V8 defaults for Node.js #####
4444

‎deps/v8/src/api/api.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10154,6 +10154,7 @@ void Isolate::Initialize(Isolate* v8_isolate,
1015410154
uintptr_t limit =
1015510155
reinterpret_cast<uintptr_t>(params.constraints.stack_limit());
1015610156
i_isolate->stack_guard()->SetStackLimit(limit);
10157+
i_isolate->set_stack_size(base::Stack::GetStackStart() - limit);
1015710158
}
1015810159

1015910160
// TODO(v8:2487): Once we got rid of Isolate::Current(), we can remove this.
@@ -10800,6 +10801,7 @@ void Isolate::SetStackLimit(uintptr_t stack_limit) {
1080010801
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(this);
1080110802
CHECK(stack_limit);
1080210803
i_isolate->stack_guard()->SetStackLimit(stack_limit);
10804+
i_isolate->set_stack_size(base::Stack::GetStackStart() - stack_limit);
1080310805
}
1080410806

1080510807
void Isolate::GetCodeRange(void** start, size_t* length_in_bytes) {

‎deps/v8/src/execution/isolate.cc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4135,14 +4135,13 @@ Isolate::Isolate(IsolateGroup* isolate_group)
41354135
next_unique_sfi_id_(0),
41364136
next_module_async_evaluation_ordinal_(
41374137
SourceTextModule::kFirstAsyncEvaluationOrdinal),
4138-
cancelable_task_manager_(new CancelableTaskManager())
4138+
cancelable_task_manager_(new CancelableTaskManager()),
41394139
#if defined(V8_ENABLE_ETW_STACK_WALKING)
4140-
,
41414140
etw_tracing_enabled_(false),
41424141
etw_trace_interpreted_frames_(v8_flags.interpreted_frames_native_stack),
4143-
etw_in_rundown_(false)
4142+
etw_in_rundown_(false),
41444143
#endif // V8_ENABLE_ETW_STACK_WALKING
4145-
{
4144+
stack_size_(v8_flags.stack_size * KB) {
41464145
TRACE_ISOLATE(constructor);
41474146
CheckIsolateLayout();
41484147

‎deps/v8/src/execution/isolate.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1739,6 +1739,9 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
17391739

17401740
bool jitless() const { return jitless_; }
17411741

1742+
void set_stack_size(size_t v) { stack_size_ = v; }
1743+
size_t stack_size() { return stack_size_; }
1744+
17421745
base::RandomNumberGenerator* random_number_generator();
17431746

17441747
base::RandomNumberGenerator* fuzzer_rng();
@@ -2879,6 +2882,9 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
28792882
// The mutex only guards adding pages, the retrieval is signal safe.
28802883
base::SpinningMutex code_pages_mutex_;
28812884

2885+
// Stack size set with ResourceConstraints or Isolate::SetStackLimit, in
2886+
// bytes. This is initialized with value of --stack-size.
2887+
size_t stack_size_;
28822888
#ifdef V8_ENABLE_WEBASSEMBLY
28832889
wasm::WasmCodeLookupCache* wasm_code_look_up_cache_ = nullptr;
28842890
std::vector<std::unique_ptr<wasm::StackMemory>> wasm_stacks_;

‎deps/v8/src/execution/simulator.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ class SimulatorStack : public v8::internal::AllStatic {
9797
v8::internal::Isolate* isolate) {
9898
uintptr_t upper_bound = base::Stack::GetStackStart();
9999
size_t size =
100-
v8_flags.stack_size * KB + wasm::StackMemory::kJSLimitOffsetKB * KB;
100+
isolate->stack_size() + wasm::StackMemory::kJSLimitOffsetKB * KB;
101101
uintptr_t lower_bound = upper_bound - size;
102102
return base::VectorOf(reinterpret_cast<uint8_t*>(lower_bound), size);
103103
}

‎deps/v8/test/cctest/test-api.cc‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17173,6 +17173,86 @@ TEST(SetStackLimitInThread) {
1717317173
}
1717417174
}
1717517175

17176+
static bool TestStackOverflow(v8::Isolate* isolate) {
17177+
v8::Isolate::Scope isolate_scope(isolate);
17178+
v8::HandleScope scope(isolate);
17179+
LocalContext context(isolate);
17180+
v8::TryCatch try_catch(isolate);
17181+
const char* code =
17182+
"var errored = false;"
17183+
"function fn(...args) {"
17184+
" try { fn(...args); }"
17185+
" catch (e) {"
17186+
// Only trigger GC once the stack is full to speedup the test.
17187+
" if (!errored) {"
17188+
" gc();"
17189+
" errored = true;"
17190+
" }"
17191+
" throw e;"
17192+
" }"
17193+
"}"
17194+
"try {"
17195+
" fn.apply(null, new Array(10).fill(1).map(() => {}));"
17196+
" false;"
17197+
"} catch (e) {"
17198+
" e.name === 'RangeError'" // StackOverflow is a RangeError
17199+
"}";
17200+
Local<Value> value = CompileRun(code);
17201+
17202+
// A StackOverflow error is thrown, without crashing.
17203+
return value->IsTrue();
17204+
}
17205+
17206+
class StackOverflowThread : public v8::base::Thread {
17207+
public:
17208+
explicit StackOverflowThread(int stack_size, int js_stack_size)
17209+
: Thread(Options("StackOverflowThread", stack_size)),
17210+
js_stack_size_(js_stack_size),
17211+
result_(false) {}
17212+
17213+
void Run() override {
17214+
uintptr_t stack_top = v8::base::Stack::GetStackStart();
17215+
// Compute isolate stack limit by js stack size.
17216+
uintptr_t stack_base = stack_top - js_stack_size_;
17217+
v8::Isolate::CreateParams create_params = CreateTestParams();
17218+
v8::Isolate* isolate = v8::Isolate::New(create_params);
17219+
isolate->SetStackLimit(stack_base);
17220+
result_ = TestStackOverflow(isolate);
17221+
isolate->Dispose();
17222+
}
17223+
17224+
int result() { return result_; }
17225+
17226+
private:
17227+
int js_stack_size_;
17228+
bool result_;
17229+
};
17230+
17231+
TEST(SetStackLimitInThreadAndStackOverflow) {
17232+
// Set a small --stack-size flag.
17233+
i::FlagScope<int> f_stack_size(&i::v8_flags.stack_size, 100);
17234+
// Trigger GC aggressively to verify that GC does not crash with stack litmit.
17235+
i::FlagScope<size_t> f_heap_size(&i::v8_flags.max_heap_size, 8);
17236+
i::FlagScope<bool> f_expose_gc(&i::v8_flags.expose_gc, true);
17237+
17238+
// ASAN requires more stack space.
17239+
#ifdef V8_USE_ADDRESS_SANITIZER
17240+
constexpr int stack_size = 32 * v8::internal::MB;
17241+
constexpr int js_stack_size = 2 * v8::internal::MB;
17242+
#else
17243+
constexpr int stack_size = 2 * v8::internal::MB;
17244+
constexpr int js_stack_size = 1 * v8::internal::MB;
17245+
#endif
17246+
// Spawn an Isolate on a thread with larger stack limits than --stack-size.
17247+
StackOverflowThread thread1(stack_size, js_stack_size);
17248+
17249+
CHECK(thread1.Start());
17250+
17251+
thread1.Join();
17252+
17253+
CHECK(thread1.result());
17254+
}
17255+
1717617256
THREADED_TEST(GetHeapStatistics) {
1717717257
LocalContext c1;
1717817258
v8::HandleScope scope(c1->GetIsolate());

0 commit comments

Comments
 (0)