Improve debugger property lookup

src/runtime.cc

 // Copyright 2006-2009 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 5501 matching lines @@
               result->holder())->FastPropertyAt(result->GetFieldIndex());
       if (value->IsTheHole()) {
         return Heap::undefined_value();
       }
       return value;
     case CONSTANT_FUNCTION:
       return result->GetConstantFunction();
     case CALLBACKS: {
       Object* structure = result->GetCallbackObject();
       if (structure->IsProxy() || structure->IsAccessorInfo()) {
-        if (Debug::debugger_entry()) {
-          // SaveContext scope. It will restore debugger context after the
-          // getter execution.
-          SaveContext save;
-          Top::set_context(*Debug::debugger_entry()->GetContext());
-          value = receiver->GetPropertyWithCallback(
-              receiver, structure, name, result->holder());
-        } else {
-          value = receiver->GetPropertyWithCallback(
-              receiver, structure, name, result->holder());
-        }
+        value = receiver->GetPropertyWithCallback(
+            receiver, structure, name, result->holder());
         if (value->IsException()) {
           value = Top::pending_exception();
           Top::clear_pending_exception();
           if (caught_exception != NULL) {
             *caught_exception = true;
           }
         }
         return value;
       } else {
         return Heap::undefined_value();
@@ skipping 25 matching lines @@
 // Items 2-4 are only filled if the property has either a getter or a setter
 // defined through __defineGetter__ and/or __defineSetter__.
 static Object* Runtime_DebugGetPropertyDetails(Arguments args) {
   HandleScope scope;
 
   ASSERT(args.length() == 2);
 
   CONVERT_ARG_CHECKED(JSObject, obj, 0);
   CONVERT_ARG_CHECKED(String, name, 1);
 
+  // Make sure to set the current context to the context before the debugger was
+  // entered (if the debugger is entered). The reason for switching context here
+  // is that for some property lookups (accessors and interceptors) callbacks
+  // into the embedding application can occour, and the embedding application
+  // could have the assumption that its own global context is the current
+  // context and not some internal debugger context.
+  SaveContext save;
+  if (Debug::InDebugger()) {
+    Top::set_context(*Debug::debugger_entry()->GetContext());
+  }
+
   // Skip the global proxy as it has no properties and always delegates to the
   // real global object.
   if (obj->IsJSGlobalProxy()) {
     obj = Handle<JSObject>(JSObject::cast(obj->GetPrototype()));
   }
 
 
   // Check if the name is trivially convertible to an index and get the element
   // if so.
   uint32_t index;
@@ skipping 14 matching lines @@
     jsproto->LocalLookup(*name, &result);
     if (result.IsProperty()) {
       break;
     }
     if (i < length - 1) {
       jsproto = Handle<JSObject>(JSObject::cast(jsproto->GetPrototype()));
     }
   }
 
   if (result.IsProperty()) {
+    // LookupResult is not GC safe as all its members are raw object pointers.
+    // When calling DebugLookupResultValue GC can happen as this might invoke
+    // callbacks. After the call to DebugLookupResultValue the callback object
+    // in the LookupResult might still be needed. Put it into a handle for later
+    // use.
+    PropertyType result_type = result.type();
+    Handle<Object> result_callback_obj;
+    if (result_type == CALLBACKS) {
+      result_callback_obj = Handle<Object>(result.GetCallbackObject());
+    }
+
+    // Find the actual value. Don't use result after this call as it's content
+    // can be invalid.
     bool caught_exception = false;
     Object* value = DebugLookupResultValue(*obj, *name, &result,
                                            &caught_exception);
     if (value->IsFailure()) return value;
     Handle<Object> value_handle(value);
+
     // If the callback object is a fixed array then it contains JavaScript
     // getter and/or setter.
-    bool hasJavaScriptAccessors = result.type() == CALLBACKS &&
-                                  result.GetCallbackObject()->IsFixedArray();
+    bool hasJavaScriptAccessors = result_type == CALLBACKS &&
+                                  result_callback_obj->IsFixedArray();
     Handle<FixedArray> details =
         Factory::NewFixedArray(hasJavaScriptAccessors ? 5 : 2);
     details->set(0, *value_handle);
     details->set(1, result.GetPropertyDetails().AsSmi());
     if (hasJavaScriptAccessors) {
       details->set(2,
                    caught_exception ? Heap::true_value() : Heap::false_value());
       details->set(3, FixedArray::cast(result.GetCallbackObject())->get(0));
       details->set(4, FixedArray::cast(result.GetCallbackObject())->get(1));
     }
@@ skipping 1421 matching lines @@
   } else {
     // Handle last resort GC and make sure to allow future allocations
     // to grow the heap without causing GCs (if possible).
     Counters::gc_last_resort_from_js.Increment();
     Heap::CollectAllGarbage();
   }
 }
 
 
 } }  // namespace v8::internal