[runtime] Remove SourceTextModule::script field

By keeping the SharedFunctionInfo around for modules with the kErrored
status, we don't need the additional script field anymore. The script
can thus be always accessed indirectly through the code object.

Removing the script field fixes context serialization of modules.

Bug: v8:11073
Change-Id: I9bb3c6b129a41e9d708547ceeb35e6d921c8eea0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2504256
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71096}

src/api/api.cc

@@ -2268,7 +2268,7 @@ Location Module::GetModuleRequestLocation(int i) const {
   CHECK_LT(i, module_request_positions->length());
   int position = i::Smi::ToInt(module_request_positions->get(i));
   i::Handle<i::Script> script(
-      i::Handle<i::SourceTextModule>::cast(self)->script(), isolate);
+      i::Handle<i::SourceTextModule>::cast(self)->GetScript(), isolate);
   i::Script::PositionInfo info;
   i::Script::GetPositionInfo(script, position, &info, i::Script::WITH_OFFSET);
   return v8::Location(info.line, info.column);
@@ -2285,12 +2285,11 @@ Local<Value> Module::GetModuleNamespace() {
 }
 
 Local<UnboundModuleScript> Module::GetUnboundModuleScript() {
-  Utils::ApiCheck(
-      GetStatus() < kEvaluating, "v8::Module::GetUnboundScript",
-      "v8::Module::GetUnboundScript must be used on an unevaluated module");
   i::Handle<i::Module> self = Utils::OpenHandle(this);
-  CHECK(self->IsSourceTextModule());
-  return ToApiHandle<UnboundModuleScript>(i::Handle<i::SharedFunctionInfo>(
+  Utils::ApiCheck(
+      self->IsSourceTextModule(), "v8::Module::GetUnboundModuleScript",
+      "v8::Module::GetUnboundModuleScript must be used on an SourceTextModule");
+  return ToApiHandle<UnboundModuleScript>(i::handle(
       i::Handle<i::SourceTextModule>::cast(self)->GetSharedFunctionInfo(),
       self->GetIsolate()));
 }
@@ -2299,14 +2298,7 @@ int Module::ScriptId() {
   i::Handle<i::Module> self = Utils::OpenHandle(this);
   Utils::ApiCheck(self->IsSourceTextModule(), "v8::Module::ScriptId",
                   "v8::Module::ScriptId must be used on an SourceTextModule");
-
-  // The SharedFunctionInfo is not available for errored modules.
-  Utils::ApiCheck(GetStatus() != kErrored, "v8::Module::ScriptId",
-                  "v8::Module::ScriptId must not be used on an errored module");
-  i::Handle<i::SharedFunctionInfo> sfi(
-      i::Handle<i::SourceTextModule>::cast(self)->GetSharedFunctionInfo(),
-      self->GetIsolate());
-  return ToApiHandle<UnboundScript>(sfi)->GetId();
+  return i::Handle<i::SourceTextModule>::cast(self)->GetScript().id();
 }
 
 bool Module::IsGraphAsync() const {

src/diagnostics/objects-debug.cc

@@ -1400,7 +1400,7 @@ void SourceTextModule::SourceTextModuleVerify(Isolate* isolate) {
   TorqueGeneratedClassVerifiers::SourceTextModuleVerify(*this, isolate);
 
   if (status() == kErrored) {
-    CHECK(code().IsSourceTextModuleInfo());
+    CHECK(code().IsSharedFunctionInfo());
   } else if (status() == kEvaluating || status() == kEvaluated) {
     CHECK(code().IsJSGeneratorObject());
   } else {

src/diagnostics/objects-printer.cc

@@ -1552,10 +1552,11 @@ void Module::ModulePrint(std::ostream& os) {  // NOLINT
 void SourceTextModule::SourceTextModulePrint(std::ostream& os) {  // NOLINT
   PrintHeader(os, "SourceTextModule");
   PrintModuleFields(*this, os);
-  os << "\n - origin: " << Brief(script().GetNameOrSourceURL());
-  os << "\n - code: " << Brief(code());
+  os << "\n - sfi/code/info: " << Brief(code());
+  Script script = GetScript();
+  os << "\n - script: " << Brief(script);
+  os << "\n - origin: " << Brief(script.GetNameOrSourceURL());
   os << "\n - requested_modules: " << Brief(requested_modules());
-  os << "\n - script: " << Brief(script());
   os << "\n - import_meta: " << Brief(import_meta());
   os << "\n";
 }
@@ -1564,6 +1565,7 @@ void SyntheticModule::SyntheticModulePrint(std::ostream& os) {  // NOLINT
   PrintHeader(os, "SyntheticModule");
   PrintModuleFields(*this, os);
   os << "\n - export_names: " << Brief(export_names());
+  os << "\n - name: " << Brief(name());
   os << "\n";
 }
 

src/heap/factory.cc

@@ -2289,9 +2289,9 @@ Handle<JSGeneratorObject> Factory::NewJSGeneratorObject(
 }
 
 Handle<SourceTextModule> Factory::NewSourceTextModule(
-    Handle<SharedFunctionInfo> code) {
+    Handle<SharedFunctionInfo> sfi) {
   Handle<SourceTextModuleInfo> module_info(
-      code->scope_info().ModuleDescriptorInfo(), isolate());
+      sfi->scope_info().ModuleDescriptorInfo(), isolate());
   Handle<ObjectHashTable> exports =
       ObjectHashTable::New(isolate(), module_info->RegularExportCount());
   Handle<FixedArray> regular_exports =
@@ -2309,14 +2309,13 @@ Handle<SourceTextModule> Factory::NewSourceTextModule(
       SourceTextModule::cast(
           New(source_text_module_map(), AllocationType::kOld)),
       isolate());
-  module->set_code(*code);
+  module->set_code(*sfi);
   module->set_exports(*exports);
   module->set_regular_exports(*regular_exports);
   module->set_regular_imports(*regular_imports);
   module->set_hash(isolate()->GenerateIdentityHash(Smi::kMaxValue));
   module->set_module_namespace(roots.undefined_value());
   module->set_requested_modules(*requested_modules);
-  module->set_script(Script::cast(code->script()));
   module->set_status(Module::kUninstantiated);
   module->set_exception(roots.the_hole_value());
   module->set_import_meta(roots.the_hole_value());
@@ -2324,7 +2323,7 @@ Handle<SourceTextModule> Factory::NewSourceTextModule(
   module->set_dfs_ancestor_index(-1);
   module->set_top_level_capability(roots.undefined_value());
   module->set_flags(0);
-  module->set_async(IsAsyncModule(code->kind()));
+  module->set_async(IsAsyncModule(sfi->kind()));
   module->set_async_evaluating(false);
   module->set_async_parent_modules(*async_parent_modules);
   module->set_pending_async_dependencies(0);

src/objects/module-inl.h

@@ -51,9 +51,7 @@ struct Module::Hash {
 };
 
 SourceTextModuleInfo SourceTextModule::info() const {
-  return status() == kErrored
-             ? SourceTextModuleInfo::cast(code())
-             : GetSharedFunctionInfo().scope_info().ModuleDescriptorInfo();
+  return GetSharedFunctionInfo().scope_info().ModuleDescriptorInfo();
 }
 
 OBJECT_CONSTRUCTORS_IMPL(SourceTextModuleInfo, FixedArray)

src/objects/module.cc

@@ -10,12 +10,14 @@
 #include "src/api/api-inl.h"
 #include "src/ast/modules.h"
 #include "src/builtins/accessors.h"
+#include "src/common/assert-scope.h"
 #include "src/heap/heap-inl.h"
 #include "src/objects/cell-inl.h"
 #include "src/objects/hash-table-inl.h"
 #include "src/objects/js-generator-inl.h"
 #include "src/objects/module-inl.h"
 #include "src/objects/objects-inl.h"
+#include "src/objects/source-text-module.h"
 #include "src/objects/synthetic-module-inl.h"
 #include "src/utils/ostreams.h"
 
@@ -26,7 +28,7 @@ namespace {
 #ifdef DEBUG
 void PrintModuleName(Module module, std::ostream& os) {
   if (module.IsSourceTextModule()) {
-    SourceTextModule::cast(module).script().GetNameOrSourceURL().Print(os);
+    SourceTextModule::cast(module).GetScript().GetNameOrSourceURL().Print(os);
   } else {
     SyntheticModule::cast(module).name().Print(os);
   }
@@ -52,7 +54,7 @@ void PrintStatusMessage(Module module, const char* message) {
 #endif  // DEBUG
 
 void SetStatusInternal(Module module, Module::Status new_status) {
-  DisallowHeapAllocation no_alloc;
+  DisallowGarbageCollection no_gc;
 #ifdef DEBUG
   PrintStatusTransition(module, new_status);
 #endif  // DEBUG
@@ -62,7 +64,7 @@ void SetStatusInternal(Module module, Module::Status new_status) {
 }  // end namespace
 
 void Module::SetStatus(Status new_status) {
-  DisallowHeapAllocation no_alloc;
+  DisallowGarbageCollection no_gc;
   DCHECK_LE(status(), new_status);
   DCHECK_NE(new_status, Module::kErrored);
   SetStatusInternal(*this, new_status);
@@ -78,11 +80,14 @@ void Module::RecordErrorUsingPendingException(Isolate* isolate,
 // static
 void Module::RecordError(Isolate* isolate, Handle<Module> module,
                          Handle<Object> error) {
+  DisallowGarbageCollection no_gc;
   DCHECK(module->exception().IsTheHole(isolate));
   DCHECK(!error->IsTheHole(isolate));
   if (module->IsSourceTextModule()) {
-    Handle<SourceTextModule> self(SourceTextModule::cast(*module), isolate);
-    self->set_code(self->info());
+    // Revert to minmal SFI in case we have already been instantiating or
+    // evaluating.
+    auto self = SourceTextModule::cast(*module);
+    self.set_code(self.GetSharedFunctionInfo());
   }
   SetStatusInternal(*module, Module::kErrored);
   if (isolate->is_catchable_by_javascript(*error)) {

src/objects/source-text-module.cc

@@ -7,6 +7,7 @@
 #include "src/api/api-inl.h"
 #include "src/ast/modules.h"
 #include "src/builtins/accessors.h"
+#include "src/common/assert-scope.h"
 #include "src/objects/js-generator-inl.h"
 #include "src/objects/module-inl.h"
 #include "src/objects/objects-inl.h"
@@ -77,27 +78,28 @@ class Module::ResolveSet
 };
 
 SharedFunctionInfo SourceTextModule::GetSharedFunctionInfo() const {
-  DisallowHeapAllocation no_alloc;
+  DisallowGarbageCollection no_gc;
   switch (status()) {
     case kUninstantiated:
     case kPreInstantiating:
-      DCHECK(code().IsSharedFunctionInfo());
       return SharedFunctionInfo::cast(code());
     case kInstantiating:
-      DCHECK(code().IsJSFunction());
       return JSFunction::cast(code()).shared();
     case kInstantiated:
     case kEvaluating:
     case kEvaluated:
-      DCHECK(code().IsJSGeneratorObject());
       return JSGeneratorObject::cast(code()).function().shared();
     case kErrored:
-      UNREACHABLE();
+      return SharedFunctionInfo::cast(code());
   }
-
   UNREACHABLE();
 }
 
+Script SourceTextModule::GetScript() const {
+  DisallowGarbageCollection no_gc;
+  return Script::cast(GetSharedFunctionInfo().script());
+}
+
 int SourceTextModule::ExportIndex(int cell_index) {
   DCHECK_EQ(SourceTextModuleDescriptor::GetCellIndexKind(cell_index),
             SourceTextModuleDescriptor::kExport);
@@ -205,7 +207,7 @@ MaybeHandle<Cell> SourceTextModule::ResolveExport(
     Handle<SourceTextModuleInfoEntry> entry =
         Handle<SourceTextModuleInfoEntry>::cast(object);
     Handle<String> import_name(String::cast(entry->import_name()), isolate);
-    Handle<Script> script(module->script(), isolate);
+    Handle<Script> script(module->GetScript(), isolate);
     MessageLocation new_loc(script, entry->beg_pos(), entry->end_pos());
 
     Handle<Cell> cell;
@@ -269,7 +271,7 @@ MaybeHandle<Cell> SourceTextModule::ResolveExportUsingStarExports(
         continue;  // Indirect export.
       }
 
-      Handle<Script> script(module->script(), isolate);
+      Handle<Script> script(module->GetScript(), isolate);
       MessageLocation new_loc(script, entry->beg_pos(), entry->end_pos());
 
       Handle<Cell> cell;
@@ -466,7 +468,7 @@ bool SourceTextModule::FinishInstantiate(
     }
   }
 
-  Handle<Script> script(module->script(), isolate);
+  Handle<Script> script(module->GetScript(), isolate);
   Handle<SourceTextModuleInfo> module_info(module->info(), isolate);
 
   // Resolve imports.

src/objects/source-text-module.h

@@ -32,6 +32,8 @@ class SourceTextModule
   // kErrored.
   SharedFunctionInfo GetSharedFunctionInfo() const;
 
+  Script GetScript() const;
+
   // Whether or not this module is an async module. Set during module creation
   // and does not change afterwards.
   DECL_BOOLEAN_ACCESSORS(async)

src/objects/source-text-module.tq

@@ -12,7 +12,7 @@ bitfield struct SourceTextModuleFlags extends uint31 {
 @generateCppClass
 extern class SourceTextModule extends Module {
   // The code representing this module, or an abstraction thereof.
-  code: SharedFunctionInfo|JSFunction|JSGeneratorObject|SourceTextModuleInfo;
+  code: SharedFunctionInfo|JSFunction|JSGeneratorObject;
 
   // Arrays of cells corresponding to regular exports and regular imports.
   // A cell's position in the array is determined by the cell index of the
@@ -26,9 +26,6 @@ extern class SourceTextModule extends Module {
   // SourceTextModuleInfo::module_requests.
   requested_modules: FixedArray;
 
-  // Script from which the module originates.
-  script: Script;
-
   // The value of import.meta inside of this module.
   // Lazily initialized on first access. It's the hole before first access and
   // a JSObject afterwards.