Revision: 3393
Author: [email protected]
Date: Tue Dec  1 06:36:45 2009
Log: Remove the last context dependent reference from the Script object

For scripts originating from a call to eval the Script object used to hold  
a reference to the function from where the eval was called together with  
the code offset within that function of the eval call. This is used by the  
stack trace and is part of the debugger protocol. In order to avoid storing  
the function the script, the position within the script and the name of the  
function calling eval is stored instead. This avoids holding context  
dependent objects in the script object.

The calculation of the position of the eval in the script holding the eval  
is now done when the eval script is compiled as it is not possible to  
postpone this unless a reference is kept to the generated code for the  
function calling eval.

BUG=http://code.google.com/p/v8/issues/detail?id=528
TEST=cctest/test-api/Regress528
Review URL: http://codereview.chromium.org/450034
http://code.google.com/p/v8/source/detail?r=3393

Modified:
  /branches/bleeding_edge/src/accessors.cc
  /branches/bleeding_edge/src/accessors.h
  /branches/bleeding_edge/src/bootstrapper.cc
  /branches/bleeding_edge/src/compiler.cc
  /branches/bleeding_edge/src/factory.cc
  /branches/bleeding_edge/src/messages.js
  /branches/bleeding_edge/src/mirror-delay.js
  /branches/bleeding_edge/src/objects-debug.cc
  /branches/bleeding_edge/src/objects-inl.h
  /branches/bleeding_edge/src/objects.h
  /branches/bleeding_edge/test/cctest/test-api.cc

=======================================
--- /branches/bleeding_edge/src/accessors.cc    Fri Nov 27 06:10:48 2009
+++ /branches/bleeding_edge/src/accessors.cc    Tue Dec  1 06:36:45 2009
@@ -349,29 +349,38 @@


  //
-// Accessors::ScriptGetEvalFromFunction
+// Accessors::ScriptGetEvalFromScript
  //


-Object* Accessors::ScriptGetEvalFromFunction(Object* object, void*) {
+Object* Accessors::ScriptGetEvalFromScript(Object* object, void*) {
    Object* script = JSValue::cast(object)->value();
-  return Script::cast(script)->eval_from_function();
+  if (!Script::cast(script)->eval_from_shared()->IsUndefined()) {
+    Handle<SharedFunctionInfo> eval_from_shared(
+         
SharedFunctionInfo::cast(Script::cast(script)->eval_from_shared()));
+
+    if (eval_from_shared->script()->IsScript()) {
+      Handle<Script>  
eval_from_script(Script::cast(eval_from_shared->script()));
+      return *GetScriptWrapper(eval_from_script);
+    }
+  }
+  return Heap::undefined_value();
  }


-const AccessorDescriptor Accessors::ScriptEvalFromFunction = {
-  ScriptGetEvalFromFunction,
+const AccessorDescriptor Accessors::ScriptEvalFromScript = {
+  ScriptGetEvalFromScript,
    IllegalSetter,
    0
  };


  //
-// Accessors::ScriptGetEvalFromPosition
+// Accessors::ScriptGetEvalFromScriptPosition
  //


-Object* Accessors::ScriptGetEvalFromPosition(Object* object, void*) {
+Object* Accessors::ScriptGetEvalFromScriptPosition(Object* object, void*) {
    HandleScope scope;
    Handle<Script> script(Script::cast(JSValue::cast(object)->value()));

@@ -383,14 +392,42 @@

    // Get the function from where eval was called and find the source  
position
    // from the instruction offset.
-  Handle<Code>  
code(JSFunction::cast(script->eval_from_function())->code());
+  Handle<Code> code(SharedFunctionInfo::cast(
+      script->eval_from_shared())->code());
    return Smi::FromInt(code->SourcePosition(code->instruction_start() +
                        script->eval_from_instructions_offset()->value()));
  }


-const AccessorDescriptor Accessors::ScriptEvalFromPosition = {
-  ScriptGetEvalFromPosition,
+const AccessorDescriptor Accessors::ScriptEvalFromScriptPosition = {
+  ScriptGetEvalFromScriptPosition,
+  IllegalSetter,
+  0
+};
+
+
+//
+// Accessors::ScriptGetEvalFromFunctionName
+//
+
+
+Object* Accessors::ScriptGetEvalFromFunctionName(Object* object, void*) {
+  Object* script = JSValue::cast(object)->value();
+  Handle<SharedFunctionInfo> shared(SharedFunctionInfo::cast(
+      Script::cast(script)->eval_from_shared()));
+
+
+  // Find the name of the function calling eval.
+  if (!shared->name()->IsUndefined()) {
+    return shared->name();
+  } else {
+    return shared->inferred_name();
+  }
+}
+
+
+const AccessorDescriptor Accessors::ScriptEvalFromFunctionName = {
+  ScriptGetEvalFromFunctionName,
    IllegalSetter,
    0
  };
=======================================
--- /branches/bleeding_edge/src/accessors.h     Mon Jun  8 03:47:49 2009
+++ /branches/bleeding_edge/src/accessors.h     Tue Dec  1 06:36:45 2009
@@ -51,8 +51,9 @@
    V(ScriptCompilationType)          \
    V(ScriptLineEnds)                 \
    V(ScriptContextData)              \
-  V(ScriptEvalFromFunction)         \
-  V(ScriptEvalFromPosition)         \
+  V(ScriptEvalFromScript)           \
+  V(ScriptEvalFromScriptPosition)   \
+  V(ScriptEvalFromFunctionName)     \
    V(ObjectPrototype)

  // Accessors contains all predefined proxy accessors.
@@ -95,8 +96,9 @@
    static Object* ScriptGetCompilationType(Object* object, void*);
    static Object* ScriptGetLineEnds(Object* object, void*);
    static Object* ScriptGetContextData(Object* object, void*);
-  static Object* ScriptGetEvalFromFunction(Object* object, void*);
-  static Object* ScriptGetEvalFromPosition(Object* object, void*);
+  static Object* ScriptGetEvalFromScript(Object* object, void*);
+  static Object* ScriptGetEvalFromScriptPosition(Object* object, void*);
+  static Object* ScriptGetEvalFromFunctionName(Object* object, void*);
    static Object* ObjectGetPrototype(Object* receiver, void*);
    static Object* ObjectSetPrototype(JSObject* receiver, Object* value,  
void*);

=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Wed Nov 25 08:46:56 2009
+++ /branches/bleeding_edge/src/bootstrapper.cc Tue Dec  1 06:36:45 2009
@@ -1111,21 +1111,29 @@
              Factory::LookupAsciiSymbol("context_data"),
              proxy_context_data,
              common_attributes);
-    Handle<Proxy> proxy_eval_from_function =
-        Factory::NewProxy(&Accessors::ScriptEvalFromFunction);
+    Handle<Proxy> proxy_eval_from_script =
+        Factory::NewProxy(&Accessors::ScriptEvalFromScript);
      script_descriptors =
          Factory::CopyAppendProxyDescriptor(
              script_descriptors,
-            Factory::LookupAsciiSymbol("eval_from_function"),
-            proxy_eval_from_function,
+            Factory::LookupAsciiSymbol("eval_from_script"),
+            proxy_eval_from_script,
              common_attributes);
-    Handle<Proxy> proxy_eval_from_position =
-        Factory::NewProxy(&Accessors::ScriptEvalFromPosition);
+    Handle<Proxy> proxy_eval_from_script_position =
+        Factory::NewProxy(&Accessors::ScriptEvalFromScriptPosition);
      script_descriptors =
          Factory::CopyAppendProxyDescriptor(
              script_descriptors,
-            Factory::LookupAsciiSymbol("eval_from_position"),
-            proxy_eval_from_position,
+            Factory::LookupAsciiSymbol("eval_from_script_position"),
+            proxy_eval_from_script_position,
+            common_attributes);
+    Handle<Proxy> proxy_eval_from_function_name =
+        Factory::NewProxy(&Accessors::ScriptEvalFromFunctionName);
+    script_descriptors =
+        Factory::CopyAppendProxyDescriptor(
+            script_descriptors,
+            Factory::LookupAsciiSymbol("eval_from_function_name"),
+            proxy_eval_from_function_name,
              common_attributes);

      Handle<Map> script_map = Handle<Map>(script_fun->initial_map());
=======================================
--- /branches/bleeding_edge/src/compiler.cc     Mon Nov 30 05:35:59 2009
+++ /branches/bleeding_edge/src/compiler.cc     Tue Dec  1 06:36:45 2009
@@ -179,7 +179,9 @@
      // called.
      if (is_eval) {
        JavaScriptFrameIterator it;
-      script->set_eval_from_function(it.frame()->function());
+      if (it.frame()->function()->IsJSFunction())
+      script->set_eval_from_shared(
+          JSFunction::cast(it.frame()->function())->shared());
        int offset = static_cast<int>(
            it.frame()->pc() - it.frame()->code()->instruction_start());
        script->set_eval_from_instructions_offset(Smi::FromInt(offset));
=======================================
--- /branches/bleeding_edge/src/factory.cc      Fri Nov 27 06:10:48 2009
+++ /branches/bleeding_edge/src/factory.cc      Tue Dec  1 06:36:45 2009
@@ -189,7 +189,7 @@
     
script->set_compilation_type(Smi::FromInt(Script::COMPILATION_TYPE_HOST));
    script->set_wrapper(*wrapper);
    script->set_line_ends(Heap::undefined_value());
-  script->set_eval_from_function(Heap::undefined_value());
+  script->set_eval_from_shared(Heap::undefined_value());
    script->set_eval_from_instructions_offset(Smi::FromInt(0));

    return script;
=======================================
--- /branches/bleeding_edge/src/messages.js     Fri Nov 27 06:10:48 2009
+++ /branches/bleeding_edge/src/messages.js     Tue Dec  1 06:36:45 2009
@@ -629,10 +629,7 @@

  CallSite.prototype.getEvalOrigin = function () {
    var script = %FunctionGetScript(this.fun);
-  if (!script || script.compilation_type != 1)
-    return null;
-  return new CallSite(null, script.eval_from_function,
-      script.eval_from_position);
+  return FormatEvalOrigin(script);
  };

  CallSite.prototype.getFunction = function () {
@@ -700,7 +697,7 @@
    if (script) {
      location = script.locationFromPosition(this.pos, true);
    }
-  return location ? location.column : null;
+  return location ? location.column + 1: null;
  };

  CallSite.prototype.isNative = function () {
@@ -719,12 +716,44 @@
    return this.fun === constructor;
  };

+function FormatEvalOrigin(script) {
+  var eval_origin = "";
+  if (script.eval_from_function_name) {
+    eval_origin += script.eval_from_function_name;
+  } else {
+    eval_origin +=  "<anonymous>";
+  }
+
+  var eval_from_script = script.eval_from_script;
+  if (eval_from_script) {
+    if (eval_from_script.compilation_type == 1) {
+      // eval script originated from another eval.
+      eval_origin += " (eval at " + FormatEvalOrigin(eval_from_script)  
+ ")";
+    } else {
+      // eval script originated from "real" scource.
+      if (eval_from_script.name) {
+        eval_origin += " (" + eval_from_script.name;
+        var location =  
eval_from_script.locationFromPosition(script.eval_from_script_position,  
true);
+        if (location) {
+          eval_origin += ":" + (location.line + 1);
+          eval_origin += ":" + (location.column + 1);
+        }
+        eval_origin += ")"
+      } else {
+        eval_origin += " (unknown source)";
+      }
+    }
+  }
+
+  return eval_origin;
+};
+
  function FormatSourcePosition(frame) {
    var fileLocation = "";
    if (frame.isNative()) {
      fileLocation = "native";
    } else if (frame.isEval()) {
-    fileLocation = "eval at " +  
FormatSourcePosition(frame.getEvalOrigin());
+    fileLocation = "eval at " + frame.getEvalOrigin();
    } else {
      var fileName = frame.getFileName();
      if (fileName) {
=======================================
--- /branches/bleeding_edge/src/mirror-delay.js Fri Oct 30 09:38:37 2009
+++ /branches/bleeding_edge/src/mirror-delay.js Tue Dec  1 06:36:45 2009
@@ -1793,16 +1793,21 @@
  };


-ScriptMirror.prototype.evalFromFunction = function() {
-  return MakeMirror(this.script_.eval_from_function);
+ScriptMirror.prototype.evalFromScript = function() {
+  return MakeMirror(this.script_.eval_from_script);
  };


+ScriptMirror.prototype.evalFromFunctionName = function() {
+  return MakeMirror(this.script_.eval_from_function_name);
+};
+
+
  ScriptMirror.prototype.evalFromLocation = function() {
-  var eval_from_function = this.evalFromFunction();
-  if (!eval_from_function.isUndefined()) {
-    var position = this.script_.eval_from_position;
-    return eval_from_function.script().locationFromPosition(position,  
true);
+  var eval_from_script = this.evalFromScript();
+  if (!eval_from_script.isUndefined()) {
+    var position = this.script_.eval_from_script_position;
+    return eval_from_script.locationFromPosition(position, true);
    }
  };

@@ -2080,12 +2085,15 @@
        // For compilation type eval emit information on the script from  
which
        // eval was called if a script is present.
        if (mirror.compilationType() == 1 &&
-          mirror.evalFromFunction().script()) {
+          mirror.evalFromScript()) {
          content.evalFromScript =
-            this.serializeReference(mirror.evalFromFunction().script());
+            this.serializeReference(mirror.evalFromScript());
          var evalFromLocation = mirror.evalFromLocation()
          content.evalFromLocation = { line: evalFromLocation.line,
                                       column: evalFromLocation.column}
+        if (mirror.evalFromFunctionName()) {
+          content.evalFromFunctionName = mirror.evalFromFunctionName();
+        }
        }
        if (mirror.context()) {
          content.context = this.serializeReference(mirror.context());
=======================================
--- /branches/bleeding_edge/src/objects-debug.cc        Fri Nov 27 06:10:48 2009
+++ /branches/bleeding_edge/src/objects-debug.cc        Tue Dec  1 06:36:45 2009
@@ -1145,10 +1145,12 @@
    compilation_type()->ShortPrint();
    PrintF("\n - line ends: ");
    line_ends()->ShortPrint();
-  PrintF("\n - eval from function: ");
-  eval_from_function()->ShortPrint();
-  PrintF("\n - eval from instructions offset: ");
-  eval_from_instructions_offset()->ShortPrint();
+  PrintF("\n - eval from script: ");
+  eval_from_script()->ShortPrint();
+  PrintF("\n - eval from script position: ");
+  eval_from_script_position()->ShortPrint();
+  PrintF("\n - eval from function name: ");
+  eval_from_function_name()->ShortPrint();
    PrintF("\n");
  }

=======================================
--- /branches/bleeding_edge/src/objects-inl.h   Fri Nov 27 06:10:48 2009
+++ /branches/bleeding_edge/src/objects-inl.h   Tue Dec  1 06:36:45 2009
@@ -2325,7 +2325,7 @@
  ACCESSORS(Script, type, Smi, kTypeOffset)
  ACCESSORS(Script, compilation_type, Smi, kCompilationTypeOffset)
  ACCESSORS(Script, line_ends, Object, kLineEndsOffset)
-ACCESSORS(Script, eval_from_function, Object, kEvalFromFunctionOffset)
+ACCESSORS(Script, eval_from_shared, Object, kEvalFromSharedOffset)
  ACCESSORS(Script, eval_from_instructions_offset, Smi,
            kEvalFrominstructionsOffsetOffset)

=======================================
--- /branches/bleeding_edge/src/objects.h       Fri Nov 27 06:10:48 2009
+++ /branches/bleeding_edge/src/objects.h       Tue Dec  1 06:36:45 2009
@@ -3005,9 +3005,9 @@
    // [line_ends]: FixedArray of line ends positions.
    DECL_ACCESSORS(line_ends, Object)

-  // [eval_from_function]: for eval scripts the funcion from which eval was
-  // called.
-  DECL_ACCESSORS(eval_from_function, Object)
+  // [eval_from_shared]: for eval scripts the shared funcion info for the
+  // function from which eval was called.
+  DECL_ACCESSORS(eval_from_shared, Object)

    // [eval_from_instructions_offset]: the instruction offset in the code  
for the
    // function from which eval was called where eval was called.
@@ -3035,9 +3035,9 @@
    static const int kCompilationTypeOffset = kTypeOffset + kPointerSize;
    static const int kLineEndsOffset = kCompilationTypeOffset + kPointerSize;
    static const int kIdOffset = kLineEndsOffset + kPointerSize;
-  static const int kEvalFromFunctionOffset = kIdOffset + kPointerSize;
+  static const int kEvalFromSharedOffset = kIdOffset + kPointerSize;
    static const int kEvalFrominstructionsOffsetOffset =
-      kEvalFromFunctionOffset + kPointerSize;
+      kEvalFromSharedOffset + kPointerSize;
    static const int kSize = kEvalFrominstructionsOffsetOffset +  
kPointerSize;

   private:
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc     Tue Dec  1 03:16:21 2009
+++ /branches/bleeding_edge/test/cctest/test-api.cc     Tue Dec  1 06:36:45 2009
@@ -8525,7 +8525,7 @@
  }


-TEST(Bug528) {
+TEST(Regress528) {
    v8::V8::Initialize();

    v8::HandleScope scope;
@@ -8580,8 +8580,8 @@
      v8::internal::Heap::CollectAllGarbage(false);
      if (GetGlobalObjectsCount() == 1) break;
    }
-  CHECK_EQ(10, gc_count);  // Should be 1 or 2 when the bug is resolved.
-  CHECK_EQ(2, GetGlobalObjectsCount());  // Should be 1 when resolved.
+  CHECK_GE(2, gc_count);
+  CHECK_EQ(1, GetGlobalObjectsCount());

    // Looking up the line number for an exception creates reference from the
    // compilation cache to the global object.

-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to