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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3393 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/accessors.cc b/src/accessors.cc
index 09a3886..56cf135 100644 (file)
--- a/src/accessors.cc
+++ b/src/accessors.cc
@@ -349,29 +349,38 @@ const AccessorDescriptor Accessors::ScriptContextData = {
 
 
 //
-// 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 @@ Object* Accessors::ScriptGetEvalFromPosition(Object* object, void*) {
 
   // 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
 };

diff --git a/src/accessors.h b/src/accessors.h
index 51d322e..7a840a1 100644 (file)
--- a/src/accessors.h
+++ b/src/accessors.h
@@ -51,8 +51,9 @@ namespace internal {
   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 @@ class Accessors : public AllStatic {
   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*);
 

diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index 1e08fd1..deda96f 100644 (file)
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -1111,21 +1111,29 @@ bool Genesis::InstallNatives() {
             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());

diff --git a/src/compiler.cc b/src/compiler.cc
index 331d242..f5ba319 100644 (file)
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -179,7 +179,9 @@ static Handle<JSFunction> MakeFunction(bool is_global,
     // 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));

diff --git a/src/factory.cc b/src/factory.cc
index c63e005..83775ef 100644 (file)
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -189,7 +189,7 @@ Handle<Script> Factory::NewScript(Handle<String> source) {
   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;

diff --git a/src/messages.js b/src/messages.js
index b0004b1..1e5053d 100644 (file)
--- a/src/messages.js
+++ b/src/messages.js
@@ -629,10 +629,7 @@ CallSite.prototype.isEval = function () {
 
 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 @@ CallSite.prototype.getColumnNumber = function () {
   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 @@ CallSite.prototype.isConstructor = function () {
   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) {

diff --git a/src/mirror-delay.js b/src/mirror-delay.js
index 82fb9c2..ba663b2 100644 (file)
--- a/src/mirror-delay.js
+++ b/src/mirror-delay.js
@@ -1793,16 +1793,21 @@ ScriptMirror.prototype.context = function() {
 };
 
 
-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 @@ JSONProtocolSerializer.prototype.serialize_ = function(mirror, reference,
       // 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());

diff --git a/src/objects-debug.cc b/src/objects-debug.cc
index c269d11..20c68f7 100644 (file)
--- a/src/objects-debug.cc
+++ b/src/objects-debug.cc
@@ -1145,10 +1145,12 @@ void Script::ScriptPrint() {
   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");
 }
 

diff --git a/src/objects-inl.h b/src/objects-inl.h
index 71e16ec..8514a41 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -2325,7 +2325,7 @@ ACCESSORS(Script, wrapper, Proxy, kWrapperOffset)
 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)
 

diff --git a/src/objects.h b/src/objects.h
index e6c44f3..671978a 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -3005,9 +3005,9 @@ class Script: public Struct {
   // [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 @@ class Script: public Struct {
   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:

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 9fe760a..6d6c174 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -8525,7 +8525,7 @@ static int GetGlobalObjectsCount() {
 }
 
 
-TEST(Bug528) {
+TEST(Regress528) {
   v8::V8::Initialize();
 
   v8::HandleScope scope;
@@ -8580,8 +8580,8 @@ TEST(Bug528) {
     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.