Speed up access to global variables from eval scopes. Traverse the
authorager@chromium.org <ager@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 24 Feb 2009 13:11:53 +0000 (13:11 +0000)
committerager@chromium.org <ager@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 24 Feb 2009 13:11:53 +0000 (13:11 +0000)
surrounding context to figure out if the variable could be global.  If
the variable could be global we check context extension objects at
runtime and use a global LoadIC if no variables have been introduced
by eval.

Fix crash bug when loading function arguments from inside eval.  The
shadowed variable in the DYNAMIC_LOCAL case does not rewrite to a slot in
that case.

Review URL: http://codereview.chromium.org/28027

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

17 files changed:
src/codegen-arm.cc
src/codegen-ia32.cc
src/compilation-cache.cc
src/compilation-cache.h
src/compiler.cc
src/compiler.h
src/contexts.cc
src/contexts.h
src/objects.cc
src/objects.h
src/runtime.cc
src/scopeinfo.cc
src/scopeinfo.h
src/scopes.cc
src/scopes.h
test/mjsunit/global-load-from-eval.js [new file with mode: 0644]
test/mjsunit/global-load-from-nested-eval.js [new file with mode: 0644]

index 31ff620..8a47f8c 100644 (file)
@@ -2022,12 +2022,16 @@ void CodeGenerator::LoadFromSlot(Slot* slot, TypeofState typeof_state) {
 
     } else if (slot->var()->mode() == Variable::DYNAMIC_LOCAL) {
       Slot* potential_slot = slot->var()->local_if_not_shadowed()->slot();
-      __ ldr(r0,
-             ContextSlotOperandCheckExtensions(potential_slot,
-                                               r1,
-                                               r2,
-                                               &slow));
-      __ b(&done);
+      // Only generate the fast case for locals that rewrite to slots.
+      // This rules out argument loads.
+      if (potential_slot != NULL) {
+        __ ldr(r0,
+               ContextSlotOperandCheckExtensions(potential_slot,
+                                                 r1,
+                                                 r2,
+                                                 &slow));
+        __ b(&done);
+      }
     }
 
     __ bind(&slow);
@@ -2074,7 +2078,8 @@ void CodeGenerator::LoadFromGlobalSlotCheckExtensions(Slot* slot,
   // Check that no extension objects have been created by calls to
   // eval from the current scope to the global scope.
   Register context = cp;
-  for (Scope* s = scope(); s != NULL; s = s->outer_scope()) {
+  Scope* s = scope();
+  while (s != NULL) {
     if (s->num_heap_slots() > 0) {
       if (s->calls_eval()) {
         // Check that extension is NULL.
@@ -2089,7 +2094,27 @@ void CodeGenerator::LoadFromGlobalSlotCheckExtensions(Slot* slot,
     }
     // If no outer scope calls eval, we do not need to check more
     // context extensions.
-    if (!s->outer_scope_calls_eval()) break;
+    if (!s->outer_scope_calls_eval() || s->is_eval_scope()) break;
+    s = s->outer_scope();
+  }
+
+  if (s->is_eval_scope()) {
+    Label next, fast;
+    if (!context.is(tmp)) __ mov(tmp, Operand(context));
+    __ bind(&next);
+    // Terminate at global context.
+    __ ldr(tmp2, FieldMemOperand(tmp, HeapObject::kMapOffset));
+    __ cmp(tmp2, Operand(Factory::global_context_map()));
+    __ b(eq, &fast);
+    // Check that extension is NULL.
+    __ ldr(tmp2, ContextOperand(tmp, Context::EXTENSION_INDEX));
+    __ tst(tmp2, tmp2);
+    __ b(ne, slow);
+    // Load next context in chain.
+    __ ldr(tmp, ContextOperand(tmp, Context::CLOSURE_INDEX));
+    __ ldr(tmp, FieldMemOperand(tmp, JSFunction::kContextOffset));
+    __ b(&next);
+    __ bind(&fast);
   }
 
   // All extension objects were empty and it is safe to use a global
index e652150..e095541 100644 (file)
@@ -2365,11 +2365,15 @@ void CodeGenerator::LoadFromSlot(Slot* slot, TypeofState typeof_state) {
 
     } else if (slot->var()->mode() == Variable::DYNAMIC_LOCAL) {
       Slot* potential_slot = slot->var()->local_if_not_shadowed()->slot();
-      __ mov(eax,
-             ContextSlotOperandCheckExtensions(potential_slot,
-                                               ebx,
-                                               &slow));
-      __ jmp(&done);
+      // Only generate the fast case for locals that rewrite to slots.
+      // This rules out argument loads.
+      if (potential_slot != NULL) {
+        __ mov(eax,
+               ContextSlotOperandCheckExtensions(potential_slot,
+                                                 ebx,
+                                                 &slow));
+        __ jmp(&done);
+      }
     }
 
     __ bind(&slow);
@@ -2414,7 +2418,8 @@ void CodeGenerator::LoadFromGlobalSlotCheckExtensions(Slot* slot,
   // Check that no extension objects have been created by calls to
   // eval from the current scope to the global scope.
   Register context = esi;
-  for (Scope* s = scope(); s != NULL; s = s->outer_scope()) {
+  Scope* s = scope();
+  while (s != NULL) {
     if (s->num_heap_slots() > 0) {
       if (s->calls_eval()) {
         // Check that extension is NULL.
@@ -2427,8 +2432,28 @@ void CodeGenerator::LoadFromGlobalSlotCheckExtensions(Slot* slot,
       context = tmp;
     }
     // If no outer scope calls eval, we do not need to check more
-    // context extensions.
-    if (!s->outer_scope_calls_eval()) break;
+    // context extensions.  If we have reached an eval scope, we check
+    // all extensions from this point.
+    if (!s->outer_scope_calls_eval() || s->is_eval_scope()) break;
+    s = s->outer_scope();
+  }
+
+  if (s->is_eval_scope()) {
+    Label next, fast;
+    if (!context.is(tmp)) __ mov(tmp, Operand(context));
+    __ bind(&next);
+    // Terminate at global context.
+    __ cmp(FieldOperand(tmp, HeapObject::kMapOffset),
+           Immediate(Factory::global_context_map()));
+    __ j(equal, &fast);
+    // Check that extension is NULL.
+    __ cmp(ContextOperand(tmp, Context::EXTENSION_INDEX), Immediate(0));
+    __ j(not_equal, slow, not_taken);
+    // Load next context in chain.
+    __ mov(tmp, ContextOperand(tmp, Context::CLOSURE_INDEX));
+    __ mov(tmp, FieldOperand(tmp, JSFunction::kContextOffset));
+    __ jmp(&next);
+    __ bind(&fast);
   }
 
   // All extension objects were empty and it is safe to use a global
index bf32621..d5a0048 100644 (file)
@@ -102,6 +102,25 @@ static Handle<JSFunction> Lookup(Handle<String> source,
 }
 
 
+static Handle<JSFunction> Lookup(Handle<String> source,
+                                 Handle<Context> context,
+                                 CompilationCache::Entry entry) {
+  // Make sure not to leak the table into the surrounding handle
+  // scope. Otherwise, we risk keeping old tables around even after
+  // having cleared the cache.
+  Object* result;
+  { HandleScope scope;
+    Handle<CompilationCacheTable> table = GetTable(entry);
+    result = table->LookupEval(*source, *context);
+  }
+  if (result->IsJSFunction()) {
+    return Handle<JSFunction>(JSFunction::cast(result));
+  } else {
+    return Handle<JSFunction>::null();
+  }
+}
+
+
 Handle<JSFunction> CompilationCache::LookupScript(Handle<String> source,
                                                   Handle<Object> name,
                                                   int line_offset,
@@ -120,9 +139,10 @@ Handle<JSFunction> CompilationCache::LookupScript(Handle<String> source,
 
 
 Handle<JSFunction> CompilationCache::LookupEval(Handle<String> source,
+                                                Handle<Context> context,
                                                 Entry entry) {
   ASSERT(entry == EVAL_GLOBAL || entry == EVAL_CONTEXTUAL);
-  Handle<JSFunction> result = Lookup(source, entry);
+  Handle<JSFunction> result = Lookup(source, context, entry);
   if (result.is_null()) {
     Counters::compilation_cache_misses.Increment();
   } else {
@@ -142,6 +162,17 @@ void CompilationCache::PutFunction(Handle<String> source,
 }
 
 
+void CompilationCache::PutEvalFunction(Handle<String> source,
+                                       Handle<Context> context,
+                                       Entry entry,
+                                       Handle<JSFunction> boilerplate) {
+  HandleScope scope;
+  ASSERT(boilerplate->IsBoilerplate());
+  Handle<CompilationCacheTable> table = GetTable(entry);
+  CALL_HEAP_FUNCTION_VOID(table->PutEval(*source, *context, *boilerplate));
+}
+
+
 Handle<FixedArray> CompilationCache::LookupRegExp(Handle<String> source,
                                                   JSRegExp::Flags flags) {
   Handle<CompilationCacheTable> table = GetTable(REGEXP);
index abe6110..3fc65c2 100644 (file)
@@ -55,10 +55,11 @@ class CompilationCache {
                                          int line_offset,
                                          int column_offset);
 
-  // Finds the function boilerplate for a source string for
-  // eval. Returns an empty handle if the cache doesn't contain a
-  // script for the given source string.
+  // Finds the function boilerplate for a source string for eval in a
+  // given context.  Returns an empty handle if the cache doesn't
+  // contain a script for the given source string.
   static Handle<JSFunction> LookupEval(Handle<String> source,
+                                       Handle<Context> context,
                                        Entry entry);
 
   // Returns the regexp data associated with the given regexp if it
@@ -78,6 +79,14 @@ class CompilationCache {
                           Entry entry,
                           Handle<JSFunction> boilerplate);
 
+  // Associate the (source, context->closure()->shared(), kind)
+  // triple with the boilerplate. This may overwrite an existing
+  // mapping.
+  static void PutEvalFunction(Handle<String> source,
+                              Handle<Context> context,
+                              Entry entry,
+                              Handle<JSFunction> boilerplate);
+
   // Clear the cache - also used to initialize the cache at startup.
   static void Clear();
 
index 737ec4e..896e9d4 100644 (file)
@@ -40,6 +40,7 @@ namespace v8 { namespace internal {
 
 static Handle<Code> MakeCode(FunctionLiteral* literal,
                              Handle<Script> script,
+                             Handle<Context> context,
                              bool is_eval) {
   ASSERT(literal != NULL);
 
@@ -55,7 +56,7 @@ static Handle<Code> MakeCode(FunctionLiteral* literal,
   // so this doesn't re-allocate variables repeatedly.
   Scope* top = literal->scope();
   while (top->outer_scope() != NULL) top = top->outer_scope();
-  top->AllocateVariables();
+  top->AllocateVariables(context);
 
 #ifdef DEBUG
   if (Bootstrapper::IsActive() ?
@@ -81,6 +82,7 @@ static Handle<Code> MakeCode(FunctionLiteral* literal,
 static Handle<JSFunction> MakeFunction(bool is_global,
                                        bool is_eval,
                                        Handle<Script> script,
+                                       Handle<Context> context,
                                        v8::Extension* extension,
                                        ScriptDataImpl* pre_data) {
   ZoneScope zone_scope(DELETE_ON_EXIT);
@@ -113,7 +115,7 @@ static Handle<JSFunction> MakeFunction(bool is_global,
   StatsRateScope timer(rate);
 
   // Compile the code.
-  Handle<Code> code = MakeCode(lit, script, is_eval);
+  Handle<Code> code = MakeCode(lit, script, context, is_eval);
 
   // Check for stack-overflow exceptions.
   if (code.is_null()) {
@@ -201,7 +203,12 @@ Handle<JSFunction> Compiler::Compile(Handle<String> source,
     }
 
     // Compile the function and add it to the cache.
-    result = MakeFunction(true, false, script, extension, pre_data);
+    result = MakeFunction(true,
+                          false,
+                          script,
+                          Handle<Context>::null(),
+                          extension,
+                          pre_data);
     if (extension == NULL && !result.is_null()) {
       CompilationCache::PutFunction(source, CompilationCache::SCRIPT, result);
     }
@@ -219,6 +226,7 @@ Handle<JSFunction> Compiler::Compile(Handle<String> source,
 
 
 Handle<JSFunction> Compiler::CompileEval(Handle<String> source,
+                                         Handle<Context> context,
                                          int line_offset,
                                          bool is_global) {
   int source_length = source->length();
@@ -233,14 +241,15 @@ Handle<JSFunction> Compiler::CompileEval(Handle<String> source,
 
   // Do a lookup in the compilation cache; if the entry is not there,
   // invoke the compiler and add the result to the cache.
-  Handle<JSFunction> result = CompilationCache::LookupEval(source, entry);
+  Handle<JSFunction> result =
+      CompilationCache::LookupEval(source, context, entry);
   if (result.is_null()) {
     // Create a script object describing the script to be compiled.
     Handle<Script> script = Factory::NewScript(source);
     script->set_line_offset(Smi::FromInt(line_offset));
-    result = MakeFunction(is_global, true, script, NULL, NULL);
+    result = MakeFunction(is_global, true, script, context, NULL, NULL);
     if (!result.is_null()) {
-      CompilationCache::PutFunction(source, entry, result);
+      CompilationCache::PutEvalFunction(source, context, entry, result);
     }
   }
 
@@ -290,7 +299,7 @@ bool Compiler::CompileLazy(Handle<SharedFunctionInfo> shared,
   StatsRateScope timer(&Counters::compile_lazy);
 
   // Compile the code.
-  Handle<Code> code = MakeCode(lit, script, false);
+  Handle<Code> code = MakeCode(lit, script, Handle<Context>::null(), false);
 
   // Check for stack-overflow exception.
   if (code.is_null()) {
index 0f4143c..4f589b7 100644 (file)
@@ -58,6 +58,7 @@ class Compiler : public AllStatic {
 
   // Compile a String source within a context for Eval.
   static Handle<JSFunction> CompileEval(Handle<String> source,
+                                        Handle<Context> context,
                                         int line_offset,
                                         bool is_global);
 
index ec84a1d..36b5488 100644 (file)
@@ -198,6 +198,42 @@ Handle<Object> Context::Lookup(Handle<String> name, ContextLookupFlags flags,
 }
 
 
+bool Context::GlobalIfNotShadowedByEval(Handle<String> name) {
+  Context* context = this;
+
+  // Check that there is no local with the given name in contexts
+  // before the global context and check that there are no context
+  // extension objects (conservative check for with statements).
+  while (!context->IsGlobalContext()) {
+    // Check if the context is a potentially a with context.
+    if (context->has_extension()) return false;
+
+    // Not a with context so it must be a function context.
+    ASSERT(context->is_function_context());
+
+    // Check non-parameter locals.
+    Handle<Code> code(context->closure()->code());
+    Variable::Mode mode;
+    int index = ScopeInfo<>::ContextSlotIndex(*code, *name, &mode);
+    ASSERT(index < 0 || index >= MIN_CONTEXT_SLOTS);
+    if (index >= 0) return false;
+
+    // Check parameter locals.
+    int param_index = ScopeInfo<>::ParameterIndex(*code, *name);
+    if (param_index >= 0) return false;
+
+    // Check context only holding the function name variable.
+    index = ScopeInfo<>::FunctionContextSlotIndex(*code, *name);
+    if (index >= 0) return false;
+    context = Context::cast(context->closure()->context());
+  }
+
+  // No local or potential with statement found so the variable is
+  // global unless it is shadowed by an eval-introduced variable.
+  return true;
+}
+
+
 #ifdef DEBUG
 bool Context::IsBootstrappingOrContext(Object* object) {
   // During bootstrapping we allow all objects to pass as
index ac4c18e..3c25bd3 100644 (file)
@@ -304,6 +304,15 @@ class Context: public FixedArray {
   Handle<Object> Lookup(Handle<String> name, ContextLookupFlags flags,
                         int* index_, PropertyAttributes* attributes);
 
+  // Determine if a local variable with the given name exists in a
+  // context.  Do not consider context extension objects.  This is
+  // used for compiling code using eval.  If the context surrounding
+  // the eval call does not have a local variable with this name and
+  // does not contain a with statement the property is global unless
+  // it is shadowed by a property in an extension object introduced by
+  // eval.
+  bool GlobalIfNotShadowedByEval(Handle<String> name);
+
   // Code generation support.
   static int SlotOffset(int index) {
     return kHeaderSize + index * kPointerSize - kHeapObjectTag;
index 20b7139..a8d4151 100644 (file)
@@ -5850,6 +5850,20 @@ int JSObject::GetEnumElementKeys(FixedArray* storage) {
 }
 
 
+// Thomas Wang, Integer Hash Functions.
+// http://www.concentric.net/~Ttwang/tech/inthash.htm
+static uint32_t ComputeIntegerHash(uint32_t key) {
+  uint32_t hash = key;
+  hash = ~hash + (hash << 15);  // hash = (hash << 15) - hash - 1;
+  hash = hash ^ (hash >> 12);
+  hash = hash + (hash << 2);
+  hash = hash ^ (hash >> 4);
+  hash = hash * 2057;  // hash = (hash + (hash << 3)) + (hash << 11);
+  hash = hash ^ (hash >> 16);
+  return hash;
+}
+
+
 // The NumberKey uses carries the uint32_t as key.
 // This avoids allocation in HasProperty.
 class NumberKey : public HashTableKey {
@@ -5861,20 +5875,7 @@ class NumberKey : public HashTableKey {
     return number_ == ToUint32(number);
   }
 
-  // Thomas Wang, Integer Hash Functions.
-  // http://www.concentric.net/~Ttwang/tech/inthash.htm
-  static uint32_t ComputeHash(uint32_t key) {
-    uint32_t hash = key;
-    hash = ~hash + (hash << 15);  // hash = (hash << 15) - hash - 1;
-    hash = hash ^ (hash >> 12);
-    hash = hash + (hash << 2);
-    hash = hash ^ (hash >> 4);
-    hash = hash * 2057;  // hash = (hash + (hash << 3)) + (hash << 11);
-    hash = hash ^ (hash >> 16);
-    return hash;
-  }
-
-  uint32_t Hash() { return ComputeHash(number_); }
+  uint32_t Hash() { return ComputeIntegerHash(number_); }
 
   HashFunction GetHashFunction() { return NumberHash; }
 
@@ -5883,7 +5884,7 @@ class NumberKey : public HashTableKey {
   }
 
   static uint32_t NumberHash(Object* obj) {
-    return ComputeHash(ToUint32(obj));
+    return ComputeIntegerHash(ToUint32(obj));
   }
 
   static uint32_t ToUint32(Object* obj) {
@@ -5921,6 +5922,70 @@ class StringKey : public HashTableKey {
   String* string_;
 };
 
+
+// StringSharedKeys are used as keys in the eval cache.
+class StringSharedKey : public HashTableKey {
+ public:
+  StringSharedKey(String* source, SharedFunctionInfo* shared)
+      : source_(source), shared_(shared) { }
+
+  bool IsMatch(Object* other) {
+    if (!other->IsFixedArray()) return false;
+    FixedArray* pair = FixedArray::cast(other);
+    SharedFunctionInfo* shared = SharedFunctionInfo::cast(pair->get(0));
+    if (shared != shared_) return false;
+    String* source = String::cast(pair->get(1));
+    return source->Equals(source_);
+  }
+
+  typedef uint32_t (*HashFunction)(Object* obj);
+
+  virtual HashFunction GetHashFunction() { return StringSharedHash; }
+
+  static uint32_t StringSharedHashHelper(String* source,
+                                         SharedFunctionInfo* shared) {
+    uint32_t hash = source->Hash();
+    if (shared->HasSourceCode()) {
+      // Instead of using the SharedFunctionInfo pointer in the hash
+      // code computation, we use a combination of the hash of the
+      // script source code and the start and end positions.  We do
+      // this to ensure that the cache entries can survive garbage
+      // collection.
+      Script* script = Script::cast(shared->script());
+      hash ^= String::cast(script->source())->Hash();
+      hash += shared->start_position();
+    }
+    return hash;
+  }
+
+  static uint32_t StringSharedHash(Object* obj) {
+    FixedArray* pair = FixedArray::cast(obj);
+    SharedFunctionInfo* shared = SharedFunctionInfo::cast(pair->get(0));
+    String* source = String::cast(pair->get(1));
+    return StringSharedHashHelper(source, shared);
+  }
+
+  virtual uint32_t Hash() {
+    return StringSharedHashHelper(source_, shared_);
+  }
+
+  virtual Object* GetObject() {
+    Object* obj = Heap::AllocateFixedArray(2);
+    if (obj->IsFailure()) return obj;
+    FixedArray* pair = FixedArray::cast(obj);
+    pair->set(0, shared_);
+    pair->set(1, source_);
+    return pair;
+  }
+
+  virtual bool IsStringKey() { return false; }
+
+ private:
+  String* source_;
+  SharedFunctionInfo* shared_;
+};
+
+
 // RegExpKey carries the source and flags of a regular expression as key.
 class RegExpKey : public HashTableKey {
  public:
@@ -6234,6 +6299,14 @@ Object* CompilationCacheTable::Lookup(String* src) {
 }
 
 
+Object* CompilationCacheTable::LookupEval(String* src, Context* context) {
+  StringSharedKey key(src, context->closure()->shared());
+  int entry = FindEntry(&key);
+  if (entry == -1) return Heap::undefined_value();
+  return get(EntryToIndex(entry) + 1);
+}
+
+
 Object* CompilationCacheTable::LookupRegExp(String* src,
                                             JSRegExp::Flags flags) {
   RegExpKey key(src, flags);
@@ -6258,6 +6331,27 @@ Object* CompilationCacheTable::Put(String* src, Object* value) {
 }
 
 
+Object* CompilationCacheTable::PutEval(String* src,
+                                       Context* context,
+                                       Object* value) {
+  StringSharedKey key(src, context->closure()->shared());
+  Object* obj = EnsureCapacity(1, &key);
+  if (obj->IsFailure()) return obj;
+
+  CompilationCacheTable* cache =
+      reinterpret_cast<CompilationCacheTable*>(obj);
+  int entry = cache->FindInsertionEntry(src, key.Hash());
+
+  Object* k = key.GetObject();
+  if (k->IsFailure()) return k;
+
+  cache->set(EntryToIndex(entry), k);
+  cache->set(EntryToIndex(entry) + 1, value);
+  cache->ElementAdded();
+  return cache;
+}
+
+
 Object* CompilationCacheTable::PutRegExp(String* src,
                                          JSRegExp::Flags flags,
                                          FixedArray* value) {
index 81cea6b..e7b6de6 100644 (file)
@@ -2994,8 +2994,10 @@ class CompilationCacheTable: public HashTable<0, 2> {
  public:
   // Find cached value for a string key, otherwise return null.
   Object* Lookup(String* src);
+  Object* LookupEval(String* src, Context* context);
   Object* LookupRegExp(String* source, JSRegExp::Flags flags);
   Object* Put(String* src, Object* value);
+  Object* PutEval(String* src, Context* context, Object* value);
   Object* PutRegExp(String* src, JSRegExp::Flags flags, FixedArray* value);
 
   static inline CompilationCacheTable* cast(Object* obj);
index db291e2..b687057 100644 (file)
@@ -4007,11 +4007,11 @@ static Object* Runtime_CompileString(Arguments args) {
   CONVERT_ARG_CHECKED(String, source, 0);
   CONVERT_ARG_CHECKED(Smi, line_offset, 1);
 
-  // Compile source string.
+  // Compile source string in the global context.
+  Handle<Context> context(Top::context()->global_context());
   Handle<JSFunction> boilerplate =
-      Compiler::CompileEval(source, line_offset->value(), true);
+      Compiler::CompileEval(source, context, line_offset->value(), true);
   if (boilerplate.is_null()) return Failure::Exception();
-  Handle<Context> context(Top::context()->global_context());
   Handle<JSFunction> fun =
       Factory::NewFunctionFromBoilerplate(boilerplate, context);
   return *fun;
@@ -4033,8 +4033,9 @@ static Object* CompileDirectEval(Handle<String> source) {
   Handle<Context> context(Context::cast(frame->context()));
   bool is_global = context->IsGlobalContext();
 
-  // Compile source string.
-  Handle<JSFunction> boilerplate = Compiler::CompileEval(source, 0, is_global);
+  // Compile source string in the current context.
+  Handle<JSFunction> boilerplate =
+      Compiler::CompileEval(source, context, 0, is_global);
   if (boilerplate.is_null()) return Failure::Exception();
   Handle<JSFunction> fun =
     Factory::NewFunctionFromBoilerplate(boilerplate, context);
@@ -5594,7 +5595,10 @@ static Object* Runtime_DebugEvaluate(Arguments args) {
       Factory::NewStringFromAscii(Vector<const char>(source_str,
                                                      source_str_length));
   Handle<JSFunction> boilerplate =
-      Compiler::CompileEval(function_source, 0, context->IsGlobalContext());
+      Compiler::CompileEval(function_source,
+                            context,
+                            0,
+                            context->IsGlobalContext());
   if (boilerplate.is_null()) return Failure::Exception();
   Handle<JSFunction> compiled_function =
       Factory::NewFunctionFromBoilerplate(boilerplate, context);
@@ -5651,7 +5655,8 @@ static Object* Runtime_DebugEvaluateGlobal(Arguments args) {
   Handle<Context> context = Top::global_context();
 
   // Compile the source to be evaluated.
-  Handle<JSFunction> boilerplate(Compiler::CompileEval(source, 0, true));
+  Handle<JSFunction> boilerplate =
+      Handle<JSFunction>(Compiler::CompileEval(source, context, 0, true));
   if (boilerplate.is_null()) return Failure::Exception();
   Handle<JSFunction> compiled_function =
       Handle<JSFunction>(Factory::NewFunctionFromBoilerplate(boilerplate,
index 3c56acc..6d2ade8 100644 (file)
@@ -50,6 +50,7 @@ static int CompareLocal(Variable* const* v, Variable* const* w) {
 template<class Allocator>
 ScopeInfo<Allocator>::ScopeInfo(Scope* scope)
     : function_name_(Factory::empty_symbol()),
+      calls_eval_(scope->calls_eval()),
       parameters_(scope->num_parameters()),
       stack_slots_(scope->num_stack_slots()),
       context_slots_(scope->num_heap_slots()),
@@ -254,6 +255,7 @@ ScopeInfo<Allocator>::ScopeInfo(Code* code)
   Object** p0 = &Memory::Object_at(code->sinfo_start());
   Object** p = p0;
   p = ReadSymbol(p, &function_name_);
+  p = ReadBool(p, &calls_eval_);
   p = ReadList<Allocator>(p, &context_slots_, &context_modes_);
   p = ReadList<Allocator>(p, &parameters_);
   p = ReadList<Allocator>(p, &stack_slots_);
@@ -267,6 +269,12 @@ static inline Object** WriteInt(Object** p, int x) {
 }
 
 
+static inline Object** WriteBool(Object** p, bool b) {
+  *p++ = Smi::FromInt(b ? 1 : 0);
+  return p;
+}
+
+
 static inline Object** WriteSymbol(Object** p, Handle<String> s) {
   *p++ = *s;
   return p;
@@ -306,8 +314,8 @@ static Object** WriteList(Object** p,
 
 template<class Allocator>
 int ScopeInfo<Allocator>::Serialize(Code* code) {
-  // function name, length & sentinel for 3 tables:
-  const int extra_slots = 1 + 2 * 3;
+  // function name, calls eval, length & sentinel for 3 tables:
+  const int extra_slots = 1 + 1 + 2 * 3;
   int size = (extra_slots +
               context_slots_.length() * 2 +
               parameters_.length() +
@@ -318,6 +326,7 @@ int ScopeInfo<Allocator>::Serialize(Code* code) {
     Object** p0 = &Memory::Object_at(code->sinfo_start());
     Object** p = p0;
     p = WriteSymbol(p, function_name_);
+    p = WriteBool(p, calls_eval_);
     p = WriteList(p, &context_slots_, &context_modes_);
     p = WriteList(p, &parameters_);
     p = WriteList(p, &stack_slots_);
@@ -338,8 +347,8 @@ void ScopeInfo<Allocator>::IterateScopeInfo(Code* code, ObjectVisitor* v) {
 
 static Object** ContextEntriesAddr(Code* code) {
   ASSERT(code->sinfo_size() > 0);
-  // +1 for function name:
-  return &Memory::Object_at(code->sinfo_start()) + 1;
+  // +2 for function name and calls eval:
+  return &Memory::Object_at(code->sinfo_start()) + 2;
 }
 
 
@@ -362,6 +371,19 @@ static Object** StackSlotEntriesAddr(Code* code) {
 
 
 template<class Allocator>
+bool ScopeInfo<Allocator>::CallsEval(Code* code) {
+  if (code->sinfo_size() > 0) {
+    // +1 for function name:
+    Object** p = &Memory::Object_at(code->sinfo_start()) + 1;
+    bool calls_eval;
+    p = ReadBool(p, &calls_eval);
+    return calls_eval;
+  }
+  return true;
+}
+
+
+template<class Allocator>
 int ScopeInfo<Allocator>::NumberOfStackSlots(Code* code) {
   if (code->sinfo_size() > 0) {
     Object** p = StackSlotEntriesAddr(code);
index f59c02c..dbe235a 100644 (file)
@@ -92,7 +92,6 @@ class ScopeInfo BASE_EMBEDDED {
   Handle<String> LocalName(int i) const;
   int NumberOfLocals() const;
 
-
   // --------------------------------------------------------------------------
   // The following functions provide quick access to scope info details
   // for runtime routines w/o the need to explicitly create a ScopeInfo
@@ -102,6 +101,9 @@ class ScopeInfo BASE_EMBEDDED {
   // encoding of it's information in a Code object, which is why these
   // functions are in this class.
 
+  // Does this scope call eval.
+  static bool CallsEval(Code* code);
+
   // Return the number of stack slots for code.
   static int NumberOfStackSlots(Code* code);
 
@@ -132,7 +134,6 @@ class ScopeInfo BASE_EMBEDDED {
   // must be a symbol (canonicalized).
   static int FunctionContextSlotIndex(Code* code, String* name);
 
-
   // --------------------------------------------------------------------------
   // Debugging support
 
@@ -142,6 +143,7 @@ class ScopeInfo BASE_EMBEDDED {
 
  private:
   Handle<String> function_name_;
+  bool calls_eval_;
   List<Handle<String>, Allocator > parameters_;
   List<Handle<String>, Allocator > stack_slots_;
   List<Handle<String>, Allocator > context_slots_;
index 86fecc8..0a381df 100644 (file)
@@ -304,7 +304,7 @@ template void Scope::CollectUsedVariables(
     List<Variable*, PreallocatedStorage>* locals);
 
 
-void Scope::AllocateVariables() {
+void Scope::AllocateVariables(Handle<Context> context) {
   ASSERT(outer_scope_ == NULL);  // eval or global scopes only
 
   // 1) Propagate scope information.
@@ -319,7 +319,7 @@ void Scope::AllocateVariables() {
   // 2) Resolve variables.
   Scope* global_scope = NULL;
   if (is_global_scope()) global_scope = this;
-  ResolveVariablesRecursively(global_scope);
+  ResolveVariablesRecursively(global_scope, context);
 
   // 3) Allocate variables.
   AllocateVariablesRecursively();
@@ -577,7 +577,9 @@ Variable* Scope::LookupRecursive(Handle<String> name,
 }
 
 
-void Scope::ResolveVariable(Scope* global_scope, VariableProxy* proxy) {
+void Scope::ResolveVariable(Scope* global_scope,
+                            Handle<Context> context,
+                            VariableProxy* proxy) {
   ASSERT(global_scope == NULL || global_scope->is_global_scope());
 
   // If the proxy is already resolved there's nothing to do
@@ -638,10 +640,16 @@ void Scope::ResolveVariable(Scope* global_scope, VariableProxy* proxy) {
 
       } else if (outer_scope_is_eval_scope_) {
         // No with statements and we did not find a local and the code
-        // is executed with a call to eval.  We don't know anything
-        // because we do not have information about the scopes
-        // surrounding the eval call.
-        var = NonLocal(proxy->name(), Variable::DYNAMIC);
+        // is executed with a call to eval.  The context contains
+        // scope information that we can use to determine if the
+        // variable is global if it is not shadowed by eval-introduced
+        // variables.
+        if (context->GlobalIfNotShadowedByEval(proxy->name())) {
+          var = NonLocal(proxy->name(), Variable::DYNAMIC_GLOBAL);
+
+        } else {
+          var = NonLocal(proxy->name(), Variable::DYNAMIC);
+        }
 
       } else {
         // No with statements and we did not find a local and the code
@@ -657,17 +665,18 @@ void Scope::ResolveVariable(Scope* global_scope, VariableProxy* proxy) {
 }
 
 
-void Scope::ResolveVariablesRecursively(Scope* global_scope) {
+void Scope::ResolveVariablesRecursively(Scope* global_scope,
+                                        Handle<Context> context) {
   ASSERT(global_scope == NULL || global_scope->is_global_scope());
 
   // Resolve unresolved variables for this scope.
   for (int i = 0; i < unresolved_.length(); i++) {
-    ResolveVariable(global_scope, unresolved_[i]);
+    ResolveVariable(global_scope, context, unresolved_[i]);
   }
 
   // Resolve unresolved variables for inner scopes.
   for (int i = 0; i < inner_scopes_.length(); i++) {
-    inner_scopes_[i]->ResolveVariablesRecursively(global_scope);
+    inner_scopes_[i]->ResolveVariablesRecursively(global_scope, context);
   }
 }
 
index 30545ba..e78bd2a 100644 (file)
@@ -170,6 +170,11 @@ class Scope: public ZoneObject {
   bool calls_eval() const  { return scope_calls_eval_; }
   bool outer_scope_calls_eval() const  { return outer_scope_calls_eval_; }
 
+  // Is this scope inside a with statement.
+  bool inside_with() const  { return scope_inside_with_; }
+  // Does this scope contain a with statement.
+  bool contains_with() const  { return scope_contains_with_; }
+
   // The scope immediately surrounding this scope, or NULL.
   Scope* outer_scope() const  { return outer_scope_; }
 
@@ -216,10 +221,15 @@ class Scope: public ZoneObject {
   template<class Allocator>
   void CollectUsedVariables(List<Variable*, Allocator>* locals);
 
-  // Resolve and fill in the allocation information for all variables in
-  // this scopes. Must be called *after* all scopes have been processed
-  // (parsed) to ensure that unresolved variables can be resolved properly.
-  void AllocateVariables();
+  // Resolve and fill in the allocation information for all variables
+  // in this scopes. Must be called *after* all scopes have been
+  // processed (parsed) to ensure that unresolved variables can be
+  // resolved properly.
+  //
+  // In the case of code compiled and run using 'eval', the context
+  // parameter is the context in which eval was called.  In all other
+  // cases the context parameter is an empty handle.
+  void AllocateVariables(Handle<Context> context);
 
   // Result of variable allocation.
   int num_stack_slots() const  { return num_stack_slots_; }
@@ -308,8 +318,11 @@ class Scope: public ZoneObject {
   Variable* LookupRecursive(Handle<String> name,
                             bool inner_lookup,
                             Variable** invalidated_local);
-  void ResolveVariable(Scope* global_scope, VariableProxy* proxy);
-  void ResolveVariablesRecursively(Scope* global_scope);
+  void ResolveVariable(Scope* global_scope,
+                       Handle<Context> context,
+                       VariableProxy* proxy);
+  void ResolveVariablesRecursively(Scope* global_scope,
+                                   Handle<Context> context);
 
   // Scope analysis.
   bool PropagateScopeInfo(bool outer_scope_calls_eval,
diff --git a/test/mjsunit/global-load-from-eval.js b/test/mjsunit/global-load-from-eval.js
new file mode 100644 (file)
index 0000000..ad40932
--- /dev/null
@@ -0,0 +1,85 @@
+// Copyright 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
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Tests global loads from eval.
+
+var x = 27;
+
+function test() {
+  function g() {
+    assertEquals(27, eval('x'));
+    function h() {
+      // Shadow with local variable.
+      var x = 22;
+      assertEquals(22, eval('x'));
+      function i(x) {
+        // Shadow with parameter.
+        assertEquals(44, eval('x'));
+        function j() {
+          assertEquals(x, eval('x'));
+          // Shadow with function name.
+          function x() {
+            assertEquals(x, eval('x'));
+          }
+          x();
+        }
+        j();
+      }
+      i(44);
+    }
+    h();
+  }
+  g();
+}
+
+test();
+
+// Test loading of globals from deeply nested eval.  This code is a
+// bit complicated, but the complication is needed to check that the
+// code that loads the global variable accounts for the fact that the
+// global variable becomes shadowed by an eval-introduced variable.
+var result = 0;
+function testDeep(source, load, test) {
+  eval(source);
+  function f() {
+    var y = 23;
+    function g() {
+      var z = 25;
+      function h() {
+        eval(load);
+        eval(test);
+      }
+      h();
+    }
+    g();
+  }
+  f();
+}
+testDeep('1', 'result = x', 'assertEquals(27, result)');
+// Because of the eval-cache, the 'result = x' code gets reused.  This
+// time in a context where the 'x' variable has been shadowed.
+testDeep('var x = 1', 'result = x', 'assertEquals(1, result)');
diff --git a/test/mjsunit/global-load-from-nested-eval.js b/test/mjsunit/global-load-from-nested-eval.js
new file mode 100644 (file)
index 0000000..3c7ff75
--- /dev/null
@@ -0,0 +1,66 @@
+// Copyright 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
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Tests global loads from nested eval.
+
+var x = 42;
+
+// Load the global.
+function test(source) {
+  eval('eval(' + source +')');
+}
+test('assertEquals(42, x)');
+
+// Shadow variable with a with statement.
+function testWith(source) {
+  with ({ x: 1 }) {
+    eval('eval(' + source +')');
+  }
+}
+testWith('assertEquals(1, x)');
+
+// Shadow variable with an eval-introduced variable.
+function testEval(source) {
+  eval('var x = 1');
+  function f() {
+    eval('eval('+ source + ')');
+  }
+  f();
+}
+testEval('assertEquals(1, x)');
+
+// Eval that does not shadow.
+function testEvalDontShadow(source) {
+  eval('1');
+  eval('eval(' + source +')');
+}
+testEvalDontShadow('assertEquals(42, x)');
+
+
+
+
+