Commit 75350f1e authored by yangguo's avatar yangguo Committed by Commit bot

Debugger: require debugger to be active when dealing with breaks.

This invariant will save us some head ache.
The changes to test-debug/DebugStub is due to the fact that it abuses
the ability to set break points in code that has no debug break slots.
This is now no longer possible.

R=ulan@chromium.org
BUG=v8:4132
LOG=N

Review URL: https://codereview.chromium.org/1181013007

Cr-Commit-Position: refs/heads/master@{#29038}
parent 2c44f4f9
......@@ -58,6 +58,23 @@ static v8::Handle<v8::Context> GetDebugEventContext(Isolate* isolate) {
}
BreakLocation::BreakLocation(Handle<DebugInfo> debug_info, RelocInfo* rinfo,
RelocInfo* original_rinfo, int position,
int statement_position)
: debug_info_(debug_info),
pc_offset_(static_cast<int>(rinfo->pc() - debug_info->code()->entry())),
original_pc_offset_(static_cast<int>(
original_rinfo->pc() - debug_info->original_code()->entry())),
rmode_(rinfo->rmode()),
original_rmode_(original_rinfo->rmode()),
data_(rinfo->data()),
original_data_(original_rinfo->data()),
position_(position),
statement_position_(statement_position) {
DCHECK(debug_info_->GetIsolate()->debug()->is_active());
}
BreakLocation::Iterator::Iterator(Handle<DebugInfo> debug_info,
BreakLocatorType type)
: debug_info_(debug_info),
......@@ -503,6 +520,8 @@ ScriptCache::ScriptCache(Isolate* isolate) : isolate_(isolate) {
Heap* heap = isolate_->heap();
HandleScope scope(isolate_);
DCHECK(isolate_->debug()->is_active());
// Perform a GC to get rid of all unreferenced scripts.
heap->CollectAllGarbage(Heap::kMakeHeapIterableMask, "ScriptCache");
......@@ -2941,16 +2960,17 @@ void Debug::SetMessageHandler(v8::Debug::MessageHandler handler) {
void Debug::UpdateState() {
is_active_ = message_handler_ != NULL || !event_listener_.is_null();
if (is_active_ || in_debug_scope()) {
bool is_active = message_handler_ != NULL || !event_listener_.is_null();
if (is_active || in_debug_scope()) {
// Note that the debug context could have already been loaded to
// bootstrap test cases.
isolate_->compilation_cache()->Disable();
is_active_ = Load();
is_active = Load();
} else if (is_loaded()) {
isolate_->compilation_cache()->Enable();
Unload();
}
is_active_ = is_active;
}
......
......@@ -129,17 +129,8 @@ class BreakLocation {
private:
BreakLocation(Handle<DebugInfo> debug_info, RelocInfo* rinfo,
RelocInfo* original_rinfo, int position, int statement_position)
: debug_info_(debug_info),
pc_offset_(static_cast<int>(rinfo->pc() - debug_info->code()->entry())),
original_pc_offset_(static_cast<int>(
original_rinfo->pc() - debug_info->original_code()->entry())),
rmode_(rinfo->rmode()),
original_rmode_(original_rinfo->rmode()),
data_(rinfo->data()),
original_data_(original_rinfo->data()),
position_(position),
statement_position_(statement_position) {}
RelocInfo* original_rinfo, int position,
int statement_position);
class Iterator {
public:
......@@ -526,7 +517,8 @@ class Debug {
static void RecordEvalCaller(Handle<Script> script);
bool CheckExecutionState(int id) {
return !debug_context().is_null() && break_id() != 0 && break_id() == id;
return is_active() && !debug_context().is_null() && break_id() != 0 &&
break_id() == id;
}
// Flags and states.
......
......@@ -2183,7 +2183,7 @@ static bool IsPositionAlignmentCodeCorrect(int alignment) {
RUNTIME_FUNCTION(Runtime_GetBreakLocations) {
HandleScope scope(isolate);
DCHECK(args.length() == 2);
RUNTIME_ASSERT(isolate->debug()->is_active());
CONVERT_ARG_HANDLE_CHECKED(JSFunction, fun, 0);
CONVERT_NUMBER_CHECKED(int32_t, statement_aligned_code, Int32, args[1]);
......@@ -2211,6 +2211,7 @@ RUNTIME_FUNCTION(Runtime_GetBreakLocations) {
RUNTIME_FUNCTION(Runtime_SetFunctionBreakPoint) {
HandleScope scope(isolate);
DCHECK(args.length() == 3);
RUNTIME_ASSERT(isolate->debug()->is_active());
CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
CONVERT_NUMBER_CHECKED(int32_t, source_position, Int32, args[1]);
RUNTIME_ASSERT(source_position >= function->shared()->start_position() &&
......@@ -2235,6 +2236,7 @@ RUNTIME_FUNCTION(Runtime_SetFunctionBreakPoint) {
RUNTIME_FUNCTION(Runtime_SetScriptBreakPoint) {
HandleScope scope(isolate);
DCHECK(args.length() == 4);
RUNTIME_ASSERT(isolate->debug()->is_active());
CONVERT_ARG_HANDLE_CHECKED(JSValue, wrapper, 0);
CONVERT_NUMBER_CHECKED(int32_t, source_position, Int32, args[1]);
RUNTIME_ASSERT(source_position >= 0);
......@@ -2266,6 +2268,7 @@ RUNTIME_FUNCTION(Runtime_SetScriptBreakPoint) {
RUNTIME_FUNCTION(Runtime_ClearBreakPoint) {
HandleScope scope(isolate);
DCHECK(args.length() == 1);
RUNTIME_ASSERT(isolate->debug()->is_active());
CONVERT_ARG_HANDLE_CHECKED(Object, break_point_object_arg, 0);
// Clear break point.
......@@ -2363,6 +2366,7 @@ RUNTIME_FUNCTION(Runtime_PrepareStep) {
RUNTIME_FUNCTION(Runtime_ClearStepping) {
HandleScope scope(isolate);
DCHECK(args.length() == 0);
RUNTIME_ASSERT(isolate->debug()->is_active());
isolate->debug()->ClearStepping();
return isolate->heap()->undefined_value();
}
......@@ -2709,6 +2713,7 @@ RUNTIME_FUNCTION(Runtime_DebugEvaluateGlobal) {
RUNTIME_FUNCTION(Runtime_DebugGetLoadedScripts) {
HandleScope scope(isolate);
DCHECK(args.length() == 0);
RUNTIME_ASSERT(isolate->debug()->is_active());
Handle<FixedArray> instances;
{
......@@ -2976,6 +2981,7 @@ RUNTIME_FUNCTION(Runtime_GetFunctionCodePositionFromSource) {
HandleScope scope(isolate);
CHECK(isolate->debug()->live_edit_enabled());
DCHECK(args.length() == 2);
RUNTIME_ASSERT(isolate->debug()->is_active());
CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
CONVERT_NUMBER_CHECKED(int32_t, source_position, Int32, args[1]);
......@@ -3117,6 +3123,8 @@ RUNTIME_FUNCTION(Runtime_DebugCallbackSupportsStepping) {
// built-in function such as Array.forEach to enable stepping into the callback.
RUNTIME_FUNCTION(Runtime_DebugPrepareStepInIfStepping) {
DCHECK(args.length() == 1);
RUNTIME_ASSERT(isolate->debug()->is_active());
Debug* debug = isolate->debug();
if (!debug->IsStepping()) return isolate->heap()->undefined_value();
......
......@@ -575,6 +575,18 @@ static inline void SimulateIncrementalMarking(i::Heap* heap) {
}
static void DummyDebugEventListener(
const v8::Debug::EventDetails& event_details) {}
static inline void EnableDebugger() {
v8::Debug::SetDebugEventListener(&DummyDebugEventListener);
}
static inline void DisableDebugger() { v8::Debug::SetDebugEventListener(NULL); }
// Helper class for new allocations tracking and checking.
// To use checking of JS allocations tracking in a test,
// just create an instance of this class.
......
......@@ -20960,6 +20960,7 @@ TEST(StreamingWithDebuggingDoesNotProduceParserCache) {
CompileRun("function break_here() { }");
i::Handle<i::JSFunction> func = i::Handle<i::JSFunction>::cast(
v8::Utils::OpenHandle(*env->Global()->Get(v8_str("break_here"))));
EnableDebugger();
v8::internal::Debug* debug = CcTest::i_isolate()->debug();
int position = 0;
debug->SetBreakPoint(func, i::Handle<i::Object>(v8::internal::Smi::FromInt(1),
......@@ -20980,6 +20981,7 @@ TEST(StreamingWithDebuggingDoesNotProduceParserCache) {
// Check that we got no cached data.
CHECK(source.GetCachedData() == NULL);
DisableDebugger();
}
......
......@@ -445,6 +445,7 @@ void CheckDebugBreakFunction(DebugLocalContext* env,
const char* source, const char* name,
int position, v8::internal::RelocInfo::Mode mode,
Code* debug_break) {
EnableDebugger();
i::Debug* debug = CcTest::i_isolate()->debug();
// Create function and set the break point.
......@@ -488,6 +489,8 @@ void CheckDebugBreakFunction(DebugLocalContext* env,
i::RelocInfo rinfo = location.rinfo();
CHECK(!rinfo.IsPatchedReturnSequence());
}
DisableDebugger();
}
......@@ -986,12 +989,10 @@ TEST(DebugStub) {
v8::internal::RelocInfo::CODE_TARGET,
CcTest::i_isolate()->builtins()->builtin(
Builtins::kStoreIC_DebugBreak));
CheckDebugBreakFunction(&env,
"function f3(){var a=x;}", "f3",
0,
v8::internal::RelocInfo::CODE_TARGET,
CcTest::i_isolate()->builtins()->builtin(
Builtins::kLoadIC_DebugBreak));
CheckDebugBreakFunction(
&env, "function f3(){x();}", "f3", 0,
v8::internal::RelocInfo::CODE_TARGET,
CcTest::i_isolate()->builtins()->builtin(Builtins::kLoadIC_DebugBreak));
// TODO(1240753): Make the test architecture independent or split
// parts of the debugger into architecture dependent files. This
......@@ -1001,29 +1002,21 @@ TEST(DebugStub) {
CheckDebugBreakFunction(
&env,
"function f4(){var index='propertyName'; var a={}; a[index] = 'x';}",
"f4",
0,
v8::internal::RelocInfo::CODE_TARGET,
"f4", 39, v8::internal::RelocInfo::CODE_TARGET,
CcTest::i_isolate()->builtins()->builtin(
Builtins::kKeyedStoreIC_DebugBreak));
CheckDebugBreakFunction(
&env,
"function f5(){var index='propertyName'; var a={}; return a[index];}",
"f5",
0,
v8::internal::RelocInfo::CODE_TARGET,
"f5", 39, v8::internal::RelocInfo::CODE_TARGET,
CcTest::i_isolate()->builtins()->builtin(
Builtins::kKeyedLoadIC_DebugBreak));
#endif
CheckDebugBreakFunction(
&env,
"function f6(a){return a==null;}",
"f6",
0,
v8::internal::RelocInfo::CODE_TARGET,
CcTest::i_isolate()->builtins()->builtin(
Builtins::kCompareNilIC_DebugBreak));
CheckDebugBreakFunction(&env, "function f6(){(0==null)()}", "f6", 0,
v8::internal::RelocInfo::CODE_TARGET,
CcTest::i_isolate()->builtins()->builtin(
Builtins::kCompareNilIC_DebugBreak));
// Check the debug break code stubs for call ICs with different number of
// parameters.
......@@ -1066,6 +1059,7 @@ TEST(DebugInfo) {
CHECK_EQ(0, v8::internal::GetDebuggedFunctions()->length());
CHECK(!HasDebugInfo(foo));
CHECK(!HasDebugInfo(bar));
EnableDebugger();
// One function (foo) is debugged.
int bp1 = SetBreakPoint(foo, 0);
CHECK_EQ(1, v8::internal::GetDebuggedFunctions()->length());
......@@ -1083,6 +1077,7 @@ TEST(DebugInfo) {
CHECK(HasDebugInfo(bar));
// No functions are debugged.
ClearBreakPoint(bp2);
DisableDebugger();
CHECK_EQ(0, v8::internal::GetDebuggedFunctions()->length());
CHECK(!HasDebugInfo(foo));
CHECK(!HasDebugInfo(bar));
......@@ -2290,11 +2285,12 @@ TEST(RemoveBreakPointInBreak) {
v8::Local<v8::Function> foo =
CompileFunction(&env, "function foo(){a=1;}", "foo");
debug_event_remove_break_point = SetBreakPoint(foo, 0);
// Register the debug event listener pasing the function
v8::Debug::SetDebugEventListener(DebugEventRemoveBreakPoint, foo);
debug_event_remove_break_point = SetBreakPoint(foo, 0);
break_point_hit_count = 0;
foo->Call(env->Global(), 0, NULL);
CHECK_EQ(1, break_point_hit_count);
......@@ -2781,11 +2777,11 @@ TEST(DebugStepLinear) {
// Run foo to allow it to get optimized.
CompileRun("a=0; b=0; c=0; foo();");
SetBreakPoint(foo, 3);
// Register a debug event listener which steps and counts.
v8::Debug::SetDebugEventListener(DebugEventStep);
SetBreakPoint(foo, 3);
step_action = StepIn;
break_point_hit_count = 0;
foo->Call(env->Global(), 0, NULL);
......@@ -5579,11 +5575,6 @@ TEST(RecursiveBreakpointsGlobal) {
}
static void DummyDebugEventListener(
const v8::Debug::EventDetails& event_details) {
}
TEST(SetDebugEventListenerOnUninitializedVM) {
v8::Debug::SetDebugEventListener(DummyDebugEventListener);
}
......@@ -5957,7 +5948,7 @@ TEST(DebugGetLoadedScripts) {
v8::String::NewExternal(env->GetIsolate(), &source_ext_str);
v8::Handle<v8::Script> evil_script(v8::Script::Compile(source));
// "use" evil_script to make the compiler happy.
(void) evil_script;
USE(evil_script);
Handle<i::ExternalTwoByteString> i_source(
i::ExternalTwoByteString::cast(*v8::Utils::OpenHandle(*source)));
// This situation can happen if source was an external string disposed
......@@ -5966,12 +5957,14 @@ TEST(DebugGetLoadedScripts) {
bool allow_natives_syntax = i::FLAG_allow_natives_syntax;
i::FLAG_allow_natives_syntax = true;
EnableDebugger();
CompileRun(
"var scripts = %DebugGetLoadedScripts();"
"var count = scripts.length;"
"for (var i = 0; i < count; ++i) {"
" scripts[i].line_ends;"
"}");
DisableDebugger();
// Must not crash while accessing line_ends.
i::FLAG_allow_natives_syntax = allow_natives_syntax;
......@@ -6539,6 +6532,8 @@ TEST(ProvisionalBreakpointOnLineOutOfRange) {
const char* script = "function f() {};";
const char* resource_name = "test_resource";
v8::Debug::SetMessageHandler(AfterCompileMessageHandler);
// Set a couple of provisional breakpoint on lines out of the script lines
// range.
int sbp1 = SetScriptBreakPointByNameFromJS(env->GetIsolate(), resource_name,
......@@ -6547,7 +6542,6 @@ TEST(ProvisionalBreakpointOnLineOutOfRange) {
SetScriptBreakPointByNameFromJS(env->GetIsolate(), resource_name, 5, 5);
after_compile_message_count = 0;
v8::Debug::SetMessageHandler(AfterCompileMessageHandler);
v8::ScriptOrigin origin(
v8::String::NewFromUtf8(env->GetIsolate(), resource_name),
......
......@@ -1382,8 +1382,10 @@ TEST(TestCodeFlushingIncrementalAbort) {
// disabled.
int position = 0;
Handle<Object> breakpoint_object(Smi::FromInt(0), isolate);
EnableDebugger();
isolate->debug()->SetBreakPoint(function, breakpoint_object, &position);
isolate->debug()->ClearAllBreakPoints();
DisableDebugger();
// Force optimization now that code flushing is disabled.
{ v8::HandleScope scope(CcTest::isolate());
......
......@@ -29,6 +29,7 @@
// Get the Debug object exposed from the debug context global object.
Debug = debug.Debug
Debug.setListener(function(){});
var function_z_text =
" function Z() {\n"
......@@ -111,3 +112,4 @@ for (var i = 0; i < breaks_ids.length; i++) {
}
assertEquals(0, Debug.scriptBreakPoints().length);
Debug.setListener(null);
......@@ -33,6 +33,7 @@
// corresponding byte-code PCs should coincide before change and after it.
Debug = debug.Debug
Debug.setListener(function() {});
eval(
"function F1() { return 5; }\n" +
......@@ -124,3 +125,5 @@ print(pcArray2);
if (pcArray1 && pcArray2) {
assertArrayEquals(pcArray1, pcArray2);
}
Debug.setListener(null);
......@@ -29,6 +29,7 @@
// Get the Debug object exposed from the debug context global object.
Debug = debug.Debug;
Debug.setListener(listener);
SlimFunction = eval(
"(function() {\n " +
......@@ -76,7 +77,6 @@ function listener(event, exec_state, event_data, data) {
}
}
Debug.setListener(listener);
var animal = SlimFunction();
......@@ -86,3 +86,5 @@ if (saved_exception) {
}
assertEquals("Capybara", animal);
Debug.setListener(null);
......@@ -28,6 +28,7 @@
// Flags: --expose-debug-as debug
// Get the Debug object exposed from the debug context global object.
Debug = debug.Debug
Debug.setListener(function(){});
// Set and remove a script break point for a named script.
var sbp = Debug.setScriptBreakPointByName("1", 2, 3);
......@@ -110,3 +111,5 @@ Debug.clearBreakPoint(sbp3);
assertEquals(1, Debug.scriptBreakPoints().length);
Debug.clearBreakPoint(sbp2);
assertEquals(0, Debug.scriptBreakPoints().length);
Debug.setListener(null);
......@@ -36,6 +36,7 @@
// Get the Debug object exposed from the debug context global object.
Debug = debug.Debug;
Debug.setListener(function(){});
Date();
RegExp();
......@@ -103,3 +104,5 @@ assertEquals(Debug.ScriptType.Normal, debug_script.type);
// Check a nonexistent script.
var dummy_script = Debug.findScript('dummy.js');
assertTrue(typeof dummy_script == 'undefined');
Debug.setListener(null);
......@@ -33,7 +33,7 @@ function DebuggerStatement() {
debugger; /*pause*/
}
function TestCase(fun, frame_number) {
function TestCase(fun, frame_number, line_number) {
var exception = false;
var codeSnippet = undefined;
var resultPositions = undefined;
......@@ -64,8 +64,13 @@ function TestCase(fun, frame_number) {
Debug.setListener(listener);
var breakpointId;
if (line_number) breakpointId = Debug.setBreakPoint(fun, line_number);
fun();
if (line_number) Debug.clearBreakPoint(breakpointId);
Debug.setListener(null);
assertTrue(!exception, exception);
......@@ -116,9 +121,7 @@ function TestCaseWithDebugger(fun) {
}
function TestCaseWithBreakpoint(fun, line_number, frame_number) {
var breakpointId = Debug.setBreakPoint(fun, line_number);
TestCase(fun, frame_number);
Debug.clearBreakPoint(breakpointId);
TestCase(fun, frame_number, line_number);
}
function TestCaseWithException(fun, frame_number) {
......
......@@ -5,6 +5,9 @@
// Flags: --allow-natives-syntax --cache=code
// Test that script ids are unique and we found the correct ones.
var Debug = %GetDebugContext().Debug;
Debug.setListener(function(){});
var scripts = %DebugGetLoadedScripts();
scripts.sort(function(a, b) { return a.id - b.id; });
var user_script_count = 0;
......@@ -15,3 +18,5 @@ scripts.reduce(function(prev, cur) {
// Found mjsunit.js and this script.
assertEquals(2, user_script_count);
Debug.setListener(null);
......@@ -48,8 +48,6 @@ function f() {
// Get the Debug object exposed from the debug context global object.
var Debug = debug.Debug
// Set breakpoint on line 6.
var bp = Debug.setBreakPoint(f, 6);
function listener(event, exec_state, event_data, data) {
if (event == Debug.DebugEvent.Break) {
......@@ -59,6 +57,10 @@ function listener(event, exec_state, event_data, data) {
// Add the debug event listener.
Debug.setListener(listener);
//Set breakpoint on line 6.
var bp = Debug.setBreakPoint(f, 6);
result = -1;
f();
assertEquals(1, result);
......@@ -107,3 +109,5 @@ assertEquals(null, exception, exception);
exception = null;
f2();
assertEquals(null, exception, exception);
Debug.setListener(null);
......@@ -25,13 +25,13 @@ function RunTest(formals_and_body, args, value1, value2) {
// Advance to the first yield.
assertIteratorResult(value1, false, obj.next());
// Add a breakpoint on line 3 (the second yield).
var bp = Debug.setBreakPoint(gen, 3);
// Enable the debugger, which should force recompilation of the generator
// function and relocation of the suspended generator activation.
Debug.setListener(listener);
// Add a breakpoint on line 3 (the second yield).
var bp = Debug.setBreakPoint(gen, 3);
// Check that the generator resumes and suspends properly.
assertIteratorResult(value2, false, obj.next());
......
......@@ -43,8 +43,6 @@ function f() {
// Get the Debug object exposed from the debug context global object.
Debug = debug.Debug
// Set breakpoint on line 6.
var bp = Debug.setBreakPoint(f, 6);
function listener(event, exec_state, event_data, data) {
if (event == Debug.DebugEvent.Break) {
......@@ -54,6 +52,10 @@ function listener(event, exec_state, event_data, data) {
// Add the debug event listener.
Debug.setListener(listener);
//Set breakpoint on line 6.
var bp = Debug.setBreakPoint(f, 6);
result = -1;
f();
assertEquals(1, result);
......
......@@ -56,11 +56,12 @@ function f() {
};
Debug = debug.Debug;
var bp = Debug.setBreakPoint(f, 0);
function listener(event, exec_state, event_data, data) {
result = exec_state.frame().evaluate("i").value();
};
Debug.setListener(listener);
var bp = Debug.setBreakPoint(f, 0);
assertThrows(function() { f(); }, RangeError);
......@@ -23,6 +23,7 @@
function sentinel() {}
Debug = debug.Debug;
Debug.setListener(function(){});
var script = Debug.findScript(sentinel);
var line = 14;
......@@ -34,3 +35,5 @@ var actual = Debug.setBreakPointByScriptIdAndPosition(
// the break point.
assertTrue(line_start <= actual);
assertTrue(actual <= line_end);
Debug.setListener(null);
......@@ -4,4 +4,9 @@
// Flags: --gc-interval=33 --expose-gc --allow-natives-syntax
var Debug = %GetDebugContext().Debug;
Debug.setListener(function(){});
%DebugGetLoadedScripts();
Debug.setListener(null);
......@@ -29,6 +29,7 @@
// Flags: --expose-debug-as debug
Debug = debug.Debug
Debug.setListener(function(){});
function f() {a=1;b=2};
function g() {
......@@ -46,3 +47,5 @@ Debug.clearBreakPoint(bp);
%OptimizeFunctionOnNextCall(Debug.setBreakPoint);
bp = Debug.setBreakPoint(f, 0, 0);
Debug.clearBreakPoint(bp);
Debug.setListener(null);
......@@ -54,8 +54,10 @@ foo();
// and (shared) unoptimized code on foo, and sets both to lazy-compile builtin.
// Clear the break point immediately after to deactivate the debugger.
// Do all of this after compile graph has been created.
Debug.setListener(function(){});
Debug.setBreakPoint(bar, 0, 0);
Debug.clearAllBreakPoints();
Debug.setListener(null);
// At this point, concurrent recompilation is still blocked.
assertUnoptimized(foo, "no sync");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment