Reviewers: ulan,

Description:
Debugger: ensure that functions with debug info have code with break slots.

This helps reasoning about setting break points. Functions that
have debug info is also guaranteed to be able to set break points.

[email protected]
BUG=v8:4132
LOG=N

Please review this at https://codereview.chromium.org/1227213003/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+22, -32 lines):
  M src/debug.cc
  M src/objects.h
  M src/objects-inl.h


Index: src/debug.cc
diff --git a/src/debug.cc b/src/debug.cc
index cdd2b8c7b949115816cbc91bc0395a74610a4efd..6d033a7416e191a80bba50cdab215f61025a506e 100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -926,9 +926,6 @@ void Debug::ClearAllBreakPoints() {

 void Debug::FloodWithOneShot(Handle<JSFunction> function,
                              BreakLocatorType type) {
-  // Do not ever break in native and extension functions.
-  if (!function->IsSubjectToDebugging()) return;
-
   PrepareForBreakPoints();

   // Make sure the function is compiled and has set up the debug info.
@@ -951,8 +948,7 @@ void Debug::FloodBoundFunctionWithOneShot(Handle<JSFunction> function) {
   Handle<Object> bindee(new_bindings->get(JSFunction::kBoundFunctionIndex),
                         isolate_);

-  if (!bindee.is_null() && bindee->IsJSFunction() &&
-      JSFunction::cast(*bindee)->IsSubjectToDebugging()) {
+  if (!bindee.is_null() && bindee->IsJSFunction()) {
     Handle<JSFunction> bindee_function(JSFunction::cast(*bindee));
     FloodWithOneShotGeneric(bindee_function);
   }
@@ -1881,20 +1877,22 @@ Handle<Object> Debug::FindSharedFunctionInfoInScript(Handle<Script> script,
 // Ensures the debug information is present for shared.
 bool Debug::EnsureDebugInfo(Handle<SharedFunctionInfo> shared,
                             Handle<JSFunction> function) {
-  Isolate* isolate = shared->GetIsolate();
+  if (!shared->IsSubjectToDebugging()) return false;

   // Return if we already have the debug info for shared.
   if (HasDebugInfo(shared)) {
     DCHECK(shared->is_compiled());
+    DCHECK(shared->code()->has_debug_break_slots());
     return true;
   }

   // There will be at least one break point when we are done.
   has_break_points_ = true;

-  // Ensure function is compiled. Return false if this failed.
-  if (!function.is_null() &&
-      !Compiler::EnsureCompiled(function, CLEAR_EXCEPTION)) {
+  if (function.is_null()) {
+    DCHECK(shared->is_compiled());
+    DCHECK(shared->code()->has_debug_break_slots());
+  } else if (!Compiler::EnsureCompiled(function, CLEAR_EXCEPTION)) {
     return false;
   }

@@ -1904,7 +1902,9 @@ bool Debug::EnsureDebugInfo(Handle<SharedFunctionInfo> shared,
   shared->feedback_vector()->ClearICSlots(*shared);

   // Create the debug info object.
-  Handle<DebugInfo> debug_info = isolate->factory()->NewDebugInfo(shared);
+  DCHECK(shared->is_compiled());
+  DCHECK(shared->code()->has_debug_break_slots());
+  Handle<DebugInfo> debug_info = isolate_->factory()->NewDebugInfo(shared);

   // Add debug info to the list.
   DebugInfoListNode* node = new DebugInfoListNode(*debug_info);
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index b21724583d879df280080fd49a5e50655d24ce9a..c5f7a2c5d9e705cdd83454a0e65455c2b3a79100 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -5565,29 +5565,22 @@ void SharedFunctionInfo::TryReenableOptimization() {
 }


-bool JSFunction::IsBuiltin() {
-  return context()->global_object()->IsJSBuiltinsObject();
+bool SharedFunctionInfo::IsSubjectToDebugging() {
+  Object* script_obj = script();
+  if (script_obj->IsUndefined()) return false;
+  Script* script = Script::cast(script_obj);
+  Script::Type type = static_cast<Script::Type>(script->type()->value());
+  return type == Script::TYPE_NORMAL;
 }


-bool JSFunction::IsFromNativeScript() {
-  Object* script = shared()->script();
-  bool native = script->IsScript() &&
- Script::cast(script)->type()->value() == Script::TYPE_NATIVE;
-  DCHECK(!IsBuiltin() || native);  // All builtins are also native.
-  return native;
-}
-
-
-bool JSFunction::IsFromExtensionScript() {
-  Object* script = shared()->script();
-  return script->IsScript() &&
-         Script::cast(script)->type()->value() == Script::TYPE_EXTENSION;
+bool JSFunction::IsBuiltin() {
+  return context()->global_object()->IsJSBuiltinsObject();
 }


 bool JSFunction::IsSubjectToDebugging() {
-  return !IsFromNativeScript() && !IsFromExtensionScript();
+  return shared()->IsSubjectToDebugging();
 }


Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index 42828c5152deb835dd3e74e96faf702e372a961f..3414007ee28c14238cf7f94e0c9784884552bebd 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -6857,6 +6857,9 @@ class SharedFunctionInfo: public HeapObject {
                                                reason));
   }

+  // Tells whether this function should be subject to debugging.
+  inline bool IsSubjectToDebugging();
+
   // Check whether or not this function is inlineable.
   bool IsInlineable();

@@ -7249,12 +7252,6 @@ class JSFunction: public JSObject {
   // Tells whether this function is builtin.
   inline bool IsBuiltin();

-  // Tells whether this function is defined in a native script.
-  inline bool IsFromNativeScript();
-
-  // Tells whether this function is defined in an extension script.
-  inline bool IsFromExtensionScript();
-
   // Tells whether this function should be subject to debugging.
   inline bool IsSubjectToDebugging();



--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to