Skip to content

Commit 83eb4f2

Browse files
committedApr 17, 2024
deps: V8: cherry-pick cd10ad7cdbe5
Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7 PR-URL: #52535 Refs: #47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
1 parent 3d4c520 commit 83eb4f2

File tree

7 files changed

+247
-27
lines changed

7 files changed

+247
-27
lines changed
 

Diff for: ‎common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737

3838
# Reset this number to 0 on major V8 upgrades.
3939
# Increment by one for each non-official patch applied to deps/v8.
40-
'v8_embedder_string': '-node.9',
40+
'v8_embedder_string': '-node.10',
4141

4242
##### V8 defaults for Node.js #####
4343

Diff for: ‎deps/v8/src/codegen/compiler.cc

+9-10
Original file line numberDiff line numberDiff line change
@@ -1720,10 +1720,8 @@ BackgroundCompileTask::BackgroundCompileTask(
17201720

17211721
BackgroundCompileTask::~BackgroundCompileTask() = default;
17221722

1723-
namespace {
1724-
17251723
void SetScriptFieldsFromDetails(Isolate* isolate, Tagged<Script> script,
1726-
ScriptDetails script_details,
1724+
const ScriptDetails& script_details,
17271725
DisallowGarbageCollection* no_gc) {
17281726
Handle<Object> script_name;
17291727
if (script_details.name_obj.ToHandle(&script_name)) {
@@ -1749,6 +1747,8 @@ void SetScriptFieldsFromDetails(Isolate* isolate, Tagged<Script> script,
17491747
}
17501748
}
17511749

1750+
namespace {
1751+
17521752
#ifdef ENABLE_SLOW_DCHECKS
17531753

17541754
// A class which traverses the object graph for a newly compiled Script and
@@ -2450,10 +2450,10 @@ void BackgroundDeserializeTask::MergeWithExistingScript() {
24502450

24512451
MaybeHandle<SharedFunctionInfo> BackgroundDeserializeTask::Finish(
24522452
Isolate* isolate, Handle<String> source,
2453-
ScriptOriginOptions origin_options) {
2453+
const ScriptDetails& script_details) {
24542454
return CodeSerializer::FinishOffThreadDeserialize(
24552455
isolate, std::move(off_thread_data_), &cached_data_, source,
2456-
origin_options, &background_merge_task_);
2456+
script_details, &background_merge_task_);
24572457
}
24582458

24592459
// ----------------------------------------------------------------------------
@@ -3630,8 +3630,8 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
36303630
"V8.CompileDeserialize");
36313631
if (deserialize_task) {
36323632
// If there's a cache consume task, finish it.
3633-
maybe_result = deserialize_task->Finish(isolate, source,
3634-
script_details.origin_options);
3633+
maybe_result =
3634+
deserialize_task->Finish(isolate, source, script_details);
36353635
// It is possible at this point that there is a Script object for this
36363636
// script in the compilation cache (held in the variable maybe_script),
36373637
// which does not match maybe_result->script(). This could happen any of
@@ -3652,8 +3652,7 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
36523652
// would be non-trivial.
36533653
} else {
36543654
maybe_result = CodeSerializer::Deserialize(
3655-
isolate, cached_data, source, script_details.origin_options,
3656-
maybe_script);
3655+
isolate, cached_data, source, script_details, maybe_script);
36573656
}
36583657

36593658
bool consuming_code_cache_succeeded = false;
@@ -3829,7 +3828,7 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
38293828
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
38303829
"V8.CompileDeserialize");
38313830
maybe_result = CodeSerializer::Deserialize(isolate, cached_data, source,
3832-
script_details.origin_options);
3831+
script_details);
38333832
bool consuming_code_cache_succeeded = false;
38343833
if (maybe_result.ToHandle(&result)) {
38353834
is_compiled_scope = result->is_compiled_scope(isolate);

Diff for: ‎deps/v8/src/codegen/compiler.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ class V8_EXPORT_PRIVATE BackgroundDeserializeTask {
682682

683683
MaybeHandle<SharedFunctionInfo> Finish(Isolate* isolate,
684684
Handle<String> source,
685-
ScriptOriginOptions origin_options);
685+
const ScriptDetails& script_details);
686686

687687
bool rejected() const { return cached_data_.rejected(); }
688688

Diff for: ‎deps/v8/src/codegen/script-details.h

+3
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ struct ScriptDetails {
3535
const ScriptOriginOptions origin_options;
3636
};
3737

38+
void SetScriptFieldsFromDetails(Isolate* isolate, Tagged<Script> script,
39+
const ScriptDetails& script_details,
40+
DisallowGarbageCollection* no_gc);
3841
} // namespace internal
3942
} // namespace v8
4043

Diff for: ‎deps/v8/src/snapshot/code-serializer.cc

+21-13
Original file line numberDiff line numberDiff line change
@@ -314,12 +314,12 @@ class StressOffThreadDeserializeThread final : public base::Thread {
314314
CodeSerializer::StartDeserializeOffThread(&local_isolate, cached_data_);
315315
}
316316

317-
MaybeHandle<SharedFunctionInfo> Finalize(Isolate* isolate,
318-
Handle<String> source,
319-
ScriptOriginOptions origin_options) {
317+
MaybeHandle<SharedFunctionInfo> Finalize(
318+
Isolate* isolate, Handle<String> source,
319+
const ScriptDetails& script_details) {
320320
return CodeSerializer::FinishOffThreadDeserialize(
321321
isolate, std::move(off_thread_data_), cached_data_, source,
322-
origin_options);
322+
script_details);
323323
}
324324

325325
private:
@@ -330,7 +330,8 @@ class StressOffThreadDeserializeThread final : public base::Thread {
330330

331331
void FinalizeDeserialization(Isolate* isolate,
332332
Handle<SharedFunctionInfo> result,
333-
const base::ElapsedTimer& timer) {
333+
const base::ElapsedTimer& timer,
334+
const ScriptDetails& script_details) {
334335
// Devtools can report time in this function as profiler overhead, since none
335336
// of the following tasks would need to happen normally.
336337
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
@@ -343,10 +344,16 @@ void FinalizeDeserialization(Isolate* isolate,
343344
log_code_creation);
344345
}
345346

347+
Handle<Script> script(Script::cast(result->script()), isolate);
348+
// Reset the script details, including host-defined options.
349+
{
350+
DisallowGarbageCollection no_gc;
351+
SetScriptFieldsFromDetails(isolate, *script, script_details, &no_gc);
352+
}
353+
346354
bool needs_source_positions = isolate->NeedsSourcePositions();
347355
if (!log_code_creation && !needs_source_positions) return;
348356

349-
Handle<Script> script(Script::cast(result->script()), isolate);
350357
if (needs_source_positions) {
351358
Script::InitLineEnds(isolate, script);
352359
}
@@ -430,13 +437,13 @@ const char* ToString(SerializedCodeSanityCheckResult result) {
430437

431438
MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
432439
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
433-
ScriptOriginOptions origin_options,
440+
const ScriptDetails& script_details,
434441
MaybeHandle<Script> maybe_cached_script) {
435442
if (v8_flags.stress_background_compile) {
436443
StressOffThreadDeserializeThread thread(isolate, cached_data);
437444
CHECK(thread.Start());
438445
thread.Join();
439-
return thread.Finalize(isolate, source, origin_options);
446+
return thread.Finalize(isolate, source, script_details);
440447
// TODO(leszeks): Compare off-thread deserialized data to on-thread.
441448
}
442449

@@ -451,7 +458,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
451458
SerializedCodeSanityCheckResult::kSuccess;
452459
const SerializedCodeData scd = SerializedCodeData::FromCachedData(
453460
isolate, cached_data,
454-
SerializedCodeData::SourceHash(source, origin_options),
461+
SerializedCodeData::SourceHash(source, script_details.origin_options),
455462
&sanity_check_result);
456463
if (sanity_check_result != SerializedCodeSanityCheckResult::kSuccess) {
457464
if (v8_flags.profile_deserialization) {
@@ -498,7 +505,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
498505
PrintF("[Deserializing from %d bytes took %0.3f ms]\n", length, ms);
499506
}
500507

501-
FinalizeDeserialization(isolate, result, timer);
508+
FinalizeDeserialization(isolate, result, timer, script_details);
502509

503510
return scope.CloseAndEscape(result);
504511
}
@@ -553,7 +560,7 @@ CodeSerializer::StartDeserializeOffThread(LocalIsolate* local_isolate,
553560
MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
554561
Isolate* isolate, OffThreadDeserializeData&& data,
555562
AlignedCachedData* cached_data, Handle<String> source,
556-
ScriptOriginOptions origin_options,
563+
const ScriptDetails& script_details,
557564
BackgroundMergeTask* background_merge_task) {
558565
base::ElapsedTimer timer;
559566
if (v8_flags.profile_deserialization || v8_flags.log_function_events) {
@@ -569,7 +576,8 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
569576
data.sanity_check_result;
570577
const SerializedCodeData scd =
571578
SerializedCodeData::FromPartiallySanityCheckedCachedData(
572-
cached_data, SerializedCodeData::SourceHash(source, origin_options),
579+
cached_data,
580+
SerializedCodeData::SourceHash(source, script_details.origin_options),
573581
&sanity_check_result);
574582
if (sanity_check_result != SerializedCodeSanityCheckResult::kSuccess) {
575583
// The only case where the deserialization result could exist despite a
@@ -642,7 +650,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
642650
length, ms);
643651
}
644652

645-
FinalizeDeserialization(isolate, result, timer);
653+
FinalizeDeserialization(isolate, result, timer, script_details);
646654

647655
DCHECK(!background_merge_task ||
648656
!background_merge_task->HasPendingForegroundWork());

Diff for: ‎deps/v8/src/snapshot/code-serializer.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#define V8_SNAPSHOT_CODE_SERIALIZER_H_
77

88
#include "src/base/macros.h"
9+
#include "src/codegen/script-details.h"
910
#include "src/snapshot/serializer.h"
1011
#include "src/snapshot/snapshot-data.h"
1112

@@ -81,7 +82,7 @@ class CodeSerializer : public Serializer {
8182

8283
V8_WARN_UNUSED_RESULT static MaybeHandle<SharedFunctionInfo> Deserialize(
8384
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
84-
ScriptOriginOptions origin_options,
85+
const ScriptDetails& script_details,
8586
MaybeHandle<Script> maybe_cached_script = {});
8687

8788
V8_WARN_UNUSED_RESULT static OffThreadDeserializeData
@@ -92,7 +93,7 @@ class CodeSerializer : public Serializer {
9293
FinishOffThreadDeserialize(
9394
Isolate* isolate, OffThreadDeserializeData&& data,
9495
AlignedCachedData* cached_data, Handle<String> source,
95-
ScriptOriginOptions origin_options,
96+
const ScriptDetails& script_details,
9697
BackgroundMergeTask* background_merge_task = nullptr);
9798

9899
uint32_t source_hash() const { return source_hash_; }

Diff for: ‎deps/v8/test/cctest/test-serialize.cc

+209
Original file line numberDiff line numberDiff line change
@@ -5437,6 +5437,215 @@ TEST(WeakArraySerializationInCodeCache) {
54375437
delete cache;
54385438
}
54395439

5440+
v8::MaybeLocal<v8::Promise> TestHostDefinedOptionFromCachedScript(
5441+
Local<v8::Context> context, Local<v8::Data> host_defined_options,
5442+
Local<v8::Value> resource_name, Local<v8::String> specifier,
5443+
Local<v8::FixedArray> import_attributes) {
5444+
CHECK(host_defined_options->IsFixedArray());
5445+
auto arr = host_defined_options.As<v8::FixedArray>();
5446+
CHECK_EQ(arr->Length(), 1);
5447+
v8::Local<v8::Symbol> expected =
5448+
v8::Symbol::For(context->GetIsolate(), v8_str("hdo"));
5449+
CHECK_EQ(arr->Get(context, 0), expected);
5450+
CHECK(resource_name->Equals(context, v8_str("test_hdo")).FromJust());
5451+
CHECK(specifier->Equals(context, v8_str("foo")).FromJust());
5452+
5453+
Local<v8::Promise::Resolver> resolver =
5454+
v8::Promise::Resolver::New(context).ToLocalChecked();
5455+
resolver->Resolve(context, v8_str("hello")).ToChecked();
5456+
return resolver->GetPromise();
5457+
}
5458+
5459+
TEST(CachedFunctionHostDefinedOption) {
5460+
DisableAlwaysOpt();
5461+
LocalContext env;
5462+
v8::Isolate* isolate = env->GetIsolate();
5463+
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
5464+
i_isolate->compilation_cache()
5465+
->DisableScriptAndEval(); // Disable same-isolate code cache.
5466+
isolate->SetHostImportModuleDynamicallyCallback(
5467+
TestHostDefinedOptionFromCachedScript);
5468+
5469+
v8::HandleScope scope(isolate);
5470+
5471+
v8::Local<v8::String> source = v8_str("return import(x)");
5472+
v8::Local<v8::String> arg_str = v8_str("x");
5473+
5474+
v8::Local<v8::PrimitiveArray> hdo = v8::PrimitiveArray::New(isolate, 1);
5475+
hdo->Set(isolate, 0, v8::Symbol::For(isolate, v8_str("hdo")));
5476+
v8::ScriptOrigin origin(v8_str("test_hdo"), // resource_name
5477+
0, // resource_line_offset
5478+
0, // resource_column_offset
5479+
false, // resource_is_shared_cross_origin
5480+
-1, // script_id
5481+
{}, // source_map_url
5482+
false, // resource_is_opaque
5483+
false, // is_wasm
5484+
false, // is_module
5485+
hdo // host_defined_options
5486+
);
5487+
ScriptCompiler::CachedData* cache;
5488+
{
5489+
v8::ScriptCompiler::Source script_source(source, origin);
5490+
v8::Local<v8::Function> fun =
5491+
v8::ScriptCompiler::CompileFunction(
5492+
env.local(), &script_source, 1, &arg_str, 0, nullptr,
5493+
v8::ScriptCompiler::kNoCompileOptions)
5494+
.ToLocalChecked();
5495+
cache = v8::ScriptCompiler::CreateCodeCacheForFunction(fun);
5496+
}
5497+
5498+
{
5499+
DisallowCompilation no_compile_expected(i_isolate);
5500+
v8::ScriptCompiler::Source script_source(source, origin, cache);
5501+
v8::Local<v8::Function> fun =
5502+
v8::ScriptCompiler::CompileFunction(
5503+
env.local(), &script_source, 1, &arg_str, 0, nullptr,
5504+
v8::ScriptCompiler::kConsumeCodeCache)
5505+
.ToLocalChecked();
5506+
v8::Local<v8::Value> arg = v8_str("foo");
5507+
v8::Local<v8::Value> result =
5508+
fun->Call(env.local(), v8::Undefined(isolate), 1, &arg)
5509+
.ToLocalChecked();
5510+
CHECK(result->IsPromise());
5511+
v8::Local<v8::Promise> promise = result.As<v8::Promise>();
5512+
isolate->PerformMicrotaskCheckpoint();
5513+
v8::Local<v8::Value> resolved = promise->Result();
5514+
CHECK(resolved->IsString());
5515+
CHECK(resolved.As<v8::String>()
5516+
->Equals(env.local(), v8_str("hello"))
5517+
.FromJust());
5518+
}
5519+
}
5520+
5521+
TEST(CachedUnboundScriptHostDefinedOption) {
5522+
DisableAlwaysOpt();
5523+
LocalContext env;
5524+
v8::Isolate* isolate = env->GetIsolate();
5525+
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
5526+
i_isolate->compilation_cache()
5527+
->DisableScriptAndEval(); // Disable same-isolate code cache.
5528+
isolate->SetHostImportModuleDynamicallyCallback(
5529+
TestHostDefinedOptionFromCachedScript);
5530+
5531+
v8::HandleScope scope(isolate);
5532+
5533+
v8::Local<v8::String> source = v8_str("globalThis.foo =import('foo')");
5534+
5535+
v8::Local<v8::PrimitiveArray> hdo = v8::PrimitiveArray::New(isolate, 1);
5536+
hdo->Set(isolate, 0, v8::Symbol::For(isolate, v8_str("hdo")));
5537+
v8::ScriptOrigin origin(v8_str("test_hdo"), // resource_name
5538+
0, // resource_line_offset
5539+
0, // resource_column_offset
5540+
false, // resource_is_shared_cross_origin
5541+
-1, // script_id
5542+
{}, // source_map_url
5543+
false, // resource_is_opaque
5544+
false, // is_wasm
5545+
false, // is_module
5546+
hdo // host_defined_options
5547+
);
5548+
ScriptCompiler::CachedData* cache;
5549+
{
5550+
v8::ScriptCompiler::Source script_source(source, origin);
5551+
v8::Local<v8::UnboundScript> script =
5552+
v8::ScriptCompiler::CompileUnboundScript(
5553+
isolate, &script_source, v8::ScriptCompiler::kNoCompileOptions)
5554+
.ToLocalChecked();
5555+
cache = v8::ScriptCompiler::CreateCodeCache(script);
5556+
}
5557+
5558+
{
5559+
DisallowCompilation no_compile_expected(i_isolate);
5560+
v8::ScriptCompiler::Source script_source(source, origin, cache);
5561+
v8::Local<v8::UnboundScript> script =
5562+
v8::ScriptCompiler::CompileUnboundScript(
5563+
isolate, &script_source, v8::ScriptCompiler::kConsumeCodeCache)
5564+
.ToLocalChecked();
5565+
v8::Local<v8::Script> bound = script->BindToCurrentContext();
5566+
USE(bound->Run(env.local(), hdo).ToLocalChecked());
5567+
v8::Local<v8::Value> result =
5568+
env.local()->Global()->Get(env.local(), v8_str("foo")).ToLocalChecked();
5569+
CHECK(result->IsPromise());
5570+
v8::Local<v8::Promise> promise = result.As<v8::Promise>();
5571+
isolate->PerformMicrotaskCheckpoint();
5572+
v8::Local<v8::Value> resolved = promise->Result();
5573+
CHECK(resolved->IsString());
5574+
CHECK(resolved.As<v8::String>()
5575+
->Equals(env.local(), v8_str("hello"))
5576+
.FromJust());
5577+
}
5578+
}
5579+
5580+
v8::MaybeLocal<v8::Module> UnexpectedModuleResolveCallback(
5581+
v8::Local<v8::Context> context, v8::Local<v8::String> specifier,
5582+
v8::Local<v8::FixedArray> import_attributes,
5583+
v8::Local<v8::Module> referrer) {
5584+
CHECK_WITH_MSG(false, "Unexpected call to resolve callback");
5585+
}
5586+
5587+
TEST(CachedModuleScriptFunctionHostDefinedOption) {
5588+
DisableAlwaysOpt();
5589+
LocalContext env;
5590+
v8::Isolate* isolate = env->GetIsolate();
5591+
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
5592+
i_isolate->compilation_cache()
5593+
->DisableScriptAndEval(); // Disable same-isolate code cache.
5594+
isolate->SetHostImportModuleDynamicallyCallback(
5595+
TestHostDefinedOptionFromCachedScript);
5596+
5597+
v8::HandleScope scope(isolate);
5598+
5599+
v8::Local<v8::String> source = v8_str("globalThis.foo = import('foo')");
5600+
5601+
v8::Local<v8::PrimitiveArray> hdo = v8::PrimitiveArray::New(isolate, 1);
5602+
hdo->Set(isolate, 0, v8::Symbol::For(isolate, v8_str("hdo")));
5603+
v8::ScriptOrigin origin(v8_str("test_hdo"), // resource_name
5604+
0, // resource_line_offset
5605+
0, // resource_column_offset
5606+
false, // resource_is_shared_cross_origin
5607+
-1, // script_id
5608+
{}, // source_map_url
5609+
false, // resource_is_opaque
5610+
false, // is_wasm
5611+
true, // is_module
5612+
hdo // host_defined_options
5613+
);
5614+
ScriptCompiler::CachedData* cache;
5615+
{
5616+
v8::ScriptCompiler::Source script_source(source, origin);
5617+
v8::Local<v8::Module> mod =
5618+
v8::ScriptCompiler::CompileModule(isolate, &script_source,
5619+
v8::ScriptCompiler::kNoCompileOptions)
5620+
.ToLocalChecked();
5621+
cache = v8::ScriptCompiler::CreateCodeCache(mod->GetUnboundModuleScript());
5622+
}
5623+
5624+
{
5625+
DisallowCompilation no_compile_expected(i_isolate);
5626+
v8::ScriptCompiler::Source script_source(source, origin, cache);
5627+
v8::Local<v8::Module> mod =
5628+
v8::ScriptCompiler::CompileModule(isolate, &script_source,
5629+
v8::ScriptCompiler::kConsumeCodeCache)
5630+
.ToLocalChecked();
5631+
mod->InstantiateModule(env.local(), UnexpectedModuleResolveCallback)
5632+
.Check();
5633+
v8::Local<v8::Value> evaluted = mod->Evaluate(env.local()).ToLocalChecked();
5634+
CHECK(evaluted->IsPromise());
5635+
CHECK_EQ(evaluted.As<v8::Promise>()->State(),
5636+
v8::Promise::PromiseState::kFulfilled);
5637+
v8::Local<v8::Value> result =
5638+
env.local()->Global()->Get(env.local(), v8_str("foo")).ToLocalChecked();
5639+
v8::Local<v8::Promise> promise = result.As<v8::Promise>();
5640+
isolate->PerformMicrotaskCheckpoint();
5641+
v8::Local<v8::Value> resolved = promise->Result();
5642+
CHECK(resolved->IsString());
5643+
CHECK(resolved.As<v8::String>()
5644+
->Equals(env.local(), v8_str("hello"))
5645+
.FromJust());
5646+
}
5647+
}
5648+
54405649
TEST(CachedCompileFunction) {
54415650
DisableAlwaysOpt();
54425651
LocalContext env;

0 commit comments

Comments
 (0)
Please sign in to comment.