Revision: 11502
Author: [email protected]
Date: Thu May 3 10:31:34 2012
Log: Fix issue 825 (LiveEdit vs. function with no locals) in core and
for ia32.
Review URL: https://chromiumcodereview.appspot.com/10263002
http://code.google.com/p/v8/source/detail?r=11502
Added:
/branches/bleeding_edge/test/mjsunit/debug-liveedit-stack-padding.js
Modified:
/branches/bleeding_edge/src/arm/debug-arm.cc
/branches/bleeding_edge/src/debug.cc
/branches/bleeding_edge/src/debug.h
/branches/bleeding_edge/src/frames.h
/branches/bleeding_edge/src/ia32/debug-ia32.cc
/branches/bleeding_edge/src/liveedit.cc
/branches/bleeding_edge/src/mips/debug-mips.cc
/branches/bleeding_edge/src/x64/debug-x64.cc
/branches/bleeding_edge/test/mjsunit/mjsunit.status
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/debug-liveedit-stack-padding.js
Thu May 3 10:31:34 2012
@@ -0,0 +1,88 @@
+// Copyright 2012 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.
+
+// Flags: --expose-debug-as debug
+// Get the Debug object exposed from the debug context global object.
+
+Debug = debug.Debug;
+
+SlimFunction = eval(
+ "(function() {\n " +
+ " return 'Cat';\n" +
+ "})\n"
+);
+
+var script = Debug.findScript(SlimFunction);
+
+Debug.setScriptBreakPointById(script.id, 1, 0);
+
+var orig_animal = "'Cat'";
+var patch_pos = script.source.indexOf(orig_animal);
+var new_animal_patch = "'Capybara'";
+
+debugger_handler = (function() {
+ var already_called = false;
+ return function() {
+ if (already_called) {
+ return;
+ }
+ already_called = true;
+
+ var change_log = new Array();
+ try {
+ Debug.LiveEdit.TestApi.ApplySingleChunkPatch(script, patch_pos,
+ orig_animal.length, new_animal_patch, change_log);
+ } finally {
+ print("Change log: " + JSON.stringify(change_log) + "\n");
+ }
+ };
+})();
+
+var saved_exception = null;
+
+function listener(event, exec_state, event_data, data) {
+ if (event == Debug.DebugEvent.Break) {
+ try {
+ debugger_handler();
+ } catch (e) {
+ saved_exception = e;
+ }
+ } else {
+ print("Other: " + event);
+ }
+}
+
+Debug.setListener(listener);
+
+var animal = SlimFunction();
+
+if (saved_exception) {
+ print("Exception: " + saved_exception);
+ assertUnreachable();
+}
+
+assertEquals("Capybara", animal);
=======================================
--- /branches/bleeding_edge/src/arm/debug-arm.cc Fri Jan 27 08:09:20 2012
+++ /branches/bleeding_edge/src/arm/debug-arm.cc Thu May 3 10:31:34 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 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:
@@ -124,6 +124,8 @@
rinfo()->PatchCode(original_rinfo()->pc(),
Assembler::kDebugBreakSlotInstructions);
}
+
+const bool Debug::FramePaddingLayout::kIsSupported = false;
#define __ ACCESS_MASM(masm)
=======================================
--- /branches/bleeding_edge/src/debug.cc Tue Apr 3 08:54:07 2012
+++ /branches/bleeding_edge/src/debug.cc Thu May 3 10:31:34 2012
@@ -2225,6 +2225,13 @@
thread_local_.restarter_frame_function_pointer_ =
restarter_frame_function_pointer;
}
+
+
+const int Debug::FramePaddingLayout::kInitialSize = 1;
+
+
+// Any even value bigger than kInitialSize as needed for stack scanning.
+const int Debug::FramePaddingLayout::kPaddingValue = kInitialSize + 1;
bool Debug::IsDebugGlobal(GlobalObject* global) {
=======================================
--- /branches/bleeding_edge/src/debug.h Fri Apr 20 04:06:12 2012
+++ /branches/bleeding_edge/src/debug.h Thu May 3 10:31:34 2012
@@ -457,6 +457,50 @@
// Architecture-specific constant.
static const bool kFrameDropperSupported;
+ /**
+ * Defines layout of a stack frame that supports padding. This is a
regular
+ * internal frame that has a flexible stack structure. LiveEdit can shift
+ * its lower part up the stack, taking up the 'padding' space when
additional
+ * stack memory is required.
+ * Such frame is expected immediately above the topmost JavaScript frame.
+ *
+ * Stack Layout:
+ * --- Top
+ * LiveEdit routine frames
+ * ---
+ * C frames of debug handler
+ * ---
+ * ...
+ * ---
+ * An internal frame that has n padding words:
+ * - any number of words as needed by code -- upper part of frame
+ * - padding size: a Smi storing n -- current size of padding
+ * - padding: n words filled with kPaddingValue in form of Smi
+ * - 3 context/type words of a regular InternalFrame
+ * - fp
+ * ---
+ * Topmost JavaScript frame
+ * ---
+ * ...
+ * --- Bottom
+ */
+ class FramePaddingLayout : public AllStatic {
+ public:
+ // Architecture-specific constant.
+ static const bool kIsSupported;
+
+ // A size of frame base including fp. Padding words starts right above
+ // the base.
+ static const int kFrameBaseSize = 4;
+
+ // A number of words that should be reserved on stack for the LiveEdit
use.
+ // Normally equals 1. Stored on stack in form of Smi.
+ static const int kInitialSize;
+ // A value that padding words are filled with (in form of Smi). Going
+ // bottom-top, the first word not having this value is a counter word.
+ static const int kPaddingValue;
+ };
+
private:
explicit Debug(Isolate* isolate);
~Debug();
=======================================
--- /branches/bleeding_edge/src/frames.h Thu Apr 5 07:10:39 2012
+++ /branches/bleeding_edge/src/frames.h Thu May 3 10:31:34 2012
@@ -210,6 +210,9 @@
void set_pc(Address pc) { *pc_address() = pc; }
virtual void SetCallerFp(Address caller_fp) = 0;
+
+ // Manually changes value of fp in this object.
+ void UpdateFp(Address fp) { state_.fp = fp; }
Address* pc_address() const { return state_.pc_address; }
=======================================
--- /branches/bleeding_edge/src/ia32/debug-ia32.cc Fri Apr 27 06:05:45 2012
+++ /branches/bleeding_edge/src/ia32/debug-ia32.cc Thu May 3 10:31:34 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 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:
@@ -91,10 +91,12 @@
rinfo()->PatchCode(original_rinfo()->pc(),
Assembler::kDebugBreakSlotLength);
}
+// All debug break stubs support padding for LiveEdit.
+const bool Debug::FramePaddingLayout::kIsSupported = true;
+
#define __ ACCESS_MASM(masm)
-
static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
RegList object_regs,
RegList non_object_regs,
@@ -103,6 +105,13 @@
{
FrameScope scope(masm, StackFrame::INTERNAL);
+ // Load padding words on stack.
+ for (int i = 0; i < Debug::FramePaddingLayout::kInitialSize; i++) {
+ __ push(Immediate(Smi::FromInt(
+ Debug::FramePaddingLayout::kPaddingValue)));
+ }
+ __
push(Immediate(Smi::FromInt(Debug::FramePaddingLayout::kInitialSize)));
+
// Store the registers containing live values on the expression stack
to
// make sure that these are correctly updated during GC. Non object
values
// are stored as a smi causing it to be untouched by GC.
@@ -134,6 +143,10 @@
CEntryStub ceb(1);
__ CallStub(&ceb);
+ // Automatically find register that could be used after register
restore.
+ // We need one register for padding skip instructions.
+ Register unused_reg = { -1 };
+
// Restore the register values containing object pointers from the
// expression stack.
for (int i = kNumJSCallerSaved; --i >= 0;) {
@@ -142,14 +155,28 @@
if (FLAG_debug_code) {
__ Set(reg, Immediate(kDebugZapValue));
}
+ bool taken = reg.code() == esi.code();
if ((object_regs & (1 << r)) != 0) {
__ pop(reg);
+ taken = true;
}
if ((non_object_regs & (1 << r)) != 0) {
__ pop(reg);
__ SmiUntag(reg);
+ taken = true;
+ }
+ if (!taken) {
+ unused_reg = reg;
}
}
+
+ ASSERT(unused_reg.code() != -1);
+
+ // Read current padding counter and skip corresponding number of words.
+ __ pop(unused_reg);
+ // We divide stored value by 2 (untagging) and multiply it by word's
size.
+ STATIC_ASSERT(kSmiTagSize == 1);
+ __ lea(esp, Operand(esp, unused_reg, times_half_pointer_size, 0));
// Get rid of the internal frame.
}
=======================================
--- /branches/bleeding_edge/src/liveedit.cc Thu Feb 23 03:43:07 2012
+++ /branches/bleeding_edge/src/liveedit.cc Thu May 3 10:31:34 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 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:
@@ -30,6 +30,7 @@
#include "liveedit.h"
+#include "code-stubs.h"
#include "compilation-cache.h"
#include "compiler.h"
#include "debug.h"
@@ -1475,26 +1476,36 @@
// Check the nature of the top frame.
Isolate* isolate = Isolate::Current();
Code* pre_top_frame_code = pre_top_frame->LookupCode();
+ bool frame_has_padding;
if (pre_top_frame_code->is_inline_cache_stub() &&
pre_top_frame_code->ic_state() == DEBUG_BREAK) {
// OK, we can drop inline cache calls.
*mode = Debug::FRAME_DROPPED_IN_IC_CALL;
+ frame_has_padding = Debug::FramePaddingLayout::kIsSupported;
} else if (pre_top_frame_code ==
isolate->debug()->debug_break_slot()) {
// OK, we can drop debug break slot.
*mode = Debug::FRAME_DROPPED_IN_DEBUG_SLOT_CALL;
+ frame_has_padding = Debug::FramePaddingLayout::kIsSupported;
} else if (pre_top_frame_code ==
isolate->builtins()->builtin(
Builtins::kFrameDropper_LiveEdit)) {
// OK, we can drop our own code.
*mode = Debug::FRAME_DROPPED_IN_DIRECT_CALL;
+ frame_has_padding = false;
} else if (pre_top_frame_code ==
isolate->builtins()->builtin(Builtins::kReturn_DebugBreak)) {
*mode = Debug::FRAME_DROPPED_IN_RETURN_CALL;
+ frame_has_padding = Debug::FramePaddingLayout::kIsSupported;
} else if (pre_top_frame_code->kind() == Code::STUB &&
- pre_top_frame_code->major_key()) {
- // Entry from our unit tests, it's fine, we support this case.
+ pre_top_frame_code->major_key() == CodeStub::CEntry) {
+ // Entry from our unit tests on 'debugger' statement.
+ // It's fine, we support this case.
*mode = Debug::FRAME_DROPPED_IN_DIRECT_CALL;
+ // We don't have a padding from 'debugger' statement call.
+ // Here the stub is CEntry, it's not debug-only and can't be padded.
+ // If anyone would complain, a proxy padded stub could be added.
+ frame_has_padding = false;
} else {
return "Unknown structure of stack above changing function";
}
@@ -1504,8 +1515,48 @@
- Debug::kFrameDropperFrameSize * kPointerSize // Size of the new
frame.
+ kPointerSize; // Bigger address end is exclusive.
+ Address* top_frame_pc_address = top_frame->pc_address();
+
+ // top_frame may be damaged below this point. Do not used it.
+ ASSERT(!(top_frame = NULL));
+
if (unused_stack_top > unused_stack_bottom) {
- return "Not enough space for frame dropper frame";
+ if (frame_has_padding) {
+ int shortage_bytes = unused_stack_top - unused_stack_bottom;
+
+ Address padding_start = pre_top_frame->fp() -
+ Debug::FramePaddingLayout::kFrameBaseSize * kPointerSize;
+
+ Address padding_pointer = padding_start;
+ Smi* padding_object =
+ Smi::FromInt(Debug::FramePaddingLayout::kPaddingValue);
+ while (Memory::Object_at(padding_pointer) == padding_object) {
+ padding_pointer -= kPointerSize;
+ }
+ int padding_counter =
+ Smi::cast(Memory::Object_at(padding_pointer))->value();
+ if (padding_counter * kPointerSize < shortage_bytes) {
+ return "Not enough space for frame dropper frame "
+ "(even with padding frame)";
+ }
+ Memory::Object_at(padding_pointer) =
+ Smi::FromInt(padding_counter - shortage_bytes / kPointerSize);
+
+ StackFrame* pre_pre_frame = frames[top_frame_index - 2];
+
+ memmove(padding_start + kPointerSize - shortage_bytes,
+ padding_start + kPointerSize,
+ Debug::FramePaddingLayout::kFrameBaseSize * kPointerSize);
+
+ pre_top_frame->UpdateFp(pre_top_frame->fp() - shortage_bytes);
+ pre_pre_frame->SetCallerFp(pre_top_frame->fp());
+ unused_stack_top -= shortage_bytes;
+
+ STATIC_ASSERT(sizeof(Address) == kPointerSize);
+ top_frame_pc_address -= shortage_bytes / kPointerSize;
+ } else {
+ return "Not enough space for frame dropper frame";
+ }
}
// Committing now. After this point we should return only NULL value.
@@ -1515,7 +1566,7 @@
ASSERT(!FixTryCatchHandler(pre_top_frame, bottom_js_frame));
Handle<Code> code =
Isolate::Current()->builtins()->FrameDropper_LiveEdit();
- top_frame->set_pc(code->entry());
+ *top_frame_pc_address = code->entry();
pre_top_frame->SetCallerFp(bottom_js_frame->fp());
*restarter_frame_function_pointer =
=======================================
--- /branches/bleeding_edge/src/mips/debug-mips.cc Wed Mar 21 01:41:16 2012
+++ /branches/bleeding_edge/src/mips/debug-mips.cc Thu May 3 10:31:34 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 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:
@@ -115,6 +115,8 @@
rinfo()->PatchCode(original_rinfo()->pc(),
Assembler::kDebugBreakSlotInstructions);
}
+
+const bool Debug::FramePaddingLayout::kIsSupported = false;
#define __ ACCESS_MASM(masm)
=======================================
--- /branches/bleeding_edge/src/x64/debug-x64.cc Fri Jan 27 08:09:20 2012
+++ /branches/bleeding_edge/src/x64/debug-x64.cc Thu May 3 10:31:34 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 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:
@@ -90,6 +90,8 @@
ASSERT(IsDebugBreakSlot());
rinfo()->PatchCode(original_rinfo()->pc(),
Assembler::kDebugBreakSlotLength);
}
+
+const bool Debug::FramePaddingLayout::kIsSupported = false;
#define __ ACCESS_MASM(masm)
=======================================
--- /branches/bleeding_edge/test/mjsunit/mjsunit.status Fri Feb 24 02:59:12
2012
+++ /branches/bleeding_edge/test/mjsunit/mjsunit.status Thu May 3 10:31:34
2012
@@ -64,6 +64,7 @@
# Stack manipulations in LiveEdit are buggy - see bug 915
debug-liveedit-check-stack: SKIP
debug-liveedit-patch-positions-replace: SKIP
+debug-liveedit-stack-padding.js: SKIP
# Test Crankshaft compilation time. Expected to take too long in debug
mode.
regress/regress-1969: PASS, SKIP if $mode == debug
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
