[wasm] Check the input of grow-memory before calling the runtime.

src/compiler/wasm-compiler.cc

Index: src/compiler/wasm-compiler.cc
diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc
index 30c545ec515bb07c2459b52b11dc25d3e1fcedd9..cfeac987eb8ac472303f4262add81cac1015e728 100644
--- a/src/compiler/wasm-compiler.cc
+++ b/src/compiler/wasm-compiler.cc
@@ -1668,13 +1668,18 @@ Node* WasmGraphBuilder::BuildFloatToIntConversionInstruction(
 }
 
 Node* WasmGraphBuilder::BuildGrowMemory(Node* input) {
+  Diamond check_input_range(
+      graph(), jsgraph()->common(),
+      graph()->NewNode(
+          jsgraph()->machine()->Uint32LessThanOrEqual(), input,
+          jsgraph()->Uint32Constant(wasm::WasmModule::kMaxMemPages)),
+      BranchHint::kTrue);

[gdeepti, 2016/08/30 02:19:56]

This makes the first check in the GrowMemory runtime function (for 0 initial
memory) redundant, remove the one in wasm-runtime.cc? 

[ahaas, 2016/08/30 06:49:53]

I replaced the check in runtime-wasm.cc with a DCHECK.

+
   Runtime::FunctionId function_id = Runtime::kWasmGrowMemory;
   const Runtime::Function* function = Runtime::FunctionForId(function_id);
   CallDescriptor* desc = Linkage::GetRuntimeCallDescriptor(
       jsgraph()->zone(), function_id, function->nargs, Operator::kNoThrow,
       CallDescriptor::kNoFlags);
-  Node** control_ptr = control_;
-  Node** effect_ptr = effect_;
   wasm::ModuleEnv* module = module_;
   input = BuildChangeUint32ToSmi(input);

[titzer, 2016/08/29 16:57:58]

If I remember correctly this will also have a check inside. Can we avoid that
somehow?

[ahaas, 2016/08/30 06:49:53]

I do not understand this comment, do you mean the check in the GrowMemory
runtime function? I also wonder if it is worth to optimize code here because
GrowMemory should be quite cold.

[titzer, 2016/08/30 08:25:47]

Sorry, I was thinking that BuildChangeUint32ToSmi() had a branch inside, but
apparently it doesn't.

   Node* inputs[] = {
@@ -1683,13 +1688,19 @@ Node* WasmGraphBuilder::BuildGrowMemory(Node* input) {
           ExternalReference(function_id, jsgraph()->isolate())),  // ref
       jsgraph()->Int32Constant(function->nargs),                  // arity
       jsgraph()->HeapConstant(module->instance->context),         // context
-      *effect_ptr,
-      *control_ptr};
-  Node* node = graph()->NewNode(jsgraph()->common()->Call(desc),
+      *effect_,
+      check_input_range.if_true};
+  Node* call = graph()->NewNode(jsgraph()->common()->Call(desc),
                                 static_cast<int>(arraysize(inputs)), inputs);
-  *effect_ptr = node;
-  node = BuildChangeSmiToInt32(node);
-  return node;
+
+  Node* result = BuildChangeSmiToInt32(call);
+
+  result = check_input_range.Phi(MachineRepresentation::kWord32, result,
+                                 jsgraph()->Int32Constant(-1));
+  *effect_ = graph()->NewNode(jsgraph()->common()->EffectPhi(2), call, *effect_,
+                              check_input_range.merge);
+  *control_ = check_input_range.merge;
+  return result;
 }
 
 Node* WasmGraphBuilder::BuildI32DivS(Node* left, Node* right,