Direct call to eval passes strict mode through.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 36a8e5c2aabc360e74cb58a40acd35fb6f4ed151..1979d80e2ad49861bdb9ebb793f6e2c025bc062b 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -7982,20 +7982,23 @@ class StringKey : public HashTableKey {
 // StringSharedKeys are used as keys in the eval cache.
 class StringSharedKey : public HashTableKey {
  public:
-  StringSharedKey(String* source, SharedFunctionInfo* shared)
-      : source_(source), shared_(shared) { }
+  StringSharedKey(String* source, SharedFunctionInfo* shared, bool strict)
+      : source_(source), shared_(shared), strict_(strict) { }
 
   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;
+    bool strict = Smi::cast(pair->get(2))->value() != 0;

[Lasse Reichstein, 2011/02/03 12:36:01]

Do you need to store the strictness in a separate field? Could we store it on
the SharedFunctionInfo instead?

[Martin Maly, 2011/02/04 01:02:34]

I believe I do need to store the strict flag. The strict is stored on the
SharedFunctionInfo but the SharedFunctionInfo is not always the info of the
immediately calling JSFunction. The canonical counter-example is:

"use strict";
eval("x = 1");

When we call into the Runtime_ResolvePossiblyDirectEval I do:

StackFrameLocator locator;
JavaScriptFrame* frame = locator.FindJavaScriptFrame(0);

And observe that:

Top::context()->closure() != frame->function()

and moreover:
Top::context()->closure()->shared()->strict_mode() == false

whereas:
((JSFunction*)(frame->function()))->shared()->strict_mode() == true

So the SharedFunctionInfo we have available here as a key into the eval cache is
not sufficient piece of information to key off of ... so I have to store the
strict mode flag in the key explicitly.

Unless the disparity I describe above is symptom of some deeper problem in the
system.

+    if (strict != strict_) return false;
     String* source = String::cast(pair->get(1));
     return source->Equals(source_);
   }
 
   static uint32_t StringSharedHashHelper(String* source,
-                                         SharedFunctionInfo* shared) {
+                                         SharedFunctionInfo* shared,
+                                         bool strict) {
     uint32_t hash = source->Hash();
     if (shared->HasSourceCode()) {
       // Instead of using the SharedFunctionInfo pointer in the hash
@@ -8005,36 +8008,40 @@ class StringSharedKey : public HashTableKey {
       // collection.
       Script* script = Script::cast(shared->script());
       hash ^= String::cast(script->source())->Hash();
+      hash ^= strict ? 0x8000 : 0x800;

[Martin Maly, 2011/02/02 23:56:22]

Just a guess, there may be better (or more V8 code compliant) hash tricks. Open
to any suggestions.

[Lasse Reichstein, 2011/02/03 12:36:01]

I don't think it matters what you do for strict-mode hash, since we are unlikely
to have the same code as both strict and non-strict. I wouldn't mind a hash
collision in that case (but xoring by 0x8000 (or anything non-zero) in one case
is fine, I don't see why you xor by something non-zero in the other case).

[Martin Maly, 2011/02/04 01:02:34]

Done.

       hash += shared->start_position();
     }
     return hash;
   }
 
   uint32_t Hash() {
-    return StringSharedHashHelper(source_, shared_);
+    return StringSharedHashHelper(source_, shared_, strict_);
   }
 
   uint32_t HashForObject(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);
+    bool strict = Smi::cast(pair->get(2))->value() != 0;
+    return StringSharedHashHelper(source, shared, strict);
   }
 
   MUST_USE_RESULT MaybeObject* AsObject() {
     Object* obj;
-    { MaybeObject* maybe_obj = Heap::AllocateFixedArray(2);
+    { MaybeObject* maybe_obj = Heap::AllocateFixedArray(3);
       if (!maybe_obj->ToObject(&obj)) return maybe_obj;
     }
     FixedArray* pair = FixedArray::cast(obj);
     pair->set(0, shared_);
     pair->set(1, source_);
+    pair->set(2, Smi::FromInt(strict_ ? 1 : 0));
     return pair;
   }
 
  private:
   String* source_;
   SharedFunctionInfo* shared_;
+  bool strict_;
 };
 
 
@@ -8993,8 +9000,10 @@ Object* CompilationCacheTable::Lookup(String* src) {
 }
 
 
-Object* CompilationCacheTable::LookupEval(String* src, Context* context) {
-  StringSharedKey key(src, context->closure()->shared());
+Object* CompilationCacheTable::LookupEval(String* src,
+                                          Context* context,
+                                          bool is_strict) {
+  StringSharedKey key(src, context->closure()->shared(), is_strict);

[Martin Maly, 2011/02/02 23:56:22]

It seems weird that this function hashes by:
context->closure()->shared()
shouldn't we want to hash here by executing function? As far as I can see,
context->closure()->shared() is not always the executing JSFunction (if ever).

[Lasse Reichstein, 2011/02/03 12:36:01]

I'm not sure I understand the question.
You can have many JSFunctions created using the same SharedFunctionInfo. The
SharedFunctionInfo corresponds to the function literal in the source, so it
identifies the function code as well as the closure that it has access to.
If a two functions have the same SharedFunctionInfo, they will be able to use
the same code, because they run in the same environment.

[Martin Maly, 2011/02/04 01:02:34]

Makes sense, thanks for the explanation.

   int entry = FindEntry(&key);
   if (entry == kNotFound) return Heap::undefined_value();
   return get(EntryToIndex(entry) + 1);
@@ -9029,8 +9038,10 @@ MaybeObject* CompilationCacheTable::Put(String* src, Object* value) {
 
 MaybeObject* CompilationCacheTable::PutEval(String* src,
                                             Context* context,
-                                            Object* value) {
-  StringSharedKey key(src, context->closure()->shared());
+                                            SharedFunctionInfo* value) {
+  StringSharedKey key(src,
+                      context->closure()->shared(),
+                      value->strict_mode());
   Object* obj;
   { MaybeObject* maybe_obj = EnsureCapacity(1, &key);
     if (!maybe_obj->ToObject(&obj)) return maybe_obj;