[wasm] Ensure that only TurboFan code is serialized

We have the implicit assumption that Liftoff code will never be
serialized, and we start relying on that when implementing new features
(debugging, dynamic tiering).

This CL makes the serializer fail if the module contains any Liftoff
code. Existing tests are changed to ensure that we fully tiered up
before serializing a module (similar to the logic in Chromium).
The "wasm-clone-module" test needs to serialize the module before
enabling the debugger.

Note that chrome currently only serializes a module after it fully
tiered up, so that should be fine. If other embedders need the ability
to serialize a module in an arbitrary state, we will have to fix this
later. With this CL we will be on the safe side though and (gracefully)
fail serialization instead of accidentally serializing Liftoff code.

R=ahaas@chromium.org

Bug: v8:10777
Change-Id: I1245e5f7fda3447a544c1e3525e1239cde759174
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2336799
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69276}

src/runtime/runtime-test.cc

@@ -1324,30 +1324,29 @@ RUNTIME_FUNCTION(Runtime_SerializeDeserializeNow) {
   return ReadOnlyRoots(isolate).undefined_value();
 }
 
-// Take a compiled wasm module and serialize it into an array buffer, which is
-// then returned.
+// Wait until the given module is fully tiered up, then serialize it into an
+// array buffer.
 RUNTIME_FUNCTION(Runtime_SerializeWasmModule) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_HANDLE_CHECKED(WasmModuleObject, module_obj, 0);
 
   wasm::NativeModule* native_module = module_obj->native_module();
+  native_module->compilation_state()->WaitForTopTierFinishedForTesting();
+  DCHECK(!native_module->compilation_state()->failed());
+
   wasm::WasmSerializer wasm_serializer(native_module);
   size_t byte_length = wasm_serializer.GetSerializedNativeModuleSize();
 
-  MaybeHandle<JSArrayBuffer> result =
-      isolate->factory()->NewJSArrayBufferAndBackingStore(
-          byte_length, InitializedFlag::kUninitialized);
+  Handle<JSArrayBuffer> array_buffer =
+      isolate->factory()
+          ->NewJSArrayBufferAndBackingStore(byte_length,
+                                            InitializedFlag::kUninitialized)
+          .ToHandleChecked();
 
-  Handle<JSArrayBuffer> array_buffer;
-  if (result.ToHandle(&array_buffer) &&
-      wasm_serializer.SerializeNativeModule(
-          {reinterpret_cast<uint8_t*>(array_buffer->backing_store()),
-           byte_length})) {
-    return *array_buffer;
-  }
-
-  UNREACHABLE();
+  CHECK(wasm_serializer.SerializeNativeModule(
+      {static_cast<uint8_t*>(array_buffer->backing_store()), byte_length}));
+  return *array_buffer;
 }
 
 // Take an array buffer and attempt to reconstruct a compiled wasm module.

src/wasm/compilation-environment.h

@@ -101,7 +101,7 @@ enum class CompilationEvent : uint8_t {
 
 // The implementation of {CompilationState} lives in module-compiler.cc.
 // This is the PIMPL interface to that private class.
-class CompilationState {
+class V8_EXPORT_PRIVATE CompilationState {
  public:
   using callback_t = std::function<void(CompilationEvent)>;
 
@@ -113,15 +113,17 @@ class CompilationState {
 
   void SetWireBytesStorage(std::shared_ptr<WireBytesStorage>);
 
-  V8_EXPORT_PRIVATE std::shared_ptr<WireBytesStorage> GetWireBytesStorage()
-      const;
+  std::shared_ptr<WireBytesStorage> GetWireBytesStorage() const;
 
   void AddCallback(callback_t);
 
+  // Wait until top tier finished, or compilation failed (to avoid deadlocks).
+  void WaitForTopTierFinishedForTesting();
+
   bool failed() const;
-  V8_EXPORT_PRIVATE bool baseline_compilation_finished() const;
-  V8_EXPORT_PRIVATE bool top_tier_compilation_finished() const;
-  V8_EXPORT_PRIVATE bool recompilation_finished() const;
+  bool baseline_compilation_finished() const;
+  bool top_tier_compilation_finished() const;
+  bool recompilation_finished() const;
 
   // Override {operator delete} to avoid implicit instantiation of {operator
   // delete} with {size_t} argument. The {size_t} argument would be incorrect.

src/wasm/module-compiler.cc

@@ -688,6 +688,17 @@ void CompilationState::AddCallback(CompilationState::callback_t callback) {
   return Impl(this)->AddCallback(std::move(callback));
 }
 
+void CompilationState::WaitForTopTierFinishedForTesting() {
+  auto top_tier_finished_semaphore = std::make_shared<base::Semaphore>(0);
+  AddCallback([top_tier_finished_semaphore](CompilationEvent event) {
+    if (event == CompilationEvent::kFailedCompilation ||
+        event == CompilationEvent::kFinishedTopTierCompilation) {
+      top_tier_finished_semaphore->Signal();
+    }
+  });
+  top_tier_finished_semaphore->Wait();
+}
+
 bool CompilationState::failed() const { return Impl(this)->failed(); }
 
 bool CompilationState::baseline_compilation_finished() const {

src/wasm/wasm-serialization.cc

@@ -280,8 +280,8 @@ class V8_EXPORT_PRIVATE NativeModuleSerializer {
 
  private:
   size_t MeasureCode(const WasmCode*) const;
-  void WriteHeader(Writer* writer);
-  void WriteCode(const WasmCode*, Writer* writer);
+  void WriteHeader(Writer*);
+  bool WriteCode(const WasmCode*, Writer*);
 
   const NativeModule* const native_module_;
   Vector<WasmCode* const> code_table_;
@@ -301,6 +301,9 @@ NativeModuleSerializer::NativeModuleSerializer(
 size_t NativeModuleSerializer::MeasureCode(const WasmCode* code) const {
   if (code == nullptr) return sizeof(bool);
   DCHECK_EQ(WasmCode::kFunction, code->kind());
+  if (FLAG_wasm_lazy_compilation && code->tier() != ExecutionTier::kTurbofan) {
+    return sizeof(bool);
+  }
   return kCodeHeaderSize + code->instructions().size() +
          code->reloc_info().size() + code->source_positions().size() +
          code->protected_instructions_data().size();
@@ -322,13 +325,21 @@ void NativeModuleSerializer::WriteHeader(Writer* writer) {
   writer->Write(native_module_->num_imported_functions());
 }
 
-void NativeModuleSerializer::WriteCode(const WasmCode* code, Writer* writer) {
+bool NativeModuleSerializer::WriteCode(const WasmCode* code, Writer* writer) {
+  DCHECK_IMPLIES(!FLAG_wasm_lazy_compilation, code != nullptr);
   if (code == nullptr) {
     writer->Write(false);
-    return;
+    return true;
+  }
+  DCHECK_EQ(WasmCode::kFunction, code->kind());
+  if (code->tier() != ExecutionTier::kTurbofan) {
+    if (FLAG_wasm_lazy_compilation) {
+      writer->Write(false);
+      return true;
+    }
+    return false;
   }
   writer->Write(true);
-  DCHECK_EQ(WasmCode::kFunction, code->kind());
   // Write the size of the entire code section, followed by the code header.
   writer->Write(code->constant_pool_offset());
   writer->Write(code->safepoint_table_offset());
@@ -415,6 +426,7 @@ void NativeModuleSerializer::WriteCode(const WasmCode* code, Writer* writer) {
   if (code_start != serialized_code_start) {
     memcpy(serialized_code_start, code_start, code_size);
   }
+  return true;
 }
 
 bool NativeModuleSerializer::Write(Writer* writer) {
@@ -424,7 +436,7 @@ bool NativeModuleSerializer::Write(Writer* writer) {
   WriteHeader(writer);
 
   for (WasmCode* code : code_table_) {
-    WriteCode(code, writer);
+    if (!WriteCode(code, writer)) return false;
   }
   return true;
 }

test/cctest/cctest.status

@@ -182,6 +182,17 @@
   'test-cpu-profiler/MultipleIsolates': [SKIP],
 }],  # variant == nooptimization and (arch == arm or arch == arm64) and simulator_run
 
+##############################################################################
+['variant == nooptimization', {
+  # Wasm serialization relies on TurboFan to be available, hence does not work
+  # in the 'nooptimization' variant.
+  'test-wasm-serialization/*': [SKIP],
+  'test-streaming-compilation/SingleThreadedTestDeserializationBypassesCompilation': [SKIP],
+  'test-streaming-compilation/SingleThreadedTestDeserializationFails': [SKIP],
+  'test-streaming-compilation/AsyncTestDeserializationFails': [SKIP],
+  'test-streaming-compilation/AsyncTestDeserializationBypassesCompilation': [SKIP],
+}],  # variant == nooptimization
+
 ##############################################################################
 ['variant == no_lfa', {
   # https://crbug.com/v8/10219

test/cctest/wasm/test-streaming-compilation.cc

@@ -268,10 +268,11 @@ ZoneBuffer GetValidCompiledModuleBytes(Zone* zone, ZoneBuffer wire_bytes) {
   // Serialize the NativeModule.
   std::shared_ptr<NativeModule> native_module = tester.native_module();
   CHECK(native_module);
+  native_module->compilation_state()->WaitForTopTierFinishedForTesting();
   i::wasm::WasmSerializer serializer(native_module.get());
   size_t size = serializer.GetSerializedNativeModuleSize();
   std::vector<byte> buffer(size);
-  CHECK(serializer.SerializeNativeModule({buffer.data(), size}));
+  CHECK(serializer.SerializeNativeModule(VectorOf(buffer)));
   ZoneBuffer result(zone, size);
   result.write(buffer.data(), size);
   return result;

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

@@ -148,6 +148,10 @@ class WasmSerializationTest {
       // Check that the native module exists at this point.
       CHECK(weak_native_module.lock());
 
+      auto* native_module = module_object->native_module();
+      native_module->compilation_state()->WaitForTopTierFinishedForTesting();
+      DCHECK(!native_module->compilation_state()->failed());
+
       v8::Local<v8::Object> v8_module_obj =
           v8::Utils::ToLocal(Handle<JSObject>::cast(module_object));
       CHECK(v8_module_obj->IsWasmModuleObject());
@@ -163,6 +167,7 @@ class WasmSerializationTest {
       wire_bytes_ = {bytes_copy, uncompiled_bytes.size()};
       // keep alive data_ until the end
       data_ = compiled_module.Serialize();
+      CHECK_LT(0, data_.size);
     }
     // Dispose of serialization isolate to destroy the reference to the
     // NativeModule, which removes it from the module cache in the wasm engine

test/mjsunit/regress/wasm/regress-808848.js

@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax
+// The test needs --wasm-tier-up because we can't serialize and deserialize
+// Liftoff code.
+// Flags: --allow-natives-syntax --wasm-tier-up
 
 load('test/mjsunit/wasm/wasm-module-builder.js');
 

test/mjsunit/regress/wasm/regress-808980.js

@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --throws
+// The test needs --wasm-tier-up because we can't serialize and deserialize
+// Liftoff code.
+// Flags: --allow-natives-syntax --throws --wasm-tier-up
 
 load('test/mjsunit/wasm/wasm-module-builder.js');
 let kTableSize = 3;

test/mjsunit/wasm/compiled-module-serialization.js

@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --expose-wasm --allow-natives-syntax --expose-gc
+// The test needs --wasm-tier-up because we can't serialize and deserialize
+// Liftoff code.
+// Flags: --expose-wasm --allow-natives-syntax --expose-gc --wasm-tier-up
 
 load("test/mjsunit/wasm/wasm-module-builder.js");
 

test/mjsunit/wasm/print-code.js

@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --print-wasm-code
+// The test needs --wasm-tier-up because we can't serialize and deserialize
+// Liftoff code.
+// Flags: --allow-natives-syntax --print-wasm-code --wasm-tier-up
 
 // Just test that printing the code of the following wasm modules does not
 // crash.