[wasm] Do not add too much code at once

Especially on arm64 we have a rather low code space limit (128MB), so it
can happen that a background thread generates more code in one batch
than can be held in a single code space. This case is not implemented
yet.

This CL implements this by never batch-adding more than half of a code
space.

In order to test the implementation, we add a new flag called
--wasm-max-code-space-size-mb which can be used to artificially lower
the code space size limits in tests.

R=jkummerow@chromium.org

Bug: v8:13436
Change-Id: I18a3457fda724129fb1bb8c44a9815df265b6b2c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4023072
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84245}

src/common/globals.h

@@ -384,6 +384,23 @@ constexpr int kMaxDoubleStringLength = 24;
 // Just below 4GB, such that {kMaxWasmCodeMemory} fits in a 32-bit size_t.
 constexpr uint32_t kMaxCommittedWasmCodeMB = 4095;
 
+// The actual maximum code space size used can be configured with
+// --max-wasm-code-space-size. This constant is the default value, and at the
+// same time the maximum allowed value (checked by the WasmCodeManager).
+#if V8_TARGET_ARCH_ARM64
+// ARM64 only supports direct calls within a 128 MB range.
+constexpr uint32_t kDefaultMaxWasmCodeSpaceSizeMb = 128;
+#elif V8_TARGET_ARCH_PPC64
+// Branches only take 26 bits.
+constexpr uint32_t kDefaultMaxWasmCodeSpaceSizeMb = 32;
+#else
+// Use 1024 MB limit for code spaces on other platforms. This is smaller than
+// the total allowed code space (kMaxWasmCodeMemory) to avoid unnecessarily
+// big reservations, and to ensure that distances within a code space fit
+// within a 32-bit signed integer.
+constexpr uint32_t kDefaultMaxWasmCodeSpaceSizeMb = 1024;
+#endif
+
 #if V8_HOST_ARCH_64_BIT
 constexpr int kSystemPointerSizeLog2 = 3;
 constexpr intptr_t kIntptrSignBit =

src/flags/flag-definitions.h

@@ -1031,6 +1031,8 @@ DEFINE_UINT(wasm_max_table_size, wasm::kV8MaxWasmTableSize,
             "maximum table size of a wasm instance")
 DEFINE_UINT(wasm_max_committed_code_mb, kMaxCommittedWasmCodeMB,
             "maximum committed code space for wasm (in MB)")
+DEFINE_UINT(wasm_max_code_space_size_mb, kDefaultMaxWasmCodeSpaceSizeMb,
+            "maximum size of a single wasm code space")
 DEFINE_BOOL(wasm_tier_up, true,
             "enable tier up to the optimizing compiler (requires --liftoff to "
             "have an effect)")

src/wasm/wasm-code-manager.cc

@@ -515,9 +515,6 @@ int WasmCode::GetSourcePositionBefore(int offset) {
   return position;
 }
 
-// static
-constexpr size_t WasmCodeAllocator::kMaxCodeSpaceSize;
-
 WasmCodeAllocator::WasmCodeAllocator(std::shared_ptr<Counters> async_counters)
     : protect_code_memory_(!V8_HAS_PTHREAD_JIT_WRITE_PROTECT &&
                            v8_flags.wasm_write_protect_code_memory &&
@@ -622,20 +619,21 @@ size_t ReservationSize(size_t code_size_estimate, int num_declared_functions,
                         minimum_size),
                total_reserved / 4);
 
-  if (V8_UNLIKELY(minimum_size > WasmCodeAllocator::kMaxCodeSpaceSize)) {
+  const size_t max_code_space_size =
+      size_t{v8_flags.wasm_max_code_space_size_mb} * MB;
+  if (V8_UNLIKELY(minimum_size > max_code_space_size)) {
     auto oom_detail = base::FormattedString{}
                       << "required reservation minimum (" << minimum_size
                       << ") is bigger than supported maximum ("
-                      << WasmCodeAllocator::kMaxCodeSpaceSize << ")";
+                      << max_code_space_size << ")";
     V8::FatalProcessOutOfMemory(nullptr,
                                 "Exceeding maximum wasm code space size",
                                 oom_detail.PrintToArray().data());
     UNREACHABLE();
   }
 
-  // Limit by the maximum supported code space size.
-  size_t reserve_size =
-      std::min(WasmCodeAllocator::kMaxCodeSpaceSize, suggested_size);
+  // Limit by the maximum code space size.
+  size_t reserve_size = std::min(max_code_space_size, suggested_size);
 
   return reserve_size;
 }
@@ -1791,6 +1789,9 @@ NativeModule::JumpTablesRef NativeModule::FindJumpTablesForRegionLocked(
     base::AddressRegion code_region) const {
   allocation_mutex_.AssertHeld();
   auto jump_table_usable = [code_region](const WasmCode* jump_table) {
+    // We only ever need to check for suitable jump tables if
+    // {kNeedsFarJumpsBetweenCodeSpaces} is true.
+    if constexpr (!kNeedsFarJumpsBetweenCodeSpaces) UNREACHABLE();
     Address table_start = jump_table->instruction_start();
     Address table_end = table_start + jump_table->instructions().size();
     // Compute the maximum distance from anywhere in the code region to anywhere
@@ -1798,11 +1799,13 @@ NativeModule::JumpTablesRef NativeModule::FindJumpTablesForRegionLocked(
     size_t max_distance = std::max(
         code_region.end() > table_start ? code_region.end() - table_start : 0,
         table_end > code_region.begin() ? table_end - code_region.begin() : 0);
-    // We can allow a max_distance that is equal to kMaxCodeSpaceSize, because
-    // every call or jump will target an address *within* the region, but never
-    // exactly the end of the region. So all occuring offsets are actually
-    // smaller than max_distance.
-    return max_distance <= WasmCodeAllocator::kMaxCodeSpaceSize;
+    // kDefaultMaxWasmCodeSpaceSizeMb is <= the maximum near call distance on
+    // the current platform.
+    // We can allow a max_distance that is equal to
+    // kDefaultMaxWasmCodeSpaceSizeMb, because every call or jump will target an
+    // address *within* the region, but never exactly the end of the region. So
+    // all occuring offsets are actually smaller than max_distance.
+    return max_distance <= kDefaultMaxWasmCodeSpaceSizeMb * MB;
   };
 
   for (auto& code_space_data : code_space_data_) {
@@ -1894,7 +1897,12 @@ NativeModule::~NativeModule() {
 
 WasmCodeManager::WasmCodeManager()
     : max_committed_code_space_(v8_flags.wasm_max_committed_code_mb * MB),
-      critical_committed_code_space_(max_committed_code_space_ / 2) {}
+      critical_committed_code_space_(max_committed_code_space_ / 2) {
+  // Check that --wasm-max-code-space-size-mb is not set bigger than the default
+  // value. Otherwise we run into DCHECKs or other crashes later.
+  CHECK_GE(kDefaultMaxWasmCodeSpaceSizeMb,
+           v8_flags.wasm_max_code_space_size_mb);
+}
 
 WasmCodeManager::~WasmCodeManager() {
   // No more committed code space.
@@ -2331,11 +2339,38 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode(
   TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("v8.wasm.detailed"),
                "wasm.AddCompiledCode", "num", results.size());
   DCHECK(!results.empty());
+  std::vector<std::unique_ptr<WasmCode>> generated_code;
+  generated_code.reserve(results.size());
+
   // First, allocate code space for all the results.
+  // Never add more than half of a code space at once. This leaves some space
+  // for jump tables and other overhead. We could use {OverheadPerCodeSpace},
+  // but that's only an approximation, so we are conservative here and never use
+  // more than half a code space.
+  size_t max_code_batch_size = v8_flags.wasm_max_code_space_size_mb * MB / 2;
   size_t total_code_space = 0;
   for (auto& result : results) {
     DCHECK(result.succeeded());
-    total_code_space += RoundUp<kCodeAlignment>(result.code_desc.instr_size);
+    size_t new_code_space =
+        RoundUp<kCodeAlignment>(result.code_desc.instr_size);
+    if (total_code_space + new_code_space > max_code_batch_size) {
+      // Split off the first part of the {results} vector and process it
+      // separately. This method then continues with the rest.
+      size_t split_point = &result - results.begin();
+      CHECK_WITH_MSG(
+          split_point != 0,
+          "A single code object needs more than half of the code space size");
+      auto first_results = AddCompiledCode(results.SubVector(0, split_point));
+      generated_code.insert(generated_code.end(),
+                            std::make_move_iterator(first_results.begin()),
+                            std::make_move_iterator(first_results.end()));
+      // Continue processing the rest of the vector. This change to the
+      // {results} vector does not invalidate iterators (which are just
+      // pointers). In particular, the end pointer stays the same.
+      results += split_point;
+      total_code_space = 0;
+    }
+    total_code_space += new_code_space;
   }
   base::Vector<byte> code_space;
   NativeModule::JumpTablesRef jump_tables;
@@ -2353,9 +2388,6 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode(
   // {results} vector in smaller chunks).
   CHECK(jump_tables.is_valid());
 
-  std::vector<std::unique_ptr<WasmCode>> generated_code;
-  generated_code.reserve(results.size());
-
   // Now copy the generated code into the code space and relocate it.
   for (auto& result : results) {
     DCHECK_EQ(result.code_desc.buffer, result.instr_buffer->start());
@@ -2372,6 +2404,10 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode(
   }
   DCHECK_EQ(0, code_space.size());
 
+  // Check that we added the expected amount of code objects, even if we split
+  // the {results} vector.
+  DCHECK_EQ(generated_code.capacity(), generated_code.size());
+
   return generated_code;
 }
 

src/wasm/wasm-code-manager.h

@@ -520,20 +520,6 @@ const char* GetWasmCodeKindAsString(WasmCode::Kind);
 // Manages the code reservations and allocations of a single {NativeModule}.
 class WasmCodeAllocator {
  public:
-#if V8_TARGET_ARCH_ARM64
-  // ARM64 only supports direct calls within a 128 MB range.
-  static constexpr size_t kMaxCodeSpaceSize = 128 * MB;
-#elif V8_TARGET_ARCH_PPC64
-  // branches only takes 26 bits
-  static constexpr size_t kMaxCodeSpaceSize = 32 * MB;
-#else
-  // Use 1024 MB limit for code spaces on other platforms. This is smaller than
-  // the total allowed code space (kMaxWasmCodeMemory) to avoid unnecessarily
-  // big reservations, and to ensure that distances within a code space fit
-  // within a 32-bit signed integer.
-  static constexpr size_t kMaxCodeSpaceSize = 1024 * MB;
-#endif
-
   explicit WasmCodeAllocator(std::shared_ptr<Counters> async_counters);
   ~WasmCodeAllocator();
 

src/wasm/wasm-serialization.cc

@@ -738,13 +738,13 @@ DeserializationUnit NativeModuleDeserializer::ReadCode(int fn_index,
   if (current_code_space_.size() < static_cast<size_t>(code_size)) {
     // Allocate the next code space. Don't allocate more than 90% of
     // {kMaxCodeSpaceSize}, to leave some space for jump tables.
-    constexpr size_t kMaxReservation =
-        RoundUp<kCodeAlignment>(WasmCodeAllocator::kMaxCodeSpaceSize * 9 / 10);
-    size_t code_space_size = std::min(kMaxReservation, remaining_code_size_);
+    size_t max_reservation = RoundUp<kCodeAlignment>(
+        v8_flags.wasm_max_code_space_size_mb * MB * 9 / 10);
+    size_t code_space_size = std::min(max_reservation, remaining_code_size_);
     std::tie(current_code_space_, current_jump_tables_) =
         native_module_->AllocateForDeserializedCode(code_space_size);
     DCHECK_EQ(current_code_space_.size(), code_space_size);
-    DCHECK(current_jump_tables_.is_valid());
+    CHECK(current_jump_tables_.is_valid());
   }
 
   DeserializationUnit unit;

test/cctest/wasm/test-jump-table-assembler.cc

@@ -44,9 +44,10 @@ constexpr size_t kThunkBufferSize = 64 * KB;
 // is not reliable enough to guarantee that we can always achieve this with
 // separate allocations, so we generate all code in a single
 // kMaxCodeMemory-sized chunk.
-constexpr size_t kAssemblerBufferSize = WasmCodeAllocator::kMaxCodeSpaceSize;
+constexpr size_t kAssemblerBufferSize =
+    size_t{kDefaultMaxWasmCodeSpaceSizeMb} * MB;
 constexpr uint32_t kAvailableBufferSlots =
-    (WasmCodeAllocator::kMaxCodeSpaceSize - kJumpTableSize) / kThunkBufferSize;
+    (kAssemblerBufferSize - kJumpTableSize) / kThunkBufferSize;
 constexpr uint32_t kBufferSlotStartOffset =
     RoundUp<kThunkBufferSize>(kJumpTableSize);
 #else

test/mjsunit/wasm/code-space-overflow.js

@@ -0,0 +1,32 @@
+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --wasm-max-code-space-size-mb=1
+// Disable lazy compilation, so we actually generate a lot of code at once.
+// Flags: --no-wasm-lazy-compilation
+// Limit the number of background threads, so each thread generates more code.
+// Flags: --wasm-num-compilation-tasks=2
+
+// This is a regression test for https://crbug.com/v8/13436. If a single
+// background thread generates more code than fits in a single code space, we
+// need to split it into multiple code spaces.
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+// At the time of writing this test (Nov 2022), this module generated ~20MB of
+// code on x64 and ~18MB on arm64.
+const builder = new WasmModuleBuilder();
+const kNumFunctions = 1500;
+// Build a large body. Then append one instruction to get different code per
+// function (for the case that we decide to merge identical code objects in the
+// future).
+let body_template = [kExprLocalGet, 0];
+for (let i = 0; i < kNumFunctions; ++i) {
+  body_template.push(kExprCallFunction, ...wasmSignedLeb(i));
+}
+for (let i = 0; i < kNumFunctions; ++i) {
+  let body = body_template.concat([...wasmI32Const(i), kExprI32Add]);
+  builder.addFunction('f' + i, kSig_i_i).addBody(body);
+}
+builder.toModule();