From 17e6031bf0d4014111f388592f3013695580f0b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Fri, 11 Nov 2022 08:20:17 +0100 Subject: [PATCH] deps: V8: cherry-pick 031b98b25cba MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [runtime] Clear array join stack when throwing uncatchable ... exception. Array#join depends array_join_stack to avoid infinite loop and ensures symmetric pushes/pops through catch blocks to correctly maintain the elements in the join stack. However, the stack does not pop the elements and leaves in an invalid state when throwing the uncatchable termination exception. And the invalid join stack state will affect subsequent Array#join calls. Because all the terminate exception will be handled by Isolate::UnwindAndFindHandler, we could clear the array join stack when unwinding the terminate exception. Bug: v8:13259 Change-Id: I23823e823c5fe0b089528c5cf654864cea78ebeb Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3878451 Reviewed-by: Jakob Linke Commit-Queue: 王澳 Cr-Commit-Position: refs/heads/main@{#83465} Refs: https://github.com/v8/v8/commit/031b98b25cbaaa4c62d8544f5f667d33ea4076c4 Closes: https://github.com/nodejs/node/issues/44417 PR-URL: https://github.com/nodejs/node/pull/45375 Fixes: https://github.com/nodejs/node/issues/44417 Reviewed-By: Jiawen Geng Reviewed-By: Rich Trott Reviewed-By: Kohei Ueno --- deps/v8/src/execution/isolate.cc | 9 +++ ...ode-side-effecting-array-join-expected.txt | 48 +++++++++++++ ...ate-repl-mode-side-effecting-array-join.js | 32 +++++++++ .../execution/thread-termination-unittest.cc | 70 +++++++++++++++++++ 4 files changed, 159 insertions(+) create mode 100644 deps/v8/test/inspector/runtime/evaluate-repl-mode-side-effecting-array-join-expected.txt create mode 100644 deps/v8/test/inspector/runtime/evaluate-repl-mode-side-effecting-array-join.js diff --git a/deps/v8/src/execution/isolate.cc b/deps/v8/src/execution/isolate.cc index bbd6855bb3be48..2e4f402c66e31c 100644 --- a/deps/v8/src/execution/isolate.cc +++ b/deps/v8/src/execution/isolate.cc @@ -1949,6 +1949,15 @@ Object Isolate::UnwindAndFindHandler() { // Special handling of termination exceptions, uncatchable by JavaScript and // Wasm code, we unwind the handlers until the top ENTRY handler is found. bool catchable_by_js = is_catchable_by_javascript(exception); + if (!catchable_by_js && !context().is_null()) { + // Because the array join stack will not pop the elements when throwing the + // uncatchable terminate exception, we need to clear the array join stack to + // avoid leaving the stack in an invalid state. + // See also CycleProtectedArrayJoin. + raw_native_context().set_array_join_stack( + ReadOnlyRoots(this).undefined_value()); + } + int visited_frames = 0; #if V8_ENABLE_WEBASSEMBLY diff --git a/deps/v8/test/inspector/runtime/evaluate-repl-mode-side-effecting-array-join-expected.txt b/deps/v8/test/inspector/runtime/evaluate-repl-mode-side-effecting-array-join-expected.txt new file mode 100644 index 00000000000000..19ad7f863eeba9 --- /dev/null +++ b/deps/v8/test/inspector/runtime/evaluate-repl-mode-side-effecting-array-join-expected.txt @@ -0,0 +1,48 @@ +Tests that Runtime.evaluate with REPL mode correctly handles Array.prototype.join. +{ + id : + result : { + result : { + className : Array + description : Array(1) + objectId : + subtype : array + type : object + } + } +} +{ + id : + result : { + exceptionDetails : { + columnNumber : -1 + exception : { + className : EvalError + description : EvalError: Possible side-effect in debug-evaluate + objectId : + subtype : error + type : object + } + exceptionId : + lineNumber : -1 + scriptId : + text : Uncaught + } + result : { + className : EvalError + description : EvalError: Possible side-effect in debug-evaluate + objectId : + subtype : error + type : object + } + } +} +{ + id : + result : { + result : { + type : string + value : /a/ + } + } +} diff --git a/deps/v8/test/inspector/runtime/evaluate-repl-mode-side-effecting-array-join.js b/deps/v8/test/inspector/runtime/evaluate-repl-mode-side-effecting-array-join.js new file mode 100644 index 00000000000000..05259ff24f4d95 --- /dev/null +++ b/deps/v8/test/inspector/runtime/evaluate-repl-mode-side-effecting-array-join.js @@ -0,0 +1,32 @@ +// Copyright 2022 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +let {Protocol} = InspectorTest.start( + 'Tests that Runtime.evaluate with REPL mode correctly handles \ +Array.prototype.join.'); + +Protocol.Runtime.enable(); +(async function () { + await evaluateReplWithSideEffects('a=[/a/]') + await evaluateRepl('a.toString()'); + await evaluateReplWithSideEffects('a.toString()'); + + InspectorTest.completeTest(); +})(); + +async function evaluateRepl(expression) { + InspectorTest.logMessage(await Protocol.Runtime.evaluate({ + expression: expression, + replMode: true, + throwOnSideEffect: true + })); +} + +async function evaluateReplWithSideEffects(expression) { + InspectorTest.logMessage(await Protocol.Runtime.evaluate({ + expression: expression, + replMode: true, + throwOnSideEffect: false + })); +} diff --git a/deps/v8/test/unittests/execution/thread-termination-unittest.cc b/deps/v8/test/unittests/execution/thread-termination-unittest.cc index ef23af37fdce7b..f9634b4a53d7e3 100644 --- a/deps/v8/test/unittests/execution/thread-termination-unittest.cc +++ b/deps/v8/test/unittests/execution/thread-termination-unittest.cc @@ -33,6 +33,7 @@ #include "src/init/v8.h" #include "src/objects/objects-inl.h" #include "test/unittests/test-utils.h" +#include "testing/gmock-support.h" #include "testing/gtest/include/gtest/gtest.h" namespace v8 { @@ -889,6 +890,75 @@ TEST_F(ThreadTerminationTest, TerminateConsole) { CHECK(isolate()->IsExecutionTerminating()); } +TEST_F(ThreadTerminationTest, TerminationClearArrayJoinStack) { + internal::v8_flags.allow_natives_syntax = true; + HandleScope scope(isolate()); + Local global_template = + CreateGlobalTemplate(isolate(), TerminateCurrentThread, DoLoopNoCall); + { + Local context = Context::New(isolate(), nullptr, global_template); + Context::Scope context_scope(context); + { + TryCatch try_catch(isolate()); + TryRunJS( + "var error = false;" + "var a = [{toString(){if(error)loop()}}];" + "function Join(){ return a.join();}; " + "%PrepareFunctionForOptimization(Join);" + "Join();" + "%OptimizeFunctionOnNextCall(Join);" + "error = true;" + "Join();"); + CHECK(try_catch.HasTerminated()); + CHECK(isolate()->IsExecutionTerminating()); + } + EXPECT_THAT(RunJS("a[0] = 1; Join();"), testing::IsString("1")); + } + { + Local context = Context::New(isolate(), nullptr, global_template); + Context::Scope context_scope(context); + { + TryCatch try_catch(isolate()); + TryRunJS( + "var a = [{toString(){loop()}}];" + "function Join(){ return a.join();}; " + "Join();"); + CHECK(try_catch.HasTerminated()); + CHECK(isolate()->IsExecutionTerminating()); + } + EXPECT_THAT(RunJS("a[0] = 1; Join();"), testing::IsString("1")); + } + { + ConsoleImpl console; + debug::SetConsoleDelegate(isolate(), &console); + HandleScope scope(isolate()); + Local context = Context::New(isolate(), nullptr, global_template); + Context::Scope context_scope(context); + { + // setup console global. + HandleScope scope(isolate()); + Local name = String::NewFromUtf8Literal( + isolate(), "console", NewStringType::kInternalized); + Local console = context->GetExtrasBindingObject() + ->Get(context, name) + .ToLocalChecked(); + context->Global()->Set(context, name, console).FromJust(); + } + CHECK(!isolate()->IsExecutionTerminating()); + { + TryCatch try_catch(isolate()); + CHECK(!isolate()->IsExecutionTerminating()); + CHECK(TryRunJS("var a = [{toString(){terminate();console.log();fail()}}];" + "function Join() {return a.join();}" + "Join();") + .IsEmpty()); + CHECK(try_catch.HasCaught()); + CHECK(isolate()->IsExecutionTerminating()); + } + EXPECT_THAT(RunJS("a[0] = 1; Join();"), testing::IsString("1")); + } +} + class TerminatorSleeperThread : public base::Thread { public: explicit TerminatorSleeperThread(Isolate* isolate, int sleep_ms)