Skip to content

Commit 17e6031

Browse files
targosruyadorno
authored andcommitted
deps: V8: cherry-pick 031b98b25cba
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 <jgruber@chromium.org> Commit-Queue: 王澳 <wangao.james@bytedance.com> Cr-Commit-Position: refs/heads/main@{#83465} Refs: v8/v8@031b98b Closes: #44417 PR-URL: #45375 Fixes: #44417 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
1 parent 6c56c97 commit 17e6031

File tree

4 files changed

+159
-0
lines changed

4 files changed

+159
-0
lines changed

deps/v8/src/execution/isolate.cc

+9
Original file line numberDiff line numberDiff line change
@@ -1949,6 +1949,15 @@ Object Isolate::UnwindAndFindHandler() {
19491949
// Special handling of termination exceptions, uncatchable by JavaScript and
19501950
// Wasm code, we unwind the handlers until the top ENTRY handler is found.
19511951
bool catchable_by_js = is_catchable_by_javascript(exception);
1952+
if (!catchable_by_js && !context().is_null()) {
1953+
// Because the array join stack will not pop the elements when throwing the
1954+
// uncatchable terminate exception, we need to clear the array join stack to
1955+
// avoid leaving the stack in an invalid state.
1956+
// See also CycleProtectedArrayJoin.
1957+
raw_native_context().set_array_join_stack(
1958+
ReadOnlyRoots(this).undefined_value());
1959+
}
1960+
19521961
int visited_frames = 0;
19531962

19541963
#if V8_ENABLE_WEBASSEMBLY
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
Tests that Runtime.evaluate with REPL mode correctly handles Array.prototype.join.
2+
{
3+
id : <messageId>
4+
result : {
5+
result : {
6+
className : Array
7+
description : Array(1)
8+
objectId : <objectId>
9+
subtype : array
10+
type : object
11+
}
12+
}
13+
}
14+
{
15+
id : <messageId>
16+
result : {
17+
exceptionDetails : {
18+
columnNumber : -1
19+
exception : {
20+
className : EvalError
21+
description : EvalError: Possible side-effect in debug-evaluate
22+
objectId : <objectId>
23+
subtype : error
24+
type : object
25+
}
26+
exceptionId : <exceptionId>
27+
lineNumber : -1
28+
scriptId : <scriptId>
29+
text : Uncaught
30+
}
31+
result : {
32+
className : EvalError
33+
description : EvalError: Possible side-effect in debug-evaluate
34+
objectId : <objectId>
35+
subtype : error
36+
type : object
37+
}
38+
}
39+
}
40+
{
41+
id : <messageId>
42+
result : {
43+
result : {
44+
type : string
45+
value : /a/
46+
}
47+
}
48+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright 2022 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
let {Protocol} = InspectorTest.start(
6+
'Tests that Runtime.evaluate with REPL mode correctly handles \
7+
Array.prototype.join.');
8+
9+
Protocol.Runtime.enable();
10+
(async function () {
11+
await evaluateReplWithSideEffects('a=[/a/]')
12+
await evaluateRepl('a.toString()');
13+
await evaluateReplWithSideEffects('a.toString()');
14+
15+
InspectorTest.completeTest();
16+
})();
17+
18+
async function evaluateRepl(expression) {
19+
InspectorTest.logMessage(await Protocol.Runtime.evaluate({
20+
expression: expression,
21+
replMode: true,
22+
throwOnSideEffect: true
23+
}));
24+
}
25+
26+
async function evaluateReplWithSideEffects(expression) {
27+
InspectorTest.logMessage(await Protocol.Runtime.evaluate({
28+
expression: expression,
29+
replMode: true,
30+
throwOnSideEffect: false
31+
}));
32+
}

deps/v8/test/unittests/execution/thread-termination-unittest.cc

+70
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "src/init/v8.h"
3434
#include "src/objects/objects-inl.h"
3535
#include "test/unittests/test-utils.h"
36+
#include "testing/gmock-support.h"
3637
#include "testing/gtest/include/gtest/gtest.h"
3738

3839
namespace v8 {
@@ -889,6 +890,75 @@ TEST_F(ThreadTerminationTest, TerminateConsole) {
889890
CHECK(isolate()->IsExecutionTerminating());
890891
}
891892

893+
TEST_F(ThreadTerminationTest, TerminationClearArrayJoinStack) {
894+
internal::v8_flags.allow_natives_syntax = true;
895+
HandleScope scope(isolate());
896+
Local<ObjectTemplate> global_template =
897+
CreateGlobalTemplate(isolate(), TerminateCurrentThread, DoLoopNoCall);
898+
{
899+
Local<Context> context = Context::New(isolate(), nullptr, global_template);
900+
Context::Scope context_scope(context);
901+
{
902+
TryCatch try_catch(isolate());
903+
TryRunJS(
904+
"var error = false;"
905+
"var a = [{toString(){if(error)loop()}}];"
906+
"function Join(){ return a.join();}; "
907+
"%PrepareFunctionForOptimization(Join);"
908+
"Join();"
909+
"%OptimizeFunctionOnNextCall(Join);"
910+
"error = true;"
911+
"Join();");
912+
CHECK(try_catch.HasTerminated());
913+
CHECK(isolate()->IsExecutionTerminating());
914+
}
915+
EXPECT_THAT(RunJS("a[0] = 1; Join();"), testing::IsString("1"));
916+
}
917+
{
918+
Local<Context> context = Context::New(isolate(), nullptr, global_template);
919+
Context::Scope context_scope(context);
920+
{
921+
TryCatch try_catch(isolate());
922+
TryRunJS(
923+
"var a = [{toString(){loop()}}];"
924+
"function Join(){ return a.join();}; "
925+
"Join();");
926+
CHECK(try_catch.HasTerminated());
927+
CHECK(isolate()->IsExecutionTerminating());
928+
}
929+
EXPECT_THAT(RunJS("a[0] = 1; Join();"), testing::IsString("1"));
930+
}
931+
{
932+
ConsoleImpl console;
933+
debug::SetConsoleDelegate(isolate(), &console);
934+
HandleScope scope(isolate());
935+
Local<Context> context = Context::New(isolate(), nullptr, global_template);
936+
Context::Scope context_scope(context);
937+
{
938+
// setup console global.
939+
HandleScope scope(isolate());
940+
Local<String> name = String::NewFromUtf8Literal(
941+
isolate(), "console", NewStringType::kInternalized);
942+
Local<Value> console = context->GetExtrasBindingObject()
943+
->Get(context, name)
944+
.ToLocalChecked();
945+
context->Global()->Set(context, name, console).FromJust();
946+
}
947+
CHECK(!isolate()->IsExecutionTerminating());
948+
{
949+
TryCatch try_catch(isolate());
950+
CHECK(!isolate()->IsExecutionTerminating());
951+
CHECK(TryRunJS("var a = [{toString(){terminate();console.log();fail()}}];"
952+
"function Join() {return a.join();}"
953+
"Join();")
954+
.IsEmpty());
955+
CHECK(try_catch.HasCaught());
956+
CHECK(isolate()->IsExecutionTerminating());
957+
}
958+
EXPECT_THAT(RunJS("a[0] = 1; Join();"), testing::IsString("1"));
959+
}
960+
}
961+
892962
class TerminatorSleeperThread : public base::Thread {
893963
public:
894964
explicit TerminatorSleeperThread(Isolate* isolate, int sleep_ms)

0 commit comments

Comments
 (0)