Author: [email protected]
Date: Tue Apr 21 07:48:54 2009
New Revision: 1756
Modified:
branches/bleeding_edge/src/debug.cc
branches/bleeding_edge/src/debug.h
branches/bleeding_edge/src/ic-arm.cc
branches/bleeding_edge/src/ic-ia32.cc
branches/bleeding_edge/src/ic.cc
branches/bleeding_edge/src/ic.h
branches/bleeding_edge/test/cctest/test-debug.cc
Log:
Handle breaks on keyed IC loads which can have an inlined version.
For keyed IC loads setting a break point now ensures that the inlined code
is not used. When the break point is set the inlined map check is changed
to fail causing the inlined code not to be used but the IC to be called. As
long at the break point is set the map check will stay invalid.
Review URL: http://codereview.chromium.org/87025
Modified: branches/bleeding_edge/src/debug.cc
==============================================================================
--- branches/bleeding_edge/src/debug.cc (original)
+++ branches/bleeding_edge/src/debug.cc Tue Apr 21 07:48:54 2009
@@ -35,6 +35,8 @@
#include "debug.h"
#include "execution.h"
#include "global-handles.h"
+#include "ic.h"
+#include "ic-inl.h"
#include "natives.h"
#include "stub-cache.h"
#include "log.h"
@@ -289,14 +291,8 @@
// Patch the frame exit code with a break point.
SetDebugBreakAtReturn();
} else {
- // Patch the original code with the current address as the current
address
- // might have changed by the inline caching since the code was copied.
- original_rinfo()->set_target_address(rinfo()->target_address());
-
- // Patch the code to invoke the builtin debug break function matching
the
- // calling convention used by the call site.
- Handle<Code> dbgbrk_code(Debug::FindDebugBreak(rinfo()));
- rinfo()->set_target_address(dbgbrk_code->entry());
+ // Patch the IC call.
+ SetDebugBreakAtIC();
}
ASSERT(IsDebugBreak());
}
@@ -307,8 +303,8 @@
// Restore the frame exit code.
ClearDebugBreakAtReturn();
} else {
- // Patch the code to the original invoke.
- rinfo()->set_target_address(original_rinfo()->target_address());
+ // Patch the IC call.
+ ClearDebugBreakAtIC();
}
ASSERT(!IsDebugBreak());
}
@@ -361,6 +357,39 @@
}
+void BreakLocationIterator::SetDebugBreakAtIC() {
+ // Patch the original code with the current address as the current
address
+ // might have changed by the inline caching since the code was copied.
+ original_rinfo()->set_target_address(rinfo()->target_address());
+
+ RelocInfo::Mode mode = rmode();
+ if (RelocInfo::IsCodeTarget(mode)) {
+ Address target = rinfo()->target_address();
+ Handle<Code> code(Code::GetCodeFromTargetAddress(target));
+
+ // Patch the code to invoke the builtin debug break function matching
the
+ // calling convention used by the call site.
+ Handle<Code> dbgbrk_code(Debug::FindDebugBreak(code, mode));
+ rinfo()->set_target_address(dbgbrk_code->entry());
+
+ // For stubs that refer back to an inlined version clear the cached
map for
+ // the inlined case to always go through the IC. As long as the break
point
+ // is set the patching performed by the runtime system will take place
in
+ // the code copy and will therefore have no effect on the running code
+ // keeping it from using the inlined code.
+ if (code->is_keyed_load_stub() &&
KeyedLoadIC::HasInlinedVersion(pc())) {
+ KeyedLoadIC::ClearInlinedVersion(pc());
+ }
+ }
+}
+
+
+void BreakLocationIterator::ClearDebugBreakAtIC() {
+ // Patch the code to the original invoke.
+ rinfo()->set_target_address(original_rinfo()->target_address());
+}
+
+
Object* BreakLocationIterator::BreakPointObjects() {
return debug_info_->GetBreakPointObjects(code_position());
}
@@ -1056,48 +1085,42 @@
// Find the builtin to use for invoking the debug break
-Handle<Code> Debug::FindDebugBreak(RelocInfo* rinfo) {
+Handle<Code> Debug::FindDebugBreak(Handle<Code> code, RelocInfo::Mode
mode) {
// Find the builtin debug break function matching the calling convention
// used by the call site.
- RelocInfo::Mode mode = rinfo->rmode();
-
- if (RelocInfo::IsCodeTarget(mode)) {
- Address target = rinfo->target_address();
- Code* code = Code::GetCodeFromTargetAddress(target);
- if (code->is_inline_cache_stub()) {
- if (code->is_call_stub()) {
- return ComputeCallDebugBreak(code->arguments_count());
- }
- if (code->is_load_stub()) {
- return
Handle<Code>(Builtins::builtin(Builtins::LoadIC_DebugBreak));
- }
- if (code->is_store_stub()) {
- return
Handle<Code>(Builtins::builtin(Builtins::StoreIC_DebugBreak));
- }
- if (code->is_keyed_load_stub()) {
- Handle<Code> result =
-
Handle<Code>(Builtins::builtin(Builtins::KeyedLoadIC_DebugBreak));
- return result;
- }
- if (code->is_keyed_store_stub()) {
- Handle<Code> result =
-
Handle<Code>(Builtins::builtin(Builtins::KeyedStoreIC_DebugBreak));
- return result;
- }
+ if (code->is_inline_cache_stub()) {
+ if (code->is_call_stub()) {
+ return ComputeCallDebugBreak(code->arguments_count());
+ }
+ if (code->is_load_stub()) {
+ return Handle<Code>(Builtins::builtin(Builtins::LoadIC_DebugBreak));
}
- if (RelocInfo::IsConstructCall(mode)) {
+ if (code->is_store_stub()) {
+ return Handle<Code>(Builtins::builtin(Builtins::StoreIC_DebugBreak));
+ }
+ if (code->is_keyed_load_stub()) {
Handle<Code> result =
-
Handle<Code>(Builtins::builtin(Builtins::ConstructCall_DebugBreak));
+
Handle<Code>(Builtins::builtin(Builtins::KeyedLoadIC_DebugBreak));
return result;
}
- if (code->kind() == Code::STUB) {
- ASSERT(code->major_key() == CodeStub::CallFunction ||
- code->major_key() == CodeStub::StackCheck);
+ if (code->is_keyed_store_stub()) {
Handle<Code> result =
-
Handle<Code>(Builtins::builtin(Builtins::StubNoRegisters_DebugBreak));
+
Handle<Code>(Builtins::builtin(Builtins::KeyedStoreIC_DebugBreak));
return result;
}
}
+ if (RelocInfo::IsConstructCall(mode)) {
+ Handle<Code> result =
+
Handle<Code>(Builtins::builtin(Builtins::ConstructCall_DebugBreak));
+ return result;
+ }
+ if (code->kind() == Code::STUB) {
+ ASSERT(code->major_key() == CodeStub::CallFunction ||
+ code->major_key() == CodeStub::StackCheck);
+ Handle<Code> result =
+
Handle<Code>(Builtins::builtin(Builtins::StubNoRegisters_DebugBreak));
+ return result;
+ }
UNREACHABLE();
return Handle<Code>::null();
@@ -1839,7 +1862,7 @@
v8::TryCatch try_catch;
fun_name = v8::String::New("processDebugRequest");
fun = v8::Function::Cast(*cmd_processor->Get(fun_name));
-
+
request = v8::String::New(command.text().start(),
command.text().length());
command.text().Dispose();
@@ -2208,7 +2231,7 @@
MessageQueue::~MessageQueue() {
- while(!IsEmpty()) {
+ while (!IsEmpty()) {
Message m = Get();
m.Dispose();
}
Modified: branches/bleeding_edge/src/debug.h
==============================================================================
--- branches/bleeding_edge/src/debug.h (original)
+++ branches/bleeding_edge/src/debug.h Tue Apr 21 07:48:54 2009
@@ -132,6 +132,10 @@
private:
void SetDebugBreak();
void ClearDebugBreak();
+
+ void SetDebugBreakAtIC();
+ void ClearDebugBreakAtIC();
+
bool IsDebugBreakAtReturn();
void SetDebugBreakAtReturn();
void ClearDebugBreakAtReturn();
@@ -205,7 +209,7 @@
static bool IsBreakStub(Code* code);
// Find the builtin to use for invoking the debug break
- static Handle<Code> FindDebugBreak(RelocInfo* rinfo);
+ static Handle<Code> FindDebugBreak(Handle<Code> code, RelocInfo::Mode
mode);
static Handle<Object> GetSourceBreakLocations(
Handle<SharedFunctionInfo> shared);
Modified: branches/bleeding_edge/src/ic-arm.cc
==============================================================================
--- branches/bleeding_edge/src/ic-arm.cc (original)
+++ branches/bleeding_edge/src/ic-arm.cc Tue Apr 21 07:48:54 2009
@@ -507,6 +507,8 @@
// TODO(181): Implement map patching once loop nesting is tracked on
// the ARM platform so we can generate inlined fast-case code for
// array indexing in loops.
+bool KeyedLoadIC::HasInlinedVersion(Address address) { return false; }
+void KeyedLoadIC::ClearInlinedVersion(Address address) { }
void KeyedLoadIC::PatchInlinedMapCheck(Address address, Object* value) { }
Modified: branches/bleeding_edge/src/ic-ia32.cc
==============================================================================
--- branches/bleeding_edge/src/ic-ia32.cc (original)
+++ branches/bleeding_edge/src/ic-ia32.cc Tue Apr 21 07:48:54 2009
@@ -729,8 +729,24 @@
}
+// One byte opcode for test eax,0xXXXXXXXX.
+static const byte kTestEaxByte = 0xA9;
+
+
+bool KeyedLoadIC::HasInlinedVersion(Address address) {
+ Address test_instruction_address = address + 4; // 4 = stub address
+ return *test_instruction_address == kTestEaxByte;
+}
+
+
+void KeyedLoadIC::ClearInlinedVersion(Address address) {
+ // Insert null as the map to check for to make sure the map check fails
+ // sending control flow to the IC instead of the inlined version.
+ PatchInlinedMapCheck(address, Heap::null_value());
+}
+
+
void KeyedLoadIC::PatchInlinedMapCheck(Address address, Object* value) {
- static const byte kTestEaxByte = 0xA9;
Address test_instruction_address = address + 4; // 4 = stub address
// The keyed load has a fast inlined case if the IC call instruction
// is immediately followed by a test instruction.
@@ -744,7 +760,7 @@
// bytes of the 7-byte operand-immediate compare instruction, so
// we add 3 to the offset to get the map address.
Address map_address = test_instruction_address + offset_value + 3;
- // patch the map check.
+ // Patch the map check.
(*(reinterpret_cast<Object**>(map_address))) = value;
}
}
Modified: branches/bleeding_edge/src/ic.cc
==============================================================================
--- branches/bleeding_edge/src/ic.cc (original)
+++ branches/bleeding_edge/src/ic.cc Tue Apr 21 07:48:54 2009
@@ -237,7 +237,7 @@
// Make sure to also clear the map used in inline fast cases. If we
// do not clear these maps, cached code can keep objects alive
// through the embedded maps.
- PatchInlinedMapCheck(address, Heap::null_value());
+ ClearInlinedVersion(address);
SetTargetAtAddress(address, initialize_stub());
}
Modified: branches/bleeding_edge/src/ic.h
==============================================================================
--- branches/bleeding_edge/src/ic.h (original)
+++ branches/bleeding_edge/src/ic.h Tue Apr 21 07:48:54 2009
@@ -254,6 +254,12 @@
static void GeneratePreMonomorphic(MacroAssembler* masm);
static void GenerateGeneric(MacroAssembler* masm);
+ // Check if this IC corresponds to an inlined version.
+ static bool HasInlinedVersion(Address address);
+
+ // Clear the use of the inlined version.
+ static void ClearInlinedVersion(Address address);
+
private:
static void Generate(MacroAssembler* masm, const ExternalReference& f);
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 21 07:48:54
2009
@@ -2177,6 +2177,53 @@
}
+// Test of the stepping mechanism for keyed load in a loop.
+TEST(DebugStepKeyedLoadLoop) {
+ v8::HandleScope scope;
+ DebugLocalContext env;
+
+ // Create a function for testing stepping of keyed load. The
statement 'y=1'
+ // is there to have more than one breakable statement in the loop,
TODO(315).
+ v8::Local<v8::Function> foo = CompileFunction(
+ &env,
+ "function foo(a) {\n"
+ " var x;\n"
+ " var len = a.length;\n"
+ " for (var i = 0; i < len; i++) {\n"
+ " y = 1;\n"
+ " x = a[i];\n"
+ " }\n"
+ "}\n",
+ "foo");
+
+ // Create array [0,1,2,3,4,5,6,7,8,9]
+ v8::Local<v8::Array> a = v8::Array::New(10);
+ for (int i = 0; i < 10; i++) {
+ a->Set(v8::Number::New(i), v8::Number::New(i));
+ }
+
+ // Call function without any break points to ensure inlining is in place.
+ const int kArgc = 1;
+ v8::Handle<v8::Value> args[kArgc] = { a };
+ foo->Call(env->Global(), kArgc, args);
+
+ // Register a debug event listener which steps and counts.
+ v8::Debug::SetDebugEventListener(DebugEventStep);
+
+ // Setup break point and step through the function.
+ SetBreakPoint(foo, 3);
+ step_action = StepNext;
+ break_point_hit_count = 0;
+ foo->Call(env->Global(), kArgc, args);
+
+ // With stepping all break locations are hit.
+ CHECK_EQ(22, break_point_hit_count);
+
+ v8::Debug::SetDebugEventListener(NULL);
+ CheckDebuggerUnloaded();
+}
+
+
// Test the stepping mechanism with different ICs.
TEST(DebugStepLinearMixedICs) {
v8::HandleScope scope;
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
