From a3e0834ee407342f9335bd7d1d65934a781e8b59 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 1 Feb 2024 12:53:59 +0100 Subject: [PATCH] deps: V8: cherry-pick efb1133eb894 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [api] Add v8::ScriptCompiler::CachedData::CompatibilityCheck() This patch adds a new API v8::ScriptCompiler::CachedData::CompatibilityCheck() in order to allow embedders to check if the code cache can be used in the current isolate without looking up for the source code. It also returns more detailed reasons about why the code cache cannot be used when it's bound to be rejected. This makes it possible to enforce portability checks in case code code becomes CPU-dependent in the future. Refs: https://github.com/nodejs/node/issues/42566#issuecomment-1735862123 Change-Id: Ia1d677b949050add961af6fbf62c44342c061312 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4905290 Reviewed-by: Marja Hölttä Reviewed-by: Toon Verwaest Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#90833} Refs: https://github.com/v8/v8/commit/efb1133eb894a283ff0341c4e44d43e913911b6c PR-URL: https://github.com/nodejs/node/pull/51551 Reviewed-By: Richard Lau Reviewed-By: Michaël Zasso Reviewed-By: Luigi Pinca --- common.gypi | 2 +- deps/v8/include/v8-script.h | 21 ++++++++ deps/v8/src/api/api.cc | 12 +++++ deps/v8/src/snapshot/code-serializer.h | 19 ++----- deps/v8/test/cctest/test-serialize.cc | 72 ++++++++++++++++++++++++++ 5 files changed, 109 insertions(+), 17 deletions(-) diff --git a/common.gypi b/common.gypi index 7dea2696d2bc40..876b0f14566b5d 100644 --- a/common.gypi +++ b/common.gypi @@ -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.12', + 'v8_embedder_string': '-node.13', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8-script.h b/deps/v8/include/v8-script.h index 759bd754d797be..596aac7f95ef47 100644 --- a/deps/v8/include/v8-script.h +++ b/deps/v8/include/v8-script.h @@ -394,6 +394,27 @@ class V8_EXPORT ScriptCompiler { CachedData(const uint8_t* data, int length, BufferPolicy buffer_policy = BufferNotOwned); ~CachedData(); + + enum CompatibilityCheckResult { + // Don't change order/existing values of this enum since it keys into the + // `code_cache_reject_reason` histogram. Append-only! + kSuccess = 0, + kMagicNumberMismatch = 1, + kVersionMismatch = 2, + kSourceMismatch = 3, + kFlagsMismatch = 5, + kChecksumMismatch = 6, + kInvalidHeader = 7, + kLengthMismatch = 8, + kReadOnlySnapshotChecksumMismatch = 9, + + // This should always point at the last real enum value. + kLast = kReadOnlySnapshotChecksumMismatch + }; + + // Check if the CachedData can be loaded in the given isolate. + CompatibilityCheckResult CompatibilityCheck(Isolate* isolate); + // TODO(marja): Async compilation; add constructors which take a callback // which will be called when V8 no longer needs the data. const uint8_t* data; diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index 4f583896edea86..95b343b90855da 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -1951,6 +1951,18 @@ ScriptCompiler::CachedData::~CachedData() { } } +ScriptCompiler::CachedData::CompatibilityCheckResult +ScriptCompiler::CachedData::CompatibilityCheck(Isolate* isolate) { + i::AlignedCachedData aligned(data, length); + i::Isolate* i_isolate = reinterpret_cast(isolate); + i::SerializedCodeSanityCheckResult result; + i::SerializedCodeData scd = + i::SerializedCodeData::FromCachedDataWithoutSource( + i_isolate->AsLocalIsolate(), &aligned, &result); + return static_cast( + result); +} + ScriptCompiler::StreamedSource::StreamedSource( std::unique_ptr stream, Encoding encoding) : impl_(new i::ScriptStreamingData(std::move(stream), encoding)) {} diff --git a/deps/v8/src/snapshot/code-serializer.h b/deps/v8/src/snapshot/code-serializer.h index 4637d7ed91c3f9..6e0924b63bfd27 100644 --- a/deps/v8/src/snapshot/code-serializer.h +++ b/deps/v8/src/snapshot/code-serializer.h @@ -49,22 +49,9 @@ class V8_EXPORT_PRIVATE AlignedCachedData { int length_; }; -enum class SerializedCodeSanityCheckResult { - // Don't change order/existing values of this enum since it keys into the - // `code_cache_reject_reason` histogram. Append-only! - kSuccess = 0, - kMagicNumberMismatch = 1, - kVersionMismatch = 2, - kSourceMismatch = 3, - kFlagsMismatch = 5, - kChecksumMismatch = 6, - kInvalidHeader = 7, - kLengthMismatch = 8, - kReadOnlySnapshotChecksumMismatch = 9, - - // This should always point at the last real enum value. - kLast = kReadOnlySnapshotChecksumMismatch -}; +typedef v8::ScriptCompiler::CachedData::CompatibilityCheckResult + SerializedCodeSanityCheckResult; + // If this fails, update the static_assert AND the code_cache_reject_reason // histogram definition. static_assert(static_cast(SerializedCodeSanityCheckResult::kLast) == 9); diff --git a/deps/v8/test/cctest/test-serialize.cc b/deps/v8/test/cctest/test-serialize.cc index 694d6d61101a09..2a1930fd52d154 100644 --- a/deps/v8/test/cctest/test-serialize.cc +++ b/deps/v8/test/cctest/test-serialize.cc @@ -2799,6 +2799,78 @@ TEST(CodeSerializerFlagChange) { isolate2->Dispose(); } +TEST(CachedDataCompatibilityCheck) { + { + v8::Isolate::CreateParams create_params; + create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); + v8::Isolate* isolate = v8::Isolate::New(create_params); + // Hand-craft a zero-filled cached data which cannot be valid. + int length = 64; + uint8_t* payload = new uint8_t[length]; + memset(payload, 0, length); + v8::ScriptCompiler::CachedData cache( + payload, length, v8::ScriptCompiler::CachedData::BufferOwned); + { + v8::Isolate::Scope iscope(isolate); + v8::ScriptCompiler::CachedData::CompatibilityCheckResult result = + cache.CompatibilityCheck(isolate); + CHECK_NE(result, v8::ScriptCompiler::CachedData::kSuccess); + } + isolate->Dispose(); + } + + const char* js_source = "function f() { return 'abc'; }; f() + 'def'"; + std::unique_ptr cache; + { + v8::Isolate::CreateParams create_params; + create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); + v8::Isolate* isolate = v8::Isolate::New(create_params); + { + v8::Isolate::Scope iscope(isolate); + v8::HandleScope scope(isolate); + v8::Local context = v8::Context::New(isolate); + v8::Context::Scope context_scope(context); + v8::ScriptCompiler::Source source(v8_str(js_source), + {isolate, v8_str("test")}); + v8::Local script = + v8::ScriptCompiler::CompileUnboundScript( + isolate, &source, v8::ScriptCompiler::kEagerCompile) + .ToLocalChecked(); + cache.reset(ScriptCompiler::CreateCodeCache(script)); + } + isolate->Dispose(); + } + + { + v8::Isolate::CreateParams create_params; + create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); + v8::Isolate* isolate = v8::Isolate::New(create_params); + { + v8::Isolate::Scope iscope(isolate); + v8::ScriptCompiler::CachedData::CompatibilityCheckResult result = + cache->CompatibilityCheck(isolate); + CHECK_EQ(result, v8::ScriptCompiler::CachedData::kSuccess); + } + isolate->Dispose(); + } + + { + v8_flags.allow_natives_syntax = + true; // Flag change should trigger cache reject. + FlagList::EnforceFlagImplications(); + v8::Isolate::CreateParams create_params; + create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); + v8::Isolate* isolate = v8::Isolate::New(create_params); + { + v8::Isolate::Scope iscope(isolate); + v8::ScriptCompiler::CachedData::CompatibilityCheckResult result = + cache->CompatibilityCheck(isolate); + CHECK_EQ(result, v8::ScriptCompiler::CachedData::kFlagsMismatch); + } + isolate->Dispose(); + } +} + TEST(CodeSerializerBitFlip) { i::v8_flags.verify_snapshot_checksum = true; const char* js_source = "function f() { return 'abc'; }; f() + 'def'";