Fix a bug that occurs when functions are defined with more than 16,382 parameters.

Review URL: http://codereview.chromium.org/6447007

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6697 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/ia32/codegen-ia32.cc

@@ -3771,14 +3771,15 @@ void CodeGenerator::GenerateReturnSequence(Result* return_value) {
   // Leave the frame and return popping the arguments and the
   // receiver.
   frame_->Exit();
-  masm_->ret((scope()->num_parameters() + 1) * kPointerSize);
+  int arguments_bytes = (scope()->num_parameters() + 1) * kPointerSize;
+  __ Ret(arguments_bytes, ecx);
   DeleteFrame();
 
 #ifdef ENABLE_DEBUGGER_SUPPORT
-  // Check that the size of the code used for returning matches what is
-  // expected by the debugger.
-  ASSERT_EQ(Assembler::kJSReturnSequenceLength,
-            masm_->SizeOfCodeGeneratedSince(&check_exit_codesize));
+  // Check that the size of the code used for returning is large enough
+  // for the debugger's requirements.
+  ASSERT(Assembler::kJSReturnSequenceLength <=
+         masm_->SizeOfCodeGeneratedSince(&check_exit_codesize));
 #endif
 }
 

src/ia32/full-codegen-ia32.cc

@@ -310,12 +310,14 @@ void FullCodeGenerator::EmitReturnSequence() {
     // patch with the code required by the debugger.
     __ mov(esp, ebp);
     __ pop(ebp);
-    __ ret((scope()->num_parameters() + 1) * kPointerSize);
+
+    int arguments_bytes = (scope()->num_parameters() + 1) * kPointerSize;
+    __ Ret(arguments_bytes, ecx);
 #ifdef ENABLE_DEBUGGER_SUPPORT
-    // Check that the size of the code used for returning matches what is
-    // expected by the debugger.
-    ASSERT_EQ(Assembler::kJSReturnSequenceLength,
-              masm_->SizeOfCodeGeneratedSince(&check_exit_codesize));
+    // Check that the size of the code used for returning is large enough
+    // for the debugger's requirements.
+    ASSERT(Assembler::kJSReturnSequenceLength <=
+           masm_->SizeOfCodeGeneratedSince(&check_exit_codesize));
 #endif
   }
 }

src/ia32/lithium-codegen-ia32.cc

@@ -1893,7 +1893,7 @@ void LCodeGen::DoReturn(LReturn* instr) {
   }
   __ mov(esp, ebp);
   __ pop(ebp);
-  __ ret((ParameterCount() + 1) * kPointerSize);
+  __ Ret((ParameterCount() + 1) * kPointerSize, ecx);
 }
 
 

src/ia32/macro-assembler-ia32.cc

@@ -1587,6 +1587,20 @@ void MacroAssembler::Ret() {
 }
 
 
+void MacroAssembler::Ret(int bytes_dropped, Register scratch) {
+  if (is_uint16(bytes_dropped)) {
+    ret(bytes_dropped);
+  } else {
+    pop(scratch);
+    add(Operand(esp), Immediate(bytes_dropped));
+    push(scratch);
+    ret(0);
+  }
+}
+
+
+
+
 void MacroAssembler::Drop(int stack_elements) {
   if (stack_elements > 0) {
     add(Operand(esp), Immediate(stack_elements * kPointerSize));

src/ia32/macro-assembler-ia32.h

@@ -550,6 +550,10 @@ class MacroAssembler: public Assembler {
 
   void Ret();
 
+  // Return and drop arguments from stack, where the number of arguments
+  // may be bigger than 2^16 - 1.  Requires a scratch register.
+  void Ret(int bytes_dropped, Register scratch);
+
   // Emit code to discard a non-negative number of pointer-sized elements
   // from the stack, clobbering only the esp register.
   void Drop(int element_count);

src/messages.js

@@ -211,6 +211,7 @@ function FormatMessage(message) {
       invalid_preparser_data:       ["Invalid preparser data for function ", "%0"],
       strict_mode_with:             ["Strict mode code may not include a with statement"],
       strict_catch_variable:        ["Catch variable may not be eval or arguments in strict mode"],
+      too_many_parameters:          ["Too many parameters in function definition"],
       strict_param_name:            ["Parameter name eval or arguments is not allowed in strict mode"],
       strict_param_dupe:            ["Strict mode function may not have duplicate parameter names"],
       strict_var_name:              ["Variable name may not be eval or arguments in strict mode"],

src/parser.cc

@@ -333,6 +333,9 @@ TemporaryScope::~TemporaryScope() {
 }
 
 
+const int Parser::kMaxNumFunctionParameters;
+
+
 Handle<String> Parser::LookupSymbol(int symbol_id) {
   // Length of symbol cache is the number of identified symbols.
   // If we are larger than that, or negative, it's not a cached symbol.
@@ -3499,6 +3502,12 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
       Variable* parameter = top_scope_->DeclareLocal(param_name, Variable::VAR);
       top_scope_->AddParameter(parameter);
       num_parameters++;
+      if (num_parameters > kMaxNumFunctionParameters) {
+        ReportMessageAt(scanner().location(), "too_many_parameters",
+                        Vector<const char*>::empty());
+        *ok = false;
+        return NULL;
+      }
       done = (peek() == Token::RPAREN);
       if (!done) Expect(Token::COMMA, CHECK_OK);
     }

src/parser.h

@@ -436,6 +436,11 @@ class Parser {
                        Vector<Handle<String> > args);
 
  protected:
+  // Limit on number of function parameters is chosen arbitrarily.
+  // Code::Flags uses only the low 17 bits of num-parameters to
+  // construct a hashable id, so if more than 2^17 are allowed, this
+  // should be checked.
+  static const int kMaxNumFunctionParameters = 32766;
   FunctionLiteral* ParseLazy(Handle<SharedFunctionInfo> info,
                              UC16CharacterStream* source,
                              ZoneScope* zone_scope);

src/x64/codegen-x64.cc

@@ -2993,21 +2993,22 @@ void CodeGenerator::GenerateReturnSequence(Result* return_value) {
   // Leave the frame and return popping the arguments and the
   // receiver.
   frame_->Exit();
-  masm_->ret((scope()->num_parameters() + 1) * kPointerSize);
+  int arguments_bytes = (scope()->num_parameters() + 1) * kPointerSize;
+  __ Ret(arguments_bytes, rcx);
   DeleteFrame();
 
 #ifdef ENABLE_DEBUGGER_SUPPORT
   // Add padding that will be overwritten by a debugger breakpoint.
-  // frame_->Exit() generates "movq rsp, rbp; pop rbp; ret k"
+  // The shortest return sequence generated is "movq rsp, rbp; pop rbp; ret k"
   // with length 7 (3 + 1 + 3).
   const int kPadding = Assembler::kJSReturnSequenceLength - 7;
   for (int i = 0; i < kPadding; ++i) {
     masm_->int3();
   }
-  // Check that the size of the code used for returning matches what is
-  // expected by the debugger.
-  ASSERT_EQ(Assembler::kJSReturnSequenceLength,
-            masm_->SizeOfCodeGeneratedSince(&check_exit_codesize));
+  // Check that the size of the code used for returning is large enough
+  // for the debugger's requirements.
+  ASSERT(Assembler::kJSReturnSequenceLength <=
+         masm_->SizeOfCodeGeneratedSince(&check_exit_codesize));
 #endif
 }
 

src/x64/full-codegen-x64.cc

@@ -297,19 +297,22 @@ void FullCodeGenerator::EmitReturnSequence() {
     // patch with the code required by the debugger.
     __ movq(rsp, rbp);
     __ pop(rbp);
-    __ ret((scope()->num_parameters() + 1) * kPointerSize);
+
+    int arguments_bytes = (scope()->num_parameters() + 1) * kPointerSize;
+    __ Ret(arguments_bytes, rcx);
+
 #ifdef ENABLE_DEBUGGER_SUPPORT
     // Add padding that will be overwritten by a debugger breakpoint.  We
-    // have just generated "movq rsp, rbp; pop rbp; ret k" with length 7
+    // have just generated at least 7 bytes: "movq rsp, rbp; pop rbp; ret k"
     // (3 + 1 + 3).
     const int kPadding = Assembler::kJSReturnSequenceLength - 7;
     for (int i = 0; i < kPadding; ++i) {
       masm_->int3();
     }
-    // Check that the size of the code used for returning matches what is
-    // expected by the debugger.
-    ASSERT_EQ(Assembler::kJSReturnSequenceLength,
-            masm_->SizeOfCodeGeneratedSince(&check_exit_codesize));
+    // Check that the size of the code used for returning is large enough
+    // for the debugger's requirements.
+    ASSERT(Assembler::kJSReturnSequenceLength <=
+           masm_->SizeOfCodeGeneratedSince(&check_exit_codesize));
 #endif
   }
 }

src/x64/lithium-codegen-x64.cc

@@ -1650,7 +1650,7 @@ void LCodeGen::DoReturn(LReturn* instr) {
   }
   __ movq(rsp, rbp);
   __ pop(rbp);
-  __ ret((ParameterCount() + 1) * kPointerSize);
+  __ Ret((ParameterCount() + 1) * kPointerSize, rcx);
 }
 
 

src/x64/macro-assembler-x64.cc

@@ -1541,6 +1541,18 @@ void MacroAssembler::Ret() {
 }
 
 
+void MacroAssembler::Ret(int bytes_dropped, Register scratch) {
+  if (is_uint16(bytes_dropped)) {
+    ret(bytes_dropped);
+  } else {
+    pop(scratch);
+    addq(rsp, Immediate(bytes_dropped));
+    push(scratch);
+    ret(0);
+  }
+}
+
+
 void MacroAssembler::FCmp() {
   fucomip();
   fstp(0);

src/x64/macro-assembler-x64.h

@@ -920,6 +920,10 @@ class MacroAssembler: public Assembler {
 
   void Ret();
 
+  // Return and drop arguments from stack, where the number of arguments
+  // may be bigger than 2^16 - 1.  Requires a scratch register.
+  void Ret(int bytes_dropped, Register scratch);
+
   Handle<Object> CodeObject() { return code_object_; }
 
 

test/mjsunit/regress/regress-1122.js

@@ -0,0 +1,55 @@
+// Copyright 2011 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Test that we can handle functions with up to 32766 arguments, and that
+// functions with more arguments throw an exception.
+
+// See http://code.google.com/p/v8/issues/detail?id=1122.
+
+function function_with_n_args(n) {
+  test_prefix = 'prefix ';
+  test_suffix = ' suffix';
+  var source = 'test_prefix + (function f(';
+  for (var arg = 0; arg < n ; arg++) {
+    if (arg != 0) source += ',';
+    source += 'arg' + arg;
+  }
+  source += ') { return arg' + (n - n % 2) / 2 + '; })(';
+  for (var arg = 0; arg < n ; arg++) {
+    if (arg != 0) source += ',';
+    source += arg;
+  }
+  source += ') + test_suffix';
+  return eval(source);
+}
+
+assertEquals('prefix 4000 suffix', function_with_n_args(8000));
+assertEquals('prefix 9000 suffix', function_with_n_args(18000));
+assertEquals('prefix 16000 suffix', function_with_n_args(32000));
+
+assertThrows(function_with_n_args(35000));
+assertThrows(function_with_n_args(100000));