Commit 2eb25376 authored by Marja Hölttä's avatar Marja Hölttä Committed by V8 LUCI CQ

[rab/gsab] Re-enable serializing flags with ValueSerializer

Also:
- Refactor the ValueSerializer tests using raw data, so that we test all
valid versions for each test (not only one hard-coded one)
- Mark some tests as backwards compatibility tests, to make it less
likely that somebody updates them not realizing they are backwards
compatibility tests.

Bug: v8:11111, v8:12532
Change-Id: I670849de07742c8d442249ef4f013781e4ee9255
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3386802Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78681}
parent b601d6bb
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
namespace v8 { namespace v8 {
constexpr uint32_t CurrentValueSerializerFormatVersion() { return 13; } constexpr uint32_t CurrentValueSerializerFormatVersion() { return 14; }
} // namespace v8 } // namespace v8
......
...@@ -49,6 +49,7 @@ namespace internal { ...@@ -49,6 +49,7 @@ namespace internal {
// Version 12: regexp and string objects share normal string encoding // Version 12: regexp and string objects share normal string encoding
// Version 13: host objects have an explicit tag (rather than handling all // Version 13: host objects have an explicit tag (rather than handling all
// unknown tags) // unknown tags)
// Version 14: flags for JSArrayBufferViews
// //
// WARNING: Increasing this value is a change which cannot safely be rolled // WARNING: Increasing this value is a change which cannot safely be rolled
// back without breaking compatibility with data stored on disk. It is // back without breaking compatibility with data stored on disk. It is
...@@ -57,7 +58,7 @@ namespace internal { ...@@ -57,7 +58,7 @@ namespace internal {
// //
// Recent changes are routinely reverted in preparation for branch, and this // Recent changes are routinely reverted in preparation for branch, and this
// has been the cause of at least one bug in the past. // has been the cause of at least one bug in the past.
static const uint32_t kLatestVersion = 13; static const uint32_t kLatestVersion = 14;
static_assert(kLatestVersion == v8::CurrentValueSerializerFormatVersion(), static_assert(kLatestVersion == v8::CurrentValueSerializerFormatVersion(),
"Exported format version must match latest version."); "Exported format version must match latest version.");
...@@ -938,11 +939,7 @@ Maybe<bool> ValueSerializer::WriteJSArrayBufferView(JSArrayBufferView view) { ...@@ -938,11 +939,7 @@ Maybe<bool> ValueSerializer::WriteJSArrayBufferView(JSArrayBufferView view) {
WriteVarint(static_cast<uint8_t>(tag)); WriteVarint(static_cast<uint8_t>(tag));
WriteVarint(static_cast<uint32_t>(view.byte_offset())); WriteVarint(static_cast<uint32_t>(view.byte_offset()));
WriteVarint(static_cast<uint32_t>(view.byte_length())); WriteVarint(static_cast<uint32_t>(view.byte_length()));
// TODO(crbug.com/v8/12532): Re-enable the flags serialization logic below. WriteVarint(static_cast<uint32_t>(view.bit_field()));
// Bump the serialization format version number when doing so, and preserve
// logic and tests for reading from the old format.
//
// WriteVarint(static_cast<uint32_t>(view.bit_field()));
return ThrowIfOutOfMemory(); return ThrowIfOutOfMemory();
} }
...@@ -1868,8 +1865,6 @@ MaybeHandle<JSArrayBufferView> ValueDeserializer::ReadJSArrayBufferView( ...@@ -1868,8 +1865,6 @@ MaybeHandle<JSArrayBufferView> ValueDeserializer::ReadJSArrayBufferView(
uint32_t byte_offset = 0; uint32_t byte_offset = 0;
uint32_t byte_length = 0; uint32_t byte_length = 0;
uint32_t flags = 0; uint32_t flags = 0;
// TODO(crbug.com/v8/12532): Read `flags` from the serialized value, when we
// restore the logic for serializing them.
if (!ReadVarint<uint8_t>().To(&tag) || if (!ReadVarint<uint8_t>().To(&tag) ||
!ReadVarint<uint32_t>().To(&byte_offset) || !ReadVarint<uint32_t>().To(&byte_offset) ||
!ReadVarint<uint32_t>().To(&byte_length) || !ReadVarint<uint32_t>().To(&byte_length) ||
...@@ -1877,6 +1872,9 @@ MaybeHandle<JSArrayBufferView> ValueDeserializer::ReadJSArrayBufferView( ...@@ -1877,6 +1872,9 @@ MaybeHandle<JSArrayBufferView> ValueDeserializer::ReadJSArrayBufferView(
byte_length > buffer_byte_length - byte_offset) { byte_length > buffer_byte_length - byte_offset) {
return MaybeHandle<JSArrayBufferView>(); return MaybeHandle<JSArrayBufferView>();
} }
if (version_ >= 14 && !ReadVarint<uint32_t>().To(&flags)) {
return MaybeHandle<JSArrayBufferView>();
}
uint32_t id = next_id_++; uint32_t id = next_id_++;
ExternalArrayType external_array_type = kExternalInt8Array; ExternalArrayType external_array_type = kExternalInt8Array;
unsigned element_size = 0; unsigned element_size = 0;
......
...@@ -523,6 +523,7 @@ v8_source_set("unittests_sources") { ...@@ -523,6 +523,7 @@ v8_source_set("unittests_sources") {
"../..:v8_libbase", "../..:v8_libbase",
"../..:v8_libplatform", "../..:v8_libplatform",
"../..:v8_shared_internal_headers", "../..:v8_shared_internal_headers",
"../..:v8_version",
"../../third_party/inspector_protocol:crdtp_test", "../../third_party/inspector_protocol:crdtp_test",
"//build/win:default_exe_manifest", "//build/win:default_exe_manifest",
"//testing/gmock", "//testing/gmock",
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment