Support scope information and evaluation in optimized frames

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index 117e5ab36f6f7b7743ea43ef2e666220ee090e23..3a2e8c65530ad199b08ab6948db529a6b7516343 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -9976,6 +9976,64 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameCount) {
 }
 
 
+class FrameInspector {
+ public:
+  FrameInspector(JavaScriptFrame* frame,
+                 int inlined_frame_index,
+                 Isolate* isolate)
+      : frame_(frame), deoptimized_frame_(NULL), isolate_(isolate) {
+    // Calculate the deoptimized frame.
+    if (frame->is_optimized()) {
+      deoptimized_frame_ = Deoptimizer::DebuggerInspectableFrame(
+          frame, inlined_frame_index, isolate);
+    }
+  }
+
+  ~FrameInspector() {
+    // Get rid of the calculated deoptimized frame if any.
+    if (frame_->is_optimized()) {
+      Deoptimizer::DeleteDebuggerInspectableFrame(deoptimized_frame_,
+                                                  isolate_);
+    }
+  }
+
+  int GetParametersCount() {
+    return frame_->is_optimized()
+        ? deoptimized_frame_->parameters_count()
+        : frame_->ComputeParametersCount();
+  }
+  int expression_count() { return deoptimized_frame_->expression_count(); }
+  Object* GetFunction() {
+    return frame_->is_optimized()
+        ? deoptimized_frame_->GetFunction()
+        : frame_->function();
+  }
+  Object* GetParameter(int index) {
+    return frame_->is_optimized()
+        ? deoptimized_frame_->GetParameter(index)
+        : frame_->GetParameter(index);
+  }
+  Object* GetExpression(int index) {
+    return frame_->is_optimized()
+        ? deoptimized_frame_->GetExpression(index)
+        : frame_->GetExpression(index);
+  }
+
+  // Sometimes replacing the frame is required if there is an
+  // arguments adapter frame.
+  void set_frame(JavaScriptFrame* frame) {

[Sven Panne, 2011/07/13 10:53:46]

Is it OK (or even intended for correct execution of the code below) that frame_
and deoptimized_frame_ are "out of sync" after this call? I don't understand
enough details to decide this. If it *is* OK, perhaps a comment about this might
be helpful.

[Søren Thygesen Gjesse, 2011/07/13 11:12:37]

Well, this is somewhat odd. The issue is that when inspecting a JavaScript frame
(not inlined) it always have the correct number of arguments. If the provided
parameters are different the frame below holds the provided arguments. And as a
JavaScriptFrameIterator can only expose one JavaScriptFrame it is replaced. I
will elaborate the comment and narrow down the usage of this.

An even better solution might be to have the FrameInspector control the
JavaScriptFrameIterator, but that will be a separate change.

+    frame_ = frame;
+  }
+
+ private:
+  JavaScriptFrame* frame_;
+  DeoptimizedFrameInfo* deoptimized_frame_;
+  Isolate* isolate_;
+
+  DISALLOW_COPY_AND_ASSIGN(FrameInspector);
+};
+
+
 static const int kFrameDetailsFrameIdIndex = 0;
 static const int kFrameDetailsReceiverIndex = 1;
 static const int kFrameDetailsFunctionIndex = 2;
@@ -10024,8 +10082,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameDetails) {
     return heap->undefined_value();
   }
 
-  int deoptimized_frame_index = -1;  // Frame index in optimized frame.
-  DeoptimizedFrameInfo* deoptimized_frame = NULL;
+  int inlined_frame_index = 0;  // Inlined frame index in optimized frame.
 
   int count = 0;
   JavaScriptFrameIterator it(isolate, id);
@@ -10036,13 +10093,10 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameDetails) {
   if (it.done()) return heap->undefined_value();
 
   if (it.frame()->is_optimized()) {
-    deoptimized_frame_index =
+    inlined_frame_index =
         it.frame()->GetInlineCount() - (index - count) - 1;
-    deoptimized_frame = Deoptimizer::DebuggerInspectableFrame(
-        it.frame(),
-        deoptimized_frame_index,
-        isolate);
   }
+  FrameInspector frame_inspector(it.frame(), inlined_frame_index, isolate);
 
   // Traverse the saved contexts chain to find the active context for the
   // selected frame.
@@ -10061,7 +10115,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameDetails) {
 
   // Check for constructor frame. Inlined frames cannot be construct calls.
   bool inlined_frame =
-      it.frame()->is_optimized() && deoptimized_frame_index != 0;
+      it.frame()->is_optimized() && inlined_frame_index != 0;
   bool constructor = !inlined_frame && it.frame()->IsConstructor();
 
   // Get scope info and read from it for local variable information.
@@ -10083,14 +10137,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameDetails) {
   for (; i < info.number_of_stack_slots(); ++i) {
     // Use the value from the stack.
     locals->set(i * 2, *info.LocalName(i));
-    if (it.frame()->is_optimized()) {
-      // Get the value from the deoptimized frame.
-      locals->set(i * 2 + 1,
-                  deoptimized_frame->GetExpression(i));
-    } else {
-      // Get the value from the stack.
-      locals->set(i * 2 + 1, it.frame()->GetExpression(i));
-    }
+    locals->set(i * 2 + 1, frame_inspector.GetExpression(i));
   }
   if (i < info.NumberOfLocals()) {
     // Get the context containing declarations.
@@ -10148,17 +10195,19 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameDetails) {
   // of arguments matching the functions parameters. The rest of the
   // information (except for what is collected above) is the same.
   it.AdvanceToArgumentsFrame();
+  frame_inspector.set_frame(it.frame());
 
   // Find the number of arguments to fill. At least fill the number of
   // parameters for the function and fill more if more parameters are provided.
   int argument_count = info.number_of_parameters();
+  if (argument_count < frame_inspector.GetParametersCount()) {
+    argument_count = frame_inspector.GetParametersCount();
+  }
+#ifdef DEBUG
   if (it.frame()->is_optimized()) {
-    ASSERT_EQ(argument_count, deoptimized_frame->parameters_count());
-  } else {
-    if (argument_count < it.frame()->ComputeParametersCount()) {
-      argument_count = it.frame()->ComputeParametersCount();
-    }
+    ASSERT_EQ(argument_count, frame_inspector.GetParametersCount());
   }
+#endif
 
   // Calculate the size of the result.
   int details_size = kFrameDetailsFirstDynamicIndex +
@@ -10170,13 +10219,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameDetails) {
   details->set(kFrameDetailsFrameIdIndex, *frame_id);
 
   // Add the function (same as in function frame).
-  if (it.frame()->is_optimized()) {
-    // Get the function from the deoptimized frame.
-    details->set(kFrameDetailsFunctionIndex, deoptimized_frame->GetFunction());
-  } else {
-    // Get the function from the stack.
-    details->set(kFrameDetailsFunctionIndex, it.frame()->function());
-  }
+  details->set(kFrameDetailsFunctionIndex, frame_inspector.GetFunction());
 
   // Add the arguments count.
   details->set(kFrameDetailsArgumentCountIndex, Smi::FromInt(argument_count));
@@ -10208,9 +10251,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameDetails) {
   }
   if (it.frame()->is_optimized()) {
     flags |= 1 << 1;
-    if (deoptimized_frame_index > 0) {
-      flags |= 1 << 2;
-    }
+    flags |= inlined_frame_index << 2;
   }
   details->set(kFrameDetailsFlagsIndex, Smi::FromInt(flags));
 
@@ -10227,16 +10268,11 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameDetails) {
     }
 
     // Parameter value.
-    if (it.frame()->is_optimized()) {
-      // Get the value from the deoptimized frame.
-      details->set(details_index++, deoptimized_frame->GetParameter(i));
+    if (i < it.frame()->ComputeParametersCount()) {
+      // Get the value from the stack.
+      details->set(details_index++, frame_inspector.GetParameter(i));
     } else {
-      if (i < it.frame()->ComputeParametersCount()) {
-        // Get the value from the stack.
-        details->set(details_index++, it.frame()->GetParameter(i));
-      } else {
-        details->set(details_index++, heap->undefined_value());
-      }
+      details->set(details_index++, heap->undefined_value());
     }
   }
 
@@ -10268,12 +10304,6 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFrameDetails) {
   }
   details->set(kFrameDetailsReceiverIndex, *receiver);
 
-  // Get rid of the calculated deoptimized frame if any.
-  if (deoptimized_frame != NULL) {
-    Deoptimizer::DeleteDebuggerInspectableFrame(deoptimized_frame,
-                                                isolate);
-  }
-
   ASSERT_EQ(details_size, details_index);
   return *isolate->factory()->NewJSArrayWithElements(details);
 }
@@ -10309,12 +10339,15 @@ static bool CopyContextLocalsToScopeObject(
 
 // Create a plain JSObject which materializes the local scope for the specified
 // frame.
-static Handle<JSObject> MaterializeLocalScope(Isolate* isolate,
-                                              JavaScriptFrame* frame) {
+static Handle<JSObject> MaterializeLocalScope(
+    Isolate* isolate,
+    JavaScriptFrame* frame,
+    int inlined_frame_index) {
   Handle<JSFunction> function(JSFunction::cast(frame->function()));
   Handle<SharedFunctionInfo> shared(function->shared());
   Handle<SerializedScopeInfo> serialized_scope_info(shared->scope_info());
   ScopeInfo<> scope_info(*serialized_scope_info);
+  FrameInspector frame_inspector(frame, inlined_frame_index, isolate);
 
   // Allocate and initialize a JSObject with all the arguments, stack locals
   // heap locals and extension properties of the debugged function.
@@ -10327,7 +10360,7 @@ static Handle<JSObject> MaterializeLocalScope(Isolate* isolate,
         isolate,
         SetProperty(local_scope,
                     scope_info.parameter_name(i),
-                    Handle<Object>(frame->GetParameter(i), isolate),
+                    Handle<Object>(frame_inspector.GetParameter(i)),
                     NONE,
                     kNonStrictMode),
         Handle<JSObject>());
@@ -10339,7 +10372,7 @@ static Handle<JSObject> MaterializeLocalScope(Isolate* isolate,
         isolate,
         SetProperty(local_scope,
                     scope_info.stack_slot_name(i),
-                    Handle<Object>(frame->GetExpression(i), isolate),
+                    Handle<Object>(frame_inspector.GetExpression(i)),
                     NONE,
                     kNonStrictMode),
         Handle<JSObject>());
@@ -10459,9 +10492,12 @@ class ScopeIterator {
     ScopeTypeCatch
   };
 
-  ScopeIterator(Isolate* isolate, JavaScriptFrame* frame)
+  ScopeIterator(Isolate* isolate,
+                JavaScriptFrame* frame,
+                int inlined_frame_index)
     : isolate_(isolate),
       frame_(frame),
+      inlined_frame_index_(inlined_frame_index),
       function_(JSFunction::cast(frame->function())),
       context_(Context::cast(frame->context())),
       local_done_(false),
@@ -10546,7 +10582,7 @@ class ScopeIterator {
         return Handle<JSObject>(CurrentContext()->global());
       case ScopeIterator::ScopeTypeLocal:
         // Materialize the content of the local scope into a JSObject.
-        return MaterializeLocalScope(isolate_, frame_);
+        return MaterializeLocalScope(isolate_, frame_, inlined_frame_index_);
       case ScopeIterator::ScopeTypeWith:
         // Return the with object.
         return Handle<JSObject>(JSObject::cast(CurrentContext()->extension()));
@@ -10626,6 +10662,7 @@ class ScopeIterator {
  private:
   Isolate* isolate_;
   JavaScriptFrame* frame_;
+  int inlined_frame_index_;
   Handle<JSFunction> function_;
   Handle<Context> context_;
   bool local_done_;
@@ -10654,7 +10691,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetScopeCount) {
 
   // Count the visible scopes.
   int n = 0;
-  for (ScopeIterator it(isolate, frame); !it.Done(); it.Next()) {
+  for (ScopeIterator it(isolate, frame, 0);
+       !it.Done();
+       it.Next()) {
     n++;
   }
 
@@ -10669,14 +10708,15 @@ static const int kScopeDetailsSize = 2;
 // Return an array with scope details
 // args[0]: number: break id
 // args[1]: number: frame index
-// args[2]: number: scope index
+// args[2]: number: inlined frame index
+// args[3]: number: scope index
 //
 // The array returned contains the following information:
 // 0: Scope type
 // 1: Scope object
 RUNTIME_FUNCTION(MaybeObject*, Runtime_GetScopeDetails) {
   HandleScope scope(isolate);
-  ASSERT(args.length() == 3);
+  ASSERT(args.length() == 4);
 
   // Check arguments.
   Object* check;
@@ -10685,7 +10725,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetScopeDetails) {
     if (!maybe_check->ToObject(&check)) return maybe_check;
   }
   CONVERT_CHECKED(Smi, wrapped_id, args[1]);
-  CONVERT_NUMBER_CHECKED(int, index, Int32, args[2]);
+  CONVERT_NUMBER_CHECKED(int, inlined_frame_index, Int32, args[2]);
+  CONVERT_NUMBER_CHECKED(int, index, Int32, args[3]);
 
   // Get the frame where the debugging is performed.
   StackFrame::Id id = UnwrapFrameId(wrapped_id);
@@ -10694,7 +10735,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetScopeDetails) {
 
   // Find the requested scope.
   int n = 0;
-  ScopeIterator it(isolate, frame);
+  ScopeIterator it(isolate, frame, inlined_frame_index);
   for (; !it.Done() && n < index; it.Next()) {
     n++;
   }
@@ -10724,7 +10765,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugPrintScopes) {
   // Print the scopes for the top frame.
   StackFrameLocator locator;
   JavaScriptFrame* frame = locator.FindJavaScriptFrame(0);
-  for (ScopeIterator it(isolate, frame); !it.Done(); it.Next()) {
+  for (ScopeIterator it(isolate, frame, 0);
+       !it.Done();
+       it.Next()) {
     it.DebugPrint();
   }
 #endif
@@ -11117,6 +11160,7 @@ static Handle<Context> CopyWithContextChain(Isolate* isolate,
 // Runtime_DebugEvaluate.
 static Handle<Object> GetArgumentsObject(Isolate* isolate,
                                          JavaScriptFrame* frame,
+                                         int inlined_frame_index,
                                          Handle<JSFunction> function,
                                          Handle<SerializedScopeInfo> scope_info,
                                          const ScopeInfo<>* sinfo,
@@ -11128,6 +11172,7 @@ static Handle<Object> GetArgumentsObject(Isolate* isolate,
   if (sinfo->number_of_stack_slots() > 0) {
     index = scope_info->StackSlotIndex(isolate->heap()->arguments_symbol());
     if (index != -1) {
+      CHECK(false);
       return Handle<Object>(frame->GetExpression(index), isolate);
     }
   }
@@ -11140,7 +11185,9 @@ static Handle<Object> GetArgumentsObject(Isolate* isolate,
     }
   }
 
-  const int length = frame->ComputeParametersCount();
+  FrameInspector frame_inspector(frame, inlined_frame_index, isolate);
+
+  int length = frame_inspector.GetParametersCount();
   Handle<JSObject> arguments =
       isolate->factory()->NewArgumentsObject(function, length);
   Handle<FixedArray> array = isolate->factory()->NewFixedArray(length);
@@ -11148,7 +11195,7 @@ static Handle<Object> GetArgumentsObject(Isolate* isolate,
   AssertNoAllocation no_gc;
   WriteBarrierMode mode = array->GetWriteBarrierMode(no_gc);
   for (int i = 0; i < length; i++) {
-    array->set(i, frame->GetParameter(i), mode);
+    array->set(i, frame_inspector.GetParameter(i), mode);
   }
   arguments->set_elements(*array);
   return arguments;
@@ -11175,7 +11222,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) {
 
   // Check the execution state and decode arguments frame and source to be
   // evaluated.
-  ASSERT(args.length() == 5);
+  ASSERT(args.length() == 6);
   Object* check_result;
   { MaybeObject* maybe_check_result = Runtime_CheckExecutionState(
       RUNTIME_ARGUMENTS(isolate, args));
@@ -11184,9 +11231,10 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) {
     }
   }
   CONVERT_CHECKED(Smi, wrapped_id, args[1]);
-  CONVERT_ARG_CHECKED(String, source, 2);
-  CONVERT_BOOLEAN_CHECKED(disable_break, args[3]);
-  Handle<Object> additional_context(args[4]);
+  CONVERT_NUMBER_CHECKED(int, inlined_frame_index, Int32, args[2]);
+  CONVERT_ARG_CHECKED(String, source, 3);
+  CONVERT_BOOLEAN_CHECKED(disable_break, args[4]);
+  Handle<Object> additional_context(args[5]);
 
   // Handle the processing of break.
   DisableBreak disable_break_save(disable_break);
@@ -11226,7 +11274,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) {
 #endif
 
   // Materialize the content of the local scope into a JSObject.
-  Handle<JSObject> local_scope = MaterializeLocalScope(isolate, frame);
+  Handle<JSObject> local_scope = MaterializeLocalScope(
+      isolate, frame, inlined_frame_index);
   RETURN_IF_EMPTY_HANDLE(isolate, local_scope);
 
   // Allocate a new context for the debug evaluation and set the extension
@@ -11275,7 +11324,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugEvaluate) {
                       &has_pending_exception);
   if (has_pending_exception) return Failure::Exception();
 
-  Handle<Object> arguments = GetArgumentsObject(isolate, frame,
+  Handle<Object> arguments = GetArgumentsObject(isolate,
+                                                frame, inlined_frame_index,
                                                 function, scope_info,
                                                 &sinfo, function_context);