From 8ed81aca8a3b57d52e40a9f8e22c5aa3e6f07a86 Mon Sep 17 00:00:00 2001
From: "sgjesse@chromium.org"
 <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Thu, 28 May 2009 11:30:54 +0000
Subject: [PATCH] Improve debugger property lookup.

before performing debugger property lookup make sure the current context is set to the context active before the debugger was entered.

Make the use of the LookupResult GC safe in debugger property lookup.
Review URL: http://codereview.chromium.org/115855

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2071 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/runtime.cc | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/src/runtime.cc b/src/runtime.cc
index 1f67c4d1c..2fcdff156 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5519,17 +5519,8 @@ static Object* DebugLookupResultValue(Object* receiver, String* name,
     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();
@@ -5575,6 +5566,17 @@ static Object* Runtime_DebugGetPropertyDetails(Arguments args) {
   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()) {
@@ -5609,15 +5611,29 @@ static Object* Runtime_DebugGetPropertyDetails(Arguments args) {
   }
 
   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);
-- 
2.34.1