[wasm] Support validation of asm.js modules with != 3 args.

src/builtins/x64/builtins-x64.cc

Index: src/builtins/x64/builtins-x64.cc
diff --git a/src/builtins/x64/builtins-x64.cc b/src/builtins/x64/builtins-x64.cc
index d67583730d9536d6bb9fce43bc6b44a344b0d6c4..c919a775958182ba19c74f3de7a17061bbba04aa 100644
--- a/src/builtins/x64/builtins-x64.cc
+++ b/src/builtins/x64/builtins-x64.cc
@@ -1062,6 +1062,8 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
   Label failed;
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
+    // Preserve argument count for later compare.
+    __ movp(kScratchRegister, rax);
     // Push the number of arguments to the callee.
     __ Integer32ToSmi(rax, rax);
     __ Push(rax);
@@ -1072,16 +1074,43 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
     // The function.
     __ Push(rdi);
     // Copy arguments from caller (stdlib, foreign, heap).
-    for (int i = 2; i >= 0; --i) {
-      __ Push(Operand(
-          rbp, StandardFrameConstants::kCallerSPOffset + i * kPointerSize));
+    Label args_done;
+    for (int j = 0; j < 4; ++j) {

[Michael Starzinger, 2016/08/11 11:44:10]

Does this work correctly in the case where we instantiate the module with more
than 3 arguments? Can we add a test case for that. I am specifically thinking
about a case where the module has more than three formal parameters, like the
following:

function Module(stdlib, foreign, heap, crap1, crap2) {
  'use asm';
  return {};
}
var m = Module({},{},heap);

[bradn, 2016/08/12 01:17:09]

The validator rejects the module above as invalid and falls back.
Added a test and that seems to work.

+      Label over;
+      if (j < 3) {
+        __ cmpp(kScratchRegister, Immediate(j));
+        __ j(not_equal, &over, Label::kNear);
+      }
+      for (int i = j - 1; i >= 0; --i) {
+        __ Push(Operand(
+            rbp, StandardFrameConstants::kCallerSPOffset + i * kPointerSize));
+      }
+      for (int i = 0; i < 3 - j; ++i) {
+        __ PushRoot(Heap::kUndefinedValueRootIndex);
+      }
+      if (j < 3) {
+        __ jmp(&args_done, Label::kNear);
+        __ bind(&over);
+      }
     }
+    __ bind(&args_done);
+
     // Call runtime, on success unwind frame, and parent frame.
     __ CallRuntime(Runtime::kInstantiateAsmJs, 4);
     // A smi 0 is returned on failure, an object on success.
     __ JumpIfSmi(rax, &failed, Label::kNear);
+
+    __ Pop(kScratchRegister);
+    __ Pop(kScratchRegister);

[Michael Starzinger, 2016/08/11 11:44:10]

nit: Let's use __ Drop(2) instead of the multi-pop. Probably applies to all
archs.

[bradn, 2016/08/12 01:17:09]

Good idea. Done.

+    __ Pop(kScratchRegister);
+    __ SmiToInteger32(kScratchRegister, kScratchRegister);
     scope.GenerateLeaveFrame();
-    __ ret(4 * kPointerSize);
+
+    __ Pop(rbx);

[Michael Starzinger, 2016/08/11 11:44:10]

nit: Lets use PopReturnAddressTo for readability.

[bradn, 2016/08/12 01:17:09]

Done.

+    __ incp(kScratchRegister);
+    __ leap(rsp, Operand(rsp, kScratchRegister, times_pointer_size, 0));
+    __ Push(rbx);

[Michael Starzinger, 2016/08/11 11:44:10]

nit: Lets use PushReturnAddressFrom for readability.

[bradn, 2016/08/12 01:17:09]

Done.

+    __ ret(0);
 
     __ bind(&failed);
     // Restore target function and new target.