From a92ecf00814b1b94d5cc800201dee91099904112 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 22 Mar 2021 18:47:10 +0200 Subject: [PATCH] deps: v8 backport 9689b17687b MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [top-level-await] Implement spec fix for cycle root detection Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0 PR-URL: https://github.com/nodejs/node/pull/37865 Reviewed-By: Michaël Zasso --- common.gypi | 2 +- deps/v8/src/diagnostics/objects-printer.cc | 1 + deps/v8/src/heap/factory.cc | 1 + deps/v8/src/objects/module-inl.h | 8 +++ deps/v8/src/objects/source-text-module.cc | 71 ++++--------------- deps/v8/src/objects/source-text-module.h | 7 +- deps/v8/src/objects/source-text-module.tq | 5 ++ ...modules-import-rqstd-order-async-cycle.mjs | 12 ++++ .../harmony/modules-skip-async-cycle-1.mjs | 14 ++++ .../harmony/modules-skip-async-cycle-2.mjs | 12 ++++ .../harmony/modules-skip-async-cycle-3.mjs | 12 ++++ .../harmony/modules-skip-async-cycle-leaf.mjs | 13 ++++ .../modules-skip-async-cycle-start.mjs | 13 ++++ 13 files changed, 109 insertions(+), 62 deletions(-) create mode 100644 deps/v8/test/mjsunit/harmony/modules-import-rqstd-order-async-cycle.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-1.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-2.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-3.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-leaf.mjs create mode 100644 deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-start.mjs diff --git a/common.gypi b/common.gypi index cae6662364984c..154ba97d9f46cf 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.19', + 'v8_embedder_string': '-node.20', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/diagnostics/objects-printer.cc b/deps/v8/src/diagnostics/objects-printer.cc index 00ef81f56a6bfc..74aee11d4ce53b 100644 --- a/deps/v8/src/diagnostics/objects-printer.cc +++ b/deps/v8/src/diagnostics/objects-printer.cc @@ -1621,6 +1621,7 @@ void SourceTextModule::SourceTextModulePrint(std::ostream& os) { // NOLINT os << "\n - requested_modules: " << Brief(requested_modules()); os << "\n - script: " << Brief(script()); os << "\n - import_meta: " << Brief(import_meta()); + os << "\n - cycle_root: " << Brief(cycle_root()); os << "\n"; } diff --git a/deps/v8/src/heap/factory.cc b/deps/v8/src/heap/factory.cc index a87c4995cf2144..7ce1c1e54ebcd2 100644 --- a/deps/v8/src/heap/factory.cc +++ b/deps/v8/src/heap/factory.cc @@ -2486,6 +2486,7 @@ Handle Factory::NewSourceTextModule( module->set_flags(0); module->set_async(IsAsyncModule(code->kind())); module->set_async_evaluating(false); + module->set_cycle_root(roots.the_hole_value()); module->set_async_parent_modules(*async_parent_modules); module->set_pending_async_dependencies(0); return module; diff --git a/deps/v8/src/objects/module-inl.h b/deps/v8/src/objects/module-inl.h index ce03c39500575e..80bfa7b4d6b2ae 100644 --- a/deps/v8/src/objects/module-inl.h +++ b/deps/v8/src/objects/module-inl.h @@ -111,6 +111,14 @@ class UnorderedModuleSet ZoneAllocator>(zone)) {} }; +Handle SourceTextModule::GetCycleRoot( + Isolate* isolate) const { + CHECK_GE(status(), kEvaluated); + DCHECK(!cycle_root().IsTheHole(isolate)); + Handle root(SourceTextModule::cast(cycle_root()), isolate); + return root; +} + void SourceTextModule::AddAsyncParentModule(Isolate* isolate, Handle module, Handle parent) { diff --git a/deps/v8/src/objects/source-text-module.cc b/deps/v8/src/objects/source-text-module.cc index f1b0c9c2c48633..692e5c85674a22 100644 --- a/deps/v8/src/objects/source-text-module.cc +++ b/deps/v8/src/objects/source-text-module.cc @@ -398,6 +398,7 @@ bool SourceTextModule::MaybeTransitionComponent( DCHECK_LE(module->dfs_ancestor_index(), module->dfs_index()); if (module->dfs_ancestor_index() == module->dfs_index()) { // This is the root of its strongly connected component. + Handle cycle_root = module; Handle ancestor; do { ancestor = stack->front(); @@ -407,6 +408,9 @@ bool SourceTextModule::MaybeTransitionComponent( if (new_status == kInstantiated) { if (!SourceTextModule::RunInitializationCode(isolate, ancestor)) return false; + } else if (new_status == kEvaluated) { + DCHECK(ancestor->cycle_root().IsTheHole(isolate)); + ancestor->set_cycle_root(*cycle_root); } ancestor->SetStatus(new_status); } while (*ancestor != *module); @@ -617,9 +621,9 @@ MaybeHandle SourceTextModule::EvaluateMaybeAsync( CHECK(module->status() == kInstantiated || module->status() == kEvaluated); // 3. If module.[[Status]] is "evaluated", set module to - // GetAsyncCycleRoot(module). + // module.[[CycleRoot]]. if (module->status() == kEvaluated) { - module = GetAsyncCycleRoot(isolate, module); + module = module->GetCycleRoot(isolate); } // 4. If module.[[TopLevelCapability]] is not undefined, then @@ -734,37 +738,27 @@ void SourceTextModule::AsyncModuleExecutionFulfilled( for (int i = 0; i < module->AsyncParentModuleCount(); i++) { Handle m = module->GetAsyncParentModule(isolate, i); - // a. If module.[[DFSIndex]] is not equal to module.[[DFSAncestorIndex]], - // then - if (module->dfs_index() != module->dfs_ancestor_index()) { - // i. Assert: m.[[DFSAncestorIndex]] is equal to - // module.[[DFSAncestorIndex]]. - DCHECK_LE(m->dfs_ancestor_index(), module->dfs_ancestor_index()); - } - // b. Decrement m.[[PendingAsyncDependencies]] by 1. + // a. Decrement m.[[PendingAsyncDependencies]] by 1. m->DecrementPendingAsyncDependencies(); - // c. If m.[[PendingAsyncDependencies]] is 0 and m.[[EvaluationError]] is + // b. If m.[[PendingAsyncDependencies]] is 0 and m.[[EvaluationError]] is // undefined, then if (!m->HasPendingAsyncDependencies() && m->status() == kEvaluated) { // i. Assert: m.[[AsyncEvaluating]] is true. DCHECK(m->async_evaluating()); - // ii. Let cycleRoot be ! GetAsyncCycleRoot(m). - auto cycle_root = GetAsyncCycleRoot(isolate, m); - - // iii. If cycleRoot.[[EvaluationError]] is not undefined, + // ii. If m.[[CycleRoot]].[[EvaluationError]] is not undefined, // return undefined. - if (cycle_root->status() == kErrored) { + if (m->GetCycleRoot(isolate)->status() == kErrored) { return; } - // iv. If m.[[Async]] is true, then + // iii. If m.[[Async]] is true, then if (m->async()) { // 1. Perform ! ExecuteAsyncModule(m). ExecuteAsyncModule(isolate, m); } else { - // v. Otherwise, + // iv. Otherwise, // 1. Let result be m.ExecuteModule(). // 2. If result is a normal completion, Handle unused_result; @@ -1044,8 +1038,8 @@ MaybeHandle SourceTextModule::InnerModuleEvaluation( required_module->dfs_ancestor_index())); } else { // iv. Otherwise, - // 1. Set requiredModule to GetAsyncCycleRoot(requiredModule). - required_module = GetAsyncCycleRoot(isolate, required_module); + // 1. Set requiredModule to requiredModule.[[CycleRoot]]. + required_module = required_module->GetCycleRoot(isolate); // 2. Assert: requiredModule.[[Status]] is "evaluated". CHECK_GE(required_module->status(), kEvaluated); @@ -1103,43 +1097,6 @@ MaybeHandle SourceTextModule::InnerModuleEvaluation( return result; } -Handle SourceTextModule::GetAsyncCycleRoot( - Isolate* isolate, Handle module) { - // 1. Assert: module.[[Status]] is "evaluated". - CHECK_GE(module->status(), kEvaluated); - - // 2. If module.[[AsyncParentModules]] is an empty List, return module. - if (module->AsyncParentModuleCount() == 0) { - return module; - } - - // 3. Repeat, while module.[[DFSIndex]] is greater than - // module.[[DFSAncestorIndex]], - while (module->dfs_index() > module->dfs_ancestor_index()) { - // a. Assert: module.[[AsyncParentModules]] is a non-empty List. - DCHECK_GT(module->AsyncParentModuleCount(), 0); - - // b. Let nextCycleModule be the first element of - // module.[[AsyncParentModules]]. - Handle next_cycle_module = - module->GetAsyncParentModule(isolate, 0); - - // c. Assert: nextCycleModule.[[DFSAncestorIndex]] is less than or equal - // to module.[[DFSAncestorIndex]]. - DCHECK_LE(next_cycle_module->dfs_ancestor_index(), - module->dfs_ancestor_index()); - - // d. Set module to nextCycleModule - module = next_cycle_module; - } - - // 4. Assert: module.[[DFSIndex]] is equal to module.[[DFSAncestorIndex]]. - DCHECK_EQ(module->dfs_index(), module->dfs_ancestor_index()); - - // 5. Return module. - return module; -} - void SourceTextModule::Reset(Isolate* isolate, Handle module) { Factory* factory = isolate->factory(); diff --git a/deps/v8/src/objects/source-text-module.h b/deps/v8/src/objects/source-text-module.h index 7e64668a7ed86c..d716674c12d01a 100644 --- a/deps/v8/src/objects/source-text-module.h +++ b/deps/v8/src/objects/source-text-module.h @@ -78,6 +78,9 @@ class SourceTextModule Handle module, Handle parent); + // Get the non-hole cycle root. Only valid when status >= kEvaluated. + inline Handle GetCycleRoot(Isolate* isolate) const; + // Returns a SourceTextModule, the // ith parent in depth first traversal order of a given async child. inline Handle GetAsyncParentModule(Isolate* isolate, @@ -163,10 +166,6 @@ class SourceTextModule Isolate* isolate, Handle module, ZoneForwardList>* stack, Status new_status); - // Implementation of spec GetAsyncCycleRoot. - static V8_WARN_UNUSED_RESULT Handle GetAsyncCycleRoot( - Isolate* isolate, Handle module); - // Implementation of spec ExecuteModule is broken up into // InnerExecuteAsyncModule for asynchronous modules and ExecuteModule // for synchronous modules. diff --git a/deps/v8/src/objects/source-text-module.tq b/deps/v8/src/objects/source-text-module.tq index fda0138695f380..091a267cb6f01a 100644 --- a/deps/v8/src/objects/source-text-module.tq +++ b/deps/v8/src/objects/source-text-module.tq @@ -29,6 +29,11 @@ extern class SourceTextModule extends Module { // a JSObject afterwards. import_meta: TheHole|JSObject; + // The first visited module of a cycle. For modules not in a cycle, this is + // the module itself. It's the hole before the module state transitions to + // kEvaluated. + cycle_root: SourceTextModule|TheHole; + async_parent_modules: ArrayList; top_level_capability: JSPromise|Undefined; diff --git a/deps/v8/test/mjsunit/harmony/modules-import-rqstd-order-async-cycle.mjs b/deps/v8/test/mjsunit/harmony/modules-import-rqstd-order-async-cycle.mjs new file mode 100644 index 00000000000000..e9aa18e577d4c9 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-import-rqstd-order-async-cycle.mjs @@ -0,0 +1,12 @@ +// Copyright 2021 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. + +// Flags: --harmony-top-level-await + +import "modules-skip-async-cycle-start.mjs" + +assertEquals(globalThis.test_order, [ + '2', 'async before', 'async after', '1', + '3', 'start', +]); diff --git a/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-1.mjs b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-1.mjs new file mode 100644 index 00000000000000..b2f7ecac865917 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-1.mjs @@ -0,0 +1,14 @@ +// Copyright 2021 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. + +// Flags: --harmony-top-level-await + +import "modules-skip-async-cycle-2.mjs"; +import "modules-skip-async-cycle-leaf.mjs"; + +if (globalThis.test_order === undefined) { + globalThis.test_order = []; +} +globalThis.test_order.push('1'); + diff --git a/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-2.mjs b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-2.mjs new file mode 100644 index 00000000000000..391927ae128ebd --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-2.mjs @@ -0,0 +1,12 @@ +// Copyright 2021 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. + +// Flags: --harmony-top-level-await + +import "modules-skip-async-cycle-1.mjs"; + +if (globalThis.test_order === undefined) { + globalThis.test_order = []; +} +globalThis.test_order.push('2'); diff --git a/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-3.mjs b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-3.mjs new file mode 100644 index 00000000000000..f61dcdf55a2af4 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-3.mjs @@ -0,0 +1,12 @@ +// Copyright 2021 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. + +// Flags: --harmony-top-level-await + +import "modules-skip-async-cycle-2.mjs"; + +if (globalThis.test_order === undefined) { + globalThis.test_order = []; +} +globalThis.test_order.push('3'); diff --git a/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-leaf.mjs b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-leaf.mjs new file mode 100644 index 00000000000000..7f8e8589f36679 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-leaf.mjs @@ -0,0 +1,13 @@ +// Copyright 2021 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. + +// Flags: --harmony-top-level-await + +if (globalThis.test_order === undefined) { + globalThis.test_order = []; +} + +globalThis.test_order.push('async before'); +await 0; +globalThis.test_order.push('async after'); diff --git a/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-start.mjs b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-start.mjs new file mode 100644 index 00000000000000..feeb6c6b2aebdf --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/modules-skip-async-cycle-start.mjs @@ -0,0 +1,13 @@ +// Copyright 2021 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. + +// Flags: --harmony-top-level-await + +import "modules-skip-async-cycle-1.mjs"; +import "modules-skip-async-cycle-3.mjs"; + +if (globalThis.test_order === undefined) { + globalThis.test_order = []; +} +globalThis.test_order.push('start');