[wasm] Make BuildCCall variadic to avoid buffer space and copies

The typical pattern for using BuildCCall was to create a stack-allocated array
of arguments, which was passed to BuildCCall. BuildCCall then would allocate
scratch space using WasmGraphBuilder::Buffer and copy the arguments into the
scratch space. This copy is unnecessary and also the use of Buffer can lead to
silently overwriting scratch space that may be used higher up the stack.

Now BuildCCall takes all of the functions parameters as arguments and stores
this directly in a stack-allocated buffer, avoiding the use of Buffer at all and
all its associated pitfalls.

Change-Id: Ia0de9a90350d7fcbbb1b05d28e735d790a5f9143
Reviewed-on: https://chromium-review.googlesource.com/701638Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48350}

src/compiler/wasm-compiler.cc

@@ -1467,9 +1467,8 @@ Node* WasmGraphBuilder::BuildBitCountingCall(Node* input, ExternalReference ref,
   sig_builder.AddParam(MachineType::Pointer());
 
   Node* function = graph()->NewNode(jsgraph()->common()->ExternalConstant(ref));
-  Node* args[] = {function, stack_slot_param};
 
-  return BuildCCall(sig_builder.Build(), args);
+  return BuildCCall(sig_builder.Build(), function, stack_slot_param);
 }
 
 Node* WasmGraphBuilder::BuildI32Ctz(Node* input) {
@@ -1603,12 +1602,14 @@ Node* WasmGraphBuilder::BuildCFuncInstruction(ExternalReference ref,
                               *control_);
 
   Node* function = graph()->NewNode(jsgraph()->common()->ExternalConstant(ref));
-  Node** args = Buffer(5);
-  args[0] = function;
-  args[1] = stack_slot_param0;
-  int input_count = 1;
 
-  if (input1 != nullptr) {
+  if (input1 == nullptr) {
+    const int input_count = 1;
+    Signature<MachineType>::Builder sig_builder(jsgraph()->zone(), 0,
+                                                input_count);
+    sig_builder.AddParam(MachineType::Pointer());
+    BuildCCall(sig_builder.Build(), function, stack_slot_param0);
+  } else {
     Node* stack_slot_param1 = graph()->NewNode(
         jsgraph()->machine()->StackSlot(type.representation()));
     const Operator* store_op1 = jsgraph()->machine()->Store(
@@ -1616,17 +1617,15 @@ Node* WasmGraphBuilder::BuildCFuncInstruction(ExternalReference ref,
     *effect_ = graph()->NewNode(store_op1, stack_slot_param1,
                                 jsgraph()->Int32Constant(0), input1, *effect_,
                                 *control_);
-    args[2] = stack_slot_param1;
-    ++input_count;
-  }
 
-  Signature<MachineType>::Builder sig_builder(jsgraph()->zone(), 0,
-                                              input_count);
-  sig_builder.AddParam(MachineType::Pointer());
-  if (input1 != nullptr) {
+    const int input_count = 2;
+    Signature<MachineType>::Builder sig_builder(jsgraph()->zone(), 0,
+                                                input_count);
+    sig_builder.AddParam(MachineType::Pointer());
     sig_builder.AddParam(MachineType::Pointer());
+    BuildCCall(sig_builder.Build(), function, stack_slot_param0,
+               stack_slot_param1);
   }
-  BuildCCall(sig_builder.Build(), args);
 
   const Operator* load_op = jsgraph()->machine()->Load(type);
 
@@ -1677,8 +1676,8 @@ Node* WasmGraphBuilder::BuildIntToFloatConversionInstruction(
   sig_builder.AddParam(MachineType::Pointer());
   sig_builder.AddParam(MachineType::Pointer());
   Node* function = graph()->NewNode(jsgraph()->common()->ExternalConstant(ref));
-  Node* args[] = {function, stack_slot_param, stack_slot_result};
-  BuildCCall(sig_builder.Build(), args);
+  BuildCCall(sig_builder.Build(), function, stack_slot_param,
+             stack_slot_result);
   const Operator* load_op = jsgraph()->machine()->Load(result_type);
   Node* load =
       graph()->NewNode(load_op, stack_slot_result, jsgraph()->Int32Constant(0),
@@ -1777,9 +1776,10 @@ Node* WasmGraphBuilder::BuildFloatToIntConversionInstruction(
   sig_builder.AddParam(MachineType::Pointer());
   sig_builder.AddParam(MachineType::Pointer());
   Node* function = graph()->NewNode(jsgraph()->common()->ExternalConstant(ref));
-  Node* args[] = {function, stack_slot_param, stack_slot_result};
   ZeroCheck32(wasm::kTrapFloatUnrepresentable,
-              BuildCCall(sig_builder.Build(), args), position);
+              BuildCCall(sig_builder.Build(), function, stack_slot_param,
+                         stack_slot_result),
+              position);
   const Operator* load_op = jsgraph()->machine()->Load(result_type);
   Node* load =
       graph()->NewNode(load_op, stack_slot_result, jsgraph()->Int32Constant(0),
@@ -2288,9 +2288,8 @@ Node* WasmGraphBuilder::BuildDiv64Call(Node* left, Node* right,
   sig_builder.AddParam(MachineType::Pointer());
 
   Node* function = graph()->NewNode(jsgraph()->common()->ExternalConstant(ref));
-  Node* args[] = {function, stack_slot_dst, stack_slot_src};
-
-  Node* call = BuildCCall(sig_builder.Build(), args);
+  Node* call =
+      BuildCCall(sig_builder.Build(), function, stack_slot_dst, stack_slot_src);
 
   ZeroCheck32(static_cast<wasm::TrapReason>(trap_zero), call, position);
   TrapIfEq32(wasm::kTrapDivUnrepresentable, call, -1, position);
@@ -2302,40 +2301,18 @@ Node* WasmGraphBuilder::BuildDiv64Call(Node* left, Node* right,
   return load;
 }
 
-Node* WasmGraphBuilder::BuildCCall(MachineSignature* sig, Node* const* args) {
-  const size_t params = sig->parameter_count();
-  const size_t extra = 2;  // effect and control inputs.
-  const size_t count = 1 + params + extra;
-
-  // Reallocate the buffer to make space for extra inputs.
-  Node** new_args = Realloc(args, 1 + params, count);
-  return BuildCCallWithBuffer(sig, new_args, count);
-}
-
-// Builds a CCall using the caller-provided buffer rather than calling Buffer()
-// and copying. The args array contains the function to call and any
-// parameters. There should be space for two more arguments at the end, which
-// BuildCCallWithBuffer will fill with the effect and control nodes.
-//
-// TODO(eholk): Refactor this as a variadic template that knows the right number
-// of arguments.
-Node* WasmGraphBuilder::BuildCCallWithBuffer(MachineSignature* sig, Node** args,
-                                             size_t args_len) {
-  const size_t params = sig->parameter_count();
-  const size_t extra = 2;  // effect and control inputs.
-  const size_t count = 1 + params + extra;
-
-  DCHECK_LE(count, args_len);
-
-  // Add effect and control inputs.
-  args[params + 1] = *effect_;
-  args[params + 2] = *control_;
+template <typename... Args>
+Node* WasmGraphBuilder::BuildCCall(MachineSignature* sig, Node* function,
+                                   Args... args) {
+  DCHECK_LE(sig->return_count(), 1);
+  DCHECK_EQ(sizeof...(args), sig->parameter_count());
+  Node* const call_args[] = {function, args..., *effect_, *control_};
 
   CallDescriptor* desc =
       Linkage::GetSimplifiedCDescriptor(jsgraph()->zone(), sig);
 
   const Operator* op = jsgraph()->common()->Call(desc);
-  Node* call = graph()->NewNode(op, static_cast<int>(count), args);
+  Node* call = graph()->NewNode(op, arraysize(call_args), call_args);
   *effect_ = call;
   return call;
 }
@@ -3299,10 +3276,9 @@ Node* WasmGraphBuilder::BuildModifyThreadInWasmFlag(bool new_value) {
                 : ExternalReference::wasm_clear_thread_in_wasm_flag(
                       jsgraph()->isolate());
   MachineSignature::Builder sig_builder(jsgraph()->zone(), 0, 0);
-  Node* args[] = {graph()->NewNode(jsgraph()->common()->ExternalConstant(ref)),
-                  nullptr,  // Extra space for CCall effect and control inputs
-                  nullptr};
-  return BuildCCallWithBuffer(sig_builder.Build(), args, arraysize(args));
+  return BuildCCall(
+      sig_builder.Build(),
+      graph()->NewNode(jsgraph()->common()->ExternalConstant(ref)));
 }
 
 // Only call this function for code which is not reused across instantiations,

src/compiler/wasm-compiler.h

@@ -406,9 +406,8 @@ class WasmGraphBuilder {
   Node* MaskShiftCount32(Node* node);
   Node* MaskShiftCount64(Node* node);
 
-  Node* BuildCCall(MachineSignature* sig, Node* const* args);
-  Node* BuildCCallWithBuffer(MachineSignature* sig, Node** args,
-                             size_t args_len);
+  template <typename... Args>
+  Node* BuildCCall(MachineSignature* sig, Node* function, Args... args);
   Node* BuildWasmCall(wasm::FunctionSig* sig, Node** args, Node*** rets,
                       wasm::WasmCodePosition position);