From cdd984ef48c2a7f1bebdcc0a28c0a7d01fa87a84 Mon Sep 17 00:00:00 2001 From: Clemens Backes Date: Tue, 11 Aug 2020 13:36:55 +0200 Subject: [PATCH] Reland "[wasm] Ensure that only TurboFan code is serialized" This is a reland of 60ee70bb40efea6f05476dc19f7a5b490193a107. The wasm c-api flakes were fixed in https://crrev.com/c/2349293. Original change's description: > [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 > Reviewed-by: Andreas Haas > Cr-Commit-Position: refs/heads/master@{#69276} Bug: v8:10777 Change-Id: I2a7c1429812ca46d88a2902b8e0a7b7e3d638b56 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2349290 Reviewed-by: Andreas Haas Commit-Queue: Clemens Backes Cr-Commit-Position: refs/heads/master@{#69335} --- src/wasm/wasm-serialization.cc | 26 ++++++++++++++----- test/cctest/cctest.status | 11 ++++++++ test/mjsunit/regress/wasm/regress-808848.js | 4 ++- test/mjsunit/regress/wasm/regress-808980.js | 4 ++- .../wasm/compiled-module-serialization.js | 4 ++- test/mjsunit/wasm/print-code.js | 4 ++- test/wasm-api-tests/serialize.cc | 1 + 7 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/wasm/wasm-serialization.cc b/src/wasm/wasm-serialization.cc index 8df5d4c88e..6a2ce6c953 100644 --- a/src/wasm/wasm-serialization.cc +++ b/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 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,23 @@ 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()); + // Only serialize TurboFan code, as Liftoff code can contain breakpoints or + // non-relocatable constants. + 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 +428,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 +438,7 @@ bool NativeModuleSerializer::Write(Writer* writer) { WriteHeader(writer); for (WasmCode* code : code_table_) { - WriteCode(code, writer); + if (!WriteCode(code, writer)) return false; } return true; } diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index f5b05b3975..40e45534b8 100644 --- a/test/cctest/cctest.status +++ b/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 diff --git a/test/mjsunit/regress/wasm/regress-808848.js b/test/mjsunit/regress/wasm/regress-808848.js index 269489059f..21374e9cff 100644 --- a/test/mjsunit/regress/wasm/regress-808848.js +++ b/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'); diff --git a/test/mjsunit/regress/wasm/regress-808980.js b/test/mjsunit/regress/wasm/regress-808980.js index 6487a35cd3..5cb3dcb70e 100644 --- a/test/mjsunit/regress/wasm/regress-808980.js +++ b/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; diff --git a/test/mjsunit/wasm/compiled-module-serialization.js b/test/mjsunit/wasm/compiled-module-serialization.js index 859a3095ae..63568fe657 100644 --- a/test/mjsunit/wasm/compiled-module-serialization.js +++ b/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"); diff --git a/test/mjsunit/wasm/print-code.js b/test/mjsunit/wasm/print-code.js index c604ca75f0..876a545846 100644 --- a/test/mjsunit/wasm/print-code.js +++ b/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. diff --git a/test/wasm-api-tests/serialize.cc b/test/wasm-api-tests/serialize.cc index 710f123625..a8718402c6 100644 --- a/test/wasm-api-tests/serialize.cc +++ b/test/wasm-api-tests/serialize.cc @@ -27,6 +27,7 @@ TEST_F(WasmCapiTest, Serialize) { Compile(); vec serialized = module()->serialize(); + EXPECT_TRUE(serialized); // Serialization succeeded. own deserialized = Module::deserialize(store(), serialized); own callback_type =