Revert "[wasm] Remove WasmInstructionBuffer"

This reverts commit ac654646.

Reason for revert: Breaks ASAN no-inline - https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Clusterfuzz%20Linux64%20ASAN%20no%20inline%20-%20release%20builder/22909/overview

Original change's description:
> [wasm] Remove WasmInstructionBuffer
>
> {WasmInstructionBuffer} was basically a wrapper around {AssemblerBuffer}
> which remembered the last {AssemblerBuffer} on {Grow()}. Since the
> {Assembler} itself already keeps track of the latest {AssemblerBuffer},
> this functionality is mostly redundant. All we need instead is a method
> to retrieve the {AssemblerBuffer} from the {Assembler}.
>
> This CL thus removes {WasmInstructionBuffer} and instead adds
> {AssemblerBase::ReleaseBuffer}.
>
> R=​jkummerow@chromium.org, mslekova@chromium.org
> CC=​dlehmann@google.com
>
> Bug: v8:11714
> Change-Id: Id07945b67992802a6177bf09e5f5c5be08f657b0
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2982013
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Maya Lekova <mslekova@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#75336}

Bug: v8:11714
Change-Id: Iff32952f712ab2f0f9a16d91906d0135c084f4df
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2982014
Auto-Submit: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#75337}

src/codegen/assembler.cc

@@ -99,10 +99,9 @@ namespace {
 class DefaultAssemblerBuffer : public AssemblerBuffer {
  public:
   explicit DefaultAssemblerBuffer(int size)
-      : buffer_(base::OwnedVector<uint8_t>::NewForOverwrite(
-            std::max(AssemblerBase::kMinimalBufferSize, size))) {
+      : buffer_(base::OwnedVector<uint8_t>::NewForOverwrite(size)) {
 #ifdef DEBUG
-    ZapCode(reinterpret_cast<Address>(buffer_.start()), buffer_.size());
+    ZapCode(reinterpret_cast<Address>(buffer_.start()), size);
 #endif
   }
 

src/codegen/assembler.h

@@ -292,15 +292,6 @@ class V8_EXPORT_PRIVATE AssemblerBase : public Malloced {
   int buffer_size() const { return buffer_->size(); }
   int instruction_size() const { return pc_offset(); }
 
-  std::unique_ptr<AssemblerBuffer> ReleaseBuffer() {
-    std::unique_ptr<AssemblerBuffer> buffer = std::move(buffer_);
-    DCHECK_NULL(buffer_);
-    // Reset fields to prevent accidental further modifications of the buffer.
-    buffer_start_ = nullptr;
-    pc_ = nullptr;
-    return buffer;
-  }
-
   // This function is called when code generation is aborted, so that
   // the assembler could clean up internal data structures.
   virtual void AbortedCodeGeneration() {}

src/compiler/backend/code-generator.cc

@@ -48,7 +48,8 @@ CodeGenerator::CodeGenerator(
     int start_source_position, JumpOptimizationInfo* jump_opt,
     PoisoningMitigationLevel poisoning_level, const AssemblerOptions& options,
     Builtin builtin, size_t max_unoptimized_frame_height,
-    size_t max_pushed_argument_count, const char* debug_name)
+    size_t max_pushed_argument_count, std::unique_ptr<AssemblerBuffer> buffer,
+    const char* debug_name)
     : zone_(codegen_zone),
       isolate_(isolate),
       frame_access_state_(nullptr),
@@ -61,7 +62,7 @@ CodeGenerator::CodeGenerator(
       current_block_(RpoNumber::Invalid()),
       start_source_position_(start_source_position),
       current_source_position_(SourcePosition::Unknown()),
-      tasm_(isolate, options, CodeObjectRequired::kNo),
+      tasm_(isolate, options, CodeObjectRequired::kNo, std::move(buffer)),
       resolver_(this),
       safepoints_(codegen_zone),
       handlers_(codegen_zone),

src/compiler/backend/code-generator.h

@@ -127,7 +127,8 @@ class V8_EXPORT_PRIVATE CodeGenerator final : public GapResolver::Assembler {
       int start_source_position, JumpOptimizationInfo* jump_opt,
       PoisoningMitigationLevel poisoning_level, const AssemblerOptions& options,
       Builtin builtin, size_t max_unoptimized_frame_height,
-      size_t max_pushed_argument_count, const char* debug_name = nullptr);
+      size_t max_pushed_argument_count, std::unique_ptr<AssemblerBuffer> = {},
+      const char* debug_name = nullptr);
 
   // Generate native code. After calling AssembleCode, call FinalizeCode to
   // produce the actual code object. If an error occurs during either phase,

src/compiler/pipeline.cc

@@ -546,14 +546,15 @@ class PipelineData {
     start_source_position_ = position;
   }
 
-  void InitializeCodeGenerator(Linkage* linkage) {
+  void InitializeCodeGenerator(Linkage* linkage,
+                               std::unique_ptr<AssemblerBuffer> buffer) {
     DCHECK_NULL(code_generator_);
     code_generator_ = new CodeGenerator(
         codegen_zone(), frame(), linkage, sequence(), info(), isolate(),
         osr_helper_, start_source_position_, jump_optimization_info_,
         info()->GetPoisoningMitigationLevel(), assembler_options_,
         info_->builtin(), max_unoptimized_frame_height(),
-        max_pushed_argument_count(),
+        max_pushed_argument_count(), std::move(buffer),
         FLAG_trace_turbo_stack_accesses ? debug_name_.get() : nullptr);
   }
 
@@ -703,7 +704,8 @@ class PipelineImpl final {
   bool SelectInstructions(Linkage* linkage);
 
   // Step C. Run the code assembly pass.
-  void AssembleCode(Linkage* linkage);
+  void AssembleCode(Linkage* linkage,
+                    std::unique_ptr<AssemblerBuffer> buffer = {});
 
   // Step D. Run the code finalization pass.
   MaybeHandle<Code> FinalizeCode(bool retire_broker = true);
@@ -3153,6 +3155,11 @@ wasm::WasmCompilationResult Pipeline::GenerateCodeForWasmNativeStub(
   wasm::WasmEngine* wasm_engine = wasm::GetWasmEngine();
   ZoneStats zone_stats(wasm_engine->allocator());
   NodeOriginTable* node_positions = graph->zone()->New<NodeOriginTable>(graph);
+  // {instruction_buffer} must live longer than {PipelineData}, since
+  // {PipelineData} will reference the {instruction_buffer} via the
+  // {AssemblerBuffer} of the {Assembler} contained in the {CodeGenerator}.
+  std::unique_ptr<wasm::WasmInstructionBuffer> instruction_buffer =
+      wasm::WasmInstructionBuffer::New();
   PipelineData data(&zone_stats, wasm_engine, &info, mcgraph, nullptr,
                     source_positions, node_positions, options);
   std::unique_ptr<PipelineStatistics> pipeline_statistics;
@@ -3193,14 +3200,14 @@ wasm::WasmCompilationResult Pipeline::GenerateCodeForWasmNativeStub(
 
   Linkage linkage(call_descriptor);
   CHECK(pipeline.SelectInstructions(&linkage));
-  pipeline.AssembleCode(&linkage);
+  pipeline.AssembleCode(&linkage, instruction_buffer->CreateView());
 
   CodeGenerator* code_generator = pipeline.code_generator();
   wasm::WasmCompilationResult result;
   code_generator->tasm()->GetCode(
       nullptr, &result.code_desc, code_generator->safepoint_table_builder(),
       static_cast<int>(code_generator->GetHandlerTableOffset()));
-  result.instr_buffer = code_generator->tasm()->ReleaseBuffer();
+  result.instr_buffer = instruction_buffer->ReleaseBuffer();
   result.source_positions = code_generator->GetSourcePositionTable();
   result.protected_instructions_data =
       code_generator->GetProtectedInstructionsData();
@@ -3251,6 +3258,11 @@ void Pipeline::GenerateCodeForWasmFunction(
   ZoneStats zone_stats(wasm_engine->allocator());
   std::unique_ptr<PipelineStatistics> pipeline_statistics(
       CreatePipelineStatistics(function_body, module, info, &zone_stats));
+  // {instruction_buffer} must live longer than {PipelineData}, since
+  // {PipelineData} will reference the {instruction_buffer} via the
+  // {AssemblerBuffer} of the {Assembler} contained in the {CodeGenerator}.
+  std::unique_ptr<wasm::WasmInstructionBuffer> instruction_buffer =
+      wasm::WasmInstructionBuffer::New();
   PipelineData data(&zone_stats, wasm_engine, info, mcgraph,
                     pipeline_statistics.get(), source_positions, node_origins,
                     WasmAssemblerOptions());
@@ -3298,7 +3310,7 @@ void Pipeline::GenerateCodeForWasmFunction(
 
   Linkage linkage(call_descriptor);
   if (!pipeline.SelectInstructions(&linkage)) return;
-  pipeline.AssembleCode(&linkage);
+  pipeline.AssembleCode(&linkage, instruction_buffer->CreateView());
 
   auto result = std::make_unique<wasm::WasmCompilationResult>();
   CodeGenerator* code_generator = pipeline.code_generator();
@@ -3306,7 +3318,7 @@ void Pipeline::GenerateCodeForWasmFunction(
       nullptr, &result->code_desc, code_generator->safepoint_table_builder(),
       static_cast<int>(code_generator->GetHandlerTableOffset()));
 
-  result->instr_buffer = code_generator->tasm()->ReleaseBuffer();
+  result->instr_buffer = instruction_buffer->ReleaseBuffer();
   result->frame_slot_count = code_generator->frame()->GetTotalFrameSlotCount();
   result->tagged_parameter_slots = call_descriptor->GetTaggedParameterSlots();
   result->source_positions = code_generator->GetSourcePositionTable();
@@ -3701,10 +3713,11 @@ std::ostream& operator<<(std::ostream& out,
   return out;
 }
 
-void PipelineImpl::AssembleCode(Linkage* linkage) {
+void PipelineImpl::AssembleCode(Linkage* linkage,
+                                std::unique_ptr<AssemblerBuffer> buffer) {
   PipelineData* data = this->data_;
   data->BeginPhaseKind("V8.TFCodeGeneration");
-  data->InitializeCodeGenerator(linkage);
+  data->InitializeCodeGenerator(linkage, std::move(buffer));
 
   UnparkedScopeIfNeeded unparked_scope(data->broker(), FLAG_code_comments);
 

src/wasm/baseline/liftoff-compiler.cc

@@ -491,10 +491,6 @@ class LiftoffCompiler {
                  handler_table_offset_);
   }
 
-  std::unique_ptr<AssemblerBuffer> ReleaseBuffer() {
-    return asm_.ReleaseBuffer();
-  }
-
   base::OwnedVector<uint8_t> GetSourcePositionTable() {
     return source_position_table_builder_.ToSourcePositionTableVector();
   }
@@ -6230,10 +6226,9 @@ WasmCompilationResult ExecuteLiftoffCompilation(
   size_t code_size_estimate =
       WasmCodeManager::EstimateLiftoffCodeSize(func_body_size);
   // Allocate the initial buffer a bit bigger to avoid reallocation during code
-  // generation. Overflows when casting to int are fine, as we will allocate at
-  // least {AssemblerBase::kMinimalBufferSize} anyway, so in the worst case we
-  // have to grow more often.
-  int initial_buffer_size = static_cast<int>(128 + code_size_estimate * 4 / 3);
+  // generation.
+  std::unique_ptr<wasm::WasmInstructionBuffer> instruction_buffer =
+      wasm::WasmInstructionBuffer::New(128 + code_size_estimate * 4 / 3);
   std::unique_ptr<DebugSideTableBuilder> debug_sidetable_builder;
   if (debug_sidetable) {
     debug_sidetable_builder = std::make_unique<DebugSideTableBuilder>();
@@ -6241,7 +6236,7 @@ WasmCompilationResult ExecuteLiftoffCompilation(
   DCHECK_IMPLIES(max_steps, for_debugging == kForDebugging);
   WasmFullDecoder<Decoder::kBooleanValidation, LiftoffCompiler> decoder(
       &zone, env->module, env->enabled_features, detected, func_body,
-      call_descriptor, env, &zone, NewAssemblerBuffer(initial_buffer_size),
+      call_descriptor, env, &zone, instruction_buffer->CreateView(),
       debug_sidetable_builder.get(), for_debugging, func_index, breakpoints,
       dead_breakpoint, max_steps, nondeterminism);
   decoder.Decode();
@@ -6264,7 +6259,7 @@ WasmCompilationResult ExecuteLiftoffCompilation(
 
   WasmCompilationResult result;
   compiler->GetCode(&result.code_desc);
-  result.instr_buffer = compiler->ReleaseBuffer();
+  result.instr_buffer = instruction_buffer->ReleaseBuffer();
   result.source_positions = compiler->GetSourcePositionTable();
   result.protected_instructions_data = compiler->GetProtectedInstructionsData();
   result.frame_slot_count = compiler->GetTotalFrameSlotCountForGC();

src/wasm/function-compiler.cc

@@ -22,6 +22,96 @@ namespace v8 {
 namespace internal {
 namespace wasm {
 
+namespace {
+
+class WasmInstructionBufferImpl {
+ public:
+  class View : public AssemblerBuffer {
+   public:
+    View(base::Vector<uint8_t> buffer, WasmInstructionBufferImpl* holder)
+        : buffer_(buffer), holder_(holder) {}
+
+    ~View() override {
+      if (buffer_.begin() == holder_->old_buffer_.start()) {
+        DCHECK_EQ(buffer_.size(), holder_->old_buffer_.size());
+        holder_->old_buffer_ = {};
+      }
+    }
+
+    byte* start() const override { return buffer_.begin(); }
+
+    int size() const override { return static_cast<int>(buffer_.size()); }
+
+    std::unique_ptr<AssemblerBuffer> Grow(int new_size) override {
+      // If we grow, we must be the current buffer of {holder_}.
+      DCHECK_EQ(buffer_.begin(), holder_->buffer_.start());
+      DCHECK_EQ(buffer_.size(), holder_->buffer_.size());
+      DCHECK_NULL(holder_->old_buffer_);
+
+      DCHECK_LT(size(), new_size);
+
+      holder_->old_buffer_ = std::move(holder_->buffer_);
+      holder_->buffer_ = base::OwnedVector<uint8_t>::NewForOverwrite(new_size);
+      return std::make_unique<View>(holder_->buffer_.as_vector(), holder_);
+    }
+
+   private:
+    const base::Vector<uint8_t> buffer_;
+    WasmInstructionBufferImpl* const holder_;
+  };
+
+  explicit WasmInstructionBufferImpl(size_t size)
+      : buffer_(base::OwnedVector<uint8_t>::NewForOverwrite(size)) {}
+
+  std::unique_ptr<AssemblerBuffer> CreateView() {
+    DCHECK_NOT_NULL(buffer_);
+    return std::make_unique<View>(buffer_.as_vector(), this);
+  }
+
+  std::unique_ptr<uint8_t[]> ReleaseBuffer() {
+    DCHECK_NULL(old_buffer_);
+    DCHECK_NOT_NULL(buffer_);
+    return buffer_.ReleaseData();
+  }
+
+  bool released() const { return buffer_ == nullptr; }
+
+ private:
+  // The current buffer used to emit code.
+  base::OwnedVector<uint8_t> buffer_;
+
+  // While the buffer is grown, we need to temporarily also keep the old buffer
+  // alive.
+  base::OwnedVector<uint8_t> old_buffer_;
+};
+
+WasmInstructionBufferImpl* Impl(WasmInstructionBuffer* buf) {
+  return reinterpret_cast<WasmInstructionBufferImpl*>(buf);
+}
+
+}  // namespace
+
+// PIMPL interface WasmInstructionBuffer for WasmInstBufferImpl
+WasmInstructionBuffer::~WasmInstructionBuffer() {
+  Impl(this)->~WasmInstructionBufferImpl();
+}
+
+std::unique_ptr<AssemblerBuffer> WasmInstructionBuffer::CreateView() {
+  return Impl(this)->CreateView();
+}
+
+std::unique_ptr<uint8_t[]> WasmInstructionBuffer::ReleaseBuffer() {
+  return Impl(this)->ReleaseBuffer();
+}
+
+// static
+std::unique_ptr<WasmInstructionBuffer> WasmInstructionBuffer::New(size_t size) {
+  return std::unique_ptr<WasmInstructionBuffer>{
+      reinterpret_cast<WasmInstructionBuffer*>(new WasmInstructionBufferImpl(
+          std::max(size_t{AssemblerBase::kMinimalBufferSize}, size)))};
+}
+// End of PIMPL interface WasmInstructionBuffer for WasmInstBufferImpl
+
 // static
 ExecutionTier WasmCompilationUnit::GetBaselineExecutionTier(
     const WasmModule* module) {

src/wasm/function-compiler.h

@@ -11,7 +11,6 @@
 
 #include <memory>
 
-#include "src/codegen/assembler.h"
 #include "src/codegen/code-desc.h"
 #include "src/trap-handler/trap-handler.h"
 #include "src/wasm/compilation-environment.h"
@@ -23,6 +22,7 @@
 namespace v8 {
 namespace internal {
 
+class AssemblerBuffer;
 class Counters;
 class OptimizedCompilationJob;
 
@@ -33,6 +33,24 @@ class WasmCode;
 class WasmEngine;
 struct WasmFunction;
 
+class WasmInstructionBuffer final {
+ public:
+  WasmInstructionBuffer() = delete;
+  WasmInstructionBuffer(const WasmInstructionBuffer&) = delete;
+  WasmInstructionBuffer& operator=(const WasmInstructionBuffer&) = delete;
+  ~WasmInstructionBuffer();
+  std::unique_ptr<AssemblerBuffer> CreateView();
+  std::unique_ptr<uint8_t[]> ReleaseBuffer();
+
+  // Allocate a new {WasmInstructionBuffer}. The size is the maximum of {size}
+  // and {AssemblerBase::kMinimalSize}.
+  static std::unique_ptr<WasmInstructionBuffer> New(size_t size = 0);
+
+  // Override {operator delete} to avoid implicit instantiation of {operator
+  // delete} with {size_t} argument. The {size_t} argument would be incorrect.
+  void operator delete(void* ptr) { ::operator delete(ptr); }
+};
+
 struct WasmCompilationResult {
  public:
   MOVE_ONLY_WITH_DEFAULT_CONSTRUCTORS(WasmCompilationResult);
@@ -47,7 +65,7 @@ struct WasmCompilationResult {
   operator bool() const { return succeeded(); }
 
   CodeDesc code_desc;
-  std::unique_ptr<AssemblerBuffer> instr_buffer;
+  std::unique_ptr<uint8_t[]> instr_buffer;
   uint32_t frame_slot_count = 0;
   uint32_t tagged_parameter_slots = 0;
   base::OwnedVector<byte> source_positions;

src/wasm/wasm-code-manager.cc

@@ -2099,7 +2099,7 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode(
   // {allocator_mutex_} if we try to switch for each code individually.
   CodeSpaceWriteScope code_space_write_scope(this);
   for (auto& result : results) {
-    DCHECK_EQ(result.code_desc.buffer, result.instr_buffer->start());
+    DCHECK_EQ(result.code_desc.buffer, result.instr_buffer.get());
     size_t code_size = RoundUp<kCodeAlignment>(result.code_desc.instr_size);
     base::Vector<byte> this_code_space = code_space.SubVector(0, code_size);
     code_space += code_size;