[modules] Change ScriptOrModule to custom Struct

Due to caching issues we will not be able to store host-defined options
directly on the Script anymore. ScriptOrModule can thus no longer be
a i::Script.

NodeJS keeps weak references from ScriptOrModule to their import meta
data. This CL changes ScriptOrModule to be a temporary struct which has
a different lifetime. As a temporary fix until the API is fully updated
we introduce the v8_scriptormodule_legacy_lifetime compile-time flag.
It keeps references to ScriptOrModule alive on the Script to restore the
previous behavior (at an additional memory cost).

Bug: chromium:1244145
Change-Id: I1dc42d25930d7bc4f22ee3c9bba93d89425be406
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3211575
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77382}

BUILD.gn

@@ -343,6 +343,12 @@ declare_args() {
   # Enable global allocation site tracking.
   v8_allocation_site_tracking = true
 
+  # TODO(cbruni, v8:12302): Remove once API is migrated
+  # Enable legacy mode for ScriptOrModule's lifetime. By default it's a
+  # temporary object, if enabled it will be kept alive by the parent Script.
+  # This is only used by nodejs.
+  v8_scriptormodule_legacy_lifetime = false
+
   # If enabled, the receiver is always included in the actual and formal
   # parameter count of function with JS linkage.
   # TODO(v8:11112): Remove once all architectures support the flag and it is
@@ -962,6 +968,9 @@ config("features") {
   if (v8_allocation_site_tracking) {
     defines += [ "V8_ALLOCATION_SITE_TRACKING" ]
   }
+  if (v8_scriptormodule_legacy_lifetime) {
+    defines += [ "V8_SCRIPTORMODULE_LEGACY_LIFETIME" ]
+  }
   if (v8_advanced_bigint_algorithms) {
     defines += [ "V8_ADVANCED_BIGINT_ALGORITHMS" ]
   }

src/api/api-inl.h

@@ -111,7 +111,7 @@ MAKE_TO_LOCAL(CallableToLocal, JSReceiver, Function)
 MAKE_TO_LOCAL(ToLocalPrimitive, Object, Primitive)
 MAKE_TO_LOCAL(FixedArrayToLocal, FixedArray, FixedArray)
 MAKE_TO_LOCAL(PrimitiveArrayToLocal, FixedArray, PrimitiveArray)
-MAKE_TO_LOCAL(ScriptOrModuleToLocal, Script, ScriptOrModule)
+MAKE_TO_LOCAL(ToLocal, ScriptOrModule, ScriptOrModule)
 
 #undef MAKE_TO_LOCAL_TYPED_ARRAY
 #undef MAKE_TO_LOCAL

src/api/api.cc

@@ -2093,16 +2093,16 @@ MaybeLocal<Value> Script::Run(Local<Context> context) {
 }
 
 Local<Value> ScriptOrModule::GetResourceName() {
-  i::Handle<i::Script> obj = Utils::OpenHandle(this);
-  i::Isolate* isolate = obj->GetIsolate();
+  i::Handle<i::ScriptOrModule> obj = Utils::OpenHandle(this);
+  i::Isolate* isolate = i::GetIsolateFromWritableObject(*obj);
   ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
-  i::Handle<i::Object> val(obj->name(), isolate);
+  i::Handle<i::Object> val(obj->resource_name(), isolate);
   return ToApiHandle<Value>(val);
 }
 
 Local<PrimitiveArray> ScriptOrModule::GetHostDefinedOptions() {
-  i::Handle<i::Script> obj = Utils::OpenHandle(this);
-  i::Isolate* isolate = obj->GetIsolate();
+  i::Handle<i::ScriptOrModule> obj = Utils::OpenHandle(this);
+  i::Isolate* isolate = i::GetIsolateFromWritableObject(*obj);
   ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
   i::Handle<i::FixedArray> val(obj->host_defined_options(), isolate);
   return ToApiHandle<PrimitiveArray>(val);
@@ -2646,16 +2646,26 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
     RETURN_ON_FAILED_EXECUTION(Function);
     result = handle_scope.Escape(Utils::CallableToLocal(scoped_result));
   }
-
+  // TODO(cbruni): remove script_or_module_out paramater
   if (script_or_module_out != nullptr) {
     i::Handle<i::JSFunction> function =
         i::Handle<i::JSFunction>::cast(Utils::OpenHandle(*result));
     i::Isolate* isolate = function->GetIsolate();
     i::Handle<i::SharedFunctionInfo> shared(function->shared(), isolate);
     i::Handle<i::Script> script(i::Script::cast(shared->script()), isolate);
-    *script_or_module_out = v8::Utils::ScriptOrModuleToLocal(script);
+    // TODO(cbruni, v8:12302): Avoid creating tempory ScriptOrModule objects.
+    auto script_or_module = i::Handle<i::ScriptOrModule>::cast(
+        isolate->factory()->NewStruct(i::SCRIPT_OR_MODULE_TYPE));
+    script_or_module->set_resource_name(script->name());
+    script_or_module->set_host_defined_options(script->host_defined_options());
+#ifdef V8_SCRIPTORMODULE_LEGACY_LIFETIME
+    i::Handle<i::ArrayList> list =
+        i::handle(script->script_or_modules(), isolate);
+    list = i::ArrayList::Add(isolate, list, script_or_module);
+    script->set_script_or_modules(*list);
+#endif  // V8_SCRIPTORMODULE_LEGACY_LIFETIME
+    *script_or_module_out = v8::Utils::ToLocal(script_or_module);
   }
-
   return result;
 }
 

src/api/api.h

@@ -141,7 +141,7 @@ class RegisteredExtension {
   V(Primitive, Object)                         \
   V(PrimitiveArray, FixedArray)                \
   V(BigInt, BigInt)                            \
-  V(ScriptOrModule, Script)                    \
+  V(ScriptOrModule, ScriptOrModule)            \
   V(FixedArray, FixedArray)                    \
   V(ModuleRequest, ModuleRequest)              \
   IF_WASM(V, WasmMemoryObject, WasmMemoryObject)
@@ -254,8 +254,8 @@ class Utils {
       v8::internal::Handle<v8::internal::FixedArray> obj);
   static inline Local<PrimitiveArray> PrimitiveArrayToLocal(
       v8::internal::Handle<v8::internal::FixedArray> obj);
-  static inline Local<ScriptOrModule> ScriptOrModuleToLocal(
-      v8::internal::Handle<v8::internal::Script> obj);
+  static inline Local<ScriptOrModule> ToLocal(
+      v8::internal::Handle<v8::internal::ScriptOrModule> obj);
 
 #define DECLARE_OPEN_HANDLE(From, To)                              \
   static inline v8::internal::Handle<v8::internal::To> OpenHandle( \

src/diagnostics/objects-printer.cc

@@ -2079,6 +2079,12 @@ void AllocationMemento::AllocationMementoPrint(std::ostream& os) {
   }
 }
 
+void ScriptOrModule::ScriptOrModulePrint(std::ostream& os) {
+  PrintHeader(os, "ScriptOrModule");
+  os << "\n - host_defined_options: " << Brief(host_defined_options());
+  os << "\n - resource_name: " << Brief(resource_name());
+}
+
 void Script::ScriptPrint(std::ostream& os) {
   PrintHeader(os, "Script");
   os << "\n - source: " << Brief(source());

src/execution/isolate.cc

@@ -4338,7 +4338,7 @@ MaybeHandle<JSPromise> Isolate::RunHostImportModuleDynamicallyCallback(
     Handle<Script> referrer, Handle<Object> specifier,
     MaybeHandle<Object> maybe_import_assertions_argument) {
   v8::Local<v8::Context> api_context =
-      v8::Utils::ToLocal(Handle<Context>(native_context()));
+      v8::Utils::ToLocal(Handle<Context>::cast(native_context()));
   if (host_import_module_dynamically_with_import_assertions_callback_ ==
       nullptr) {
     Handle<Object> exception =
@@ -4357,21 +4357,25 @@ MaybeHandle<JSPromise> Isolate::RunHostImportModuleDynamicallyCallback(
 
   v8::Local<v8::Promise> promise;
   Handle<FixedArray> import_assertions_array;
-  if (GetImportAssertionsFromArgument(maybe_import_assertions_argument)
-          .ToHandle(&import_assertions_array)) {
-    ASSIGN_RETURN_ON_SCHEDULED_EXCEPTION_VALUE(
-        this, promise,
-        host_import_module_dynamically_with_import_assertions_callback_(
-            api_context, v8::Utils::ScriptOrModuleToLocal(referrer),
-            v8::Utils::ToLocal(specifier_str),
-            ToApiHandle<v8::FixedArray>(import_assertions_array)),
-        MaybeHandle<JSPromise>());
-    return v8::Utils::OpenHandle(*promise);
-  } else {
+  if (!GetImportAssertionsFromArgument(maybe_import_assertions_argument)
+           .ToHandle(&import_assertions_array)) {
     Handle<Object> exception(pending_exception(), this);
     clear_pending_exception();
     return NewRejectedPromise(this, api_context, exception);
   }
+  // TODO(cbruni, v8:12302): Avoid creating tempory ScriptOrModule objects.
+  auto script_or_module = i::Handle<i::ScriptOrModule>::cast(
+      this->factory()->NewStruct(i::SCRIPT_OR_MODULE_TYPE));
+  script_or_module->set_resource_name(referrer->name());
+  script_or_module->set_host_defined_options(referrer->host_defined_options());
+  ASSIGN_RETURN_ON_SCHEDULED_EXCEPTION_VALUE(
+      this, promise,
+      host_import_module_dynamically_with_import_assertions_callback_(
+          api_context, v8::Utils::ToLocal(script_or_module),
+          v8::Utils::ToLocal(specifier_str),
+          ToApiHandle<v8::FixedArray>(import_assertions_array)),
+      MaybeHandle<JSPromise>());
+  return v8::Utils::OpenHandle(*promise);
 }
 
 MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument(

src/heap/factory-base.cc

@@ -228,6 +228,9 @@ Handle<Script> FactoryBase<Impl>::NewScriptWithId(
   DCHECK(source->IsString() || source->IsUndefined());
   // Create and initialize script object.
   ReadOnlyRoots roots = read_only_roots();
+#ifdef V8_SCRIPTORMODULE_LEGACY_LIFETIME
+  Handle<ArrayList> list = NewArrayList(0);
+#endif
   Handle<Script> script = handle(
       NewStructInternal<Script>(SCRIPT_TYPE, AllocationType::kOld), isolate());
   {
@@ -248,6 +251,9 @@ Handle<Script> FactoryBase<Impl>::NewScriptWithId(
                                   SKIP_WRITE_BARRIER);
     raw.set_flags(0);
     raw.set_host_defined_options(roots.empty_fixed_array(), SKIP_WRITE_BARRIER);
+#ifdef V8_SCRIPTORMODULE_LEGACY_LIFETIME
+    raw.set_script_or_modules(*list);
+#endif
   }
 
   if (script_id != Script::kTemporaryScriptId) {
@@ -258,6 +264,15 @@ Handle<Script> FactoryBase<Impl>::NewScriptWithId(
   return script;
 }
 
+template <typename Impl>
+Handle<ArrayList> FactoryBase<Impl>::NewArrayList(int size) {
+  Handle<FixedArray> fixed_array = NewFixedArray(size + ArrayList::kFirstIndex);
+  fixed_array->set_map_no_write_barrier(read_only_roots().array_list_map());
+  Handle<ArrayList> result = Handle<ArrayList>::cast(fixed_array);
+  result->SetLength(0);
+  return result;
+}
+
 template <typename Impl>
 Handle<SharedFunctionInfo> FactoryBase<Impl>::NewSharedFunctionInfoForLiteral(
     FunctionLiteral* literal, Handle<Script> script, bool is_toplevel) {

src/heap/factory-base.h

@@ -153,6 +153,8 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) FactoryBase
   Handle<Script> NewScriptWithId(Handle<PrimitiveHeapObject> source,
                                  int script_id);
 
+  Handle<ArrayList> NewArrayList(int size);
+
   Handle<SharedFunctionInfo> NewSharedFunctionInfoForLiteral(
       FunctionLiteral* literal, Handle<Script> script, bool is_toplevel);
 

src/heap/factory.cc

@@ -1300,6 +1300,9 @@ void Factory::AddToScriptList(Handle<Script> script) {
 Handle<Script> Factory::CloneScript(Handle<Script> script) {
   Heap* heap = isolate()->heap();
   int script_id = isolate()->GetNextScriptId();
+#ifdef V8_SCRIPTORMODULE_LEGACY_LIFETIME
+  Handle<ArrayList> list = ArrayList::New(isolate(), 0);
+#endif
   Handle<Script> new_script_handle =
       Handle<Script>::cast(NewStruct(SCRIPT_TYPE, AllocationType::kOld));
   {
@@ -1321,7 +1324,11 @@ Handle<Script> Factory::CloneScript(Handle<Script> script) {
     new_script.set_eval_from_position(old_script.eval_from_position());
     new_script.set_flags(old_script.flags());
     new_script.set_host_defined_options(old_script.host_defined_options());
+#ifdef V8_SCRIPTORMODULE_LEGACY_LIFETIME
+    new_script.set_script_or_modules(*list);
+#endif
   }
+
   Handle<WeakArrayList> scripts = script_list();
   scripts = WeakArrayList::AddToEnd(isolate(), scripts,
                                     MaybeObjectHandle::Weak(new_script_handle));

src/objects/fixed-array.h

@@ -480,12 +480,13 @@ class ArrayList : public TorqueGeneratedArrayList<ArrayList, FixedArray> {
 
   static const int kHeaderFields = 1;
 
- private:
-  static Handle<ArrayList> EnsureSpace(Isolate* isolate,
-                                       Handle<ArrayList> array, int length);
   static const int kLengthIndex = 0;
   static const int kFirstIndex = 1;
   STATIC_ASSERT(kHeaderFields == kFirstIndex);
+
+ private:
+  static Handle<ArrayList> EnsureSpace(Isolate* isolate,
+                                       Handle<ArrayList> array, int length);
   TQ_OBJECT_CONSTRUCTORS(ArrayList)
 };
 

src/objects/module-inl.h

@@ -22,6 +22,7 @@ namespace internal {
 
 TQ_OBJECT_CONSTRUCTORS_IMPL(Module)
 TQ_OBJECT_CONSTRUCTORS_IMPL(JSModuleNamespace)
+TQ_OBJECT_CONSTRUCTORS_IMPL(ScriptOrModule)
 
 NEVER_READ_ONLY_SPACE_IMPL(Module)
 NEVER_READ_ONLY_SPACE_IMPL(ModuleRequest)

src/objects/module.h

@@ -177,6 +177,13 @@ class JSModuleNamespace
   TQ_OBJECT_CONSTRUCTORS(JSModuleNamespace)
 };
 
+class ScriptOrModule
+    : public TorqueGeneratedScriptOrModule<ScriptOrModule, Struct> {
+ public:
+  DECL_PRINTER(ScriptOrModule)
+  TQ_OBJECT_CONSTRUCTORS(ScriptOrModule)
+};
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/module.tq

@@ -18,3 +18,8 @@ extern class Module extends HeapObject {
 }
 
 extern class JSModuleNamespace extends JSSpecialObject { module: Module; }
+
+extern class ScriptOrModule extends Struct {
+  resource_name: Object;
+  host_defined_options: FixedArray;
+}

src/objects/objects-definitions.h

@@ -144,6 +144,7 @@ namespace internal {
   V(_, REG_EXP_BOILERPLATE_DESCRIPTION_TYPE, RegExpBoilerplateDescription,     \
     regexp_boilerplate_description)                                            \
   V(_, SCRIPT_TYPE, Script, script)                                            \
+  V(_, SCRIPT_OR_MODULE_TYPE, ScriptOrModule, script_or_module)                \
   V(_, SOURCE_TEXT_MODULE_INFO_ENTRY_TYPE, SourceTextModuleInfoEntry,          \
     module_info_entry)                                                         \
   V(_, STACK_FRAME_INFO_TYPE, StackFrameInfo, stack_frame_info)                \

src/objects/objects.cc

@@ -4014,13 +4014,7 @@ Handle<ArrayList> ArrayList::Add(Isolate* isolate, Handle<ArrayList> array,
 
 // static
 Handle<ArrayList> ArrayList::New(Isolate* isolate, int size) {
-  Handle<FixedArray> fixed_array =
-      isolate->factory()->NewFixedArray(size + kFirstIndex);
-  fixed_array->set_map_no_write_barrier(
-      ReadOnlyRoots(isolate).array_list_map());
-  Handle<ArrayList> result = Handle<ArrayList>::cast(fixed_array);
-  result->SetLength(0);
-  return result;
+  return isolate->factory()->NewArrayList(size);
 }
 
 Handle<FixedArray> ArrayList::Elements(Isolate* isolate,

src/objects/script.tq

@@ -59,4 +59,9 @@ extern class Script extends Struct {
 
   // [host_defined_options]: Options defined by the embedder.
   host_defined_options: FixedArray;
+
+  // TODO(cbruni, v8:12302): remove once module callback API is updated.
+  // Used to make sure we are backwards compatible with node to gaurantee
+  // the same lifetime for ScriptOrModule as the Script they originated.
+  @if(V8_SCRIPTORMODULE_LEGACY_LIFETIME) script_or_modules: ArrayList;
 }

src/torque/torque-parser.cc

@@ -50,6 +50,11 @@ class BuildFlags : public ContextualClass<BuildFlags> {
     build_flags_["TAGGED_SIZE_8_BYTES"] = TAGGED_SIZE_8_BYTES;
     build_flags_["TRUE_FOR_TESTING"] = true;
     build_flags_["FALSE_FOR_TESTING"] = false;
+#ifdef V8_SCRIPTORMODULE_LEGACY_LIFETIME
+    build_flags_["V8_SCRIPTORMODULE_LEGACY_LIFETIME"] = true;
+#else
+    build_flags_["V8_SCRIPTORMODULE_LEGACY_LIFETIME"] = false;
+#endif
   }
   static bool GetFlag(const std::string& name, const char* production) {
     auto it = Get().build_flags_.find(name);

test/cctest/test-compiler.cc

@@ -659,7 +659,7 @@ TEST(CompileFunctionInContextScriptOrigin) {
   LocalContext env;
   v8::ScriptOrigin origin(isolate, v8_str("test"), 22, 41);
   v8::ScriptCompiler::Source script_source(v8_str("throw new Error()"), origin);
-  Local<ScriptOrModule> script;
+  Local<v8::ScriptOrModule> script;
   v8::Local<v8::Function> fun =
       v8::ScriptCompiler::CompileFunctionInContext(
           env.local(), &script_source, 0, nullptr, 0, nullptr,