[snapshot] Check if a cached data has wrapped arguments

legendecas · V8 LUCI CQ · commit 96ee9bb9e830 · 2025-01-03T07:23:59.000-08:00
Fixes that ScriptCompiler::CreateCodeCacheForFunction aborts on a deserialized shared function info from a cached data accepted with ScriptCompiler::CompileFunction. If the wrapped argument list does not match, the cached data should be rejected. Refs: nodejs/node#56366 Change-Id: I3f0376216791d866fac8eed1ce88dfa05e919b48 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6140933 Commit-Queue: Chengzhong Wu <cwu631@bloomberg.net> Reviewed-by: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/main@{#97942}
diff --git a/src/snapshot/code-serializer.cc b/src/snapshot/code-serializer.cc
@@ -71,9 +71,16 @@ ScriptCompiler::CachedData* CodeSerializer::Serialize(
 
   // Serialize code object.
   DirectHandle<String> source(Cast<String>(script->source()), isolate);
+  DirectHandle<FixedArray> wrapped_arguments;
+  if (script->is_wrapped()) {
+    wrapped_arguments =
+        DirectHandle<FixedArray>(script->wrapped_arguments(), isolate);
+  }
+
   HandleScope scope(isolate);
-  CodeSerializer cs(isolate, SerializedCodeData::SourceHash(
-                                 source, script->origin_options()));
+  CodeSerializer cs(isolate,
+                    SerializedCodeData::SourceHash(source, wrapped_arguments,
+                                                   script->origin_options()));
   DisallowGarbageCollection no_gc;
 
 #ifndef DEBUG
@@ -489,11 +496,17 @@ MaybeDirectHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
 
   HandleScope scope(isolate);
 
+  DirectHandle<FixedArray> wrapped_arguments;
+  if (!script_details.wrapped_arguments.is_null()) {
+    wrapped_arguments = script_details.wrapped_arguments.ToHandleChecked();
+  }
+
   SerializedCodeSanityCheckResult sanity_check_result =
       SerializedCodeSanityCheckResult::kSuccess;
   const SerializedCodeData scd = SerializedCodeData::FromCachedData(
       isolate, cached_data,
-      SerializedCodeData::SourceHash(source, script_details.origin_options),
+      SerializedCodeData::SourceHash(source, wrapped_arguments,
+                                     script_details.origin_options),
       &sanity_check_result);
   if (sanity_check_result != SerializedCodeSanityCheckResult::kSuccess) {
     if (v8_flags.profile_deserialization) {
@@ -604,6 +617,11 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
 
   HandleScope scope(isolate);
 
+  DirectHandle<FixedArray> wrapped_arguments;
+  if (!script_details.wrapped_arguments.is_null()) {
+    wrapped_arguments = script_details.wrapped_arguments.ToHandleChecked();
+  }
+
   // Do a source sanity check now that we have the source. It's important for
   // FromPartiallySanityCheckedCachedData call that the sanity_check_result
   // holds the result of the off-thread sanity check.
@@ -612,7 +630,8 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
   const SerializedCodeData scd =
       SerializedCodeData::FromPartiallySanityCheckedCachedData(
           cached_data,
-          SerializedCodeData::SourceHash(source, script_details.origin_options),
+          SerializedCodeData::SourceHash(source, wrapped_arguments,
+                                         script_details.origin_options),
           &sanity_check_result);
   if (sanity_check_result != SerializedCodeSanityCheckResult::kSuccess) {
     // The only case where the deserialization result could exist despite a
@@ -793,15 +812,17 @@ SerializedCodeSanityCheckResult SerializedCodeData::SanityCheckWithoutSource(
   return SerializedCodeSanityCheckResult::kSuccess;
 }
 
-uint32_t SerializedCodeData::SourceHash(DirectHandle<String> source,
-                                        ScriptOriginOptions origin_options) {
+uint32_t SerializedCodeData::SourceHash(
+    DirectHandle<String> source, DirectHandle<FixedArray> wrapped_arguments,
+    ScriptOriginOptions origin_options) {
   const uint32_t source_length = source->length();
+  const uint32_t has_wrapped_arguments = !wrapped_arguments.is_null();
 
   static constexpr uint32_t kModuleFlagMask = (1 << 31);
   const uint32_t is_module = origin_options.IsModule() ? kModuleFlagMask : 0;
   DCHECK_EQ(0, source_length & kModuleFlagMask);
 
-  return source_length | is_module;
+  return source_length | is_module | has_wrapped_arguments << 1;
 }
 
 // Return ScriptData object and relinquish ownership over it to the caller.
diff --git a/src/snapshot/code-serializer.h b/src/snapshot/code-serializer.h
@@ -153,6 +153,7 @@ class SerializedCodeData : public SerializedData {
   base::Vector<const uint8_t> Payload() const;
 
   static uint32_t SourceHash(DirectHandle<String> source,
+                             DirectHandle<FixedArray> wrapped_arguments,
                              ScriptOriginOptions origin_options);
 
  private:
diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc
@@ -6202,6 +6202,71 @@ TEST(CachedCompileFunction) {
   }
 }
 
+TEST(InvalidCachedCompileFunction) {
+  DisableAlwaysOpt();
+  LocalContext env;
+  Isolate* isolate = CcTest::i_isolate();
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
+
+  v8::HandleScope scope(CcTest::isolate());
+
+  v8::Local<v8::String> source = v8_str("");
+  ScriptCompiler::CachedData* script_cache;
+  {
+    v8::ScriptCompiler::Source script_source(source);
+    v8::Local<v8::UnboundScript> script =
+        v8::ScriptCompiler::CompileUnboundScript(
+            reinterpret_cast<v8::Isolate*>(isolate), &script_source,
+            v8::ScriptCompiler::kEagerCompile)
+            .ToLocalChecked();
+    script_cache = v8::ScriptCompiler::CreateCodeCache(script);
+  }
+
+  ScriptCompiler::CachedData* fun_cache;
+  {
+    v8::ScriptCompiler::Source script_source(source, script_cache);
+    v8::Local<v8::Function> fun =
+        v8::ScriptCompiler::CompileFunction(
+            env.local(), &script_source, 0, nullptr, 0, nullptr,
+            v8::ScriptCompiler::kConsumeCodeCache)
+            .ToLocalChecked();
+    // Check that the cached data can be re-created.
+    fun_cache = v8::ScriptCompiler::CreateCodeCacheForFunction(fun);
+    // Check that the cached data without wrapped arguments is rejected.
+    CHECK(script_source.GetCachedData()->rejected);
+  }
+
+  {
+    DisallowCompilation no_compile_expected(isolate);
+    v8::ScriptCompiler::Source script_source(source, fun_cache);
+    v8::Local<v8::Function> fun =
+        v8::ScriptCompiler::CompileFunction(
+            env.local(), &script_source, 0, nullptr, 0, nullptr,
+            v8::ScriptCompiler::kConsumeCodeCache)
+            .ToLocalChecked();
+    v8::Local<v8::Value> result =
+        fun->Call(env.local(), v8::Undefined(CcTest::isolate()), 0, nullptr)
+            .ToLocalChecked();
+    CHECK(result->IsUndefined());
+    fun_cache = v8::ScriptCompiler::CreateCodeCacheForFunction(fun);
+  }
+
+  {
+    v8::ScriptCompiler::Source script_source(source, fun_cache);
+    v8::Local<v8::UnboundScript> script =
+        v8::ScriptCompiler::CompileUnboundScript(
+            reinterpret_cast<v8::Isolate*>(isolate), &script_source,
+            v8::ScriptCompiler::kConsumeCodeCache)
+            .ToLocalChecked();
+    // Check that the cached data can be re-created.
+    script_cache = v8::ScriptCompiler::CreateCodeCache(script);
+    // Check that the cached data with wrapped arguments is rejected.
+    CHECK(script_source.GetCachedData()->rejected);
+    delete script_cache;
+  }
+}
+
 TEST(CachedCompileFunctionRespectsEager) {
   DisableAlwaysOpt();
   LocalContext env;
