Author: [email protected]
Date: Tue Apr 7 02:54:53 2009
New Revision: 1683
Added:
branches/bleeding_edge/test/mjsunit/regress/regress-269.js (props
changed)
- copied unchanged from r1682,
/branches/bleeding_edge/test/mjsunit/bugs/bug-269.js
Removed:
branches/bleeding_edge/test/mjsunit/bugs/bug-269.js
Modified:
branches/bleeding_edge/src/debug.cc
branches/bleeding_edge/test/cctest/test-debug.cc
branches/bleeding_edge/test/mjsunit/mjsunit.status
Log:
Fixed the step in handling for function.apply.
The generic step-in mechanism floods the function called with break points
to ensure a break is hit when entering the function. This generic mechanism
was also used for function.apply. The code for function.apply contains a
keyed load IC which was patched when stepping into function.apply. However
function.apply enteres an internal frame not a JavaScript frame. This
caused the logic for returning from the break in function.apply to fail as
it forced a jump to the IC on the top JavaScript frame. The top JavaScript
frame was the frame for the function calling function.apply not the frame
for the apply function. Now returning from the break point in the keyed
load IC in the apply code caused a jump to the code for the call IC for the
function calling function.apply in the first place. Not a pretty sight.
Step-in now handles function.apply as a separate case where the actual
JavaScript function called through apply is flodded with breakpoints
instead of the function.apply function.
BUG=269
[email protected]
Review URL: http://codereview.chromium.org/63055
Modified: branches/bleeding_edge/src/debug.cc
==============================================================================
--- branches/bleeding_edge/src/debug.cc (original)
+++ branches/bleeding_edge/src/debug.cc Tue Apr 7 02:54:53 2009
@@ -1160,7 +1160,28 @@
if (fp == Debug::step_in_fp()) {
// Don't allow step into functions in the native context.
if (function->context()->global() != Top::context()->builtins()) {
-
Debug::FloodWithOneShot(Handle<SharedFunctionInfo>(function->shared()));
+ if (function->shared()->code() ==
+ Builtins::builtin(Builtins::FunctionApply)) {
+ // Handle function.apply separately to flood the function to be
called
+ // and not the code for Builtins::FunctionApply. At the point of
the
+ // call IC to call Builtins::FunctionApply the expression stack
has the
+ // following content:
+ // symbol "apply"
+ // function apply was called on
+ // receiver for apply (first parameter to apply)
+ // arguments array for apply (second parameter to apply)
+ JavaScriptFrameIterator it;
+ ASSERT(it.frame()->fp() == fp);
+ ASSERT(it.frame()->GetExpression(1)->IsJSFunction());
+ if (it.frame()->GetExpression(1)->IsJSFunction()) {
+ Handle<JSFunction>
+
actual_function(JSFunction::cast(it.frame()->GetExpression(1)));
+ Handle<SharedFunctionInfo>
actual_shared(actual_function->shared());
+ Debug::FloodWithOneShot(actual_shared);
+ }
+ } else {
+
Debug::FloodWithOneShot(Handle<SharedFunctionInfo>(function->shared()));
+ }
}
}
}
Modified: branches/bleeding_edge/test/cctest/test-debug.cc
==============================================================================
--- branches/bleeding_edge/test/cctest/test-debug.cc (original)
+++ branches/bleeding_edge/test/cctest/test-debug.cc Tue Apr 7 02:54:53
2009
@@ -2406,6 +2406,45 @@
}
+// Test that step in works with function.apply.
+TEST(DebugStepFunctionApply) {
+ v8::HandleScope scope;
+ DebugLocalContext env;
+
+ // Create a function for testing stepping.
+ v8::Local<v8::Function> foo = CompileFunction(
+ &env,
+ "function bar(x, y, z) { if (x == 1) { a = y; b = z; } }"
+ "function foo(){ debugger; bar.apply(this, [1,2,3]); }",
+ "foo");
+
+ // Register a debug event listener which steps and counts.
+ v8::Debug::SetDebugEventListener(DebugEventStep);
+
+ step_action = StepIn;
+ break_point_hit_count = 0;
+ foo->Call(env->Global(), 0, NULL);
+
+ // With stepping all break locations are hit.
+ CHECK_EQ(6, break_point_hit_count);
+
+ v8::Debug::SetDebugEventListener(NULL);
+ CheckDebuggerUnloaded();
+
+ // Register a debug event listener which just counts.
+ v8::Debug::SetDebugEventListener(DebugEventBreakPointHitCount);
+
+ break_point_hit_count = 0;
+ foo->Call(env->Global(), 0, NULL);
+
+ // Without stepping only the debugger statement is hit.
+ CHECK_EQ(1, break_point_hit_count);
+
+ v8::Debug::SetDebugEventListener(NULL);
+ CheckDebuggerUnloaded();
+}
+
+
// Test break on exceptions. For each exception break combination the
number
// of debug event exception callbacks and message callbacks are collected.
The
// number of debug event exception callbacks are used to check that the
Modified: branches/bleeding_edge/test/mjsunit/mjsunit.status
==============================================================================
--- branches/bleeding_edge/test/mjsunit/mjsunit.status (original)
+++ branches/bleeding_edge/test/mjsunit/mjsunit.status Tue Apr 7 02:54:53
2009
@@ -36,9 +36,6 @@
big-object-literal: PASS, SKIP if ($arch == arm)
-# Bug realiably triggers a debug assertion and crashed in release mode.
-bugs/bug-269: CRASH, FAIL if $mode == debug
-
[ $arch == arm ]
# Slow tests which times out in debug mode.
@@ -64,7 +61,7 @@
debug-step: SKIP
debug-breakpoints: PASS || FAIL
debug-handle: CRASH, FAIL if $mode == debug
-bugs/bug-269: SKIP
+regress/regress-269: SKIP
# Bug number 130 http://code.google.com/p/v8/issues/detail?id=130
# Fails on real ARM hardware but not on the simulator.
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
