Fix issue 825 (LiveEdit vs. function with no locals) in core and for ia32.

Review URL: https://chromiumcodereview.appspot.com/10263002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11502 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
peter.rybin@gmail.com 2012-05-03 17:31:34 +00:00
parent e443c69b8d
commit 1719a1499a
10 changed files with 237 additions and 10 deletions

View File

@ -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:
@ -125,6 +125,8 @@ void BreakLocationIterator::ClearDebugBreakAtSlot() {
Assembler::kDebugBreakSlotInstructions);
}
const bool Debug::FramePaddingLayout::kIsSupported = false;
#define __ ACCESS_MASM(masm)

View File

@ -2227,6 +2227,13 @@ void Debug::FramesHaveBeenDropped(StackFrame::Id new_break_frame_id,
}
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) {
return IsLoaded() && global == debug_context()->global();
}

View File

@ -457,6 +457,50 @@ class Debug {
// 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();

View File

@ -211,6 +211,9 @@ class StackFrame BASE_EMBEDDED {
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; }
// Get the id of this stack frame.

View File

@ -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 @@ void BreakLocationIterator::ClearDebugBreakAtSlot() {
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 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
{
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 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
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,15 +155,29 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
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.
}

View File

@ -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 @@ static const char* DropFrames(Vector<StackFrame*> frames,
// 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 @@ static const char* DropFrames(Vector<StackFrame*> frames,
- 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 @@ static const char* DropFrames(Vector<StackFrame*> frames,
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 =

View File

@ -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:
@ -116,6 +116,8 @@ void BreakLocationIterator::ClearDebugBreakAtSlot() {
Assembler::kDebugBreakSlotInstructions);
}
const bool Debug::FramePaddingLayout::kIsSupported = false;
#define __ ACCESS_MASM(masm)

View File

@ -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,6 +91,8 @@ void BreakLocationIterator::ClearDebugBreakAtSlot() {
rinfo()->PatchCode(original_rinfo()->pc(), Assembler::kDebugBreakSlotLength);
}
const bool Debug::FramePaddingLayout::kIsSupported = false;
#define __ ACCESS_MASM(masm)

View File

@ -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);

View File

@ -64,6 +64,7 @@ regress/regress-524: (PASS || TIMEOUT), SKIP if $mode == debug
# 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