Fix DebugEvaluate for generators.

src/runtime.cc

 // 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
@@ skipping 11399 matching lines @@
 // Create a plain JSObject which materializes the local scope for the specified
 // frame.
 static Handle<JSObject> MaterializeStackLocalsWithFrameInspector(
     Isolate* isolate,
     Handle<JSObject> target,
     Handle<JSFunction> function,
     FrameInspector* frame_inspector) {
   Handle<SharedFunctionInfo> shared(function->shared());
   Handle<ScopeInfo> scope_info(shared->scope_info());
 
-  // First fill all parameters.
-  for (int i = 0; i < scope_info->ParameterCount(); ++i) {
-    Handle<String> name(scope_info->ParameterName(i));
-    VariableMode mode;
-    InitializationFlag init_flag;
-    // Do not materialize the parameter if it is shadowed by a context local.
-    if (scope_info->ContextSlotIndex(*name, &mode, &init_flag) != -1) continue;
+  // First fill all parameters.  Parameters in generators are context-allocated.
+  if (!shared->is_generator()) {
+    for (int i = 0; i < scope_info->ParameterCount(); ++i) {
+      Handle<String> name(scope_info->ParameterName(i));
+      VariableMode mode;
+      InitializationFlag init_flag;
+      // Do not materialize the parameter if it is shadowed by a context local.
+      if (scope_info->ContextSlotIndex(*name, &mode, &init_flag) != -1) {

[Michael Starzinger, 2014/03/24 10:24:28]

I am little bit confused here. For generators we should already context-allocate
all parameter slots, which in turn means this check should hit for all of them.
So I don't see how the assert below would ever hit unless the scoping
information is off.

+        continue;
+      }
 
-    Handle<Object> value(i < frame_inspector->GetParametersCount()
-                             ? frame_inspector->GetParameter(i)
-                             : isolate->heap()->undefined_value(),
-                         isolate);
-    ASSERT(!value->IsTheHole());
+      Handle<Object> value(i < frame_inspector->GetParametersCount()
+                               ? frame_inspector->GetParameter(i)
+                               : isolate->heap()->undefined_value(),
+                           isolate);
+      ASSERT(!value->IsTheHole());
 
-    RETURN_IF_EMPTY_HANDLE_VALUE(
-        isolate,
-        Runtime::SetObjectProperty(isolate, target, name, value, NONE, SLOPPY),
-        Handle<JSObject>());
+      RETURN_IF_EMPTY_HANDLE_VALUE(
+          isolate,
+          Runtime::SetObjectProperty(
+              isolate, target, name, value, NONE, SLOPPY),
+          Handle<JSObject>());
+    }
   }
 
   // Second fill all stack locals.
   for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
     Handle<String> name(scope_info->StackLocalName(i));
     Handle<Object> value(frame_inspector->GetExpression(i), isolate);
     if (value->IsTheHole()) continue;
 
     RETURN_IF_EMPTY_HANDLE_VALUE(
         isolate,
@@ skipping 13 matching lines @@
   if (inlined_jsframe_index != 0 || frame->is_optimized()) {
     // Optimized frames are not supported.
     // TODO(yangguo): make sure all code deoptimized when debugger is active
     //                and assert that this cannot happen.
     return;
   }
 
   Handle<SharedFunctionInfo> shared(function->shared());
   Handle<ScopeInfo> scope_info(shared->scope_info());
 
-  // Parameters.
-  for (int i = 0; i < scope_info->ParameterCount(); ++i) {
-    ASSERT(!frame->GetParameter(i)->IsTheHole());
-    HandleScope scope(isolate);
-    Handle<Object> value = GetProperty(
-        isolate, target, Handle<String>(scope_info->ParameterName(i)));
-    frame->SetParameterValue(i, *value);
+  // Parameters. Parameters in generators are context-allocated.
+  if (!shared->is_generator()) {
+    for (int i = 0; i < scope_info->ParameterCount(); ++i) {
+      ASSERT(!frame->GetParameter(i)->IsTheHole());

[Michael Starzinger, 2014/03/24 10:24:28]

Likewise, instead of baking in arbitrary special handling of generators here, I
would argue that we should instead use the provided scoping information and just
skip shadowed parameters in this loop.

+      HandleScope scope(isolate);
+      Handle<Object> value = GetProperty(
+          isolate, target, Handle<String>(scope_info->ParameterName(i)));
+      frame->SetParameterValue(i, *value);
+    }
   }
 
   // Stack locals.
   for (int i = 0; i < scope_info->StackLocalCount(); ++i) {
     if (frame->GetExpression(i)->IsTheHole()) continue;
     HandleScope scope(isolate);
     Handle<Object> value = GetProperty(
         isolate, target, Handle<String>(scope_info->StackLocalName(i)));
     frame->SetExpression(i, *value);
   }
@@ skipping 3572 matching lines @@
     // Handle last resort GC and make sure to allow future allocations
     // to grow the heap without causing GCs (if possible).
     isolate->counters()->gc_last_resort_from_js()->Increment();
     isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags,
                                        "Runtime::PerformGC");
   }
 }
 
 
 } }  // namespace v8::internal