[wasm] Fix stack iteration for tagged parameters

When we pass function arguments on the stack, untagged parameters
"come first", i.e. are put to lower addresses / can be popped off
first. So when a function instructs the stack walker to visit its
parameters (belonging to its caller's frame), it must skip past
any untagged parameters at the top of the caller's frame.

Change-Id: I5a42e4850b0808237ae937c90b0cec930df8571b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2964394
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75180}

src/compiler/linkage.cc

@@ -138,15 +138,26 @@ int CallDescriptor::GetOffsetToReturns() const {
   return offset;
 }
 
-int CallDescriptor::GetTaggedParameterSlots() const {
-  int result = 0;
+uint32_t CallDescriptor::GetTaggedParameterSlots() const {
+  uint32_t count = 0;
+  uint32_t first_offset = kMaxInt;
   for (size_t i = 0; i < InputCount(); ++i) {
     LinkageLocation operand = GetInputLocation(i);
     if (!operand.IsRegister() && operand.GetType().IsTagged()) {
-      ++result;
+      ++count;
+      // Caller frame slots have negative indices and start at -1. Flip it
+      // back to a positive offset (to be added to the frame's SP to find the
+      // slot).
+      int slot_offset = -operand.GetLocation() - 1;
+      DCHECK_GE(slot_offset, 0);
+      first_offset = std::min(first_offset, static_cast<uint32_t>(slot_offset));
     }
   }
-  return result;
+  if (count > 0) {
+    DCHECK(first_offset != kMaxInt);
+    return (first_offset << 16) | (count & 0xFFFFu);
+  }
+  return 0;
 }
 
 bool CallDescriptor::CanTailCall(const CallDescriptor* callee) const {
@@ -263,18 +274,6 @@ bool Linkage::NeedsFrameStateInput(Runtime::FunctionId function) {
   return true;
 }
 
-
-bool CallDescriptor::UsesOnlyRegisters() const {
-  for (size_t i = 0; i < InputCount(); ++i) {
-    if (!GetInputLocation(i).IsRegister()) return false;
-  }
-  for (size_t i = 0; i < ReturnCount(); ++i) {
-    if (!GetReturnLocation(i).IsRegister()) return false;
-  }
-  return true;
-}
-
-
 CallDescriptor* Linkage::GetRuntimeCallDescriptor(
     Zone* zone, Runtime::FunctionId function_id, int js_parameter_count,
     Operator::Properties properties, CallDescriptor::Flags flags) {

src/compiler/linkage.h

@@ -395,8 +395,6 @@ class V8_EXPORT_PRIVATE CallDescriptor final
 
   const char* debug_name() const { return debug_name_; }
 
-  bool UsesOnlyRegisters() const;
-
   int GetStackParameterDelta(const CallDescriptor* tail_caller) const;
 
   // Returns the offset to the area below the parameter slots on the stack,
@@ -410,7 +408,8 @@ class V8_EXPORT_PRIVATE CallDescriptor final
   // If there are no parameter slots, returns 0.
   int GetOffsetToReturns() const;
 
-  int GetTaggedParameterSlots() const;
+  // Returns two 16-bit numbers packed together: (first slot << 16) | num_slots.
+  uint32_t GetTaggedParameterSlots() const;
 
   bool CanTailCall(const CallDescriptor* callee) const;
 

src/execution/frames.cc

@@ -940,7 +940,8 @@ void CommonFrame::IterateCompiledFrame(RootVisitor* v) const {
   uint32_t stack_slots = 0;
   Code code;
   bool has_tagged_outgoing_params = false;
-  uint32_t tagged_parameter_slots = 0;
+  uint16_t first_tagged_parameter_slot = 0;
+  uint16_t num_tagged_parameter_slots = 0;
   bool is_wasm = false;
 
 #if V8_ENABLE_WEBASSEMBLY
@@ -953,7 +954,8 @@ void CommonFrame::IterateCompiledFrame(RootVisitor* v) const {
     has_tagged_outgoing_params =
         wasm_code->kind() != wasm::WasmCode::kFunction &&
         wasm_code->kind() != wasm::WasmCode::kWasmToCapiWrapper;
-    tagged_parameter_slots = wasm_code->tagged_parameter_slots();
+    first_tagged_parameter_slot = wasm_code->first_tagged_parameter_slot();
+    num_tagged_parameter_slots = wasm_code->num_tagged_parameter_slots();
   }
 #endif  // V8_ENABLE_WEBASSEMBLY
 
@@ -1091,10 +1093,11 @@ void CommonFrame::IterateCompiledFrame(RootVisitor* v) const {
   // frame. Conceptionally these parameters belong to the parent frame. However,
   // the exact count is only known by this frame (in the presence of tail calls,
   // this information cannot be derived from the call site).
-  if (tagged_parameter_slots > 0) {
+  if (num_tagged_parameter_slots > 0) {
     FullObjectSlot tagged_parameter_base(&Memory<Address>(caller_sp()));
+    tagged_parameter_base += first_tagged_parameter_slot;
     FullObjectSlot tagged_parameter_limit =
-        tagged_parameter_base + tagged_parameter_slots;
+        tagged_parameter_base + num_tagged_parameter_slots;
 
     v->VisitRootPointers(Root::kStackRoots, nullptr, tagged_parameter_base,
                          tagged_parameter_limit);

src/wasm/wasm-code-manager.cc

@@ -284,6 +284,15 @@ void WasmCode::LogCode(Isolate* isolate, const char* source_url,
 }
 
 void WasmCode::Validate() const {
+  // The packing strategy for {tagged_parameter_slots} only works if both the
+  // max number of parameters and their max combined stack slot usage fits into
+  // their respective half of the result value.
+  STATIC_ASSERT(wasm::kV8MaxWasmFunctionParams <
+                std::numeric_limits<uint16_t>::max());
+  static constexpr int kMaxSlotsPerParam = 4;  // S128 on 32-bit platforms.
+  STATIC_ASSERT(wasm::kV8MaxWasmFunctionParams * kMaxSlotsPerParam <
+                std::numeric_limits<uint16_t>::max());
+
 #ifdef DEBUG
   // Scope for foreign WasmCode pointers.
   WasmCodeRefScope code_ref_scope;
@@ -1056,7 +1065,8 @@ void NativeModule::UseLazyStub(uint32_t func_index) {
 
 std::unique_ptr<WasmCode> NativeModule::AddCode(
     int index, const CodeDesc& desc, int stack_slots,
-    int tagged_parameter_slots, Vector<const byte> protected_instructions_data,
+    uint32_t tagged_parameter_slots,
+    Vector<const byte> protected_instructions_data,
     Vector<const byte> source_position_table, WasmCode::Kind kind,
     ExecutionTier tier, ForDebugging for_debugging) {
   Vector<byte> code_space;
@@ -1076,7 +1086,8 @@ std::unique_ptr<WasmCode> NativeModule::AddCode(
 
 std::unique_ptr<WasmCode> NativeModule::AddCodeWithCodeSpace(
     int index, const CodeDesc& desc, int stack_slots,
-    int tagged_parameter_slots, Vector<const byte> protected_instructions_data,
+    uint32_t tagged_parameter_slots,
+    Vector<const byte> protected_instructions_data,
     Vector<const byte> source_position_table, WasmCode::Kind kind,
     ExecutionTier tier, ForDebugging for_debugging,
     Vector<uint8_t> dst_code_bytes, const JumpTablesRef& jump_tables) {
@@ -1284,7 +1295,7 @@ NativeModule::AllocateForDeserializedCode(size_t total_code_size) {
 
 std::unique_ptr<WasmCode> NativeModule::AddDeserializedCode(
     int index, Vector<byte> instructions, int stack_slots,
-    int tagged_parameter_slots, int safepoint_table_offset,
+    uint32_t tagged_parameter_slots, int safepoint_table_offset,
     int handler_table_offset, int constant_pool_offset,
     int code_comments_offset, int unpadded_binary_size,
     Vector<const byte> protected_instructions_data,

src/wasm/wasm-code-manager.h

@@ -255,7 +255,15 @@ class V8_EXPORT_PRIVATE WasmCode final {
   int code_comments_offset() const { return code_comments_offset_; }
   int unpadded_binary_size() const { return unpadded_binary_size_; }
   int stack_slots() const { return stack_slots_; }
-  int tagged_parameter_slots() const { return tagged_parameter_slots_; }
+  uint16_t first_tagged_parameter_slot() const {
+    return tagged_parameter_slots_ >> 16;
+  }
+  uint16_t num_tagged_parameter_slots() const {
+    return tagged_parameter_slots_ & 0xFFFF;
+  }
+  uint32_t raw_tagged_parameter_slots_for_serialization() const {
+    return tagged_parameter_slots_;
+  }
   bool is_liftoff() const { return tier() == ExecutionTier::kLiftoff; }
   bool contains(Address pc) const {
     return reinterpret_cast<Address>(instructions_) <= pc &&
@@ -346,7 +354,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
   friend class NativeModule;
 
   WasmCode(NativeModule* native_module, int index, Vector<byte> instructions,
-           int stack_slots, int tagged_parameter_slots,
+           int stack_slots, uint32_t tagged_parameter_slots,
            int safepoint_table_offset, int handler_table_offset,
            int constant_pool_offset, int code_comments_offset,
            int unpadded_binary_size,
@@ -417,9 +425,10 @@ class V8_EXPORT_PRIVATE WasmCode final {
   const int index_;
   const int constant_pool_offset_;
   const int stack_slots_;
-  // Number of tagged parameters passed to this function via the stack. This
-  // value is used by the stack walker (e.g. GC) to find references.
-  const int tagged_parameter_slots_;
+  // Number and position of tagged parameters passed to this function via the
+  // stack, packed into a single uint32. These values are used by the stack
+  // walker (e.g. GC) to find references.
+  const uint32_t tagged_parameter_slots_;
   // We care about safepoint data for wasm-to-js functions, since there may be
   // stack/register tagged values for large number conversions.
   const int safepoint_table_offset_;
@@ -566,7 +575,8 @@ class V8_EXPORT_PRIVATE NativeModule final {
   // code below, i.e. it can be called concurrently from background threads.
   // The returned code still needs to be published via {PublishCode}.
   std::unique_ptr<WasmCode> AddCode(int index, const CodeDesc& desc,
-                                    int stack_slots, int tagged_parameter_slots,
+                                    int stack_slots,
+                                    uint32_t tagged_parameter_slots,
                                     Vector<const byte> protected_instructions,
                                     Vector<const byte> source_position_table,
                                     WasmCode::Kind kind, ExecutionTier tier,
@@ -597,7 +607,7 @@ class V8_EXPORT_PRIVATE NativeModule final {
 
   std::unique_ptr<WasmCode> AddDeserializedCode(
       int index, Vector<byte> instructions, int stack_slots,
-      int tagged_parameter_slots, int safepoint_table_offset,
+      uint32_t tagged_parameter_slots, int safepoint_table_offset,
       int handler_table_offset, int constant_pool_offset,
       int code_comments_offset, int unpadded_binary_size,
       Vector<const byte> protected_instructions_data,
@@ -788,7 +798,7 @@ class V8_EXPORT_PRIVATE NativeModule final {
 
   std::unique_ptr<WasmCode> AddCodeWithCodeSpace(
       int index, const CodeDesc& desc, int stack_slots,
-      int tagged_parameter_slots,
+      uint32_t tagged_parameter_slots,
       Vector<const byte> protected_instructions_data,
       Vector<const byte> source_position_table, WasmCode::Kind kind,
       ExecutionTier tier, ForDebugging for_debugging,

src/wasm/wasm-serialization.cc

@@ -352,7 +352,7 @@ bool NativeModuleSerializer::WriteCode(const WasmCode* code, Writer* writer) {
   writer->Write(code->code_comments_offset());
   writer->Write(code->unpadded_binary_size());
   writer->Write(code->stack_slots());
-  writer->Write(code->tagged_parameter_slots());
+  writer->Write(code->raw_tagged_parameter_slots_for_serialization());
   writer->Write(code->instructions().length());
   writer->Write(code->reloc_info().length());
   writer->Write(code->source_positions().length());
@@ -698,7 +698,7 @@ DeserializationUnit NativeModuleDeserializer::ReadCode(int fn_index,
   int code_comment_offset = reader->Read<int>();
   int unpadded_binary_size = reader->Read<int>();
   int stack_slot_count = reader->Read<int>();
-  int tagged_parameter_slots = reader->Read<int>();
+  uint32_t tagged_parameter_slots = reader->Read<uint32_t>();
   int code_size = reader->Read<int>();
   int reloc_size = reader->Read<int>();
   int source_position_size = reader->Read<int>();

test/mjsunit/wasm/tagged-stack-parameters.js

+// Copyright 2021 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: --allow-natives-syntax --expose-gc --experimental-wasm-gc
+
+d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
+
+var instance = (function () {
+  let builder = new WasmModuleBuilder();
+
+  let struct_index = builder.addStruct([makeField(kWasmI32, true)]);
+  let struct_ref = wasmRefType(struct_index);
+
+  let gc_function_index =
+    builder.addImport("imports", "gc_func", kSig_v_v);
+
+  let struct_consumer =
+    builder.addFunction("struct_consumer", makeSig([
+        struct_ref, struct_ref], [kWasmI32]))
+      .addBody([
+        kExprLocalGet, 0, kGCPrefix, kExprStructGet, struct_index, 0,
+        kExprLocalGet, 1, kGCPrefix, kExprStructGet, struct_index, 0,
+        kExprI32Add]);
+
+  let many_params = builder.addFunction("many_params", makeSig([
+      kWasmI32, kWasmI32, kWasmI32, kWasmI32, kWasmI32, kWasmI32, kWasmI32,
+      struct_ref, struct_ref], [kWasmI32]))
+    .addBody([
+      kExprCallFunction, gc_function_index,
+      kExprLocalGet, 7, kExprLocalGet, 8,
+      kExprCallFunction, struct_consumer.index]);
+
+  builder.addFunction("main", kSig_i_v)
+    .addBody([
+      // Seven i32 parameters that look like tagged pointers.
+      ...wasmI32Const(1), ...wasmI32Const(3), ...wasmI32Const(5),
+      ...wasmI32Const(7), ...wasmI32Const(9), ...wasmI32Const(11),
+      ...wasmI32Const(13),
+      // Two structs (i.e. actual tagged pointers).
+      ...wasmI32Const(20), kGCPrefix, kExprRttCanon, struct_index,
+      kGCPrefix, kExprStructNewWithRtt, struct_index,
+      ...wasmI32Const(22), kGCPrefix, kExprRttCanon, struct_index,
+      kGCPrefix, kExprStructNewWithRtt, struct_index,
+      kExprCallFunction, many_params.index,
+    ])
+    .exportFunc();
+
+  return builder.instantiate({imports: {
+    gc_func: () => gc(),
+  }});
+})();
+
+assertEquals(42, instance.exports.main());