[wasm] Enable wasm frame inspection for debugging

src/runtime/runtime-debug.cc

Index: src/runtime/runtime-debug.cc
diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc
index dcdffb748051a55f51d983823b21f8af7c34a9fc..c9137b317cf2b156271d6d23acac5b1373af8193 100644
--- a/src/runtime/runtime-debug.cc
+++ b/src/runtime/runtime-debug.cc
@@ -14,6 +14,7 @@
 #include "src/interpreter/interpreter.h"
 #include "src/isolate-inl.h"
 #include "src/runtime/runtime.h"
+#include "src/wasm/wasm-module.h"
 
 namespace v8 {
 namespace internal {
@@ -453,12 +454,16 @@ RUNTIME_FUNCTION(Runtime_GetFrameCount) {
     return Smi::FromInt(0);
   }
 
-  for (JavaScriptFrameIterator it(isolate, id); !it.done(); it.Advance()) {
+  for (StackTraceFrameIterator it(isolate, id); !it.done(); it.Advance()) {
     List<FrameSummary> frames(FLAG_max_inlining_levels + 1);
-    it.frame()->Summarize(&frames);
-    for (int i = frames.length() - 1; i >= 0; i--) {
-      // Omit functions from native and extension scripts.
-      if (frames[i].function()->shared()->IsSubjectToDebugging()) n++;
+    if (it.is_wasm()) {
+      n++;
+    } else {
+      it.javascript_frame()->Summarize(&frames);
+      for (int i = frames.length() - 1; i >= 0; i--) {
+        // Omit functions from native and extension scripts.
+        if (frames[i].function()->shared()->IsSubjectToDebugging()) n++;
+      }
     }
   }
   return Smi::FromInt(n);
@@ -468,14 +473,14 @@ RUNTIME_FUNCTION(Runtime_GetFrameCount) {
 static const int kFrameDetailsFrameIdIndex = 0;
 static const int kFrameDetailsReceiverIndex = 1;
 static const int kFrameDetailsFunctionIndex = 2;
-static const int kFrameDetailsArgumentCountIndex = 3;
-static const int kFrameDetailsLocalCountIndex = 4;
-static const int kFrameDetailsSourcePositionIndex = 5;
-static const int kFrameDetailsConstructCallIndex = 6;
-static const int kFrameDetailsAtReturnIndex = 7;
-static const int kFrameDetailsFlagsIndex = 8;
-static const int kFrameDetailsFirstDynamicIndex = 9;
-
+static const int kFrameDetailsScriptIndex = 3;
+static const int kFrameDetailsArgumentCountIndex = 4;
+static const int kFrameDetailsLocalCountIndex = 5;
+static const int kFrameDetailsSourcePositionIndex = 6;
+static const int kFrameDetailsConstructCallIndex = 7;
+static const int kFrameDetailsAtReturnIndex = 8;
+static const int kFrameDetailsFlagsIndex = 9;
+static const int kFrameDetailsFirstDynamicIndex = 10;
 
 // Return an array with frame details
 // args[0]: number: break id
@@ -485,12 +490,13 @@ static const int kFrameDetailsFirstDynamicIndex = 9;
 // 0: Frame id
 // 1: Receiver
 // 2: Function
-// 3: Argument count
-// 4: Local count
-// 5: Source position
-// 6: Constructor call
-// 7: Is at return
-// 8: Flags
+// 3: Script
+// 4: Argument count
+// 5: Local count
+// 6: Source position
+// 7: Constructor call
+// 8: Is at return
+// 9: Flags
 // Arguments name, value
 // Locals name, value
 // Return value if any
@@ -510,7 +516,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
     return heap->undefined_value();
   }
 
-  JavaScriptFrameIterator it(isolate, id);
+  StackTraceFrameIterator it(isolate, id);
   // Inlined frame index in optimized frame, starting from outer function.
   int inlined_jsframe_index =
       DebugFrameHelper::FindIndexedNonNativeFrame(&it, index);
@@ -534,61 +540,67 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
   // Check for constructor frame.
   bool constructor = frame_inspector.IsConstructor();
 
-  // Get scope info and read from it for local variable information.
-  Handle<JSFunction> function =
-      Handle<JSFunction>::cast(frame_inspector.GetFunction());
-  RUNTIME_ASSERT(function->shared()->IsSubjectToDebugging());
-  Handle<SharedFunctionInfo> shared(function->shared());
-  Handle<ScopeInfo> scope_info(shared->scope_info());
-  DCHECK(*scope_info != ScopeInfo::Empty(isolate));
+  Handle<ScopeInfo> scope_info;
 
   // Get the locals names and values into a temporary array.
-  int local_count = scope_info->LocalCount();
-  for (int slot = 0; slot < scope_info->LocalCount(); ++slot) {
-    // Hide compiler-introduced temporary variables, whether on the stack or on
-    // the context.
-    if (ScopeInfo::VariableIsSynthetic(scope_info->LocalName(slot))) {
-      local_count--;
+  Handle<FixedArray> locals;

[Yang, 2016/06/24 05:56:37]

Can we re-organize this file like this?

if (it.is_wasm()) {
  [all wasm-relevant stuff goes here]

} else {
  JavaScriptFrame* js_frame = ...
  [everything else goes here]
}

I would also put the check for IsBreakAtReturn into the else-block and avoid
changing its argument type to StandardFrame.

[Clemens Hammacher, 2016/06/24 09:23:31]

Done.

+  if (it.is_wasm()) {
+    // TODO(clemensh): add names and values of local variables.
+    locals = isolate->factory()->NewFixedArray(0);
+  } else {
+    // Get scope info and read from it for local variable information.
+    SharedFunctionInfo* shared = frame_inspector.GetFunction()->shared();
+    RUNTIME_ASSERT(shared->IsSubjectToDebugging());
+    scope_info = handle(shared->scope_info(), isolate);
+    DCHECK(*scope_info != ScopeInfo::Empty(isolate));
+
+    int local_count = scope_info->LocalCount();
+    for (int slot = 0; slot < scope_info->LocalCount(); ++slot) {
+      // Hide compiler-introduced temporary variables, whether on the stack or
+      // on the context.
+      if (ScopeInfo::VariableIsSynthetic(scope_info->LocalName(slot))) {
+        local_count--;
+      }
     }
-  }
 
-  Handle<FixedArray> locals =
-      isolate->factory()->NewFixedArray(local_count * 2);
-
-  // Fill in the values of the locals.
-  int local = 0;
-  int i = 0;
-  for (; i < scope_info->StackLocalCount(); ++i) {
-    // Use the value from the stack.
-    if (ScopeInfo::VariableIsSynthetic(scope_info->LocalName(i))) continue;
-    locals->set(local * 2, scope_info->LocalName(i));
-    Handle<Object> value =
-        frame_inspector.GetExpression(scope_info->StackLocalIndex(i));
-    // TODO(yangguo): We convert optimized out values to {undefined} when they
-    // are passed to the debugger. Eventually we should handle them somehow.
-    if (value->IsOptimizedOut(isolate)) {
-      value = isolate->factory()->undefined_value();
-    }
-    locals->set(local * 2 + 1, *value);
-    local++;
-  }
-  if (local < local_count) {
-    // Get the context containing declarations.
-    Handle<Context> context(
-        Handle<Context>::cast(frame_inspector.GetContext())->closure_context());
-    for (; i < scope_info->LocalCount(); ++i) {
-      Handle<String> name(scope_info->LocalName(i));
-      if (ScopeInfo::VariableIsSynthetic(*name)) continue;
-      VariableMode mode;
-      InitializationFlag init_flag;
-      MaybeAssignedFlag maybe_assigned_flag;
-      locals->set(local * 2, *name);
-      int context_slot_index = ScopeInfo::ContextSlotIndex(
-          scope_info, name, &mode, &init_flag, &maybe_assigned_flag);
-      Object* value = context->get(context_slot_index);
-      locals->set(local * 2 + 1, value);
+    locals = isolate->factory()->NewFixedArray(local_count * 2);
+
+    // Fill in the values of the locals.
+    int local = 0;
+    int i = 0;
+    for (; i < scope_info->StackLocalCount(); ++i) {
+      // Use the value from the stack.
+      if (ScopeInfo::VariableIsSynthetic(scope_info->LocalName(i))) continue;
+      locals->set(local * 2, scope_info->LocalName(i));
+      Handle<Object> value =
+          frame_inspector.GetExpression(scope_info->StackLocalIndex(i));
+      // TODO(yangguo): We convert optimized out values to {undefined} when they
+      // are passed to the debugger. Eventually we should handle them somehow.
+      if (value->IsOptimizedOut(isolate)) {
+        value = isolate->factory()->undefined_value();
+      }
+      locals->set(local * 2 + 1, *value);
       local++;
     }
+    if (local < local_count) {
+      // Get the context containing declarations.
+      Handle<Context> context(
+          Handle<Context>::cast(frame_inspector.GetContext())
+              ->closure_context());
+      for (; i < scope_info->LocalCount(); ++i) {
+        Handle<String> name(scope_info->LocalName(i));
+        if (ScopeInfo::VariableIsSynthetic(*name)) continue;
+        VariableMode mode;
+        InitializationFlag init_flag;
+        MaybeAssignedFlag maybe_assigned_flag;
+        locals->set(local * 2, *name);
+        int context_slot_index = ScopeInfo::ContextSlotIndex(
+            scope_info, name, &mode, &init_flag, &maybe_assigned_flag);
+        Object* value = context->get(context_slot_index);
+        locals->set(local * 2 + 1, value);
+        local++;
+      }
+    }
   }
 
   // Check whether this frame is positioned at return. If not top
@@ -609,34 +621,50 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
   // the provided parameters whereas the function frame always have the number
   // of arguments matching the functions parameters. The rest of the
   // information (except for what is collected above) is the same.
-  if ((inlined_jsframe_index == 0) && it.frame()->has_adapted_arguments()) {
+  if ((inlined_jsframe_index == 0) && it.frame()->is_java_script() &&
+      it.javascript_frame()->has_adapted_arguments()) {
     it.AdvanceToArgumentsFrame();
     frame_inspector.SetArgumentsFrame(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 = scope_info->ParameterCount();
-  if (argument_count < frame_inspector.GetParametersCount()) {
-    argument_count = frame_inspector.GetParametersCount();
+  int argument_count = frame_inspector.GetParametersCount();
+  if (!it.is_wasm()) {
+    int scope_param_count = scope_info->ParameterCount();
+    if (scope_param_count > argument_count) argument_count = scope_param_count;
   }
 
   // Calculate the size of the result.
-  int details_size = kFrameDetailsFirstDynamicIndex +
-                     2 * (argument_count + local_count) + (at_return ? 1 : 0);
+  int details_size = kFrameDetailsFirstDynamicIndex + 2 * argument_count +
+                     locals->length() + (at_return ? 1 : 0);
   Handle<FixedArray> details = isolate->factory()->NewFixedArray(details_size);
 
   // Add the frame id.
   details->set(kFrameDetailsFrameIdIndex, *frame_id);
 
   // Add the function (same as in function frame).
-  details->set(kFrameDetailsFunctionIndex, *(frame_inspector.GetFunction()));
+  if (it.is_wasm()) {
+    Handle<Object> wasm_obj(it.wasm_frame()->wasm_obj(), isolate);
+    int func_index = it.wasm_frame()->function_index();
+    Handle<String> func_name =
+        wasm::GetWasmFunctionName(isolate, wasm_obj, func_index);
+    details->set(kFrameDetailsFunctionIndex, *func_name);
+  } else {
+    details->set(kFrameDetailsFunctionIndex, *frame_inspector.GetFunction());
+  }
+
+  // Add the script wrapper
+  Handle<Object> script_wrapper =
+      Script::GetWrapper(frame_inspector.GetScript());
+  details->set(kFrameDetailsScriptIndex, *script_wrapper);
 
   // Add the arguments count.
   details->set(kFrameDetailsArgumentCountIndex, Smi::FromInt(argument_count));
 
   // Add the locals count
-  details->set(kFrameDetailsLocalCountIndex, Smi::FromInt(local_count));
+  details->set(kFrameDetailsLocalCountIndex,
+               Smi::FromInt(locals->length() / 2));
 
   // Add the source position.
   if (position != RelocInfo::kNoPosition) {
@@ -669,25 +697,21 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
   int details_index = kFrameDetailsFirstDynamicIndex;
 
   // Add arguments name and value.
-  for (int i = 0; i < argument_count; i++) {
+  for (int i = 0; i < argument_count; i++, details_index += 2) {
     // Name of the argument.
-    if (i < scope_info->ParameterCount()) {
-      details->set(details_index++, scope_info->ParameterName(i));
-    } else {
-      details->set(details_index++, heap->undefined_value());
+    if (!scope_info.is_null() && i < scope_info->ParameterCount()) {
+      details->set(details_index, scope_info->ParameterName(i));
     }
 
     // Parameter value.
     if (i < frame_inspector.GetParametersCount()) {
       // Get the value from the stack.
-      details->set(details_index++, *(frame_inspector.GetParameter(i)));
-    } else {
-      details->set(details_index++, heap->undefined_value());
+      details->set(details_index + 1, *frame_inspector.GetParameter(i));
     }
   }
 
   // Add locals name and value from the temporary copy from the function frame.
-  for (int i = 0; i < local_count * 2; i++) {
+  for (int i = 0, l = locals->length(); i < l; ++i) {
     details->set(details_index++, locals->get(i));
   }
 
@@ -698,8 +722,11 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
 
   // Add the receiver (same as in function frame).
   Handle<Object> receiver(it.frame()->receiver(), isolate);
-  DCHECK(!function->shared()->IsBuiltin());
-  DCHECK_IMPLIES(is_sloppy(shared->language_mode()), receiver->IsJSReceiver());
+  DCHECK(it.is_wasm() || !frame_inspector.GetFunction()->shared()->IsBuiltin());
+  DCHECK_IMPLIES(
+      !it.is_wasm() &&
+          is_sloppy(frame_inspector.GetFunction()->shared()->language_mode()),
+      receiver->IsJSReceiver());
   details->set(kFrameDetailsReceiverIndex, *receiver);
 
   DCHECK_EQ(details_size, details_index);
@@ -795,8 +822,8 @@ RUNTIME_FUNCTION(Runtime_GetAllScopesDetails) {
 
   // Get the frame where the debugging is performed.
   StackFrame::Id id = DebugFrameHelper::UnwrapFrameId(wrapped_id);
-  JavaScriptFrameIterator frame_it(isolate, id);
-  JavaScriptFrame* frame = frame_it.frame();
+  StackTraceFrameIterator frame_it(isolate, id);
+  StandardFrame* frame = frame_it.frame();

[Yang, 2016/06/24 05:56:37]

We need to return an empty array if the frame is not javascript.

[Clemens Hammacher, 2016/06/24 09:23:31]

This happens implicitely, as the ScopeIterator will be empty for wasm frames.
You really prefer an explicit check?

[Yang, 2016/06/24 10:02:57]

It will be empty because of the check in ScopeIterator's constructor:

!frame_inspector->GetFunction()->IsJSFunction()

Correct? That seems very brittle, relying on the fact that we don't have a
JSFunction. I'd be happier if the constructor had an explicit check against wasm
frames.

[Clemens Hammacher, 2016/06/24 10:45:42]

The check is in debug-scopes.cc:25:
if (!frame_inspector->GetContext()->IsContext()) { [...] return; }

Wasm frames don't have a context, and will never (in the foreseeable future ;) )
have one.

[Yang, 2016/06/24 10:52:25]

I see. Can we at least add an assertion in ScopeIterator to guard that we don't
run into a wasm frame?

[Clemens Hammacher, 2016/06/24 15:11:58]

Done.

   FrameInspector frame_inspector(frame, inlined_jsframe_index, isolate);
 
   List<Handle<JSObject> > result(4);
@@ -1555,8 +1582,10 @@ static Handle<Object> GetJSPositionInfo(Handle<Script> script, int position,
   }
 
   Handle<String> source = handle(String::cast(script->source()), isolate);
-  Handle<String> sourceText =
-      isolate->factory()->NewSubString(source, info.line_start, info.line_end);
+  Handle<String> sourceText = info.line_end > source->length()

[Yang, 2016/06/24 05:56:37]

can we do a script->is_wasm check instead?

[Clemens Hammacher, 2016/06/24 09:23:31]

Done.

+                                  ? isolate->factory()->empty_string()
+                                  : isolate->factory()->NewSubString(
+                                        source, info.line_start, info.line_end);
 
   Handle<JSObject> jsinfo =
       isolate->factory()->NewJSObject(isolate->object_function());