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

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
 };

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*);
 

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());

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));

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;

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) {

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());

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");
 }
 

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)
 

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:

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.