Merge r176676 - Fixes inline cache fast path accessing nonexistant ge…

…tters.

<rdar://problem/18416918>
https://bugs.webkit.org/show_bug.cgi?id=136961

Reviewed by Filip Pizlo.

Fixes a bug in inline caching where getters would have been able to
modify the property they are getting during
building the inline cache and then accessing that
property through the inline cache site causing a recursive
inline cache building and allowing the fast path of the cache to
try to load a getter for the property that no longer exists.

* jit/JITOperations.cpp: Switched use of get to getPropertySlot.
* runtime/JSCJSValue.h:
added getPropertySlot for when you don't want to perform the get quite yet but want
to fill out the slot.
* runtime/JSCJSValueInlines.h: Added implementation for getPropertySlot
(JSC::JSValue::get): changed to simply call getPropertySlot
(JSC::JSValue::getPropertySlot): added.
* tests/stress/recursive_property_redefine_during_inline_caching.js: Added test case for bug.
(test):

Canonical link: https://commits.webkit.org/154760.247@webkitgtk/2.6
git-svn-id: https://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.6@178286 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,28 @@
+2014-12-02  Matthew Mirman  <mmirman@apple.com>
+
+        Fixes inline cache fast path accessing nonexistant getters.
+        <rdar://problem/18416918>
+        https://bugs.webkit.org/show_bug.cgi?id=136961
+
+        Reviewed by Filip Pizlo.
+
+        Fixes a bug in inline caching where getters would have been able to 
+        modify the property they are getting during 
+        building the inline cache and then accessing that 
+        property through the inline cache site causing a recursive 
+        inline cache building and allowing the fast path of the cache to 
+        try to load a getter for the property that no longer exists.
+
+        * jit/JITOperations.cpp: Switched use of get to getPropertySlot.
+        * runtime/JSCJSValue.h: 
+        added getPropertySlot for when you don't want to perform the get quite yet but want 
+        to fill out the slot.
+        * runtime/JSCJSValueInlines.h: Added implementation for getPropertySlot
+        (JSC::JSValue::get): changed to simply call getPropertySlot
+        (JSC::JSValue::getPropertySlot): added.
+        * tests/stress/recursive_property_redefine_during_inline_caching.js: Added test case for bug.
+        (test):
+
 2014-12-01  Michael Saboff  <msaboff@apple.com>
 
         Crash (integer overflow) beneath ByteCodeParser::handleGetById typing in search field on weather.com

Source/JavaScriptCore/jit/JITOperations.cpp

@@ -147,31 +147,28 @@ EncodedJSValue JIT_OPERATION operationGetByIdBuildList(ExecState* exec, Structur
 
     JSValue baseValue = JSValue::decode(base);
     PropertySlot slot(baseValue);
-    JSValue result = baseValue.get(exec, ident, slot);
-
+    bool hasResult = baseValue.getPropertySlot(exec, ident, slot);
+    
     if (accessType == static_cast<AccessType>(stubInfo->accessType))
         buildGetByIDList(exec, baseValue, ident, slot, *stubInfo);
 
-    return JSValue::encode(result);
+    return JSValue::encode(hasResult? slot.getValue(exec, ident) : jsUndefined());
 }
 
 EncodedJSValue JIT_OPERATION operationGetByIdOptimize(ExecState* exec, StructureStubInfo* stubInfo, EncodedJSValue base, StringImpl* uid)
 {
     VM* vm = &exec->vm();
     NativeCallFrameTracer tracer(vm, exec);
     Identifier ident = uid->isEmptyUnique() ? Identifier::from(PrivateName(uid)) : Identifier(vm, uid);
-    AccessType accessType = static_cast<AccessType>(stubInfo->accessType);
 
     JSValue baseValue = JSValue::decode(base);
     PropertySlot slot(baseValue);
     JSValue result = baseValue.get(exec, ident, slot);
 
-    if (accessType == static_cast<AccessType>(stubInfo->accessType)) {
-        if (stubInfo->seen)
-            repatchGetByID(exec, baseValue, ident, slot, *stubInfo);
-        else
-            stubInfo->seen = true;
-    }
+    if (stubInfo->seen)
+        repatchGetByID(exec, baseValue, ident, slot, *stubInfo);
+    else
+        stubInfo->seen = true;
 
     return JSValue::encode(result);
 }

Source/JavaScriptCore/runtime/JSCJSValue.h

@@ -262,6 +262,9 @@ class JSValue {
     JSValue get(ExecState*, PropertyName, PropertySlot&) const;
     JSValue get(ExecState*, unsigned propertyName) const;
     JSValue get(ExecState*, unsigned propertyName, PropertySlot&) const;
+
+    bool getPropertySlot(ExecState*, PropertyName, PropertySlot&) const;
+
     void put(ExecState*, PropertyName, JSValue, PutPropertySlot&);
     JS_EXPORT_PRIVATE void putToPrimitive(ExecState*, PropertyName, JSValue, PutPropertySlot&);
     JS_EXPORT_PRIVATE void putToPrimitiveByIndex(ExecState*, unsigned propertyName, JSValue, bool shouldThrow);

Source/JavaScriptCore/runtime/JSCJSValueInlines.h

@@ -681,20 +681,24 @@ ALWAYS_INLINE JSValue JSValue::get(ExecState* exec, PropertyName propertyName) c
 }
 
 ALWAYS_INLINE JSValue JSValue::get(ExecState* exec, PropertyName propertyName, PropertySlot& slot) const
+{
+    return getPropertySlot(exec, propertyName, slot) ? 
+        slot.getValue(exec, propertyName) : jsUndefined();
+}
+
+ALWAYS_INLINE bool JSValue::getPropertySlot(ExecState* exec, PropertyName propertyName, PropertySlot& slot) const
 {
     // If this is a primitive, we'll need to synthesize the prototype -
     // and if it's a string there are special properties to check first.
     JSObject* object;
     if (UNLIKELY(!isObject())) {
         if (isCell() && asString(*this)->getStringPropertySlot(exec, propertyName, slot))
-            return slot.getValue(exec, propertyName);
+            return true;
         object = synthesizePrototype(exec);
     } else
         object = asObject(asCell());
 
-    if (object->getPropertySlot(exec, propertyName, slot))
-        return slot.getValue(exec, propertyName);
-    return jsUndefined();
+    return object->getPropertySlot(exec, propertyName, slot);
 }
 
 ALWAYS_INLINE JSValue JSValue::get(ExecState* exec, unsigned propertyName) const

Source/JavaScriptCore/tests/stress/recursive_property_redefine_during_inline_caching.js

@@ -0,0 +1,23 @@
+// to be run with useLLInt = false
+var o = {};
+
+function getSomeProperty(){
+    return o.someProperty;
+}
+
+var count = 0;
+function test(){
+    count++;
+    if (count == 3) {
+        Object.defineProperty(this, 'someProperty', { value : "okay" });
+        return getSomeProperty();
+    }
+    return "okay";
+}
+
+o.__defineGetter__('someProperty', test)
+
+for (var i = 0; i < 4; i++) {
+    if (getSomeProperty() != "okay")
+        throw ("Error: " + i);
+}