[wasm] Fix deserializer test

The test was supposed to manipulate the serialized bytes to make them
invalid, but the value at the manipulated position was already 0, hence
the bytes stayed valid. This went unnoticed before
https://crrev.com/c/2010786, since there was a fallback anyway to
re-compile the module if deserialization fails.

This CL fixes this by using the right offset, and checking that the
value there is not already zero.

R=thibaudm@chromium.org

Change-Id: Ie0eaf2c8ee9e8c4c477f717f3d8aed8564b3adbf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2007493
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65898}

src/wasm/wasm-serialization.cc

@@ -119,13 +119,12 @@ class Reader {
   const byte* pos_;
 };
 
-constexpr size_t kVersionSize = 4 * sizeof(uint32_t);
-
-void WriteVersion(Writer* writer) {
+void WriteHeader(Writer* writer) {
   writer->Write(SerializedData::kMagicNumber);
   writer->Write(Version::Hash());
   writer->Write(static_cast<uint32_t>(CpuFeatures::SupportedFeatures()));
   writer->Write(FlagList::Hash());
+  DCHECK_EQ(WasmSerializer::kHeaderSize, writer->bytes_written());
 }
 
 // On Intel, call sites are encoded as a displacement. For linking and for
@@ -438,16 +437,16 @@ WasmSerializer::WasmSerializer(NativeModule* native_module)
 
 size_t WasmSerializer::GetSerializedNativeModuleSize() const {
   NativeModuleSerializer serializer(native_module_, VectorOf(code_table_));
-  return kVersionSize + serializer.Measure();
+  return kHeaderSize + serializer.Measure();
 }
 
 bool WasmSerializer::SerializeNativeModule(Vector<byte> buffer) const {
   NativeModuleSerializer serializer(native_module_, VectorOf(code_table_));
-  size_t measured_size = kVersionSize + serializer.Measure();
+  size_t measured_size = kHeaderSize + serializer.Measure();
   if (buffer.size() < measured_size) return false;
 
   Writer writer(buffer);
-  WriteVersion(&writer);
+  WriteHeader(&writer);
 
   if (!serializer.Write(&writer)) return false;
   DCHECK_EQ(measured_size, writer.bytes_written());
@@ -592,12 +591,13 @@ bool NativeModuleDeserializer::ReadCode(uint32_t fn_index, Reader* reader) {
   return true;
 }
 
-bool IsSupportedVersion(Vector<const byte> version) {
-  if (version.size() < kVersionSize) return false;
-  byte current_version[kVersionSize];
-  Writer writer({current_version, kVersionSize});
-  WriteVersion(&writer);
-  return memcmp(version.begin(), current_version, kVersionSize) == 0;
+bool IsSupportedVersion(Vector<const byte> header) {
+  if (header.size() < WasmSerializer::kHeaderSize) return false;
+  byte current_version[WasmSerializer::kHeaderSize];
+  Writer writer({current_version, WasmSerializer::kHeaderSize});
+  WriteHeader(&writer);
+  return memcmp(header.begin(), current_version, WasmSerializer::kHeaderSize) ==
+         0;
 }
 
 MaybeHandle<WasmModuleObject> DeserializeNativeModule(
@@ -635,7 +635,7 @@ MaybeHandle<WasmModuleObject> DeserializeNativeModule(
     NativeModuleDeserializer deserializer(shared_native_module.get());
     WasmCodeRefScope wasm_code_ref_scope;
 
-    Reader reader(data + kVersionSize);
+    Reader reader(data + WasmSerializer::kHeaderSize);
     bool error = !deserializer.Read(&reader);
     wasm_engine->UpdateNativeModuleCache(shared_native_module, error);
     if (error) return {};

src/wasm/wasm-serialization.h

@@ -25,6 +25,21 @@ class V8_EXPORT_PRIVATE WasmSerializer {
   // success and false if the given buffer it too small for serialization.
   bool SerializeNativeModule(Vector<byte> buffer) const;
 
+  // The data header consists of uint32_t-sized entries (see {WriteVersion}):
+  // [0] magic number
+  // [1] version hash
+  // [2] supported CPU features
+  // [3] flag hash
+  // ...  number of functions
+  // ... serialized functions
+  static constexpr size_t kMagicNumberOffset = 0;
+  static constexpr size_t kVersionHashOffset = kMagicNumberOffset + kUInt32Size;
+  static constexpr size_t kSupportedCPUFeaturesOffset =
+      kVersionHashOffset + kUInt32Size;
+  static constexpr size_t kFlagHashOffset =
+      kSupportedCPUFeaturesOffset + kUInt32Size;
+  static constexpr size_t kHeaderSize = 4 * kUInt32Size;
+
  private:
   NativeModule* native_module_;
   std::vector<WasmCode*> code_table_;

test/cctest/wasm/test-wasm-serialization.cc

@@ -53,7 +53,7 @@ class WasmSerializationTest {
   void InvalidateVersion() {
     uint32_t* slot = reinterpret_cast<uint32_t*>(
         const_cast<uint8_t*>(serialized_bytes_.data()) +
-        SerializedCodeData::kVersionHashOffset);
+        WasmSerializer::kVersionHashOffset);
     *slot = Version::Hash() + 1;
   }
 
@@ -61,11 +61,12 @@ class WasmSerializationTest {
     memset(const_cast<uint8_t*>(wire_bytes_.data()), 0, wire_bytes_.size() / 2);
   }
 
-  void InvalidateLength() {
-    uint32_t* slot = reinterpret_cast<uint32_t*>(
-        const_cast<uint8_t*>(serialized_bytes_.data()) +
-        SerializedCodeData::kPayloadLengthOffset);
-    *slot = 0u;
+  void InvalidateNumFunctions() {
+    Address num_functions_slot =
+        reinterpret_cast<Address>(serialized_bytes_.data()) +
+        WasmSerializer::kHeaderSize;
+    CHECK_EQ(1, base::ReadUnalignedValue<uint32_t>(num_functions_slot));
+    base::WriteUnalignedValue<uint32_t>(num_functions_slot, 0);
   }
 
   MaybeHandle<WasmModuleObject> Deserialize() {
@@ -207,13 +208,12 @@ TEST(DeserializeNoSerializedData) {
   test.CollectGarbage();
 }
 
-TEST(DeserializeInvalidLength) {
+TEST(DeserializeInvalidNumFunctions) {
   WasmSerializationTest test;
   {
     HandleScope scope(CcTest::i_isolate());
-    test.InvalidateLength();
-    // TODO(clemensb): Check why this still deserialized correctly.
-    // CHECK(test.Deserialize().is_null());
+    test.InvalidateNumFunctions();
+    CHECK(test.Deserialize().is_null());
   }
   test.CollectGarbage();
 }