Skip to content

Commit

Permalink
deps: V8: backport 96ee9bb9e830
Browse files Browse the repository at this point in the history
Original commit message:

    [snapshot] Check if a cached data has wrapped arguments

    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: #56366
    Change-Id: I3f0376216791d866fac8eed1ce88dfa05e919b48
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6140933
    Commit-Queue: Chengzhong Wu <[email protected]>
    Reviewed-by: Leszek Swirski <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#97942}

Refs: v8/v8@96ee9bb
Co-authored-by: Joyee Cheung <[email protected]>
  • Loading branch information
legendecas and joyeecheung committed Jan 23, 2025
1 parent eca93e1 commit 0398e4b
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 8 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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 #####

Expand Down
35 changes: 28 additions & 7 deletions deps/v8/src/snapshot/code-serializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,16 @@ ScriptCompiler::CachedData* CodeSerializer::Serialize(

// Serialize code object.
Handle<String> source(String::cast(script->source()), isolate);
Handle<FixedArray> wrapped_arguments;
if (script->is_wrapped()) {
wrapped_arguments =
Handle<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;
cs.reference_map()->AddAttachedReference(*source);
AlignedCachedData* cached_data = cs.SerializeSharedFunctionInfo(info);
Expand Down Expand Up @@ -429,11 +436,17 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(

HandleScope scope(isolate);

Handle<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(
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)
Expand Down Expand Up @@ -542,6 +555,11 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(

HandleScope scope(isolate);

Handle<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.
Expand All @@ -550,7 +568,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
Expand Down Expand Up @@ -708,15 +727,17 @@ SerializedCodeSanityCheckResult SerializedCodeData::SanityCheckWithoutSource()
return SerializedCodeSanityCheckResult::kSuccess;
}

uint32_t SerializedCodeData::SourceHash(Handle<String> source,
ScriptOriginOptions origin_options) {
uint32_t SerializedCodeData::SourceHash(
Handle<String> source, Handle<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.
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/snapshot/code-serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ class SerializedCodeData : public SerializedData {
base::Vector<const byte> Payload() const;

static uint32_t SourceHash(Handle<String> source,
Handle<FixedArray> wrapped_arguments,
ScriptOriginOptions origin_options);

private:
Expand Down
65 changes: 65 additions & 0 deletions deps/v8/test/cctest/test-serialize.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5442,6 +5442,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;
Expand Down

0 comments on commit 0398e4b

Please sign in to comment.