Fixes operationPutByIdOptimizes such that they check that the put didn't
authormmirman@apple.com <mmirman@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Dec 2014 20:11:00 +0000 (20:11 +0000)
committermmirman@apple.com <mmirman@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Dec 2014 20:11:00 +0000 (20:11 +0000)
change the structure of the object who's property access is being
cached.
https://bugs.webkit.org/show_bug.cgi?id=139500

Reviewed by Geoffrey Garen.

* jit/JITOperations.cpp:
(JSC::operationPutByIdStrictOptimize): saved the structure before the put.
(JSC::operationPutByIdNonStrictOptimize): ditto.
(JSC::operationPutByIdDirectStrictOptimize): ditto.
(JSC::operationPutByIdDirectNonStrictOptimize): ditto.
* jit/Repatch.cpp:
(JSC::tryCachePutByID): Added argument for the old structure
(JSC::repatchPutByID): Added argument for the old structure
* jit/Repatch.h:
* tests/stress/put-by-id-build-list-order-recurse.js:
Added test that fails without this patch.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@177380 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/jit/Repatch.cpp
Source/JavaScriptCore/jit/Repatch.h
Source/JavaScriptCore/tests/stress/put-by-id-build-list-order-recurse.js [new file with mode: 0644]

index b4c8eeb..23c4935 100644 (file)
@@ -1,3 +1,24 @@
+2014-12-16  Matthew Mirman  <mmirman@apple.com>
+
+        Fixes operationPutByIdOptimizes such that they check that the put didn't
+        change the structure of the object who's property access is being
+        cached.
+        https://bugs.webkit.org/show_bug.cgi?id=139500
+
+        Reviewed by Geoffrey Garen.
+
+        * jit/JITOperations.cpp:
+        (JSC::operationPutByIdStrictOptimize): saved the structure before the put.
+        (JSC::operationPutByIdNonStrictOptimize): ditto.
+        (JSC::operationPutByIdDirectStrictOptimize): ditto.
+        (JSC::operationPutByIdDirectNonStrictOptimize): ditto.
+        * jit/Repatch.cpp:
+        (JSC::tryCachePutByID): Added argument for the old structure
+        (JSC::repatchPutByID): Added argument for the old structure
+        * jit/Repatch.h:
+        * tests/stress/put-by-id-build-list-order-recurse.js: 
+        Added test that fails without this patch.
+
 2014-12-15  Chris Dumez  <cdumez@apple.com>
 
         [iOS] Add feature counting support
index ce749ed..caa34dd 100644 (file)
@@ -273,14 +273,15 @@ void JIT_OPERATION operationPutByIdStrictOptimize(ExecState* exec, StructureStub
     JSValue value = JSValue::decode(encodedValue);
     JSValue baseValue = JSValue::decode(encodedBase);
     PutPropertySlot slot(baseValue, true, exec->codeBlock()->putByIdContext());
-    
+
+    Structure* structure = baseValue.isCell() ? baseValue.asCell()->structure(*vm) : nullptr;
     baseValue.put(exec, ident, value, slot);
     
     if (accessType != static_cast<AccessType>(stubInfo->accessType))
         return;
     
     if (stubInfo->seen)
-        repatchPutByID(exec, baseValue, ident, slot, *stubInfo, NotDirect);
+        repatchPutByID(exec, baseValue, structure, ident, slot, *stubInfo, NotDirect);
     else
         stubInfo->seen = true;
 }
@@ -296,14 +297,15 @@ void JIT_OPERATION operationPutByIdNonStrictOptimize(ExecState* exec, StructureS
     JSValue value = JSValue::decode(encodedValue);
     JSValue baseValue = JSValue::decode(encodedBase);
     PutPropertySlot slot(baseValue, false, exec->codeBlock()->putByIdContext());
-    
+
+    Structure* structure = baseValue.isCell() ? baseValue.asCell()->structure(*vm) : nullptr;    
     baseValue.put(exec, ident, value, slot);
     
     if (accessType != static_cast<AccessType>(stubInfo->accessType))
         return;
     
     if (stubInfo->seen)
-        repatchPutByID(exec, baseValue, ident, slot, *stubInfo, NotDirect);
+        repatchPutByID(exec, baseValue, structure, ident, slot, *stubInfo, NotDirect);
     else
         stubInfo->seen = true;
 }
@@ -320,13 +322,14 @@ void JIT_OPERATION operationPutByIdDirectStrictOptimize(ExecState* exec, Structu
     JSObject* baseObject = asObject(JSValue::decode(encodedBase));
     PutPropertySlot slot(baseObject, true, exec->codeBlock()->putByIdContext());
     
+    Structure* structure = baseObject->structure(*vm);
     baseObject->putDirect(exec->vm(), ident, value, slot);
     
     if (accessType != static_cast<AccessType>(stubInfo->accessType))
         return;
     
     if (stubInfo->seen)
-        repatchPutByID(exec, baseObject, ident, slot, *stubInfo, Direct);
+        repatchPutByID(exec, baseObject, structure, ident, slot, *stubInfo, Direct);
     else
         stubInfo->seen = true;
 }
@@ -343,13 +346,14 @@ void JIT_OPERATION operationPutByIdDirectNonStrictOptimize(ExecState* exec, Stru
     JSObject* baseObject = asObject(JSValue::decode(encodedBase));
     PutPropertySlot slot(baseObject, false, exec->codeBlock()->putByIdContext());
     
+    Structure* structure = baseObject->structure(*vm);
     baseObject->putDirect(exec->vm(), ident, value, slot);
     
     if (accessType != static_cast<AccessType>(stubInfo->accessType))
         return;
     
     if (stubInfo->seen)
-        repatchPutByID(exec, baseObject, ident, slot, *stubInfo, Direct);
+        repatchPutByID(exec, baseObject, structure, ident, slot, *stubInfo, Direct);
     else
         stubInfo->seen = true;
 }
index fd6a374..3c9b145 100644 (file)
@@ -1224,7 +1224,7 @@ static void emitPutTransitionStub(
             structure);
 }
 
-static InlineCacheAction tryCachePutByID(ExecState* exec, JSValue baseValue, const Identifier& ident, const PutPropertySlot& slot, StructureStubInfo& stubInfo, PutKind putKind)
+static InlineCacheAction tryCachePutByID(ExecState* exec, JSValue baseValue, Structure* structure, const Identifier& ident, const PutPropertySlot& slot, StructureStubInfo& stubInfo, PutKind putKind)
 {
     if (Options::forceICFailure())
         return GiveUpOnCache;
@@ -1235,7 +1235,10 @@ static InlineCacheAction tryCachePutByID(ExecState* exec, JSValue baseValue, con
     if (!baseValue.isCell())
         return GiveUpOnCache;
     JSCell* baseCell = baseValue.asCell();
-    Structure* structure = baseCell->structure(*vm);
+    
+    if (baseCell->structure(*vm)->id() != structure->id())
+        return GiveUpOnCache;
+
     Structure* oldStructure = structure->previousID();
     
     if (!slot.isCacheablePut() && !slot.isCacheableCustom() && !slot.isCacheableSetter())
@@ -1333,11 +1336,11 @@ static InlineCacheAction tryCachePutByID(ExecState* exec, JSValue baseValue, con
     return GiveUpOnCache;
 }
 
-void repatchPutByID(ExecState* exec, JSValue baseValue, const Identifier& propertyName, const PutPropertySlot& slot, StructureStubInfo& stubInfo, PutKind putKind)
+void repatchPutByID(ExecState* exec, JSValue baseValue, Structure* structure, const Identifier& propertyName, const PutPropertySlot& slot, StructureStubInfo& stubInfo, PutKind putKind)
 {
     GCSafeConcurrentJITLocker locker(exec->codeBlock()->m_lock, exec->vm().heap);
     
-    if (tryCachePutByID(exec, baseValue, propertyName, slot, stubInfo, putKind) == GiveUpOnCache)
+    if (tryCachePutByID(exec, baseValue, structure, propertyName, slot, stubInfo, putKind) == GiveUpOnCache)
         repatchCall(exec->codeBlock(), stubInfo.callReturnLocation, appropriateGenericPutByIdFunction(slot, putKind));
 }
 
index cb33420..f37ac7b 100644 (file)
@@ -36,7 +36,7 @@ namespace JSC {
 void repatchGetByID(ExecState*, JSValue, const Identifier&, const PropertySlot&, StructureStubInfo&);
 void buildGetByIDList(ExecState*, JSValue, const Identifier&, const PropertySlot&, StructureStubInfo&);
 void buildGetByIDProtoList(ExecState*, JSValue, const Identifier&, const PropertySlot&, StructureStubInfo&);
-void repatchPutByID(ExecState*, JSValue, const Identifier&, const PutPropertySlot&, StructureStubInfo&, PutKind);
+void repatchPutByID(ExecState*, JSValue, Structure*, const Identifier&, const PutPropertySlot&, StructureStubInfo&, PutKind);
 void buildPutByIdList(ExecState*, JSValue, Structure*, const Identifier&, const PutPropertySlot&, StructureStubInfo&, PutKind);
 void repatchIn(ExecState*, JSCell*, const Identifier&, bool wasFound, const PropertySlot&, StructureStubInfo&);
 void linkFor(ExecState*, CallLinkInfo&, CodeBlock*, JSFunction* callee, MacroAssemblerCodePtr, CodeSpecializationKind, RegisterPreservationMode);
diff --git a/Source/JavaScriptCore/tests/stress/put-by-id-build-list-order-recurse.js b/Source/JavaScriptCore/tests/stress/put-by-id-build-list-order-recurse.js
new file mode 100644 (file)
index 0000000..09d0219
--- /dev/null
@@ -0,0 +1,35 @@
+var count = 0;
+
+function setter(value) {
+    Object.defineProperty(
+        this, "f", {
+        enumerable: true,
+                configurable: true,
+                writable: true,
+                value: 32
+                });
+    var o = Object.create(this);
+    var currentCount = count++;
+    var str = "for (var i = " + currentCount + "; i < " + (100 + currentCount) + "; ++i)\n"
+            + "    o.f\n";
+    eval(str);
+}
+
+function foo(o) {
+    o.f = 42;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 1000; ++i) {
+    var o = {};
+    o.__defineSetter__("f", setter);
+
+    foo(o);
+    if (o.f != 32)
+        throw ("Error 1: "+o.f);
+
+    foo(o);
+    if (o.f != 42)
+        throw ("Error 2: "+o.f);
+}
\ No newline at end of file