Revision: 6697
Author: [email protected]
Date: Wed Feb 9 04:46:22 2011
Log: Fix a bug that occurs when functions are defined with more than 16,382
parameters.
Review URL: http://codereview.chromium.org/6447007
http://code.google.com/p/v8/source/detail?r=6697
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-1122.js
Modified:
/branches/bleeding_edge/src/ia32/codegen-ia32.cc
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
/branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc
/branches/bleeding_edge/src/ia32/macro-assembler-ia32.h
/branches/bleeding_edge/src/messages.js
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/parser.h
/branches/bleeding_edge/src/x64/codegen-x64.cc
/branches/bleeding_edge/src/x64/full-codegen-x64.cc
/branches/bleeding_edge/src/x64/lithium-codegen-x64.cc
/branches/bleeding_edge/src/x64/macro-assembler-x64.cc
/branches/bleeding_edge/src/x64/macro-assembler-x64.h
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-1122.js Wed Feb 9
04:46:22 2011
@@ -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));
=======================================
--- /branches/bleeding_edge/src/ia32/codegen-ia32.cc Fri Feb 4 10:15:49
2011
+++ /branches/bleeding_edge/src/ia32/codegen-ia32.cc Wed Feb 9 04:46:22
2011
@@ -3771,14 +3771,15 @@
// 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
}
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Tue Feb 8
06:00:22 2011
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Wed Feb 9
04:46:22 2011
@@ -310,12 +310,14 @@
// 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
}
}
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Wed Feb 9
04:39:15 2011
+++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Wed Feb 9
04:46:22 2011
@@ -1893,7 +1893,7 @@
}
__ mov(esp, ebp);
__ pop(ebp);
- __ ret((ParameterCount() + 1) * kPointerSize);
+ __ Ret((ParameterCount() + 1) * kPointerSize, ecx);
}
=======================================
--- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Tue Feb 8
09:25:40 2011
+++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Wed Feb 9
04:46:22 2011
@@ -1587,6 +1587,20 @@
}
+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));
=======================================
--- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.h Thu Feb 3
04:50:50 2011
+++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.h Wed Feb 9
04:46:22 2011
@@ -550,6 +550,10 @@
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);
=======================================
--- /branches/bleeding_edge/src/messages.js Mon Feb 7 23:49:59 2011
+++ /branches/bleeding_edge/src/messages.js Wed Feb 9 04:46:22 2011
@@ -211,6 +211,7 @@
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"],
=======================================
--- /branches/bleeding_edge/src/parser.cc Tue Feb 8 10:46:13 2011
+++ /branches/bleeding_edge/src/parser.cc Wed Feb 9 04:46:22 2011
@@ -331,6 +331,9 @@
TemporaryScope::~TemporaryScope() {
*variable_ = parent_;
}
+
+
+const int Parser::kMaxNumFunctionParameters;
Handle<String> Parser::LookupSymbol(int symbol_id) {
@@ -3499,6 +3502,12 @@
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);
}
=======================================
--- /branches/bleeding_edge/src/parser.h Fri Feb 4 10:36:37 2011
+++ /branches/bleeding_edge/src/parser.h Wed Feb 9 04:46:22 2011
@@ -436,6 +436,11 @@
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);
=======================================
--- /branches/bleeding_edge/src/x64/codegen-x64.cc Fri Feb 4 10:15:49 2011
+++ /branches/bleeding_edge/src/x64/codegen-x64.cc Wed Feb 9 04:46:22 2011
@@ -2993,21 +2993,22 @@
// 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
}
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Tue Feb 8 11:42:24
2011
+++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Wed Feb 9 04:46:22
2011
@@ -297,19 +297,22 @@
// 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
}
}
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Tue Feb 8
06:37:50 2011
+++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Wed Feb 9
04:46:22 2011
@@ -1650,7 +1650,7 @@
}
__ movq(rsp, rbp);
__ pop(rbp);
- __ ret((ParameterCount() + 1) * kPointerSize);
+ __ Ret((ParameterCount() + 1) * kPointerSize, rcx);
}
=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Tue Feb 8
09:25:40 2011
+++ /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Wed Feb 9
04:46:22 2011
@@ -1539,6 +1539,18 @@
void MacroAssembler::Ret() {
ret(0);
}
+
+
+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() {
=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.h Fri Feb 4
06:09:03 2011
+++ /branches/bleeding_edge/src/x64/macro-assembler-x64.h Wed Feb 9
04:46:22 2011
@@ -920,6 +920,10 @@
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_; }
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
