From ca936dae9e0f0fc6a36bf9c79effef2313b32bae Mon Sep 17 00:00:00 2001 From: "kmillikin@chromium.org" Date: Wed, 2 Feb 2011 15:08:29 +0000 Subject: [PATCH] More of the fix for V8 issue 1079. The arguments property of functions, if we find an optimized frame for the function, is always a freshly allocated object. We never try to find an existing arguments object. Review URL: http://codereview.chromium.org/6349050 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6581 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/accessors.cc | 83 ++++++++++++---------------- test/mjsunit/regress/regress-1079.js | 2 +- 2 files changed, 37 insertions(+), 48 deletions(-) diff --git a/src/accessors.cc b/src/accessors.cc index c7d9cfe94c..2b205d5d74 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -675,46 +675,36 @@ static void ComputeSlotMappingForArguments(JavaScriptFrame* frame, int inlined_frame_index, Vector* args_slots) { AssertNoAllocation no_gc; - int deopt_index = AstNode::kNoNumber; - DeoptimizationInputData* data = static_cast(frame)->GetDeoptimizationData(&deopt_index); - TranslationIterator it(data->TranslationByteArray(), data->TranslationIndex(deopt_index)->value()); - Translation::Opcode opcode = static_cast(it.Next()); ASSERT(opcode == Translation::BEGIN); int frame_count = it.Next(); - USE(frame_count); ASSERT(frame_count > inlined_frame_index); - int frames_to_skip = inlined_frame_index; while (true) { opcode = static_cast(it.Next()); - // Skip over operands to advance to the next opcode. it.Skip(Translation::NumberOfOperandsFor(opcode)); - if (opcode == Translation::FRAME) { if (frames_to_skip == 0) { - // We reached frame corresponding to inlined function in question. - // Process translation commands for arguments. - - // Skip translation command for receiver. + // We reached the frame corresponding to the inlined function + // in question. Process the translation commands for the + // arguments. + // + // Skip the translation command for the receiver. it.Skip(Translation::NumberOfOperandsFor( static_cast(it.Next()))); - // Compute slots for arguments. for (int i = 0; i < args_slots->length(); ++i) { (*args_slots)[i] = ComputeSlotForNextArgument(&it, data, frame); } - return; } - frames_to_skip--; } } @@ -727,16 +717,11 @@ static MaybeObject* ConstructArgumentsObjectForInlinedFunction( JavaScriptFrame* frame, Handle inlined_function, int inlined_frame_index) { - int args_count = inlined_function->shared()->formal_parameter_count(); - ScopedVector args_slots(args_count); - ComputeSlotMappingForArguments(frame, inlined_frame_index, &args_slots); - Handle arguments = Factory::NewArgumentsObject(inlined_function, args_count); - Handle array = Factory::NewFixedArray(args_count); for (int i = 0; i < args_count; ++i) { Handle value = args_slots[i].GetValue(); @@ -766,39 +751,43 @@ MaybeObject* Accessors::FunctionGetArguments(Object* object, void*) { if (functions[i] != *function) continue; if (i > 0) { - // Function in question was inlined. + // The function in question was inlined. Inlined functions have the + // correct number of arguments and no allocated arguments object, so + // we can construct a fresh one by interpreting the function's + // deoptimization input data. return ConstructArgumentsObjectForInlinedFunction(frame, function, i); - } else { + } + + if (!frame->is_optimized()) { // If there is an arguments variable in the stack, we return that. - int index = function->shared()->scope_info()-> - StackSlotIndex(Heap::arguments_symbol()); + Handle info(function->shared()->scope_info()); + int index = info->StackSlotIndex(Heap::arguments_symbol()); if (index >= 0) { - Handle arguments = - Handle(frame->GetExpression(index)); + Handle arguments(frame->GetExpression(index)); if (!arguments->IsArgumentsMarker()) return *arguments; } - - // If there isn't an arguments variable in the stack, we need to - // find the frame that holds the actual arguments passed to the - // function on the stack. - it.AdvanceToArgumentsFrame(); - frame = it.frame(); - - // Get the number of arguments and construct an arguments object - // mirror for the right frame. - const int length = frame->GetProvidedParametersCount(); - Handle arguments = Factory::NewArgumentsObject(function, - length); - Handle array = Factory::NewFixedArray(length); - - // Copy the parameters to the arguments object. - ASSERT(array->length() == length); - for (int i = 0; i < length; i++) array->set(i, frame->GetParameter(i)); - arguments->set_elements(*array); - - // Return the freshly allocated arguments object. - return *arguments; } + + // If there is no arguments variable in the stack or we have an + // optimized frame, we find the frame that holds the actual arguments + // passed to the function. + it.AdvanceToArgumentsFrame(); + frame = it.frame(); + + // Get the number of arguments and construct an arguments object + // mirror for the right frame. + const int length = frame->GetProvidedParametersCount(); + Handle arguments = Factory::NewArgumentsObject(function, + length); + Handle array = Factory::NewFixedArray(length); + + // Copy the parameters to the arguments object. + ASSERT(array->length() == length); + for (int i = 0; i < length; i++) array->set(i, frame->GetParameter(i)); + arguments->set_elements(*array); + + // Return the freshly allocated arguments object. + return *arguments; } functions.Rewind(0); } diff --git a/test/mjsunit/regress/regress-1079.js b/test/mjsunit/regress/regress-1079.js index 6863eb12d4..f3f3ce5bab 100644 --- a/test/mjsunit/regress/regress-1079.js +++ b/test/mjsunit/regress/regress-1079.js @@ -40,6 +40,6 @@ function unoptimized() { } for (var i = 0; i < 100000; ++i) { - optimized(1, 2, 3) + assertEquals(3, optimized(1, 2, 3).length); }