diff --git a/benchmark/vm/compile-script-in-isolate-cache.js b/benchmark/vm/compile-script-in-isolate-cache.js new file mode 100644 index 00000000000000..7eceb0eba0d215 --- /dev/null +++ b/benchmark/vm/compile-script-in-isolate-cache.js @@ -0,0 +1,35 @@ +'use strict'; + +// This benchmarks compiling scripts that hit the in-isolate compilation +// cache (by having the same source). +const common = require('../common.js'); +const fs = require('fs'); +const vm = require('vm'); +const fixtures = require('../../test/common/fixtures.js'); +const scriptPath = fixtures.path('snapshot', 'typescript.js'); + +const bench = common.createBenchmark(main, { + type: ['with-dynamic-import-callback', 'without-dynamic-import-callback'], + n: [100], +}); + +const scriptSource = fs.readFileSync(scriptPath, 'utf8'); + +function main({ n, type }) { + let script; + bench.start(); + const options = {}; + switch (type) { + case 'with-dynamic-import-callback': + // Use a dummy callback for now until we really need to benchmark it. + options.importModuleDynamically = async () => {}; + break; + case 'without-dynamic-import-callback': + break; + } + for (let i = 0; i < n; i++) { + script = new vm.Script(scriptSource, options); + } + bench.end(n); + script.runInThisContext(); +} diff --git a/common.gypi b/common.gypi index dba2631f218581..a58d48b0c5eb9a 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.28', + 'v8_embedder_string': '-node.29', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8-object.h b/deps/v8/include/v8-object.h index bad299fc42948d..49369317b72c39 100644 --- a/deps/v8/include/v8-object.h +++ b/deps/v8/include/v8-object.h @@ -20,6 +20,8 @@ class Function; class FunctionTemplate; template class PropertyCallbackInfo; +class Module; +class UnboundScript; /** * A private symbol @@ -480,6 +482,21 @@ class V8_EXPORT Object : public Value { /** Sets the value in an internal field. */ void SetInternalField(int index, Local value); + /** + * Warning: These are Node.js-specific extentions used to avoid breaking + * changes in Node.js v18.x. They do not exist in V8 upstream and will + * not exist in Node.js v21.x. Node.js embedders and addon authors should + * not use them from v18.x. + */ +#ifndef NODE_WANT_INTERNALS + V8_DEPRECATED("This extention should only be used by Node.js core") +#endif + void SetInternalFieldForNodeCore(int index, Local value); +#ifndef NODE_WANT_INTERNALS + V8_DEPRECATED("This extention should only be used by Node.js core") +#endif + void SetInternalFieldForNodeCore(int index, Local value); + /** * Gets a 2-byte-aligned native pointer from an internal field. This field * must have been set by SetAlignedPointerInInternalField, everything else diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index 8d224bea2e2261..3b1a81680b0323 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -5948,14 +5948,33 @@ Local v8::Object::SlowGetInternalField(int index) { return Utils::ToLocal(value); } -void v8::Object::SetInternalField(int index, v8::Local value) { - i::Handle obj = Utils::OpenHandle(this); +template +void SetInternalFieldImpl(v8::Object* receiver, int index, v8::Local value) { + i::Handle obj = Utils::OpenHandle(receiver); const char* location = "v8::Object::SetInternalField()"; if (!InternalFieldOK(obj, index, location)) return; i::Handle val = Utils::OpenHandle(*value); i::Handle::cast(obj)->SetEmbedderField(index, *val); } +void v8::Object::SetInternalField(int index, v8::Local value) { + SetInternalFieldImpl(this, index, value); +} + +/** + * These are Node.js-specific extentions used to avoid breaking changes in + * Node.js v20.x. + */ +void v8::Object::SetInternalFieldForNodeCore(int index, + v8::Local value) { + SetInternalFieldImpl(this, index, value); +} + +void v8::Object::SetInternalFieldForNodeCore(int index, + v8::Local value) { + SetInternalFieldImpl(this, index, value); +} + void* v8::Object::SlowGetAlignedPointerFromInternalField(int index) { i::Handle obj = Utils::OpenHandle(this); const char* location = "v8::Object::GetAlignedPointerFromInternalField()"; diff --git a/deps/v8/src/builtins/base.tq b/deps/v8/src/builtins/base.tq index c34d9f6884cfb5..fe67d403b87879 100644 --- a/deps/v8/src/builtins/base.tq +++ b/deps/v8/src/builtins/base.tq @@ -438,9 +438,9 @@ extern enum MessageTemplate { kWasmTrapArrayOutOfBounds, kWasmTrapArrayTooLarge, kWeakRefsRegisterTargetAndHoldingsMustNotBeSame, - kWeakRefsRegisterTargetMustBeObject, - kWeakRefsUnregisterTokenMustBeObject, - kWeakRefsWeakRefConstructorTargetMustBeObject, + kInvalidWeakRefsRegisterTarget, + kInvalidWeakRefsUnregisterToken, + kInvalidWeakRefsWeakRefConstructorTarget, ... } @@ -906,10 +906,10 @@ macro Float64IsNaN(n: float64): bool { // The type of all tagged values that can safely be compared with TaggedEqual. @if(V8_ENABLE_WEBASSEMBLY) type TaggedWithIdentity = JSReceiver | FixedArrayBase | Oddball | Map | - WeakCell | Context | EmptyString | WasmInternalFunction; + WeakCell | Context | EmptyString | Symbol | WasmInternalFunction; @ifnot(V8_ENABLE_WEBASSEMBLY) type TaggedWithIdentity = JSReceiver | FixedArrayBase | Oddball | Map | - WeakCell | Context | EmptyString; + WeakCell | Context | EmptyString | Symbol; extern operator '==' macro TaggedEqual(TaggedWithIdentity, Object): bool; extern operator '==' macro TaggedEqual(Object, TaggedWithIdentity): bool; diff --git a/deps/v8/src/builtins/builtins-collections-gen.cc b/deps/v8/src/builtins/builtins-collections-gen.cc index 00cad7b314b756..d83038f7f65c40 100644 --- a/deps/v8/src/builtins/builtins-collections-gen.cc +++ b/deps/v8/src/builtins/builtins-collections-gen.cc @@ -22,130 +22,6 @@ namespace internal { template using TVariable = compiler::TypedCodeAssemblerVariable; -class BaseCollectionsAssembler : public CodeStubAssembler { - public: - explicit BaseCollectionsAssembler(compiler::CodeAssemblerState* state) - : CodeStubAssembler(state) {} - - virtual ~BaseCollectionsAssembler() = default; - - protected: - enum Variant { kMap, kSet, kWeakMap, kWeakSet }; - - // Adds an entry to a collection. For Maps, properly handles extracting the - // key and value from the entry (see LoadKeyValue()). - void AddConstructorEntry(Variant variant, TNode context, - TNode collection, TNode add_function, - TNode key_value, - Label* if_may_have_side_effects = nullptr, - Label* if_exception = nullptr, - TVariable* var_exception = nullptr); - - // Adds constructor entries to a collection. Choosing a fast path when - // possible. - void AddConstructorEntries(Variant variant, TNode context, - TNode native_context, - TNode collection, - TNode initial_entries); - - // Fast path for adding constructor entries. Assumes the entries are a fast - // JS array (see CodeStubAssembler::BranchIfFastJSArray()). - void AddConstructorEntriesFromFastJSArray(Variant variant, - TNode context, - TNode native_context, - TNode collection, - TNode fast_jsarray, - Label* if_may_have_side_effects); - - // Adds constructor entries to a collection using the iterator protocol. - void AddConstructorEntriesFromIterable(Variant variant, - TNode context, - TNode native_context, - TNode collection, - TNode iterable); - - // Constructs a collection instance. Choosing a fast path when possible. - TNode AllocateJSCollection(TNode context, - TNode constructor, - TNode new_target); - - // Fast path for constructing a collection instance if the constructor - // function has not been modified. - TNode AllocateJSCollectionFast(TNode constructor); - - // Fallback for constructing a collection instance if the constructor function - // has been modified. - TNode AllocateJSCollectionSlow(TNode context, - TNode constructor, - TNode new_target); - - // Allocates the backing store for a collection. - virtual TNode AllocateTable( - Variant variant, TNode at_least_space_for) = 0; - - // Main entry point for a collection constructor builtin. - void GenerateConstructor(Variant variant, - Handle constructor_function_name, - TNode new_target, TNode argc, - TNode context); - - // Retrieves the collection function that adds an entry. `set` for Maps and - // `add` for Sets. - TNode GetAddFunction(Variant variant, TNode context, - TNode collection); - - // Retrieves the collection constructor function. - TNode GetConstructor(Variant variant, - TNode native_context); - - // Retrieves the initial collection function that adds an entry. Should only - // be called when it is certain that a collection prototype's map hasn't been - // changed. - TNode GetInitialAddFunction(Variant variant, - TNode native_context); - - // Checks whether {collection}'s initial add/set function has been modified - // (depending on {variant}, loaded from {native_context}). - void GotoIfInitialAddFunctionModified(Variant variant, - TNode native_context, - TNode collection, - Label* if_modified); - - // Gets root index for the name of the add/set function. - RootIndex GetAddFunctionNameIndex(Variant variant); - - // Retrieves the offset to access the backing table from the collection. - int GetTableOffset(Variant variant); - - // Estimates the number of entries the collection will have after adding the - // entries passed in the constructor. AllocateTable() can use this to avoid - // the time of growing/rehashing when adding the constructor entries. - TNode EstimatedInitialSize(TNode initial_entries, - TNode is_fast_jsarray); - - void GotoIfCannotBeWeakKey(const TNode obj, - Label* if_cannot_be_weak_key); - - // Determines whether the collection's prototype has been modified. - TNode HasInitialCollectionPrototype(Variant variant, - TNode native_context, - TNode collection); - - // Gets the initial prototype map for given collection {variant}. - TNode GetInitialCollectionPrototype(Variant variant, - TNode native_context); - - // Loads an element from a fixed array. If the element is the hole, returns - // `undefined`. - TNode LoadAndNormalizeFixedArrayElement(TNode elements, - TNode index); - - // Loads an element from a fixed double array. If the element is the hole, - // returns `undefined`. - TNode LoadAndNormalizeFixedDoubleArrayElement( - TNode elements, TNode index); -}; - void BaseCollectionsAssembler::AddConstructorEntry( Variant variant, TNode context, TNode collection, TNode add_function, TNode key_value, @@ -523,14 +399,25 @@ TNode BaseCollectionsAssembler::EstimatedInitialSize( [=] { return IntPtrConstant(0); }); } -void BaseCollectionsAssembler::GotoIfCannotBeWeakKey( - const TNode obj, Label* if_cannot_be_weak_key) { - GotoIf(TaggedIsSmi(obj), if_cannot_be_weak_key); +// https://tc39.es/ecma262/#sec-canbeheldweakly +void BaseCollectionsAssembler::GotoIfCannotBeHeldWeakly( + const TNode obj, Label* if_cannot_be_held_weakly) { + Label check_symbol_key(this); + Label end(this); + GotoIf(TaggedIsSmi(obj), if_cannot_be_held_weakly); TNode instance_type = LoadMapInstanceType(LoadMap(CAST(obj))); - GotoIfNot(IsJSReceiverInstanceType(instance_type), if_cannot_be_weak_key); + GotoIfNot(IsJSReceiverInstanceType(instance_type), &check_symbol_key); // TODO(v8:12547) Shared structs should only be able to point to shared values // in weak collections. For now, disallow them as weak collection keys. - GotoIf(IsJSSharedStructInstanceType(instance_type), if_cannot_be_weak_key); + GotoIf(IsJSSharedStructInstanceType(instance_type), if_cannot_be_held_weakly); + Goto(&end); + Bind(&check_symbol_key); + GotoIfNot(IsSymbolInstanceType(instance_type), if_cannot_be_held_weakly); + TNode flags = LoadSymbolFlags(CAST(obj)); + GotoIf(Word32And(flags, Symbol::IsInPublicSymbolTableBit::kMask), + if_cannot_be_held_weakly); + Goto(&end); + Bind(&end); } TNode BaseCollectionsAssembler::GetInitialCollectionPrototype( @@ -2427,67 +2314,6 @@ TF_BUILTIN(FindOrderedHashMapEntry, CollectionsBuiltinsAssembler) { Return(SmiConstant(-1)); } -class WeakCollectionsBuiltinsAssembler : public BaseCollectionsAssembler { - public: - explicit WeakCollectionsBuiltinsAssembler(compiler::CodeAssemblerState* state) - : BaseCollectionsAssembler(state) {} - - protected: - void AddEntry(TNode table, TNode key_index, - TNode key, TNode value, - TNode number_of_elements); - - TNode AllocateTable(Variant variant, - TNode at_least_space_for) override; - - // Generates and sets the identity for a JSRececiver. - TNode CreateIdentityHash(TNode receiver); - TNode EntryMask(TNode capacity); - - // Builds code that finds the EphemeronHashTable entry for a {key} using the - // comparison code generated by {key_compare}. The key index is returned if - // the {key} is found. - using KeyComparator = - std::function entry_key, Label* if_same)>; - TNode FindKeyIndex(TNode table, TNode key_hash, - TNode entry_mask, - const KeyComparator& key_compare); - - // Builds code that finds an EphemeronHashTable entry available for a new - // entry. - TNode FindKeyIndexForInsertion(TNode table, - TNode key_hash, - TNode entry_mask); - - // Builds code that finds the EphemeronHashTable entry with key that matches - // {key} and returns the entry's key index. If {key} cannot be found, jumps to - // {if_not_found}. - TNode FindKeyIndexForKey(TNode table, TNode key, - TNode hash, - TNode entry_mask, - Label* if_not_found); - - TNode InsufficientCapacityToAdd(TNode capacity, - TNode number_of_elements, - TNode number_of_deleted); - TNode KeyIndexFromEntry(TNode entry); - - TNode LoadNumberOfElements(TNode table, - int offset); - TNode LoadNumberOfDeleted(TNode table, - int offset = 0); - TNode LoadTable(TNode collection); - TNode LoadTableCapacity(TNode table); - - void RemoveEntry(TNode table, TNode key_index, - TNode number_of_elements); - TNode ShouldRehash(TNode number_of_elements, - TNode number_of_deleted); - TNode ShouldShrink(TNode capacity, - TNode number_of_elements); - TNode ValueIndexFromKeyIndex(TNode key_index); -}; - void WeakCollectionsBuiltinsAssembler::AddEntry( TNode table, TNode key_index, TNode key, TNode value, TNode number_of_elements) { @@ -2503,6 +2329,25 @@ void WeakCollectionsBuiltinsAssembler::AddEntry( SmiFromIntPtr(number_of_elements)); } +TNode WeakCollectionsBuiltinsAssembler::GetHash( + const TNode key, Label* if_no_hash) { + TVARIABLE(IntPtrT, var_hash); + Label if_symbol(this); + Label return_result(this); + GotoIfNot(IsJSReceiver(key), &if_symbol); + var_hash = LoadJSReceiverIdentityHash(CAST(key), if_no_hash); + Goto(&return_result); + Bind(&if_symbol); + CSA_DCHECK(this, IsSymbol(key)); + CSA_DCHECK(this, Word32BinaryNot( + Word32And(LoadSymbolFlags(CAST(key)), + Symbol::IsInPublicSymbolTableBit::kMask))); + var_hash = ChangeInt32ToIntPtr(LoadNameHash(CAST(key), nullptr)); + Goto(&return_result); + Bind(&return_result); + return var_hash.value(); +} + TNode WeakCollectionsBuiltinsAssembler::AllocateTable( Variant variant, TNode at_least_space_for) { // See HashTable::New(). @@ -2728,18 +2573,17 @@ TF_BUILTIN(WeakMapLookupHashIndex, WeakCollectionsBuiltinsAssembler) { auto table = Parameter(Descriptor::kTable); auto key = Parameter(Descriptor::kKey); - Label if_cannot_be_weak_key(this); + Label if_cannot_be_held_weakly(this); - GotoIfCannotBeWeakKey(key, &if_cannot_be_weak_key); + GotoIfCannotBeHeldWeakly(key, &if_cannot_be_held_weakly); - TNode hash = - LoadJSReceiverIdentityHash(CAST(key), &if_cannot_be_weak_key); + TNode hash = GetHash(CAST(key), &if_cannot_be_held_weakly); TNode capacity = LoadTableCapacity(table); TNode key_index = FindKeyIndexForKey( - table, key, hash, EntryMask(capacity), &if_cannot_be_weak_key); + table, key, hash, EntryMask(capacity), &if_cannot_be_held_weakly); Return(SmiTag(ValueIndexFromKeyIndex(key_index))); - BIND(&if_cannot_be_weak_key); + BIND(&if_cannot_be_held_weakly); Return(SmiConstant(-1)); } @@ -2794,23 +2638,22 @@ TF_BUILTIN(WeakCollectionDelete, WeakCollectionsBuiltinsAssembler) { auto collection = Parameter(Descriptor::kCollection); auto key = Parameter(Descriptor::kKey); - Label call_runtime(this), if_cannot_be_weak_key(this); + Label call_runtime(this), if_cannot_be_held_weakly(this); - GotoIfCannotBeWeakKey(key, &if_cannot_be_weak_key); + GotoIfCannotBeHeldWeakly(key, &if_cannot_be_held_weakly); - TNode hash = - LoadJSReceiverIdentityHash(CAST(key), &if_cannot_be_weak_key); + TNode hash = GetHash(CAST(key), &if_cannot_be_held_weakly); TNode table = LoadTable(collection); TNode capacity = LoadTableCapacity(table); TNode key_index = FindKeyIndexForKey( - table, key, hash, EntryMask(capacity), &if_cannot_be_weak_key); + table, key, hash, EntryMask(capacity), &if_cannot_be_held_weakly); TNode number_of_elements = LoadNumberOfElements(table, -1); GotoIf(ShouldShrink(capacity, number_of_elements), &call_runtime); RemoveEntry(table, key_index, number_of_elements); Return(TrueConstant()); - BIND(&if_cannot_be_weak_key); + BIND(&if_cannot_be_held_weakly); Return(FalseConstant()); BIND(&call_runtime); @@ -2823,10 +2666,10 @@ TF_BUILTIN(WeakCollectionDelete, WeakCollectionsBuiltinsAssembler) { TF_BUILTIN(WeakCollectionSet, WeakCollectionsBuiltinsAssembler) { auto context = Parameter(Descriptor::kContext); auto collection = Parameter(Descriptor::kCollection); - auto key = Parameter(Descriptor::kKey); + auto key = Parameter(Descriptor::kKey); auto value = Parameter(Descriptor::kValue); - CSA_DCHECK(this, IsJSReceiver(key)); + CSA_DCHECK(this, Word32Or(IsJSReceiver(key), IsSymbol(key))); Label call_runtime(this), if_no_hash(this), if_not_found(this); @@ -2834,7 +2677,7 @@ TF_BUILTIN(WeakCollectionSet, WeakCollectionsBuiltinsAssembler) { TNode capacity = LoadTableCapacity(table); TNode entry_mask = EntryMask(capacity); - TVARIABLE(IntPtrT, var_hash, LoadJSReceiverIdentityHash(key, &if_no_hash)); + TVARIABLE(IntPtrT, var_hash, GetHash(key, &if_no_hash)); TNode key_index = FindKeyIndexForKey(table, key, var_hash.value(), entry_mask, &if_not_found); @@ -2843,6 +2686,7 @@ TF_BUILTIN(WeakCollectionSet, WeakCollectionsBuiltinsAssembler) { BIND(&if_no_hash); { + CSA_DCHECK(this, IsJSReceiver(key)); var_hash = SmiUntag(CreateIdentityHash(key)); Goto(&if_not_found); } @@ -2891,7 +2735,7 @@ TF_BUILTIN(WeakMapPrototypeSet, WeakCollectionsBuiltinsAssembler) { "WeakMap.prototype.set"); Label throw_invalid_key(this); - GotoIfCannotBeWeakKey(key, &throw_invalid_key); + GotoIfCannotBeHeldWeakly(key, &throw_invalid_key); Return( CallBuiltin(Builtin::kWeakCollectionSet, context, receiver, key, value)); @@ -2909,7 +2753,7 @@ TF_BUILTIN(WeakSetPrototypeAdd, WeakCollectionsBuiltinsAssembler) { "WeakSet.prototype.add"); Label throw_invalid_value(this); - GotoIfCannotBeWeakKey(value, &throw_invalid_value); + GotoIfCannotBeHeldWeakly(value, &throw_invalid_value); Return(CallBuiltin(Builtin::kWeakCollectionSet, context, receiver, value, TrueConstant())); diff --git a/deps/v8/src/builtins/builtins-collections-gen.h b/deps/v8/src/builtins/builtins-collections-gen.h index a132557e3cd0a4..53ea23f12d9f6b 100644 --- a/deps/v8/src/builtins/builtins-collections-gen.h +++ b/deps/v8/src/builtins/builtins-collections-gen.h @@ -20,6 +20,192 @@ void BranchIfIterableWithOriginalValueSetIterator( TNode context, compiler::CodeAssemblerLabel* if_true, compiler::CodeAssemblerLabel* if_false); +class BaseCollectionsAssembler : public CodeStubAssembler { + public: + explicit BaseCollectionsAssembler(compiler::CodeAssemblerState* state) + : CodeStubAssembler(state) {} + + virtual ~BaseCollectionsAssembler() = default; + + void GotoIfCannotBeHeldWeakly(const TNode obj, + Label* if_cannot_be_held_weakly); + + protected: + enum Variant { kMap, kSet, kWeakMap, kWeakSet }; + + // Adds an entry to a collection. For Maps, properly handles extracting the + // key and value from the entry (see LoadKeyValue()). + void AddConstructorEntry(Variant variant, TNode context, + TNode collection, TNode add_function, + TNode key_value, + Label* if_may_have_side_effects = nullptr, + Label* if_exception = nullptr, + TVariable* var_exception = nullptr); + + // Adds constructor entries to a collection. Choosing a fast path when + // possible. + void AddConstructorEntries(Variant variant, TNode context, + TNode native_context, + TNode collection, + TNode initial_entries); + + // Fast path for adding constructor entries. Assumes the entries are a fast + // JS array (see CodeStubAssembler::BranchIfFastJSArray()). + void AddConstructorEntriesFromFastJSArray(Variant variant, + TNode context, + TNode native_context, + TNode collection, + TNode fast_jsarray, + Label* if_may_have_side_effects); + + // Adds constructor entries to a collection using the iterator protocol. + void AddConstructorEntriesFromIterable(Variant variant, + TNode context, + TNode native_context, + TNode collection, + TNode iterable); + + // Constructs a collection instance. Choosing a fast path when possible. + TNode AllocateJSCollection(TNode context, + TNode constructor, + TNode new_target); + + // Fast path for constructing a collection instance if the constructor + // function has not been modified. + TNode AllocateJSCollectionFast(TNode constructor); + + // Fallback for constructing a collection instance if the constructor function + // has been modified. + TNode AllocateJSCollectionSlow(TNode context, + TNode constructor, + TNode new_target); + + // Allocates the backing store for a collection. + virtual TNode AllocateTable( + Variant variant, TNode at_least_space_for) = 0; + + // Main entry point for a collection constructor builtin. + void GenerateConstructor(Variant variant, + Handle constructor_function_name, + TNode new_target, TNode argc, + TNode context); + + // Retrieves the collection function that adds an entry. `set` for Maps and + // `add` for Sets. + TNode GetAddFunction(Variant variant, TNode context, + TNode collection); + + // Retrieves the collection constructor function. + TNode GetConstructor(Variant variant, + TNode native_context); + + // Retrieves the initial collection function that adds an entry. Should only + // be called when it is certain that a collection prototype's map hasn't been + // changed. + TNode GetInitialAddFunction(Variant variant, + TNode native_context); + + // Checks whether {collection}'s initial add/set function has been modified + // (depending on {variant}, loaded from {native_context}). + void GotoIfInitialAddFunctionModified(Variant variant, + TNode native_context, + TNode collection, + Label* if_modified); + + // Gets root index for the name of the add/set function. + RootIndex GetAddFunctionNameIndex(Variant variant); + + // Retrieves the offset to access the backing table from the collection. + int GetTableOffset(Variant variant); + + // Estimates the number of entries the collection will have after adding the + // entries passed in the constructor. AllocateTable() can use this to avoid + // the time of growing/rehashing when adding the constructor entries. + TNode EstimatedInitialSize(TNode initial_entries, + TNode is_fast_jsarray); + + // Determines whether the collection's prototype has been modified. + TNode HasInitialCollectionPrototype(Variant variant, + TNode native_context, + TNode collection); + + // Gets the initial prototype map for given collection {variant}. + TNode GetInitialCollectionPrototype(Variant variant, + TNode native_context); + + // Loads an element from a fixed array. If the element is the hole, returns + // `undefined`. + TNode LoadAndNormalizeFixedArrayElement(TNode elements, + TNode index); + + // Loads an element from a fixed double array. If the element is the hole, + // returns `undefined`. + TNode LoadAndNormalizeFixedDoubleArrayElement( + TNode elements, TNode index); +}; + +class WeakCollectionsBuiltinsAssembler : public BaseCollectionsAssembler { + public: + explicit WeakCollectionsBuiltinsAssembler(compiler::CodeAssemblerState* state) + : BaseCollectionsAssembler(state) {} + + protected: + void AddEntry(TNode table, TNode key_index, + TNode key, TNode value, + TNode number_of_elements); + + TNode AllocateTable(Variant variant, + TNode at_least_space_for) override; + + TNode GetHash(const TNode key, Label* if_no_hash); + // Generates and sets the identity for a JSRececiver. + TNode CreateIdentityHash(TNode receiver); + TNode EntryMask(TNode capacity); + + // Builds code that finds the EphemeronHashTable entry for a {key} using the + // comparison code generated by {key_compare}. The key index is returned if + // the {key} is found. + using KeyComparator = + std::function entry_key, Label* if_same)>; + TNode FindKeyIndex(TNode table, TNode key_hash, + TNode entry_mask, + const KeyComparator& key_compare); + + // Builds code that finds an EphemeronHashTable entry available for a new + // entry. + TNode FindKeyIndexForInsertion(TNode table, + TNode key_hash, + TNode entry_mask); + + // Builds code that finds the EphemeronHashTable entry with key that matches + // {key} and returns the entry's key index. If {key} cannot be found, jumps to + // {if_not_found}. + TNode FindKeyIndexForKey(TNode table, TNode key, + TNode hash, + TNode entry_mask, + Label* if_not_found); + + TNode InsufficientCapacityToAdd(TNode capacity, + TNode number_of_elements, + TNode number_of_deleted); + TNode KeyIndexFromEntry(TNode entry); + + TNode LoadNumberOfElements(TNode table, + int offset); + TNode LoadNumberOfDeleted(TNode table, + int offset = 0); + TNode LoadTable(TNode collection); + TNode LoadTableCapacity(TNode table); + + void RemoveEntry(TNode table, TNode key_index, + TNode number_of_elements); + TNode ShouldRehash(TNode number_of_elements, + TNode number_of_deleted); + TNode ShouldShrink(TNode capacity, + TNode number_of_elements); + TNode ValueIndexFromKeyIndex(TNode key_index); +}; + } // namespace internal } // namespace v8 diff --git a/deps/v8/src/builtins/builtins-weak-refs.cc b/deps/v8/src/builtins/builtins-weak-refs.cc index aee330b4bd4f3c..eef9de1cb1d66f 100644 --- a/deps/v8/src/builtins/builtins-weak-refs.cc +++ b/deps/v8/src/builtins/builtins-weak-refs.cc @@ -9,31 +9,28 @@ namespace v8 { namespace internal { +// https://tc39.es/ecma262/#sec-finalization-registry.prototype.unregister BUILTIN(FinalizationRegistryUnregister) { HandleScope scope(isolate); const char* method_name = "FinalizationRegistry.prototype.unregister"; // 1. Let finalizationGroup be the this value. // - // 2. If Type(finalizationGroup) is not Object, throw a TypeError - // exception. - // - // 3. If finalizationGroup does not have a [[Cells]] internal slot, - // throw a TypeError exception. + // 2. Perform ? RequireInternalSlot(finalizationRegistry, [[Cells]]). CHECK_RECEIVER(JSFinalizationRegistry, finalization_registry, method_name); Handle unregister_token = args.atOrUndefined(isolate, 1); - // 4. If Type(unregisterToken) is not Object, throw a TypeError exception. - if (!unregister_token->IsJSReceiver()) { + // 3. If CanBeHeldWeakly(unregisterToken) is false, throw a TypeError + // exception. + if (!unregister_token->CanBeHeldWeakly()) { THROW_NEW_ERROR_RETURN_FAILURE( - isolate, - NewTypeError(MessageTemplate::kWeakRefsUnregisterTokenMustBeObject, - unregister_token)); + isolate, NewTypeError(MessageTemplate::kInvalidWeakRefsUnregisterToken, + unregister_token)); } bool success = JSFinalizationRegistry::Unregister( - finalization_registry, Handle::cast(unregister_token), + finalization_registry, Handle::cast(unregister_token), isolate); return *isolate->factory()->ToBoolean(success); diff --git a/deps/v8/src/builtins/cast.tq b/deps/v8/src/builtins/cast.tq index 5cb6e0bc92e0ed..0d347e3dd35cdc 100644 --- a/deps/v8/src/builtins/cast.tq +++ b/deps/v8/src/builtins/cast.tq @@ -697,6 +697,21 @@ Cast(o: HeapObject): JSReceiver|Null } } +Cast(implicit context: Context)(o: Object): JSReceiver|Symbol + labels CastError { + typeswitch (o) { + case (o: JSReceiver): { + return o; + } + case (o: Symbol): { + return o; + } + case (Object): { + goto CastError; + } + } +} + Cast(o: Object): Smi|PromiseReaction labels CastError { typeswitch (o) { case (o: Smi): { diff --git a/deps/v8/src/builtins/finalization-registry.tq b/deps/v8/src/builtins/finalization-registry.tq index 38cae7ed20b9ff..c4e7f038ade07f 100644 --- a/deps/v8/src/builtins/finalization-registry.tq +++ b/deps/v8/src/builtins/finalization-registry.tq @@ -1,6 +1,7 @@ // Copyright 2020 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. +#include "src/builtins/builtins-collections-gen.h" namespace runtime { extern runtime @@ -15,6 +16,9 @@ extern transitioning macro RemoveFinalizationRegistryCellFromUnregisterTokenMap( JSFinalizationRegistry, WeakCell): void; +extern macro WeakCollectionsBuiltinsAssembler::GotoIfCannotBeHeldWeakly(JSAny): + void labels NotWeakKey; + macro SplitOffTail(weakCell: WeakCell): WeakCell|Undefined { const weakCellTail = weakCell.next; weakCell.next = Undefined; @@ -125,6 +129,7 @@ FinalizationRegistryConstructor( return finalizationRegistry; } +// https://tc39.es/ecma262/#sec-finalization-registry.prototype.register transitioning javascript builtin FinalizationRegistryRegister( js-implicit context: NativeContext, receiver: JSAny)(...arguments): JSAny { @@ -134,33 +139,32 @@ FinalizationRegistryRegister( ThrowTypeError( MessageTemplate::kIncompatibleMethodReceiver, 'FinalizationRegistry.prototype.register', receiver); - // 3. If Type(target) is not Object, throw a TypeError exception. - const target = Cast(arguments[0]) otherwise ThrowTypeError( - MessageTemplate::kWeakRefsRegisterTargetMustBeObject); + // 3. If CanBeHeldWeakly(target) is false, throw a TypeError exception. + GotoIfCannotBeHeldWeakly(arguments[0]) + otherwise ThrowTypeError(MessageTemplate::kInvalidWeakRefsRegisterTarget); + + const target = UnsafeCast<(JSReceiver | Symbol)>(arguments[0]); const heldValue = arguments[1]; // 4. If SameValue(target, heldValue), throw a TypeError exception. if (target == heldValue) { ThrowTypeError( MessageTemplate::kWeakRefsRegisterTargetAndHoldingsMustNotBeSame); } - // 5. If Type(unregisterToken) is not Object, + // 5. If CanBeHeldWeakly(unregisterToken) is false, // a. If unregisterToken is not undefined, throw a TypeError exception. // b. Set unregisterToken to empty. const unregisterTokenRaw = arguments[2]; - let unregisterToken: JSReceiver|Undefined; - typeswitch (unregisterTokenRaw) { - case (Undefined): { - unregisterToken = Undefined; - } - case (unregisterTokenObj: JSReceiver): { - unregisterToken = unregisterTokenObj; - } - case (JSAny): deferred { - ThrowTypeError( - MessageTemplate::kWeakRefsUnregisterTokenMustBeObject, - unregisterTokenRaw); - } + let unregisterToken: JSReceiver|Undefined|Symbol; + + if (IsUndefined(unregisterTokenRaw)) { + unregisterToken = Undefined; + } else { + GotoIfCannotBeHeldWeakly(unregisterTokenRaw) + otherwise ThrowTypeError( + MessageTemplate::kInvalidWeakRefsUnregisterToken, unregisterTokenRaw); + unregisterToken = UnsafeCast<(JSReceiver | Symbol)>(unregisterTokenRaw); } + // 6. Let cell be the Record { [[WeakRefTarget]] : target, [[HeldValue]]: // heldValue, [[UnregisterToken]]: unregisterToken }. // Allocate the WeakCell object in the old space, because 1) WeakCell weakness diff --git a/deps/v8/src/builtins/weak-ref.tq b/deps/v8/src/builtins/weak-ref.tq index 56d3fc1c4314bf..ea0842c1ac2dd2 100644 --- a/deps/v8/src/builtins/weak-ref.tq +++ b/deps/v8/src/builtins/weak-ref.tq @@ -2,15 +2,18 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include 'src/builtins/builtins-collections-gen.h' + namespace runtime { -extern runtime JSWeakRefAddToKeptObjects(implicit context: Context)(JSReceiver): - void; +extern runtime JSWeakRefAddToKeptObjects(implicit context: Context)( + JSReceiver | Symbol): void; } // namespace runtime namespace weakref { +// https://tc39.es/ecma262/#sec-weak-ref-target transitioning javascript builtin WeakRefConstructor( js-implicit context: NativeContext, receiver: JSAny, newTarget: JSAny, @@ -19,15 +22,17 @@ WeakRefConstructor( if (newTarget == Undefined) { ThrowTypeError(MessageTemplate::kConstructorNotFunction, 'WeakRef'); } - // 2. If Type(target) is not Object, throw a TypeError exception. - const weakTarget = Cast(weakTarget) otherwise - ThrowTypeError( - MessageTemplate::kWeakRefsWeakRefConstructorTargetMustBeObject); + + // 2. If CanBeHeldWeakly(weakTarget) is false, throw a TypeError exception. + GotoIfCannotBeHeldWeakly(weakTarget) otherwise ThrowTypeError( + MessageTemplate::kInvalidWeakRefsWeakRefConstructorTarget); + // 3. Let weakRef be ? OrdinaryCreateFromConstructor(NewTarget, // "%WeakRefPrototype%", « [[WeakRefTarget]] »). const map = GetDerivedMap(target, UnsafeCast(newTarget)); const weakRef = UnsafeCast(AllocateFastOrSlowJSObjectFromMap(map)); // 4. Perfom ! AddToKeptObjects(target). + const weakTarget = UnsafeCast<(JSReceiver | Symbol)>(weakTarget); runtime::JSWeakRefAddToKeptObjects(weakTarget); // 5. Set weakRef.[[WeakRefTarget]] to target. weakRef.target = weakTarget; @@ -52,7 +57,8 @@ WeakRefDeref(js-implicit context: NativeContext, receiver: JSAny)(): JSAny { if (target != Undefined) { // JSWeakRefAddToKeptObjects might allocate and cause a GC, but it // won't clear `target` since we hold it here on the stack. - runtime::JSWeakRefAddToKeptObjects(UnsafeCast(target)); + runtime::JSWeakRefAddToKeptObjects( + UnsafeCast<(JSReceiver | Symbol)>(target)); } return target; } diff --git a/deps/v8/src/common/message-template.h b/deps/v8/src/common/message-template.h index 22fde49ffd1673..a37d278d6e17c9 100644 --- a/deps/v8/src/common/message-template.h +++ b/deps/v8/src/common/message-template.h @@ -633,17 +633,15 @@ namespace internal { T(TraceEventPhaseError, "Trace event phase must be a number.") \ T(TraceEventIDError, "Trace event id must be a number.") \ /* Weak refs */ \ - T(WeakRefsUnregisterTokenMustBeObject, \ - "unregisterToken ('%') must be an object") \ + T(InvalidWeakRefsUnregisterToken, "Invalid unregisterToken ('%')") \ T(WeakRefsCleanupMustBeCallable, \ "FinalizationRegistry: cleanup must be callable") \ - T(WeakRefsRegisterTargetMustBeObject, \ - "FinalizationRegistry.prototype.register: target must be an object") \ + T(InvalidWeakRefsRegisterTarget, \ + "FinalizationRegistry.prototype.register: invalid target") \ T(WeakRefsRegisterTargetAndHoldingsMustNotBeSame, \ "FinalizationRegistry.prototype.register: target and holdings must not " \ "be same") \ - T(WeakRefsWeakRefConstructorTargetMustBeObject, \ - "WeakRef: target must be an object") \ + T(InvalidWeakRefsWeakRefConstructorTarget, "WeakRef: invalid target") \ T(OptionalChainingNoNew, "Invalid optional chain from new expression") \ T(OptionalChainingNoSuper, "Invalid optional chain from super property") \ T(OptionalChainingNoTemplate, "Invalid tagged template on optional chain") \ diff --git a/deps/v8/src/diagnostics/objects-debug.cc b/deps/v8/src/diagnostics/objects-debug.cc index 42e9dff7e363fd..a4ab4e6299bef8 100644 --- a/deps/v8/src/diagnostics/objects-debug.cc +++ b/deps/v8/src/diagnostics/objects-debug.cc @@ -1240,7 +1240,7 @@ void JSSharedStruct::JSSharedStructVerify(Isolate* isolate) { void WeakCell::WeakCellVerify(Isolate* isolate) { CHECK(IsWeakCell()); - CHECK(target().IsJSReceiver() || target().IsUndefined(isolate)); + CHECK(target().IsUndefined(isolate) || target().CanBeHeldWeakly()); CHECK(prev().IsWeakCell() || prev().IsUndefined(isolate)); if (prev().IsWeakCell()) { @@ -1268,7 +1268,7 @@ void WeakCell::WeakCellVerify(Isolate* isolate) { void JSWeakRef::JSWeakRefVerify(Isolate* isolate) { CHECK(IsJSWeakRef()); JSObjectVerify(isolate); - CHECK(target().IsUndefined(isolate) || target().IsJSReceiver()); + CHECK(target().IsUndefined(isolate) || target().CanBeHeldWeakly()); } void JSFinalizationRegistry::JSFinalizationRegistryVerify(Isolate* isolate) { diff --git a/deps/v8/src/flags/flag-definitions.h b/deps/v8/src/flags/flag-definitions.h index f02ef309d69e45..c101c5730c98f2 100644 --- a/deps/v8/src/flags/flag-definitions.h +++ b/deps/v8/src/flags/flag-definitions.h @@ -318,7 +318,7 @@ DEFINE_BOOL(harmony_shipping, true, "enable all shipped harmony features") #endif // Features that are complete (but still behind the --harmony flag). -#define HARMONY_STAGED_BASE(V) \ +#define HARMONY_STAGED_BASE(V) \ V(harmony_array_grouping, "harmony array grouping") #ifdef V8_INTL_SUPPORT diff --git a/deps/v8/src/heap/heap.cc b/deps/v8/src/heap/heap.cc index 02eefd9e4fdc78..53871476c72905 100644 --- a/deps/v8/src/heap/heap.cc +++ b/deps/v8/src/heap/heap.cc @@ -6924,7 +6924,7 @@ void Heap::RemoveDirtyFinalizationRegistriesOnContext(NativeContext context) { set_dirty_js_finalization_registries_list_tail(prev); } -void Heap::KeepDuringJob(Handle target) { +void Heap::KeepDuringJob(Handle target) { DCHECK(weak_refs_keep_during_job().IsUndefined() || weak_refs_keep_during_job().IsOrderedHashSet()); Handle table; diff --git a/deps/v8/src/heap/heap.h b/deps/v8/src/heap/heap.h index b0a8757ca96ab6..a3817f90ed3014 100644 --- a/deps/v8/src/heap/heap.h +++ b/deps/v8/src/heap/heap.h @@ -955,7 +955,7 @@ class Heap { return is_finalization_registry_cleanup_task_posted_; } - V8_EXPORT_PRIVATE void KeepDuringJob(Handle target); + V8_EXPORT_PRIVATE void KeepDuringJob(Handle target); void ClearKeptObjects(); // =========================================================================== diff --git a/deps/v8/src/heap/mark-compact.cc b/deps/v8/src/heap/mark-compact.cc index ef0b67ca2b6274..0baf08ee74a35b 100644 --- a/deps/v8/src/heap/mark-compact.cc +++ b/deps/v8/src/heap/mark-compact.cc @@ -3027,7 +3027,7 @@ void MarkCompactCollector::ClearJSWeakRefs() { }; HeapObject target = HeapObject::cast(weak_cell.target()); if (!non_atomic_marking_state()->IsBlackOrGrey(target)) { - DCHECK(!target.IsUndefined()); + DCHECK(target.CanBeHeldWeakly()); // The value of the WeakCell is dead. JSFinalizationRegistry finalization_registry = JSFinalizationRegistry::cast(weak_cell.finalization_registry()); @@ -3049,6 +3049,7 @@ void MarkCompactCollector::ClearJSWeakRefs() { HeapObject unregister_token = weak_cell.unregister_token(); if (!non_atomic_marking_state()->IsBlackOrGrey(unregister_token)) { + DCHECK(unregister_token.CanBeHeldWeakly()); // The unregister token is dead. Remove any corresponding entries in the // key map. Multiple WeakCell with the same token will have all their // unregister_token field set to undefined when processing the first @@ -3057,7 +3058,7 @@ void MarkCompactCollector::ClearJSWeakRefs() { JSFinalizationRegistry finalization_registry = JSFinalizationRegistry::cast(weak_cell.finalization_registry()); finalization_registry.RemoveUnregisterToken( - JSReceiver::cast(unregister_token), isolate(), + unregister_token, isolate(), JSFinalizationRegistry::kKeepMatchedCellsInRegistry, gc_notify_updated_slot); } else { diff --git a/deps/v8/src/inspector/value-mirror.cc b/deps/v8/src/inspector/value-mirror.cc index c2fa9b46cc0067..a345a8db38675e 100644 --- a/deps/v8/src/inspector/value-mirror.cc +++ b/deps/v8/src/inspector/value-mirror.cc @@ -656,6 +656,18 @@ class SymbolMirror final : public ValueMirror { .build(); } + void buildEntryPreview( + v8::Local context, int* nameLimit, int* indexLimit, + std::unique_ptr* preview) const override { + *preview = + ObjectPreview::create() + .setType(RemoteObject::TypeEnum::Symbol) + .setDescription(descriptionForSymbol(context, m_symbol)) + .setOverflow(false) + .setProperties(std::make_unique>()) + .build(); + } + v8::Local v8Value() const override { return m_symbol; } protocol::Response buildWebDriverValue( diff --git a/deps/v8/src/objects/js-weak-refs-inl.h b/deps/v8/src/objects/js-weak-refs-inl.h index 76e6e075e5ded6..0385d59efb7e56 100644 --- a/deps/v8/src/objects/js-weak-refs-inl.h +++ b/deps/v8/src/objects/js-weak-refs-inl.h @@ -5,10 +5,9 @@ #ifndef V8_OBJECTS_JS_WEAK_REFS_INL_H_ #define V8_OBJECTS_JS_WEAK_REFS_INL_H_ -#include "src/objects/js-weak-refs.h" - #include "src/api/api-inl.h" #include "src/heap/heap-write-barrier-inl.h" +#include "src/objects/js-weak-refs.h" #include "src/objects/smi-inl.h" // Has to be the last include (doesn't have include guards): @@ -55,7 +54,7 @@ void JSFinalizationRegistry::RegisterWeakCellWithUnregisterToken( bool JSFinalizationRegistry::Unregister( Handle finalization_registry, - Handle unregister_token, Isolate* isolate) { + Handle unregister_token, Isolate* isolate) { // Iterate through the doubly linked list of WeakCells associated with the // key. Each WeakCell will be in the "active_cells" or "cleared_cells" list of // its FinalizationRegistry; remove it from there. @@ -66,7 +65,7 @@ bool JSFinalizationRegistry::Unregister( template bool JSFinalizationRegistry::RemoveUnregisterToken( - JSReceiver unregister_token, Isolate* isolate, + HeapObject unregister_token, Isolate* isolate, RemoveUnregisterTokenMode removal_mode, GCNotifyUpdatedSlotCallback gc_notify_updated_slot) { // This method is called from both FinalizationRegistry#unregister and for @@ -171,7 +170,7 @@ void WeakCell::Nullify(Isolate* isolate, // only called for WeakCells which haven't been unregistered yet, so they will // be in the active_cells list. (The caller must guard against calling this // for unregistered WeakCells by checking that the target is not undefined.) - DCHECK(target().IsJSReceiver()); + DCHECK(target().CanBeHeldWeakly()); set_target(ReadOnlyRoots(isolate).undefined_value()); JSFinalizationRegistry fr = @@ -217,7 +216,7 @@ void WeakCell::RemoveFromFinalizationRegistryCells(Isolate* isolate) { // It's important to set_target to undefined here. This guards that we won't // call Nullify (which assumes that the WeakCell is in active_cells). - DCHECK(target().IsUndefined() || target().IsJSReceiver()); + DCHECK(target().IsUndefined() || target().CanBeHeldWeakly()); set_target(ReadOnlyRoots(isolate).undefined_value()); JSFinalizationRegistry fr = diff --git a/deps/v8/src/objects/js-weak-refs.h b/deps/v8/src/objects/js-weak-refs.h index f678234ff81afc..64ff9573f6b2df 100644 --- a/deps/v8/src/objects/js-weak-refs.h +++ b/deps/v8/src/objects/js-weak-refs.h @@ -37,7 +37,7 @@ class JSFinalizationRegistry Handle weak_cell, Isolate* isolate); inline static bool Unregister( Handle finalization_registry, - Handle unregister_token, Isolate* isolate); + Handle unregister_token, Isolate* isolate); // RemoveUnregisterToken is called from both Unregister and during GC. Since // it modifies slots in key_map and WeakCells and the normal write barrier is @@ -49,7 +49,7 @@ class JSFinalizationRegistry }; template inline bool RemoveUnregisterToken( - JSReceiver unregister_token, Isolate* isolate, + HeapObject unregister_token, Isolate* isolate, RemoveUnregisterTokenMode removal_mode, GCNotifyUpdatedSlotCallback gc_notify_updated_slot); diff --git a/deps/v8/src/objects/js-weak-refs.tq b/deps/v8/src/objects/js-weak-refs.tq index c687ab50016776..c9fa8ee30bdb57 100644 --- a/deps/v8/src/objects/js-weak-refs.tq +++ b/deps/v8/src/objects/js-weak-refs.tq @@ -20,8 +20,8 @@ extern class JSFinalizationRegistry extends JSObject { extern class WeakCell extends HeapObject { finalization_registry: Undefined|JSFinalizationRegistry; - target: Undefined|JSReceiver; - unregister_token: Undefined|JSReceiver; + target: Undefined|JSReceiver|Symbol; + unregister_token: Undefined|JSReceiver|Symbol; holdings: JSAny; // For storing doubly linked lists of WeakCells in JSFinalizationRegistry's @@ -38,5 +38,6 @@ extern class WeakCell extends HeapObject { key_list_prev: Undefined|WeakCell; key_list_next: Undefined|WeakCell; } - -extern class JSWeakRef extends JSObject { target: Undefined|JSReceiver; } +extern class JSWeakRef extends JSObject { + target: Undefined|JSReceiver|Symbol; +} diff --git a/deps/v8/src/objects/objects-inl.h b/deps/v8/src/objects/objects-inl.h index 82d8776ef34ca8..a1f73d23a9ca86 100644 --- a/deps/v8/src/objects/objects-inl.h +++ b/deps/v8/src/objects/objects-inl.h @@ -1206,6 +1206,20 @@ MaybeHandle Object::Share(Isolate* isolate, Handle value, throw_if_cannot_be_shared); } +// https://tc39.es/ecma262/#sec-canbeheldweakly +bool Object::CanBeHeldWeakly() const { + if (IsJSReceiver()) { + // TODO(v8:12547) Shared structs and arrays should only be able to point + // to shared values in weak collections. For now, disallow them as weak + // collection keys. + if (FLAG_harmony_struct) { + return !IsJSSharedStruct(); + } + return true; + } + return IsSymbol() && !Symbol::cast(*this).is_in_public_symbol_table(); +} + Handle ObjectHashTableShape::AsHandle(Handle key) { return key; } diff --git a/deps/v8/src/objects/objects.h b/deps/v8/src/objects/objects.h index 316f870e31f33c..a3ad333f0a91d9 100644 --- a/deps/v8/src/objects/objects.h +++ b/deps/v8/src/objects/objects.h @@ -770,6 +770,11 @@ class Object : public TaggedImpl { Handle value, ShouldThrow throw_if_cannot_be_shared); + // Whether this Object can be held weakly, i.e. whether it can be used as a + // key in WeakMap, as a key in WeakSet, as the target of a WeakRef, or as a + // target or unregister token of a FinalizationRegistry. + inline bool CanBeHeldWeakly() const; + protected: inline Address field_address(size_t offset) const { return ptr() + offset - kHeapObjectTag; diff --git a/deps/v8/src/runtime/runtime-collections.cc b/deps/v8/src/runtime/runtime-collections.cc index 38279e2d0e27d6..98619ff0a35f07 100644 --- a/deps/v8/src/runtime/runtime-collections.cc +++ b/deps/v8/src/runtime/runtime-collections.cc @@ -82,7 +82,7 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) { int hash = args.smi_value_at(2); #ifdef DEBUG - DCHECK(key->IsJSReceiver()); + DCHECK(key->CanBeHeldWeakly()); DCHECK(EphemeronHashTable::IsKey(ReadOnlyRoots(isolate), *key)); Handle table( EphemeronHashTable::cast(weak_collection->table()), isolate); @@ -105,7 +105,7 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionSet) { int hash = args.smi_value_at(3); #ifdef DEBUG - DCHECK(key->IsJSReceiver()); + DCHECK(key->CanBeHeldWeakly()); DCHECK(EphemeronHashTable::IsKey(ReadOnlyRoots(isolate), *key)); Handle table( EphemeronHashTable::cast(weak_collection->table()), isolate); diff --git a/deps/v8/src/runtime/runtime-test.cc b/deps/v8/src/runtime/runtime-test.cc index fcaada5711d487..7dcc05ab485408 100644 --- a/deps/v8/src/runtime/runtime-test.cc +++ b/deps/v8/src/runtime/runtime-test.cc @@ -27,6 +27,7 @@ #include "src/logging/counters.h" #include "src/objects/heap-object-inl.h" #include "src/objects/js-array-inl.h" +#include "src/objects/js-collection-inl.h" #include "src/objects/js-function-inl.h" #include "src/objects/js-regexp-inl.h" #include "src/objects/managed-inl.h" @@ -1730,5 +1731,14 @@ RUNTIME_FUNCTION(Runtime_SharedGC) { return ReadOnlyRoots(isolate).undefined_value(); } +RUNTIME_FUNCTION(Runtime_GetWeakCollectionSize) { + HandleScope scope(isolate); + DCHECK_EQ(1, args.length()); + Handle collection = args.at(0); + + return Smi::FromInt( + EphemeronHashTable::cast(collection->table()).NumberOfElements()); +} + } // namespace internal } // namespace v8 diff --git a/deps/v8/src/runtime/runtime-weak-refs.cc b/deps/v8/src/runtime/runtime-weak-refs.cc index f3c6f63ebcb6d7..ff60813b434bef 100644 --- a/deps/v8/src/runtime/runtime-weak-refs.cc +++ b/deps/v8/src/runtime/runtime-weak-refs.cc @@ -2,10 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "src/runtime/runtime-utils.h" - #include "src/execution/arguments-inl.h" #include "src/objects/js-weak-refs-inl.h" +#include "src/runtime/runtime-utils.h" namespace v8 { namespace internal { @@ -44,7 +43,8 @@ RUNTIME_FUNCTION( RUNTIME_FUNCTION(Runtime_JSWeakRefAddToKeptObjects) { HandleScope scope(isolate); DCHECK_EQ(1, args.length()); - Handle object = args.at(0); + Handle object = args.at(0); + DCHECK(object->CanBeHeldWeakly()); isolate->heap()->KeepDuringJob(object); diff --git a/deps/v8/src/runtime/runtime.h b/deps/v8/src/runtime/runtime.h index 877b277a5e26cb..d0f81cf2f7672a 100644 --- a/deps/v8/src/runtime/runtime.h +++ b/deps/v8/src/runtime/runtime.h @@ -498,6 +498,7 @@ namespace internal { F(GetInitializerFunction, 1, 1) \ F(GetOptimizationStatus, 1, 1) \ F(GetUndetectable, 0, 1) \ + F(GetWeakCollectionSize, 1, 1) \ F(GlobalPrint, 1, 1) \ F(HasDictionaryElements, 1, 1) \ F(HasDoubleElements, 1, 1) \ diff --git a/deps/v8/test/inspector/debugger/object-preview-internal-properties-expected.txt b/deps/v8/test/inspector/debugger/object-preview-internal-properties-expected.txt index 56f992237a2ebd..1bdef8231b2523 100644 --- a/deps/v8/test/inspector/debugger/object-preview-internal-properties-expected.txt +++ b/deps/v8/test/inspector/debugger/object-preview-internal-properties-expected.txt @@ -159,6 +159,88 @@ expression: new WeakSet([{}]) ] +Running test: symbolsAsKeysInEntries +expression: new Map([[Symbol('key1'), 1]]) +{ + name : size + type : number + value : 1 +} +[[Entries]]: +[ + [0] : { + key : { + description : Symbol(key1) + overflow : false + properties : [ + ] + type : symbol + } + value : { + description : 1 + overflow : false + properties : [ + ] + type : number + } + } +] + +expression: new Set([Symbol('key2')]) +{ + name : size + type : number + value : 1 +} +[[Entries]]: +[ + [0] : { + value : { + description : Symbol(key2) + overflow : false + properties : [ + ] + type : symbol + } + } +] + +expression: new WeakMap([[Symbol('key3'), 2]]) +[[Entries]]: +[ + [0] : { + key : { + description : Symbol(key3) + overflow : false + properties : [ + ] + type : symbol + } + value : { + description : 2 + overflow : false + properties : [ + ] + type : number + } + } +] + +expression: new WeakSet([Symbol('key4')]) +[[Entries]]: +[ + [0] : { + value : { + description : Symbol(key4) + overflow : false + properties : [ + ] + type : symbol + } + } +] + + Running test: iteratorObject expression: (new Map([[1,2]])).entries() [[Entries]]: diff --git a/deps/v8/test/inspector/debugger/object-preview-internal-properties.js b/deps/v8/test/inspector/debugger/object-preview-internal-properties.js index f542683aa49159..026c4bc1c364c9 100644 --- a/deps/v8/test/inspector/debugger/object-preview-internal-properties.js +++ b/deps/v8/test/inspector/debugger/object-preview-internal-properties.js @@ -1,8 +1,6 @@ // Copyright 2016 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-class-fields let {session, contextGroup, Protocol} = InspectorTest.start("Check internal properties reported in object preview."); @@ -45,6 +43,15 @@ InspectorTest.runTestSuite([ .then(next); }, + function symbolsAsKeysInEntries(next) + { + checkExpression("new Map([[Symbol('key1'), 1]])") + .then(() => checkExpression("new Set([Symbol('key2')])")) + .then(() => checkExpression("new WeakMap([[Symbol('key3'), 2]])")) + .then(() => checkExpression("new WeakSet([Symbol('key4')])")) + .then(next); + }, + function iteratorObject(next) { checkExpression("(new Map([[1,2]])).entries()") diff --git a/deps/v8/test/message/fail/weak-refs-register1.out b/deps/v8/test/message/fail/weak-refs-register1.out index aa4cbc2fa22e1e..7e7bf12791be2d 100644 --- a/deps/v8/test/message/fail/weak-refs-register1.out +++ b/deps/v8/test/message/fail/weak-refs-register1.out @@ -1,6 +1,6 @@ -*%(basename)s:*: TypeError: FinalizationRegistry.prototype.register: target must be an object +*%(basename)s:*: TypeError: FinalizationRegistry.prototype.register: invalid target fg.register(1); ^ -TypeError: FinalizationRegistry.prototype.register: target must be an object +TypeError: FinalizationRegistry.prototype.register: invalid target at FinalizationRegistry.register () at *%(basename)s:*:4 diff --git a/deps/v8/test/message/fail/weak-refs-unregister.out b/deps/v8/test/message/fail/weak-refs-unregister.out index 52945869838778..a6a66ea709d747 100644 --- a/deps/v8/test/message/fail/weak-refs-unregister.out +++ b/deps/v8/test/message/fail/weak-refs-unregister.out @@ -1,6 +1,6 @@ -*%(basename)s:*: TypeError: unregisterToken ('1') must be an object +*%(basename)s:*: TypeError: Invalid unregisterToken ('1') fg.unregister(1); ^ -TypeError: unregisterToken ('1') must be an object +TypeError: Invalid unregisterToken ('1') at FinalizationRegistry.unregister () at *%(basename)s:*:4 diff --git a/deps/v8/test/mjsunit/es6/collections.js b/deps/v8/test/mjsunit/es6/collections.js index feae6294392365..2b608e1f5da10d 100644 --- a/deps/v8/test/mjsunit/es6/collections.js +++ b/deps/v8/test/mjsunit/es6/collections.js @@ -77,7 +77,6 @@ function TestInvalidCalls(m) { assertThrows(function () { m.set(null, 0) }, TypeError); assertThrows(function () { m.set(0, 0) }, TypeError); assertThrows(function () { m.set('a-key', 0) }, TypeError); - assertThrows(function () { m.set(Symbol(), 0) }, TypeError); } TestInvalidCalls(new WeakMap); diff --git a/deps/v8/test/mjsunit/harmony/regress/regress-crbug-1372500.js b/deps/v8/test/mjsunit/harmony/regress/regress-crbug-1372500.js new file mode 100644 index 00000000000000..bccca0b40761fe --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/regress/regress-crbug-1372500.js @@ -0,0 +1,14 @@ +// 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. + +// Flags: --expose-gc + +// Register an object in a FinalizationRegistry with a Symbol as the unregister +// token. +let fr = new FinalizationRegistry(function () {}); +(function register() { + fr.register({}, "holdings", Symbol('unregisterToken')); +})(); +// The unregister token should be dead, trigger its collection. +gc(); diff --git a/deps/v8/test/mjsunit/harmony/symbol-as-weakmap-key.js b/deps/v8/test/mjsunit/harmony/symbol-as-weakmap-key.js new file mode 100644 index 00000000000000..9070b9309a5aa1 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/symbol-as-weakmap-key.js @@ -0,0 +1,108 @@ +// 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. + +// Flags: --expose-gc --allow-natives-syntax --noincremental-marking + +(function TestWeakMapWithNonRegisteredSymbolKey() { + const key = Symbol('123'); + const value = 1; + const map = new WeakMap(); + assertFalse(map.has(key)); + assertSame(undefined, map.get(key)); + assertFalse(map.delete(key)); + assertSame(map, map.set(key, value)); + assertSame(value, map.get(key)); + assertTrue(map.has(key)); + assertTrue(map.delete(key)); + assertFalse(map.has(key)); + assertSame(undefined, map.get(key)); + assertFalse(map.delete(key)); + assertFalse(map.has(key)); + assertSame(undefined, map.get(key)); +})(); + +(function TestWeakMapWithNonRegisteredSymbolKeyGC() { + const map = new WeakMap(); + + const outerKey = Symbol('234'); + const outerValue = 1; + map.set(outerKey, outerValue); + (function () { + const innerKey = Symbol('123'); + const innerValue = 1; + map.set(innerKey, innerValue); + assertTrue(map.has(innerKey)); + assertSame(innerValue, map.get(innerKey)); + })(); + gc(); + assertTrue(map.has(outerKey)); + assertSame(outerValue, map.get(outerKey)); + assertEquals(1, %GetWeakCollectionSize(map)); +})(); + +(function TestWeakMapWithRegisteredSymbolKey() { + const key = Symbol.for('123'); + const value = 1; + const map = new WeakMap(); + assertFalse(map.has(key)); + assertSame(undefined, map.get(key)); + assertFalse(map.delete(key)); + assertThrows(() => { + map.set(key, value); + }, TypeError, 'Invalid value used as weak map key'); + assertFalse(map.has(key)); + assertSame(undefined, map.get(key)); + assertFalse(map.delete(key)); + assertFalse(map.has(key)); + assertSame(undefined, map.get(key)); +})(); + +(function TestWeakSetWithNonRegisteredSymbolKey() { + const key = Symbol('123'); + const set = new WeakSet(); + assertFalse(set.has(key)); + assertFalse(set.delete(key)); + assertSame(set, set.add(key)); + assertTrue(set.has(key)); + assertTrue(set.delete(key)); + assertFalse(set.has(key)); + assertFalse(set.delete(key)); + assertFalse(set.has(key)); +})(); + +(function TestWeakSetWithNonRegisteredSymbolKeyGC() { + const set = new WeakSet(); + const outerKey = Symbol('234'); + set.add(outerKey); + (function () { + const innerKey = Symbol('123'); + set.add(innerKey); + assertTrue(set.has(innerKey)); + })(); + assertTrue(set.has(outerKey)); + gc(); + assertEquals(1, %GetWeakCollectionSize(set)); +})(); + +(function TestWeakSetWithRegisteredSymbolKey() { + const key = Symbol.for('123'); + const set = new WeakSet(); + assertFalse(set.has(key)); + assertFalse(set.delete(key)); + + assertThrows(() => { + assertSame(set, set.add(key)); + }, TypeError, 'Invalid value used in weak set'); + + assertFalse(set.has(key)); + assertFalse(set.delete(key)); + assertFalse(set.has(key)); +})(); + +(function TestFinalizationRegistryUnregister() { + const fr = new FinalizationRegistry(function() {}); + const key = {}; + fr.register(Symbol('foo'), "holdings", key); + fr.unregister(key); +})(); diff --git a/deps/v8/test/mjsunit/harmony/weakrefs/basics.js b/deps/v8/test/mjsunit/harmony/weakrefs/basics.js index f879df9a2a63b0..018dc6d5ab1def 100644 --- a/deps/v8/test/mjsunit/harmony/weakrefs/basics.js +++ b/deps/v8/test/mjsunit/harmony/weakrefs/basics.js @@ -47,11 +47,10 @@ (function TestRegisterWithNonObjectTarget() { let fg = new FinalizationRegistry(() => {}); - let message = "FinalizationRegistry.prototype.register: target must be an object"; + let message = "FinalizationRegistry.prototype.register: invalid target"; assertThrows(() => fg.register(1, "holdings"), TypeError, message); assertThrows(() => fg.register(false, "holdings"), TypeError, message); assertThrows(() => fg.register("foo", "holdings"), TypeError, message); - assertThrows(() => fg.register(Symbol(), "holdings"), TypeError, message); assertThrows(() => fg.register(null, "holdings"), TypeError, message); assertThrows(() => fg.register(undefined, "holdings"), TypeError, message); })(); @@ -97,7 +96,6 @@ assertThrows(() => fg.unregister(1), TypeError); assertThrows(() => fg.unregister(1n), TypeError); assertThrows(() => fg.unregister('one'), TypeError); - assertThrows(() => fg.unregister(Symbol()), TypeError); assertThrows(() => fg.unregister(true), TypeError); assertThrows(() => fg.unregister(false), TypeError); assertThrows(() => fg.unregister(undefined), TypeError); @@ -116,12 +114,11 @@ })(); (function TestWeakRefConstructorWithNonObject() { - let message = "WeakRef: target must be an object"; + let message = "WeakRef: invalid target"; assertThrows(() => new WeakRef(), TypeError, message); assertThrows(() => new WeakRef(1), TypeError, message); assertThrows(() => new WeakRef(false), TypeError, message); assertThrows(() => new WeakRef("foo"), TypeError, message); - assertThrows(() => new WeakRef(Symbol()), TypeError, message); assertThrows(() => new WeakRef(null), TypeError, message); assertThrows(() => new WeakRef(undefined), TypeError, message); })(); diff --git a/deps/v8/test/mjsunit/harmony/weakrefs/symbol-as-weakref-target-gc.js b/deps/v8/test/mjsunit/harmony/weakrefs/symbol-as-weakref-target-gc.js new file mode 100644 index 00000000000000..3f663d2b0e0f52 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/weakrefs/symbol-as-weakref-target-gc.js @@ -0,0 +1,39 @@ +// 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. + +// Flags: --expose-gc --noincremental-marking + +(function TestWeakRefWithSymbolGC() { + let weakRef; + { + const innerKey = Symbol('123'); + weakRef = new WeakRef(innerKey); + } + // Since the WeakRef was created during this turn, it is not cleared by GC. + gc(); + assertNotEquals(undefined, weakRef.deref()); + // Next task. + setTimeout(() => { + gc(); + assertEquals(undefined, weakRef.deref()); + }, 0); +})(); + +(function TestFinalizationRegistryWithSymbolGC() { + let cleanUpCalled = false; + const fg = new FinalizationRegistry((target) => { + assertEquals('123', target); + cleanUpCalled = true; + }); + (function () { + const innerKey = Symbol('123'); + fg.register(innerKey, '123'); + })(); + gc(); + assertFalse(cleanUpCalled); + // Check that cleanup callback was called in a follow up task. + setTimeout(() => { + assertTrue(cleanUpCalled); + }, 0); +})(); diff --git a/deps/v8/test/mjsunit/harmony/weakrefs/symbol-as-weakref-target.js b/deps/v8/test/mjsunit/harmony/weakrefs/symbol-as-weakref-target.js new file mode 100644 index 00000000000000..a969f388717a50 --- /dev/null +++ b/deps/v8/test/mjsunit/harmony/weakrefs/symbol-as-weakref-target.js @@ -0,0 +1,39 @@ +// 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. + +(function TestRegisterWithSymbolTarget() { + const fg = new FinalizationRegistry(() => { }); + fg.register(Symbol('123'), 'holdings'); + // Registered symbols cannot be the target. + assertThrows(() => fg.register(Symbol.for('123'), 'holdings'), TypeError); +})(); + +(function TestRegisterWithSymbolUnregisterToken() { + const fg = new FinalizationRegistry(() => { }); + fg.register({}, 'holdings', Symbol('123')); + // Registered symbols cannot be the unregister token. + assertThrows(() => fg.register({}, 'holdings', Symbol.for('123')), TypeError); +})(); + +(function TestRegisterSymbolAndHoldingsSameValue() { + const fg = new FinalizationRegistry(() => {}); + const obj = Symbol('123'); + // SameValue(target, holdings) not ok. + assertThrows(() => fg.register(obj, obj), TypeError); + const holdings = {a: 1}; + fg.register(obj, holdings); +})(); + +(function TestUnregisterWithSymbolUnregisterToken() { + const fg = new FinalizationRegistry(() => {}); + fg.unregister(Symbol('123')); + // Registered symbols cannot be the unregister token. + assertThrows(() => fg.unregister(Symbol.for('123')), TypeError); +})(); + +(function TestWeakRefConstructorWithSymbol() { + new WeakRef(Symbol('123')); + // Registered symbols cannot be the WeakRef target. + assertThrows(() => new WeakRef(Symbol.for('123')), TypeError); +})(); diff --git a/doc/api/errors.md b/doc/api/errors.md index fee0ed89e4b844..88116a7ef6bd60 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2985,6 +2985,12 @@ An attempt was made to use something that was already closed. While using the Performance Timing API (`perf_hooks`), no valid performance entry types are found. + + +### `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG` + +A dynamic import callback was invoked without `--experimental-vm-modules`. + ### `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING` diff --git a/doc/api/vm.md b/doc/api/vm.md index f405d71ef299d4..a8ba0903ad6d68 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -98,7 +98,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} * `importAttributes` {Object} The `"assert"` value passed to the @@ -765,6 +767,9 @@ changes: * `importModuleDynamically` {Function} Called during evaluation of this module when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. + If `--experimental-vm-modules` isn't set, this callback will be ignored + and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `module` {vm.Module} * `importAttributes` {Object} The `"assert"` value passed to the @@ -1022,7 +1027,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API, and should not be - considered stable. + considered stable. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `function` {Function} * `importAttributes` {Object} The `"assert"` value passed to the @@ -1246,7 +1253,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} * `importAttributes` {Object} The `"assert"` value passed to the @@ -1345,7 +1354,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} * `importAttributes` {Object} The `"assert"` value passed to the @@ -1425,7 +1436,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. If `--experimental-vm-modules` isn't + set, this callback will be ignored and calls to `import()` will reject with + [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][]. * `specifier` {string} specifier passed to `import()` * `script` {vm.Script} * `importAttributes` {Object} The `"assert"` value passed to the @@ -1589,6 +1602,7 @@ are not controllable through the timeout either. [Source Text Module Record]: https://tc39.es/ecma262/#sec-source-text-module-records [Synthetic Module Record]: https://heycam.github.io/webidl/#synthetic-module-records [V8 Embedder's Guide]: https://v8.dev/docs/embed#contexts +[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`]: errors.md#err_vm_dynamic_import_callback_missing_flag [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`]: errors.md#err_vm_dynamic_import_callback_missing [`ERR_VM_MODULE_STATUS`]: errors.md#err_vm_module_status [`Error`]: errors.md#class-error diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 5d488843f60ab8..6e1974b1e642b3 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1716,6 +1716,9 @@ E('ERR_VALID_PERFORMANCE_ENTRY_TYPE', 'At least one valid performance entry type is required', Error); E('ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING', 'A dynamic import callback was not specified.', TypeError); +E('ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG', + 'A dynamic import callback was invoked without --experimental-vm-modules', + TypeError); E('ERR_VM_MODULE_ALREADY_LINKED', 'Module has already been linked', Error); E('ERR_VM_MODULE_CANNOT_CREATE_CACHED_DATA', 'Cached data cannot be created for a module which has been evaluated', Error); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 316996a8c329a1..69473df97ede09 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -53,6 +53,7 @@ const { SafeMap, SafeWeakMap, String, + Symbol, StringPrototypeCharAt, StringPrototypeCharCodeAt, StringPrototypeEndsWith, @@ -85,7 +86,12 @@ const { setOwnProperty, getLazy, } = require('internal/util'); -const { internalCompileFunction } = require('internal/vm'); +const { + internalCompileFunction, + makeContextifyScript, + runScriptInThisContext, +} = require('internal/vm'); + const assert = require('internal/assert'); const fs = require('fs'); const path = require('path'); @@ -1236,7 +1242,6 @@ Module.prototype.require = function(id) { let resolvedArgv; let hasPausedEntry = false; /** @type {import('vm').Script} */ -let Script; /** * Wraps the given content in a script and runs it in a new context. @@ -1245,46 +1250,49 @@ let Script; * @param {Module} cjsModuleInstance The CommonJS loader instance */ function wrapSafe(filename, content, cjsModuleInstance) { + const hostDefinedOptionId = Symbol(`cjs:${filename}`); + async function importModuleDynamically(specifier, _, importAttributes) { + const cascadedLoader = getCascadedLoader(); + return cascadedLoader.import(specifier, normalizeReferrerURL(filename), + importAttributes); + } if (patched) { - const wrapper = Module.wrap(content); - if (Script === undefined) { - ({ Script } = require('vm')); - } - const script = new Script(wrapper, { - filename, - lineOffset: 0, - importModuleDynamically: async (specifier, _, importAttributes) => { - const cascadedLoader = getCascadedLoader(); - return cascadedLoader.import(specifier, normalizeReferrerURL(filename), - importAttributes); - }, - }); + const wrapped = Module.wrap(content); + const script = makeContextifyScript( + wrapped, // code + filename, // filename + 0, // lineOffset + 0, // columnOffset + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); // Cache the source map for the module if present. if (script.sourceMapURL) { maybeCacheSourceMap(filename, content, this, false, undefined, script.sourceMapURL); } - return script.runInThisContext({ - displayErrors: true, - }); + return runScriptInThisContext(script, true, false); } + const params = [ 'exports', 'require', 'module', '__filename', '__dirname' ]; try { - const result = internalCompileFunction(content, [ - 'exports', - 'require', - 'module', - '__filename', - '__dirname', - ], { - filename, - importModuleDynamically(specifier, _, importAttributes) { - const cascadedLoader = getCascadedLoader(); - return cascadedLoader.import(specifier, normalizeReferrerURL(filename), - importAttributes); - }, - }); + const result = internalCompileFunction( + content, // code, + filename, // filename + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + undefined, // contextExtensions + params, // params + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); // Cache the source map for the module if present. if (result.sourceMapURL) { diff --git a/lib/internal/modules/esm/create_dynamic_module.js b/lib/internal/modules/esm/create_dynamic_module.js index c0060c47e93b5a..f5077acb78ace2 100644 --- a/lib/internal/modules/esm/create_dynamic_module.js +++ b/lib/internal/modules/esm/create_dynamic_module.js @@ -69,8 +69,9 @@ import.meta.done(); if (imports.length) { reflect.imports = { __proto__: null }; } - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(m, { + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(m, { + __proto__: null, initializeImportMeta: (meta, wrap) => { meta.exports = reflect.exports; if (reflect.imports) { diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 6e42f1a5db5a8a..992311f8922c54 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -210,9 +210,10 @@ class ModuleLoader { ) { const evalInstance = (url) => { const { ModuleWrap } = internalBinding('module_wrap'); - const { setCallbackForWrap } = require('internal/modules/esm/utils'); + const { registerModule } = require('internal/modules/esm/utils'); const module = new ModuleWrap(url, undefined, source, 0, 0); - setCallbackForWrap(module, { + registerModule(module, { + __proto__: null, initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }), importModuleDynamically: (specifier, { url }, importAttributes) => { return this.import(specifier, url, importAttributes); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 39af99c6c3b5b7..d1bc2754fc3ce3 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -150,8 +150,9 @@ translators.set('module', async function moduleStrategy(url, source, isMain) { maybeCacheSourceMap(url, source); debug(`Translating StandardModule ${url}`); const module = new ModuleWrap(url, undefined, source, 0, 0); - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(module, { + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(module, { + __proto__: null, initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }), importModuleDynamically, }); diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index df729ef96c7172..b265e61aea4012 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -8,6 +8,17 @@ const { } = primordials; const { + privateSymbols: { + host_defined_option_symbol, + }, +} = internalBinding('util'); +const { + default_host_defined_options, + vm_dynamic_import_missing_flag, +} = internalBinding('symbols'); + +const { + ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG, ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING, ERR_INVALID_ARG_VALUE, } = require('internal/errors').codes; @@ -21,16 +32,8 @@ const { setImportModuleDynamicallyCallback, setInitializeImportMetaObjectCallback, } = internalBinding('module_wrap'); -const { - getModuleFromWrap, -} = require('internal/vm/module'); const assert = require('internal/assert'); -const callbackMap = new SafeWeakMap(); -function setCallbackForWrap(wrap, data) { - callbackMap.set(wrap, data); -} - let defaultConditions; /** * Returns the default conditions for ES module loading. @@ -83,36 +86,99 @@ function getConditionsSet(conditions) { return getDefaultConditionsSet(); } +/** + * @callback ImportModuleDynamicallyCallback + * @param {string} specifier + * @param {ModuleWrap|ContextifyScript|Function|vm.Module} callbackReferrer + * @param {object} attributes + * @returns { Promise } + */ + +/** + * @callback InitializeImportMetaCallback + * @param {object} meta + * @param {ModuleWrap|ContextifyScript|Function|vm.Module} callbackReferrer + */ + +/** + * @typedef {{ + * callbackReferrer: ModuleWrap|ContextifyScript|Function|vm.Module + * initializeImportMeta? : InitializeImportMetaCallback, + * importModuleDynamically? : ImportModuleDynamicallyCallback + * }} ModuleRegistry + */ + +/** + * @type {WeakMap} + */ +const moduleRegistries = new SafeWeakMap(); + +/** + * V8 would make sure that as long as import() can still be initiated from + * the referrer, the symbol referenced by |host_defined_option_symbol| should + * be alive, which in term would keep the settings object alive through the + * WeakMap, and in turn that keeps the referrer object alive, which would be + * passed into the callbacks. + * The reference goes like this: + * [v8::internal::Script] (via host defined options) ----1--> [idSymbol] + * [callbackReferrer] (via host_defined_option_symbol) ------2------^ | + * ^----------3---- (via WeakMap)------ + * 1+3 makes sure that as long as import() can still be initiated, the + * referrer wrap is still around and can be passed into the callbacks. + * 2 is only there so that we can get the id symbol to configure the + * weak map. + * @param {ModuleWrap|ContextifyScript|Function} referrer The referrer to + * get the id symbol from. This is different from callbackReferrer which + * could be set by the caller. + * @param {ModuleRegistry} registry + */ +function registerModule(referrer, registry) { + const idSymbol = referrer[host_defined_option_symbol]; + if (idSymbol === default_host_defined_options || + idSymbol === vm_dynamic_import_missing_flag) { + // The referrer is compiled without custom callbacks, so there is + // no registry to hold on to. We'll throw + // ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING when a callback is + // needed. + return; + } + // To prevent it from being GC'ed. + registry.callbackReferrer ??= referrer; + moduleRegistries.set(idSymbol, registry); +} + /** * Defines the `import.meta` object for a given module. - * @param {object} wrap - Reference to the module. + * @param {symbol} symbol - Reference to the module. * @param {Record} meta - The import.meta object to initialize. */ -function initializeImportMetaObject(wrap, meta) { - if (callbackMap.has(wrap)) { - const { initializeImportMeta } = callbackMap.get(wrap); +function initializeImportMetaObject(symbol, meta) { + if (moduleRegistries.has(symbol)) { + const { initializeImportMeta, callbackReferrer } = moduleRegistries.get(symbol); if (initializeImportMeta !== undefined) { - meta = initializeImportMeta(meta, getModuleFromWrap(wrap) || wrap); + meta = initializeImportMeta(meta, callbackReferrer); } } } /** * Asynchronously imports a module dynamically using a callback function. The native callback. - * @param {object} wrap - Reference to the module. + * @param {symbol} symbol - Reference to the module. * @param {string} specifier - The module specifier string. * @param {Record} attributes - The import attributes object. * @returns {Promise} - The imported module object. * @throws {ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING} - If the callback function is missing. */ -async function importModuleDynamicallyCallback(wrap, specifier, attributes) { - if (callbackMap.has(wrap)) { - const { importModuleDynamically } = callbackMap.get(wrap); +async function importModuleDynamicallyCallback(symbol, specifier, attributes) { + if (moduleRegistries.has(symbol)) { + const { importModuleDynamically, callbackReferrer } = moduleRegistries.get(symbol); if (importModuleDynamically !== undefined) { - return importModuleDynamically( - specifier, getModuleFromWrap(wrap) || wrap, attributes); + return importModuleDynamically(specifier, callbackReferrer, attributes); } } + if (symbol === vm_dynamic_import_missing_flag) { + throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG(); + } throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING(); } @@ -176,7 +242,7 @@ async function initializeHooks() { } module.exports = { - setCallbackForWrap, + registerModule, initializeESM, initializeHooks, getDefaultConditions, diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index c3a63f63d1d613..838ed93a3e2afb 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -1,6 +1,7 @@ 'use strict'; const { + Symbol, RegExpPrototypeExec, globalThis, } = primordials; @@ -24,7 +25,9 @@ const { emitAfter, popAsyncContext, } = require('internal/async_hooks'); - +const { + makeContextifyScript, runScriptInThisContext, +} = require('internal/vm'); // shouldAbortOnUncaughtToggle is a typed array for faster // communication with JS. const { shouldAbortOnUncaughtToggle } = internalBinding('util'); @@ -52,8 +55,7 @@ function evalModule(source, print) { function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { const CJSModule = require('internal/modules/cjs/loader').Module; - const { kVmBreakFirstLineSymbol } = require('internal/util'); - const { pathToFileURL } = require('url'); + const { pathToFileURL } = require('internal/url'); const cwd = tryGetCwd(); const origModule = globalThis.module; // Set e.g. when called from the REPL. @@ -78,16 +80,25 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) { `; globalThis.__filename = name; RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. - const result = module._compile(script, `${name}-wrapper`)(() => - require('vm').runInThisContext(body, { - filename: name, - displayErrors: true, - [kVmBreakFirstLineSymbol]: !!breakFirstLine, - importModuleDynamically(specifier, _, importAttributes) { - const loader = asyncESM.esmLoader; - return loader.import(specifier, baseUrl, importAttributes); - }, - })); + const result = module._compile(script, `${name}-wrapper`)(() => { + const hostDefinedOptionId = Symbol(name); + async function importModuleDynamically(specifier, _, importAttributes) { + const loader = asyncESM.esmLoader; + return loader.import(specifier, baseUrl, importAttributes); + } + const script = makeContextifyScript( + body, // code + name, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); + return runScriptInThisContext(script, true, !!breakFirstLine); + }); if (print) { const { log } = require('internal/console/global'); log(result); diff --git a/lib/internal/vm.js b/lib/internal/vm.js index b14ba13e7e4cfb..624e6825b8b45d 100644 --- a/lib/internal/vm.js +++ b/lib/internal/vm.js @@ -1,26 +1,31 @@ 'use strict'; const { - ArrayPrototypeForEach, + ReflectApply, + Symbol, } = primordials; const { + ContextifyScript, compileFunction, isContext: _isContext, } = internalBinding('contextify'); const { - validateArray, - validateBoolean, - validateBuffer, + runInContext, +} = ContextifyScript.prototype; +const { + default_host_defined_options, + vm_dynamic_import_missing_flag, +} = internalBinding('symbols'); +const { validateFunction, validateObject, - validateString, - validateStringArray, - validateUint32, } = require('internal/validators'); + const { - ERR_INVALID_ARG_TYPE, -} = require('internal/errors').codes; + getOptionValue, +} = require('internal/options'); + function isContext(object) { validateObject(object, 'object', { __proto__: null, allowArray: true }); @@ -28,48 +33,46 @@ function isContext(object) { return _isContext(object); } -function internalCompileFunction(code, params, options) { - validateString(code, 'code'); - if (params !== undefined) { - validateStringArray(params, 'params'); +function getHostDefinedOptionId(importModuleDynamically, filename) { + if (importModuleDynamically !== undefined) { + // Check that it's either undefined or a function before we pass + // it into the native constructor. + validateFunction(importModuleDynamically, + 'options.importModuleDynamically'); } - - const { - filename = '', - columnOffset = 0, - lineOffset = 0, - cachedData = undefined, - produceCachedData = false, - parsingContext = undefined, - contextExtensions = [], - importModuleDynamically, - } = options; - - validateString(filename, 'options.filename'); - validateUint32(columnOffset, 'options.columnOffset'); - validateUint32(lineOffset, 'options.lineOffset'); - if (cachedData !== undefined) - validateBuffer(cachedData, 'options.cachedData'); - validateBoolean(produceCachedData, 'options.produceCachedData'); - if (parsingContext !== undefined) { - if ( - typeof parsingContext !== 'object' || - parsingContext === null || - !isContext(parsingContext) - ) { - throw new ERR_INVALID_ARG_TYPE( - 'options.parsingContext', - 'Context', - parsingContext, - ); - } + if (importModuleDynamically === undefined) { + // We need a default host defined options that are the same for all + // scripts not needing custom module callbacks so that the isolate + // compilation cache can be hit. + return default_host_defined_options; + } + // We should've thrown here immediately when we introduced + // --experimental-vm-modules and importModuleDynamically, but since + // users are already using this callback to throw a similar error, + // we also defer the error to the time when an actual import() is called + // to avoid breaking them. To ensure that the isolate compilation + // cache can still be hit, use a constant sentinel symbol here. + if (!getOptionValue('--experimental-vm-modules')) { + return vm_dynamic_import_missing_flag; } - validateArray(contextExtensions, 'options.contextExtensions'); - ArrayPrototypeForEach(contextExtensions, (extension, i) => { - const name = `options.contextExtensions[${i}]`; - validateObject(extension, name, { __proto__: null, nullable: true }); + + return Symbol(filename); +} + +function registerImportModuleDynamically(referrer, importModuleDynamically) { + const { importModuleDynamicallyWrap } = require('internal/vm/module'); + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(referrer, { + __proto__: null, + importModuleDynamically: + importModuleDynamicallyWrap(importModuleDynamically), }); +} +function internalCompileFunction( + code, filename, lineOffset, columnOffset, + cachedData, produceCachedData, parsingContext, contextExtensions, + params, hostDefinedOptionId, importModuleDynamically) { const result = compileFunction( code, filename, @@ -80,6 +83,7 @@ function internalCompileFunction(code, params, options) { parsingContext, contextExtensions, params, + hostDefinedOptionId, ); if (produceCachedData) { @@ -95,21 +99,65 @@ function internalCompileFunction(code, params, options) { } if (importModuleDynamically !== undefined) { - validateFunction(importModuleDynamically, - 'options.importModuleDynamically'); - const { importModuleDynamicallyWrap } = require('internal/vm/module'); - const wrapped = importModuleDynamicallyWrap(importModuleDynamically); - const func = result.function; - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(result.cacheKey, { - importModuleDynamically: (s, _k, i) => wrapped(s, func, i), - }); + registerImportModuleDynamically(result.function, importModuleDynamically); } return result; } +function makeContextifyScript(code, + filename, + lineOffset, + columnOffset, + cachedData, + produceCachedData, + parsingContext, + hostDefinedOptionId, + importModuleDynamically) { + let script; + // Calling `ReThrow()` on a native TryCatch does not generate a new + // abort-on-uncaught-exception check. A dummy try/catch in JS land + // protects against that. + try { // eslint-disable-line no-useless-catch + script = new ContextifyScript(code, + filename, + lineOffset, + columnOffset, + cachedData, + produceCachedData, + parsingContext, + hostDefinedOptionId); + } catch (e) { + throw e; /* node-do-not-add-exception-line */ + } + + if (importModuleDynamically !== undefined) { + registerImportModuleDynamically(script, importModuleDynamically); + } + return script; +} + +// Internal version of vm.Script.prototype.runInThisContext() which skips +// argument validation. +function runScriptInThisContext(script, displayErrors, breakOnFirstLine) { + return ReflectApply( + runInContext, + script, + [ + null, // sandbox - use current context + -1, // timeout + displayErrors, // displayErrors + false, // breakOnSigint + breakOnFirstLine, // breakOnFirstLine + ], + ); +} + module.exports = { + getHostDefinedOptionId, internalCompileFunction, isContext, + makeContextifyScript, + registerImportModuleDynamically, + runScriptInThisContext, }; diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index 19d93e1abfbd42..439908a891b10c 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -12,7 +12,6 @@ const { ObjectSetPrototypeOf, ReflectApply, SafePromiseAllReturnVoid, - SafeWeakMap, Symbol, SymbolToStringTag, TypeError, @@ -70,7 +69,6 @@ const STATUS_MAP = { let globalModuleId = 0; const defaultModuleName = 'vm:module'; -const wrapToModuleMap = new SafeWeakMap(); const kWrap = Symbol('kWrap'); const kContext = Symbol('kContext'); @@ -121,17 +119,18 @@ class Module { }); } + let registry = { __proto__: null }; if (sourceText !== undefined) { this[kWrap] = new ModuleWrap(identifier, context, sourceText, options.lineOffset, options.columnOffset, options.cachedData); - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(this[kWrap], { + registry = { + __proto__: null, initializeImportMeta: options.initializeImportMeta, importModuleDynamically: options.importModuleDynamically ? importModuleDynamicallyWrap(options.importModuleDynamically) : undefined, - }); + }; } else { assert(syntheticEvaluationSteps); this[kWrap] = new ModuleWrap(identifier, context, @@ -139,7 +138,11 @@ class Module { syntheticEvaluationSteps); } - wrapToModuleMap.set(this[kWrap], this); + // This will take precedence over the referrer as the object being + // passed into the callbacks. + registry.callbackReferrer = this; + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(this[kWrap], registry); this[kContext] = context; } @@ -446,5 +449,4 @@ module.exports = { SourceTextModule, SyntheticModule, importModuleDynamicallyWrap, - getModuleFromWrap: (wrap) => wrapToModuleMap.get(wrap), }; diff --git a/lib/repl.js b/lib/repl.js index b2d143619ae093..2e54fabba2cff3 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -118,6 +118,9 @@ const { } = require('internal/util'); const { inspect } = require('internal/util/inspect'); const vm = require('vm'); + +const { runInThisContext, runInContext } = vm.Script.prototype; + const path = require('path'); const fs = require('fs'); const { Interface } = require('readline'); @@ -186,7 +189,9 @@ const { extensionFormatMap, legacyExtensionFormatMap, } = require('internal/modules/esm/formats'); - +const { + makeContextifyScript, +} = require('internal/vm'); let nextREPLResourceNumber = 1; // This prevents v8 code cache from getting confused and using a different // cache from a resource of the same name @@ -430,8 +435,6 @@ function REPLServer(prompt, } function defaultEval(code, context, file, cb) { - const asyncESM = require('internal/process/esm_loader'); - let result, script, wrappedErr; let err = null; let wrappedCmd = false; @@ -449,6 +452,21 @@ function REPLServer(prompt, wrappedCmd = true; } + const hostDefinedOptionId = Symbol(`eval:${file}`); + let parentURL; + try { + const { pathToFileURL } = require('internal/url'); + // Adding `/repl` prevents dynamic imports from loading relative + // to the parent of `process.cwd()`. + parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; + } catch { + // Continue regardless of error. + } + async function importModuleDynamically(specifier, _, importAttributes) { + const asyncESM = require('internal/process/esm_loader'); + return asyncESM.esmLoader.import(specifier, parentURL, + importAttributes); + } // `experimentalREPLAwait` is set to true by default. // Shall be false in case `--no-experimental-repl-await` flag is used. if (experimentalREPLAwait && StringPrototypeIncludes(code, 'await')) { @@ -466,28 +484,21 @@ function REPLServer(prompt, } catch (e) { let recoverableError = false; if (e.name === 'SyntaxError') { - let parentURL; - try { - const { pathToFileURL } = require('url'); - // Adding `/repl` prevents dynamic imports from loading relative - // to the parent of `process.cwd()`. - parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; - } catch { - // Continue regardless of error. - } - // Remove all "await"s and attempt running the script // in order to detect if error is truly non recoverable const fallbackCode = SideEffectFreeRegExpPrototypeSymbolReplace(/\bawait\b/g, code, ''); try { - vm.createScript(fallbackCode, { - filename: file, - displayErrors: true, - importModuleDynamically: (specifier, _, importAttributes) => { - return asyncESM.esmLoader.import(specifier, parentURL, - importAttributes); - }, - }); + makeContextifyScript( + fallbackCode, // code + file, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); } catch (fallbackError) { if (isRecoverableError(fallbackError, fallbackCode)) { recoverableError = true; @@ -507,15 +518,6 @@ function REPLServer(prompt, return cb(null); if (err === null) { - let parentURL; - try { - const { pathToFileURL } = require('url'); - // Adding `/repl` prevents dynamic imports from loading relative - // to the parent of `process.cwd()`. - parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; - } catch { - // Continue regardless of error. - } while (true) { try { if (self.replMode === module.exports.REPL_MODE_STRICT && @@ -524,14 +526,17 @@ function REPLServer(prompt, // value for statements and declarations that don't return a value. code = `'use strict'; void 0;\n${code}`; } - script = vm.createScript(code, { - filename: file, - displayErrors: true, - importModuleDynamically: (specifier, _, importAttributes) => { - return asyncESM.esmLoader.import(specifier, parentURL, - importAttributes); - }, - }); + script = makeContextifyScript( + code, // code + file, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); } catch (e) { debug('parse error %j', code, e); if (wrappedCmd) { @@ -591,9 +596,9 @@ function REPLServer(prompt, }; if (self.useGlobal) { - result = script.runInThisContext(scriptOptions); + result = ReflectApply(runInThisContext, script, [scriptOptions]); } else { - result = script.runInContext(context, scriptOptions); + result = ReflectApply(runInContext, script, [context, scriptOptions]); } } finally { if (self.breakEvalOnSigint) { diff --git a/lib/vm.js b/lib/vm.js index b48e79c282541b..6bea7e5e74de9c 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -40,13 +40,14 @@ const { ERR_INVALID_ARG_TYPE, } = require('internal/errors').codes; const { + validateArray, validateBoolean, validateBuffer, - validateFunction, validateInt32, - validateObject, validateOneOf, + validateObject, validateString, + validateStringArray, validateUint32, } = require('internal/validators'); const { @@ -55,8 +56,10 @@ const { kVmBreakFirstLineSymbol, } = require('internal/util'); const { + getHostDefinedOptionId, internalCompileFunction, isContext, + registerImportModuleDynamically, } = require('internal/vm'); const kParsingContext = Symbol('script parsing context'); @@ -87,6 +90,8 @@ class Script extends ContextifyScript { } validateBoolean(produceCachedData, 'options.produceCachedData'); + const hostDefinedOptionId = + getHostDefinedOptionId(importModuleDynamically, filename); // Calling `ReThrow()` on a native TryCatch does not generate a new // abort-on-uncaught-exception check. A dummy try/catch in JS land // protects against that. @@ -97,20 +102,14 @@ class Script extends ContextifyScript { columnOffset, cachedData, produceCachedData, - parsingContext); + parsingContext, + hostDefinedOptionId); } catch (e) { throw e; /* node-do-not-add-exception-line */ } if (importModuleDynamically !== undefined) { - validateFunction(importModuleDynamically, - 'options.importModuleDynamically'); - const { importModuleDynamicallyWrap } = require('internal/vm/module'); - const { setCallbackForWrap } = require('internal/modules/esm/utils'); - setCallbackForWrap(this, { - importModuleDynamically: - importModuleDynamicallyWrap(importModuleDynamically), - }); + registerImportModuleDynamically(this, importModuleDynamically); } } @@ -299,7 +298,54 @@ function runInThisContext(code, options) { } function compileFunction(code, params, options = kEmptyObject) { - return internalCompileFunction(code, params, options).function; + validateString(code, 'code'); + if (params !== undefined) { + validateStringArray(params, 'params'); + } + const { + filename = '', + columnOffset = 0, + lineOffset = 0, + cachedData = undefined, + produceCachedData = false, + parsingContext = undefined, + contextExtensions = [], + importModuleDynamically, + } = options; + + validateString(filename, 'options.filename'); + validateInt32(columnOffset, 'options.columnOffset'); + validateInt32(lineOffset, 'options.lineOffset'); + if (cachedData !== undefined) + validateBuffer(cachedData, 'options.cachedData'); + validateBoolean(produceCachedData, 'options.produceCachedData'); + if (parsingContext !== undefined) { + if ( + typeof parsingContext !== 'object' || + parsingContext === null || + !isContext(parsingContext) + ) { + throw new ERR_INVALID_ARG_TYPE( + 'options.parsingContext', + 'Context', + parsingContext, + ); + } + } + validateArray(contextExtensions, 'options.contextExtensions'); + ArrayPrototypeForEach(contextExtensions, (extension, i) => { + const name = `options.contextExtensions[${i}]`; + validateObject(extension, name, { __proto__: null, nullable: true }); + }); + + const hostDefinedOptionId = + getHostDefinedOptionId(importModuleDynamically, filename); + + return internalCompileFunction( + code, filename, lineOffset, columnOffset, + cachedData, produceCachedData, parsingContext, contextExtensions, + params, hostDefinedOptionId, importModuleDynamically, + ).function; } const measureMemoryModes = { diff --git a/src/base_object-inl.h b/src/base_object-inl.h index f003f1390b864f..0148c75427985e 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -127,7 +127,8 @@ template void BaseObject::InternalFieldGet( v8::Local property, const v8::PropertyCallbackInfo& info) { - info.GetReturnValue().Set(info.This()->GetInternalField(Field)); + info.GetReturnValue().Set( + info.This()->GetInternalField(Field).As()); } template diff --git a/src/env-inl.h b/src/env-inl.h index b959fb48d29ce1..2ee4e709d82d72 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -345,16 +345,6 @@ inline AliasedInt32Array& Environment::stream_base_state() { return stream_base_state_; } -inline uint32_t Environment::get_next_module_id() { - return module_id_counter_++; -} -inline uint32_t Environment::get_next_script_id() { - return script_id_counter_++; -} -inline uint32_t Environment::get_next_function_id() { - return function_id_counter_++; -} - ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope( Environment* env) : env_(env) { diff --git a/src/env.h b/src/env.h index 06250f32e14863..1f5dbc65790d97 100644 --- a/src/env.h +++ b/src/env.h @@ -698,14 +698,6 @@ class Environment : public MemoryRetainer { builtins::BuiltinLoader* builtin_loader(); std::unordered_multimap hash_to_module_map; - std::unordered_map id_to_module_map; - std::unordered_map - id_to_script_map; - std::unordered_map id_to_function_map; - - inline uint32_t get_next_module_id(); - inline uint32_t get_next_script_id(); - inline uint32_t get_next_function_id(); EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; } diff --git a/src/env_properties.h b/src/env_properties.h index 5fc5106c0c5ad2..f0ca211cf5ac44 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -21,6 +21,7 @@ V(arrow_message_private_symbol, "node:arrowMessage") \ V(contextify_context_private_symbol, "node:contextify:context") \ V(decorated_private_symbol, "node:decorated") \ + V(host_defined_option_symbol, "node:host_defined_option_symbol") \ V(napi_type_tag, "node:napi:type_tag") \ V(napi_wrapper, "node:napi:wrapper") \ V(untransferable_object_private_symbol, "node:untransferableObject") \ @@ -30,6 +31,7 @@ // Symbols are per-isolate primitives but Environment proxies them // for the sake of convenience. #define PER_ISOLATE_SYMBOL_PROPERTIES(V) \ + V(default_host_defined_options, "default_host_defined_options") \ V(fs_use_promises_symbol, "fs_use_promises_symbol") \ V(async_id_symbol, "async_id_symbol") \ V(handle_onclose_symbol, "handle_onclose") \ @@ -42,7 +44,8 @@ V(owner_symbol, "owner_symbol") \ V(onpskexchange_symbol, "onpskexchange") \ V(resource_symbol, "resource_symbol") \ - V(trigger_async_id_symbol, "trigger_async_id_symbol") + V(trigger_async_id_symbol, "trigger_async_id_symbol") \ + V(vm_dynamic_import_missing_flag, "vm_dynamic_import_missing_flag") // Strings are per-isolate primitives but Environment proxies them // for the sake of convenience. Strings should be ASCII-only. @@ -337,7 +340,6 @@ V(blocklist_constructor_template, v8::FunctionTemplate) \ V(contextify_global_template, v8::ObjectTemplate) \ V(contextify_wrapper_template, v8::ObjectTemplate) \ - V(compiled_fn_entry_template, v8::ObjectTemplate) \ V(crypto_key_object_handle_constructor, v8::FunctionTemplate) \ V(env_proxy_template, v8::ObjectTemplate) \ V(env_proxy_ctor_template, v8::FunctionTemplate) \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 21b9a702aebaf4..cf9425f4786144 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -38,13 +38,13 @@ using v8::MaybeLocal; using v8::MicrotaskQueue; using v8::Module; using v8::ModuleRequest; -using v8::Number; using v8::Object; using v8::PrimitiveArray; using v8::Promise; using v8::ScriptCompiler; using v8::ScriptOrigin; using v8::String; +using v8::Symbol; using v8::UnboundModuleScript; using v8::Undefined; using v8::Value; @@ -52,23 +52,28 @@ using v8::Value; ModuleWrap::ModuleWrap(Environment* env, Local object, Local module, - Local url) - : BaseObject(env, object), - module_(env->isolate(), module), - id_(env->get_next_module_id()) { - env->id_to_module_map.emplace(id_, this); - - Local undefined = Undefined(env->isolate()); + Local url, + Local context_object, + Local synthetic_evaluation_step) + : BaseObject(env, object), + module_(env->isolate(), module), + module_hash_(module->GetIdentityHash()) { + object->SetInternalFieldForNodeCore(kModuleSlot, module); object->SetInternalField(kURLSlot, url); - object->SetInternalField(kSyntheticEvaluationStepsSlot, undefined); - object->SetInternalField(kContextObjectSlot, undefined); + object->SetInternalField(kSyntheticEvaluationStepsSlot, + synthetic_evaluation_step); + object->SetInternalField(kContextObjectSlot, context_object); + + if (!synthetic_evaluation_step->IsUndefined()) { + synthetic_ = true; + } + MakeWeak(); + module_.SetWeak(); } ModuleWrap::~ModuleWrap() { HandleScope scope(env()->isolate()); - Local module = module_.Get(env()->isolate()); - env()->id_to_module_map.erase(id_); - auto range = env()->hash_to_module_map.equal_range(module->GetIdentityHash()); + auto range = env()->hash_to_module_map.equal_range(module_hash_); for (auto it = range.first; it != range.second; ++it) { if (it->second == this) { env()->hash_to_module_map.erase(it); @@ -78,8 +83,10 @@ ModuleWrap::~ModuleWrap() { } Local ModuleWrap::context() const { - Local obj = object()->GetInternalField(kContextObjectSlot); - if (obj.IsEmpty()) return {}; + Local obj = object()->GetInternalField(kContextObjectSlot).As(); + // If this fails, there is likely a bug e.g. ModuleWrap::context() is accessed + // before the ModuleWrap constructor completes. + CHECK(obj->IsObject()); return obj.As()->GetCreationContext().ToLocalChecked(); } @@ -94,14 +101,6 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env, return nullptr; } -ModuleWrap* ModuleWrap::GetFromID(Environment* env, uint32_t id) { - auto module_wrap_it = env->id_to_module_map.find(id); - if (module_wrap_it == env->id_to_module_map.end()) { - return nullptr; - } - return module_wrap_it->second; -} - // new ModuleWrap(url, context, source, lineOffset, columnOffset) // new ModuleWrap(url, context, exportNames, syntheticExecutionFunction) void ModuleWrap::New(const FunctionCallbackInfo& args) { @@ -146,8 +145,8 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { Local host_defined_options = PrimitiveArray::New(isolate, HostDefinedOptions::kLength); - host_defined_options->Set(isolate, HostDefinedOptions::kType, - Number::New(isolate, ScriptType::kModule)); + Local id_symbol = Symbol::New(isolate, url); + host_defined_options->Set(isolate, HostDefinedOptions::kID, id_symbol); ShouldNotAbortOnUncaughtScope no_abort_scope(env); TryCatchScope try_catch(env); @@ -227,25 +226,25 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { return; } - ModuleWrap* obj = new ModuleWrap(env, that, module, url); - - if (synthetic) { - obj->synthetic_ = true; - obj->object()->SetInternalField(kSyntheticEvaluationStepsSlot, args[3]); + if (that->SetPrivate(context, env->host_defined_option_symbol(), id_symbol) + .IsNothing()) { + return; } // Use the extras object as an object whose GetCreationContext() will be the // original `context`, since the `Context` itself strictly speaking cannot // be stored in an internal field. - obj->object()->SetInternalField(kContextObjectSlot, - context->GetExtrasBindingObject()); + Local context_object = context->GetExtrasBindingObject(); + Local synthetic_evaluation_step = + synthetic ? args[3] : Undefined(env->isolate()).As(); + + ModuleWrap* obj = new ModuleWrap( + env, that, module, url, context_object, synthetic_evaluation_step); + obj->contextify_context_ = contextify_context; env->hash_to_module_map.emplace(module->GetIdentityHash(), obj); - host_defined_options->Set(isolate, HostDefinedOptions::kID, - Number::New(isolate, obj->id())); - that->SetIntegrityLevel(context, IntegrityLevel::kFrozen); args.GetReturnValue().Set(that); } @@ -580,35 +579,16 @@ static MaybeLocal ImportModuleDynamically( Local object; - int type = options->Get(context, HostDefinedOptions::kType) - .As() - ->Int32Value(context) - .ToChecked(); - uint32_t id = options->Get(context, HostDefinedOptions::kID) - .As() - ->Uint32Value(context) - .ToChecked(); - if (type == ScriptType::kScript) { - contextify::ContextifyScript* wrap = env->id_to_script_map.find(id)->second; - object = wrap->object(); - } else if (type == ScriptType::kModule) { - ModuleWrap* wrap = ModuleWrap::GetFromID(env, id); - object = wrap->object(); - } else if (type == ScriptType::kFunction) { - auto it = env->id_to_function_map.find(id); - CHECK_NE(it, env->id_to_function_map.end()); - object = it->second->object(); - } else { - UNREACHABLE(); - } + Local id = + options->Get(context, HostDefinedOptions::kID).As(); Local attributes = createImportAttributesContainer(env, isolate, import_attributes); Local import_args[] = { - object, - Local(specifier), - attributes, + id, + Local(specifier), + attributes, }; Local result; @@ -652,7 +632,13 @@ void ModuleWrap::HostInitializeImportMetaObjectCallback( Local wrap = module_wrap->object(); Local callback = env->host_initialize_import_meta_object_callback(); - Local args[] = { wrap, meta }; + Local id; + if (!wrap->GetPrivate(context, env->host_defined_option_symbol()) + .ToLocal(&id)) { + return; + } + DCHECK(id->IsSymbol()); + Local args[] = {id, meta}; TryCatchScope try_catch(env); USE(callback->Call( context, Undefined(env->isolate()), arraysize(args), args)); @@ -684,7 +670,9 @@ MaybeLocal ModuleWrap::SyntheticModuleEvaluationStepsCallback( TryCatchScope try_catch(env); Local synthetic_evaluation_steps = - obj->object()->GetInternalField(kSyntheticEvaluationStepsSlot) + obj->object() + ->GetInternalField(kSyntheticEvaluationStepsSlot) + .As() .As(); obj->object()->SetInternalField( kSyntheticEvaluationStepsSlot, Undefined(isolate)); diff --git a/src/module_wrap.h b/src/module_wrap.h index ce4610a461a2b4..1fc801edced9c5 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -26,15 +26,14 @@ enum ScriptType : int { }; enum HostDefinedOptions : int { - kType = 8, - kID = 9, - kLength = 10, + kID = 8, + kLength = 9, }; class ModuleWrap : public BaseObject { public: enum InternalFields { - kModuleWrapBaseField = BaseObject::kInternalFieldCount, + kModuleSlot = BaseObject::kInternalFieldCount, kURLSlot, kSyntheticEvaluationStepsSlot, kContextObjectSlot, // Object whose creation context is the target Context @@ -55,9 +54,7 @@ class ModuleWrap : public BaseObject { tracker->TrackField("resolve_cache", resolve_cache_); } - inline uint32_t id() { return id_; } v8::Local context() const; - static ModuleWrap* GetFromID(node::Environment*, uint32_t id); SET_MEMORY_INFO_NAME(ModuleWrap) SET_SELF_SIZE(ModuleWrap) @@ -72,7 +69,9 @@ class ModuleWrap : public BaseObject { ModuleWrap(Environment* env, v8::Local object, v8::Local module, - v8::Local url); + v8::Local url, + v8::Local context_object, + v8::Local synthetic_evaluation_step); ~ModuleWrap() override; static void New(const v8::FunctionCallbackInfo& args); @@ -107,7 +106,7 @@ class ModuleWrap : public BaseObject { contextify::ContextifyContext* contextify_context_ = nullptr; bool synthetic_ = false; bool linked_ = false; - uint32_t id_; + int module_hash_; }; } // namespace loader diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 3da8746e2c46bb..65a7acbcfb6bc6 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -60,7 +60,6 @@ using v8::MicrotasksPolicy; using v8::Name; using v8::NamedPropertyHandlerConfiguration; using v8::Nothing; -using v8::Number; using v8::Object; using v8::ObjectTemplate; using v8::PrimitiveArray; @@ -73,11 +72,11 @@ using v8::Script; using v8::ScriptCompiler; using v8::ScriptOrigin; using v8::String; +using v8::Symbol; using v8::Uint32; using v8::UnboundScript; using v8::Value; using v8::WeakCallbackInfo; -using v8::WeakCallbackType; // The vm module executes code in a sandboxed environment with a different // global object than the rest of the code. This is achieved by applying @@ -788,10 +787,12 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { bool produce_cached_data = false; Local parsing_context = context; + Local id_symbol; if (argc > 2) { // new ContextifyScript(code, filename, lineOffset, columnOffset, - // cachedData, produceCachedData, parsingContext) - CHECK_EQ(argc, 7); + // cachedData, produceCachedData, parsingContext, + // hostDefinedOptionId) + CHECK_EQ(argc, 8); CHECK(args[2]->IsNumber()); line_offset = args[2].As()->Value(); CHECK(args[3]->IsNumber()); @@ -810,6 +811,8 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(sandbox); parsing_context = sandbox->context(); } + CHECK(args[7]->IsSymbol()); + id_symbol = args[7].As(); } ContextifyScript* contextify_script = @@ -833,10 +836,8 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { Local host_defined_options = PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); - host_defined_options->Set(isolate, loader::HostDefinedOptions::kType, - Number::New(isolate, loader::ScriptType::kScript)); - host_defined_options->Set(isolate, loader::HostDefinedOptions::kID, - Number::New(isolate, contextify_script->id())); + host_defined_options->Set( + isolate, loader::HostDefinedOptions::kID, id_symbol); ScriptOrigin origin(isolate, filename, @@ -873,13 +874,23 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { "ContextifyScript::New"); return; } + contextify_script->script_.Reset(isolate, v8_script); + contextify_script->script_.SetWeak(); + contextify_script->object()->SetInternalFieldForNodeCore(kUnboundScriptSlot, + v8_script); std::unique_ptr new_cached_data; if (produce_cached_data) { new_cached_data.reset(ScriptCompiler::CreateCodeCache(v8_script)); } + if (contextify_script->object() + ->SetPrivate(context, env->host_defined_option_symbol(), id_symbol) + .IsNothing()) { + return; + } + if (StoreCodeCacheResult(env, args.This(), compile_options, @@ -1117,17 +1128,11 @@ bool ContextifyScript::EvalMachine(Local context, ContextifyScript::ContextifyScript(Environment* env, Local object) - : BaseObject(env, object), - id_(env->get_next_script_id()) { + : BaseObject(env, object) { MakeWeak(); - env->id_to_script_map.emplace(id_, this); -} - - -ContextifyScript::~ContextifyScript() { - env()->id_to_script_map.erase(id_); } +ContextifyScript::~ContextifyScript() {} void ContextifyContext::CompileFunction( const FunctionCallbackInfo& args) { @@ -1189,6 +1194,10 @@ void ContextifyContext::CompileFunction( params_buf = args[8].As(); } + // Argument 10: host-defined option symbol + CHECK(args[9]->IsSymbol()); + Local id_symbol = args[9].As(); + // Read cache from cached data buffer ScriptCompiler::CachedData* cached_data = nullptr; if (!cached_data_buf.IsEmpty()) { @@ -1197,18 +1206,11 @@ void ContextifyContext::CompileFunction( data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength()); } - // Get the function id - uint32_t id = env->get_next_function_id(); - // Set host_defined_options Local host_defined_options = PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); host_defined_options->Set( - isolate, - loader::HostDefinedOptions::kType, - Number::New(isolate, loader::ScriptType::kFunction)); - host_defined_options->Set( - isolate, loader::HostDefinedOptions::kID, Number::New(isolate, id)); + isolate, loader::HostDefinedOptions::kID, id_symbol); ScriptOrigin origin(isolate, filename, @@ -1273,21 +1275,14 @@ void ContextifyContext::CompileFunction( } return; } - - Local cache_key; - if (!env->compiled_fn_entry_template()->NewInstance( - context).ToLocal(&cache_key)) { + if (fn->SetPrivate(context, env->host_defined_option_symbol(), id_symbol) + .IsNothing()) { return; } - CompiledFnEntry* entry = new CompiledFnEntry(env, cache_key, id, fn); - env->id_to_function_map.emplace(id, entry); Local result = Object::New(isolate); if (result->Set(parsing_context, env->function_string(), fn).IsNothing()) return; - if (result->Set(parsing_context, env->cache_key_string(), cache_key) - .IsNothing()) - return; if (result ->Set(parsing_context, env->source_map_url_string(), @@ -1312,25 +1307,6 @@ void ContextifyContext::CompileFunction( args.GetReturnValue().Set(result); } -void CompiledFnEntry::WeakCallback( - const WeakCallbackInfo& data) { - CompiledFnEntry* entry = data.GetParameter(); - delete entry; -} - -CompiledFnEntry::CompiledFnEntry(Environment* env, - Local object, - uint32_t id, - Local fn) - : BaseObject(env, object), id_(id), fn_(env->isolate(), fn) { - fn_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter); -} - -CompiledFnEntry::~CompiledFnEntry() { - env()->id_to_function_map.erase(id_); - fn_.ClearWeak(); -} - static void StartSigintWatchdog(const FunctionCallbackInfo& args) { int ret = SigintWatchdogHelper::GetInstance()->Start(); args.GetReturnValue().Set(ret == 0); @@ -1418,15 +1394,6 @@ void Initialize(Local target, SetMethodNoSideEffect( context, target, "watchdogHasPendingSigint", WatchdogHasPendingSigint); - { - Local tpl = FunctionTemplate::New(env->isolate()); - tpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "CompiledFnEntry")); - tpl->InstanceTemplate()->SetInternalFieldCount( - CompiledFnEntry::kInternalFieldCount); - - env->set_compiled_fn_entry_template(tpl->InstanceTemplate()); - } - Local constants = Object::New(env->isolate()); Local measure_memory = Object::New(env->isolate()); Local memory_execution = Object::New(env->isolate()); diff --git a/src/node_contextify.h b/src/node_contextify.h index 76c89318bb6cbf..771de6edb515d2 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -149,6 +149,11 @@ class ContextifyContext : public BaseObject { class ContextifyScript : public BaseObject { public: + enum InternalFields { + kUnboundScriptSlot = BaseObject::kInternalFieldCount, + kInternalFieldCount + }; + SET_NO_MEMORY_INFO() SET_MEMORY_INFO_NAME(ContextifyScript) SET_SELF_SIZE(ContextifyScript) @@ -171,32 +176,8 @@ class ContextifyScript : public BaseObject { std::shared_ptr microtask_queue, const v8::FunctionCallbackInfo& args); - inline uint32_t id() { return id_; } - private: v8::Global script_; - uint32_t id_; -}; - -class CompiledFnEntry final : public BaseObject { - public: - SET_NO_MEMORY_INFO() - SET_MEMORY_INFO_NAME(CompiledFnEntry) - SET_SELF_SIZE(CompiledFnEntry) - - CompiledFnEntry(Environment* env, - v8::Local object, - uint32_t id, - v8::Local fn); - ~CompiledFnEntry(); - - bool IsNotIndicativeOfMemoryLeakAtExit() const override { return true; } - - private: - uint32_t id_; - v8::Global fn_; - - static void WeakCallback(const v8::WeakCallbackInfo& data); }; v8::Maybe StoreCodeCacheResult( diff --git a/src/node_file.cc b/src/node_file.cc index 7f627ac458492c..ebbb67c226775d 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -438,7 +438,7 @@ MaybeLocal FileHandle::ClosePromise() { Local context = env()->context(); Local close_resolver = - object()->GetInternalField(FileHandle::kClosingPromiseSlot); + object()->GetInternalField(FileHandle::kClosingPromiseSlot).As(); if (!close_resolver.IsEmpty() && !close_resolver->IsUndefined()) { CHECK(close_resolver->IsPromise()); return close_resolver.As(); diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc index 5d0e2b0d4c7ba1..1a0cb082a2534f 100644 --- a/src/node_task_queue.cc +++ b/src/node_task_queue.cc @@ -50,7 +50,7 @@ static Maybe GetAssignedPromiseWrapAsyncId(Environment* env, // be an object. If it's not, we just ignore it. Ideally v8 would // have had GetInternalField returning a MaybeLocal but this works // for now. - Local promiseWrap = promise->GetInternalField(0); + Local promiseWrap = promise->GetInternalField(0).As(); if (promiseWrap->IsObject()) { Local maybe_async_id; if (!promiseWrap.As()->Get(env->context(), id_symbol) diff --git a/src/node_zlib.cc b/src/node_zlib.cc index fac116f9e6b3e2..0c4ae0fc794347 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -423,7 +423,8 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { UpdateWriteResult(); // call the write() cb - Local cb = object()->GetInternalField(kWriteJSCallback); + Local cb = + object()->GetInternalField(kWriteJSCallback).template As(); MakeCallback(cb.As(), 0, nullptr); if (pending_close_) diff --git a/src/stream_base.cc b/src/stream_base.cc index f1769ca52970fe..b9dfc645e2b49c 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -470,8 +470,9 @@ MaybeLocal StreamBase::CallJSOnreadMethod(ssize_t nread, AsyncWrap* wrap = GetAsyncWrap(); CHECK_NOT_NULL(wrap); - Local onread = wrap->object()->GetInternalField( - StreamBase::kOnReadFunctionField); + Local onread = wrap->object() + ->GetInternalField(StreamBase::kOnReadFunctionField) + .As(); CHECK(onread->IsFunction()); return wrap->MakeCallback(onread.As(), arraysize(argv), argv); } diff --git a/test/common/gc.js b/test/common/gc.js new file mode 100644 index 00000000000000..af637af7bedcd6 --- /dev/null +++ b/test/common/gc.js @@ -0,0 +1,70 @@ +'use strict'; + +// TODO(joyeecheung): merge ongc.js and gcUntil from common/index.js +// into this. + +// This function can be used to check if an object factor leaks or not, +// but it needs to be used with care: +// 1. The test should be set up with an ideally small +// --max-old-space-size or --max-heap-size, which combined with +// the maxCount parameter can reproduce a leak of the objects +// created by fn(). +// 2. This works under the assumption that if *none* of the objects +// created by fn() can be garbage-collected, the test would crash due +// to OOM. +// 3. If *any* of the objects created by fn() can be garbage-collected, +// it is considered leak-free. The FinalizationRegistry is used to +// terminate the test early once we detect any of the object is +// garbage-collected to make the test less prone to false positives. +// This may be especially important for memory management relying on +// emphemeron GC which can be inefficient to deal with extremely fast +// heap growth. +// Note that this can still produce false positives. When the test using +// this function still crashes due to OOM, inspect the heap to confirm +// if a leak is present (e.g. using heap snapshots). +// The generateSnapshotAt parameter can be used to specify a count +// interval to create the heap snapshot which may enforce a more thorough GC. +// This can be tried for code paths that require it for the GC to catch up +// with heap growth. However this type of forced GC can be in conflict with +// other logic in V8 such as bytecode aging, and it can slow down the test +// significantly, so it should be used scarcely and only as a last resort. +async function checkIfCollectable( + fn, maxCount = 4096, generateSnapshotAt = Infinity, logEvery = 128) { + let anyFinalized = false; + let count = 0; + + const f = new FinalizationRegistry(() => { + anyFinalized = true; + }); + + async function createObject() { + const obj = await fn(); + f.register(obj); + if (count++ < maxCount && !anyFinalized) { + setImmediate(createObject, 1); + } + // This can force a more thorough GC, but can slow the test down + // significantly in a big heap. Use it with care. + if (count % generateSnapshotAt === 0) { + // XXX(joyeecheung): This itself can consume a bit of JS heap memory, + // but the other alternative writeHeapSnapshot can run into disk space + // not enough problems in the CI & be slower depending on file system. + // Just do this for now as long as it works and only invent some + // internal voodoo when we absolutely have no other choice. + require('v8').getHeapSnapshot().pause().read(); + console.log(`Generated heap snapshot at ${count}`); + } + if (count % logEvery === 0) { + console.log(`Created ${count} objects`); + } + if (anyFinalized) { + console.log(`Found finalized object at ${count}, stop testing`); + } + } + + createObject(); +} + +module.exports = { + checkIfCollectable, +}; diff --git a/test/es-module/test-dynamic-import-script-lifetime.js b/test/es-module/test-dynamic-import-script-lifetime.js new file mode 100644 index 00000000000000..7ccea887109e05 --- /dev/null +++ b/test/es-module/test-dynamic-import-script-lifetime.js @@ -0,0 +1,32 @@ +// Flags: --expose-gc --experimental-vm-modules + +'use strict'; + +// This tests that vm.Script would not get GC'ed while the script can still +// initiate dynamic import. +// See https://github.com/nodejs/node/issues/43205. + +require('../common'); +const vm = require('vm'); + +const code = ` +new Promise(resolve => { + setTimeout(() => { + gc(); // vm.Script should not be GC'ed while the script is alive. + resolve(); + }, 1); +}).then(() => import('foo'));`; + +// vm.runInThisContext creates a vm.Script underneath, which should not be GC'ed +// while import() can still be initiated. +vm.runInThisContext(code, { + async importModuleDynamically() { + const m = new vm.SyntheticModule(['bar'], () => { + m.setExport('bar', 1); + }); + + await m.link(() => {}); + await m.evaluate(); + return m; + } +}); diff --git a/test/es-module/test-vm-compile-function-leak.js b/test/es-module/test-vm-compile-function-leak.js new file mode 100644 index 00000000000000..f9f04588fdc7c3 --- /dev/null +++ b/test/es-module/test-vm-compile-function-leak.js @@ -0,0 +1,16 @@ +// Flags: --max-old-space-size=16 --trace-gc +'use strict'; + +// This tests that vm.compileFunction with dynamic import callback does not leak. +// See https://github.com/nodejs/node/issues/44211 +require('../common'); +const { checkIfCollectable } = require('../common/gc'); +const vm = require('vm'); + +async function createCompiledFunction() { + return vm.compileFunction(`"${Math.random().toString().repeat(512)}"`, [], { + async importModuleDynamically() {}, + }); +} + +checkIfCollectable(createCompiledFunction, 2048); diff --git a/test/es-module/test-vm-compile-function-lineoffset.js b/test/es-module/test-vm-compile-function-lineoffset.js new file mode 100644 index 00000000000000..47846a3aa6eb8b --- /dev/null +++ b/test/es-module/test-vm-compile-function-lineoffset.js @@ -0,0 +1,34 @@ +'use strict'; + +require('../common'); + +const assert = require('assert'); +const { compileFunction } = require('node:vm'); + +const min = -2147483648; +const max = 2147483647; + +compileFunction('', [], { lineOffset: min, columnOffset: min }); +compileFunction('', [], { lineOffset: max, columnOffset: max }); + +assert.throws( + () => { + compileFunction('', [], { lineOffset: min - 1, columnOffset: max }); + }, + { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: /The value of "options\.lineOffset" is out of range/, + } +); + +assert.throws( + () => { + compileFunction('', [], { lineOffset: min, columnOffset: min - 1 }); + }, + { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: /The value of "options\.columnOffset" is out of range/, + } +); diff --git a/test/es-module/test-vm-contextified-script-leak.js b/test/es-module/test-vm-contextified-script-leak.js new file mode 100644 index 00000000000000..a8625fc94d246e --- /dev/null +++ b/test/es-module/test-vm-contextified-script-leak.js @@ -0,0 +1,16 @@ +// Flags: --max-old-space-size=16 --trace-gc +'use strict'; + +// This tests that vm.Script with dynamic import callback does not leak. +// See: https://github.com/nodejs/node/issues/33439 +require('../common'); +const { checkIfCollectable } = require('../common/gc'); +const vm = require('vm'); + +async function createContextifyScript() { + // Try to reach the maximum old space size. + return new vm.Script(`"${Math.random().toString().repeat(512)}";`, { + async importModuleDynamically() {}, + }); +} +checkIfCollectable(createContextifyScript, 2048, 512); diff --git a/test/es-module/test-vm-source-text-module-leak.js b/test/es-module/test-vm-source-text-module-leak.js new file mode 100644 index 00000000000000..d05e812ac32c95 --- /dev/null +++ b/test/es-module/test-vm-source-text-module-leak.js @@ -0,0 +1,21 @@ +// Flags: --experimental-vm-modules --max-old-space-size=16 --trace-gc +'use strict'; + +// This tests that vm.SourceTextModule() does not leak. +// See: https://github.com/nodejs/node/issues/33439 +require('../common'); +const { checkIfCollectable } = require('../common/gc'); +const vm = require('vm'); + +async function createSourceTextModule() { + // Try to reach the maximum old space size. + const m = new vm.SourceTextModule(` + const bar = new Array(512).fill("----"); + export { bar }; + `); + await m.link(() => {}); + await m.evaluate(); + return m; +} + +checkIfCollectable(createSourceTextModule, 4096, 1024); diff --git a/test/es-module/test-vm-synthetic-module-leak.js b/test/es-module/test-vm-synthetic-module-leak.js new file mode 100644 index 00000000000000..bc0e4689535327 --- /dev/null +++ b/test/es-module/test-vm-synthetic-module-leak.js @@ -0,0 +1,18 @@ +// Flags: --experimental-vm-modules --max-old-space-size=16 +'use strict'; + +// This tests that vm.SyntheticModule does not leak. +// See https://github.com/nodejs/node/issues/44211 +require('../common'); +const { checkIfCollectable } = require('../common/gc'); +const vm = require('vm'); + +async function createSyntheticModule() { + const m = new vm.SyntheticModule(['bar'], () => { + m.setExport('bar', new Array(512).fill('----')); + }); + await m.link(() => {}); + await m.evaluate(); + return m; +} +checkIfCollectable(createSyntheticModule, 4096); diff --git a/test/message/eval_messages.out b/test/message/eval_messages.out index 3f44332c03a470..e07bbe4d6acd3c 100644 --- a/test/message/eval_messages.out +++ b/test/message/eval_messages.out @@ -2,10 +2,9 @@ [eval]:1 with(this){__filename} ^^^^ + SyntaxError: Strict mode code may not include a with statement - at new Script (node:vm:*:*) - at createScript (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at makeContextifyScript (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -21,8 +20,7 @@ throw new Error("hello") Error: hello at [eval]:1:7 - at Script.runInThisContext (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -37,8 +35,7 @@ throw new Error("hello") Error: hello at [eval]:1:7 - at Script.runInThisContext (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -53,8 +50,7 @@ var x = 100; y = x; ReferenceError: y is not defined at [eval]:1:16 - at Script.runInThisContext (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) diff --git a/test/message/stdin_messages.out b/test/message/stdin_messages.out index 3fd047aacb11a2..6afc8a62d7fcd9 100644 --- a/test/message/stdin_messages.out +++ b/test/message/stdin_messages.out @@ -4,9 +4,7 @@ with(this){__filename} ^^^^ SyntaxError: Strict mode code may not include a with statement - at new Script (node:vm:*) - at createScript (node:vm:*) - at Object.runInThisContext (node:vm:*) + at makeContextifyScript (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -14,6 +12,8 @@ SyntaxError: Strict mode code may not include a with statement at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) + at process.processTicksAndRejections (node:internal/process/task_queues:*:*) Node.js * 42 @@ -24,8 +24,7 @@ throw new Error("hello") Error: hello at [stdin]:1:7 - at Script.runInThisContext (node:vm:*) - at Object.runInThisContext (node:vm:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -33,6 +32,7 @@ Error: hello at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) Node.js * [stdin]:1 @@ -41,8 +41,7 @@ throw new Error("hello") Error: hello at [stdin]:1:* - at Script.runInThisContext (node:vm:*) - at Object.runInThisContext (node:vm:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -50,6 +49,7 @@ Error: hello at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) Node.js * 100 @@ -59,8 +59,7 @@ let x = 100; y = x; ReferenceError: y is not defined at [stdin]:1:16 - at Script.runInThisContext (node:vm:*) - at Object.runInThisContext (node:vm:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -68,6 +67,7 @@ ReferenceError: y is not defined at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) Node.js * diff --git a/test/parallel/test-vm-dynamic-import-callback-missing-flag.js b/test/parallel/test-vm-dynamic-import-callback-missing-flag.js new file mode 100644 index 00000000000000..4b0d09ca3674a7 --- /dev/null +++ b/test/parallel/test-vm-dynamic-import-callback-missing-flag.js @@ -0,0 +1,28 @@ +'use strict'; + +const common = require('../common'); +const { Script, compileFunction } = require('vm'); +const assert = require('assert'); + +assert( + !process.execArgv.includes('--experimental-vm-modules'), + 'This test must be run without --experimental-vm-modules'); + +assert.rejects(async () => { + const script = new Script('import("fs")', { + importModuleDynamically: common.mustNotCall(), + }); + const imported = script.runInThisContext(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG' +}).then(common.mustCall()); + +assert.rejects(async () => { + const imported = compileFunction('return import("fs")', [], { + importModuleDynamically: common.mustNotCall(), + })(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG' +}).then(common.mustCall()); diff --git a/test/parallel/test-vm-no-dynamic-import-callback.js b/test/parallel/test-vm-no-dynamic-import-callback.js new file mode 100644 index 00000000000000..35b553d587c6e4 --- /dev/null +++ b/test/parallel/test-vm-no-dynamic-import-callback.js @@ -0,0 +1,20 @@ +'use strict'; + +const common = require('../common'); +const { Script, compileFunction } = require('vm'); +const assert = require('assert'); + +assert.rejects(async () => { + const script = new Script('import("fs")'); + const imported = script.runInThisContext(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING' +}).then(common.mustCall()); + +assert.rejects(async () => { + const imported = compileFunction('return import("fs")')(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING' +}).then(common.mustCall());