objects: IsGraphAsync should return false for source imports

legendecas · V8 LUCI CQ · commit 8c638129d872 · 2025-09-25T09:29:27.000-07:00
Fixes a type assumption that elements in `requested_modules` can be a non-Module object when it is a source phase import. Bug: 42204365 Change-Id: If889ee82a72f294a635c90efa459e47f1a830df3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6951331 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Chengzhong Wu <cwu631@bloomberg.net> Cr-Commit-Position: refs/heads/main@{#102772}
diff --git a/src/objects/module.cc b/src/objects/module.cc
@@ -488,10 +488,13 @@ bool Module::IsGraphAsync(Isolate* isolate) const {
   // Only SourceTextModules may be async.
   if (!IsSourceTextModule(*this)) return false;
   Tagged<SourceTextModule> root = Cast<SourceTextModule>(*this);
+  DCHECK(root->status() == kLinked || root->status() == kEvaluated ||
+         root->status() == kEvaluatingAsync || root->status() == kErrored);
 
   Zone zone(isolate->allocator(), ZONE_NAME);
   const size_t bucket_count = 2;
-  ZoneUnorderedSet<Tagged<Module>, Module::Hash> visited(&zone, bucket_count);
+  ZoneUnorderedSet<Tagged<SourceTextModule>, Module::Hash> visited(
+      &zone, bucket_count);
   ZoneVector<Tagged<SourceTextModule>> worklist(&zone);
   visited.insert(root);
   worklist.push_back(root);
@@ -504,10 +507,18 @@ bool Module::IsGraphAsync(Isolate* isolate) const {
     if (current->has_toplevel_await()) return true;
     Tagged<FixedArray> requested_modules = current->requested_modules();
     for (int i = 0, length = requested_modules->length(); i < length; ++i) {
-      Tagged<Module> descendant = Cast<Module>(requested_modules->get(i));
-      if (IsSourceTextModule(descendant)) {
+      Tagged<Object> raw_descendant = requested_modules->get(i);
+      // The current module must have been linked as the root has been linked.
+      // If the request is a source phase import, the descendant can be a
+      // JavaScript object, and it can not be async. Skip it.
+      // If the request is an evaluation phase import, the descendant can be
+      // either a SourceTextModule or a SyntheticModule. Visit it if it is a
+      // SourceTextModule.
+      if (IsSourceTextModule(raw_descendant)) {
+        Tagged<SourceTextModule> descendant =
+            Cast<SourceTextModule>(raw_descendant);
         const bool cycle = !visited.insert(descendant).second;
-        if (!cycle) worklist.push_back(Cast<SourceTextModule>(descendant));
+        if (!cycle) worklist.push_back(descendant);
       }
     }
   } while (!worklist.empty());
diff --git a/test/unittests/objects/modules-unittest.cc b/test/unittests/objects/modules-unittest.cc
@@ -4,6 +4,7 @@
 
 #include "include/v8-function.h"
 #include "src/flags/flags.h"
+#include "test/common/flag-utils.h"
 #include "test/unittests/test-utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #if V8_ENABLE_WEBASSEMBLY
@@ -25,6 +26,7 @@ using v8::Location;
 using v8::MaybeLocal;
 using v8::Module;
 using v8::ModuleRequest;
+using v8::Object;
 using v8::Promise;
 using v8::ScriptCompiler;
 using v8::ScriptOrigin;
@@ -1213,6 +1215,60 @@ TEST_F(ModuleTest, IsGraphAsyncTopLevelAwait) {
   cycle_two_module_global.Reset();
 }
 
+bool resolve_source_return_object_invoked = false;
+MaybeLocal<Object> ResolveSourceReturnObject(
+    Local<Context> context, Local<String> specifier,
+    Local<FixedArray> import_attributes, Local<Module> referrer) {
+  Isolate* isolate = Isolate::GetCurrent();
+
+  CHECK(!specifier.IsEmpty());
+  String::Utf8Value specifier_utf8(isolate, specifier);
+  CHECK_EQ(0, strcmp("my-mod", *specifier_utf8));
+
+  CHECK_EQ(0, import_attributes->Length());
+
+  resolve_source_return_object_invoked = true;
+
+  return Object::New(isolate);
+}
+
+TEST_F(ModuleTest, IsGraphAsyncImportSource) {
+  i::FlagScope<bool> f(&i::v8_flags.js_source_phase_imports, true);
+
+  HandleScope scope(isolate());
+
+  // Check that v8::Module::IsGraphAsync() returns false for source
+  // phase imports.
+
+  Local<String> url = NewString("www.google.com");
+  Local<String> source_text =
+      NewString("import source modSource from 'my-mod';");
+
+  ScriptOrigin origin(url, 0, 0, false, -1, Local<v8::Value>(), false, false,
+                      true);
+  ScriptCompiler::Source source(source_text, origin);
+
+  Local<Module> module =
+      ScriptCompiler::CompileModule(isolate(), &source).ToLocalChecked();
+
+  CHECK(!resolve_source_return_object_invoked);
+  CHECK(module
+            ->InstantiateModule(
+                context(),
+                [](Local<Context> context, Local<String> specifier,
+                   Local<FixedArray> import_attributes,
+                   Local<Module> referrer) -> MaybeLocal<Module> {
+                  // There is no evaluation phase import.
+                  UNREACHABLE();
+                },
+                ResolveSourceReturnObject)
+            .IsJust());
+  CHECK(resolve_source_return_object_invoked);
+
+  // IsGraphAsync should return false
+  CHECK_EQ(module->IsGraphAsync(), false);
+}
+
 TEST_F(ModuleTest, HasTopLevelAwait) {
   HandleScope scope(isolate());
   {
