[webkit-changes] [124678] trunk/Source/JavaScriptCore

Fri, 03 Aug 2012 18:01:19 -0700

Title: [124678] trunk/Source/_javascript_Core
Revision
124678
Author
[email protected]
Date
2012-08-03 18:01:27 -0700 (Fri, 03 Aug 2012)

Log Message

Crashes in dfgBuildPutByIdList when clicking on just about anything on Google Maps
https://bugs.webkit.org/show_bug.cgi?id=92691

Reviewed by Mark Hahnenberg.

The state of the stubs was changing after we determined the type (by virtue of the slow path
function that was called), since the get or put (in this case put) could cause arbitrary
side effects. Perhaps a full-blown fix would be to eliminate our reliance of the slow path
function to determine what to do, but an easier fix for now is to have the slow path give up
if its assumptions were invalidated by a side effect.

* dfg/DFGOperations.cpp:
* jit/JITStubs.cpp:
(JSC::DEFINE_STUB_FUNCTION):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (124677 => 124678)


--- trunk/Source/_javascript_Core/ChangeLog	2012-08-04 00:40:01 UTC (rev 124677)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-08-04 01:01:27 UTC (rev 124678)
@@ -1,5 +1,22 @@
 2012-08-03  Filip Pizlo  <[email protected]>
 
+        Crashes in dfgBuildPutByIdList when clicking on just about anything on Google Maps
+        https://bugs.webkit.org/show_bug.cgi?id=92691
+
+        Reviewed by Mark Hahnenberg.
+
+        The state of the stubs was changing after we determined the type (by virtue of the slow path
+        function that was called), since the get or put (in this case put) could cause arbitrary
+        side effects. Perhaps a full-blown fix would be to eliminate our reliance of the slow path
+        function to determine what to do, but an easier fix for now is to have the slow path give up
+        if its assumptions were invalidated by a side effect.
+
+        * dfg/DFGOperations.cpp:
+        * jit/JITStubs.cpp:
+        (JSC::DEFINE_STUB_FUNCTION):
+
+2012-08-03  Filip Pizlo  <[email protected]>
+
         DFG handling of get_by_id should always inject a ForceOSRExit node if there is no prediction
         https://bugs.webkit.org/show_bug.cgi?id=93162

Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (124677 => 124678)


--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp	2012-08-04 00:40:01 UTC (rev 124677)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp	2012-08-04 01:01:27 UTC (rev 124678)
@@ -423,12 +423,15 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue baseValue = JSValue::decode(base);
     PropertySlot slot(baseValue);
     JSValue result = baseValue.get(exec, *propertyName, slot);
 
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
-    dfgBuildGetByIDList(exec, baseValue, *propertyName, slot, stubInfo);
+    if (accessType == static_cast<AccessType>(stubInfo.accessType))
+        dfgBuildGetByIDList(exec, baseValue, *propertyName, slot, stubInfo);
 
     return JSValue::encode(result);
 }
@@ -439,13 +442,16 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue baseValue = JSValue::decode(base);
     PropertySlot slot(baseValue);
     JSValue result = baseValue.get(exec, *propertyName, slot);
+    
+    if (accessType == static_cast<AccessType>(stubInfo.accessType))
+        dfgBuildGetByIDProtoList(exec, baseValue, *propertyName, slot, stubInfo);
 
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
-    dfgBuildGetByIDProtoList(exec, baseValue, *propertyName, slot, stubInfo);
-
     return JSValue::encode(result);
 }
 
@@ -455,15 +461,19 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue baseValue = JSValue::decode(base);
     PropertySlot slot(baseValue);
     JSValue result = baseValue.get(exec, *propertyName, slot);
     
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
-    if (stubInfo.seen)
-        dfgRepatchGetByID(exec, baseValue, *propertyName, slot, stubInfo);
-    else
-        stubInfo.seen = true;
+    if (accessType == static_cast<AccessType>(stubInfo.accessType)) {
+        if (stubInfo.seen)
+            dfgRepatchGetByID(exec, baseValue, *propertyName, slot, stubInfo);
+        else
+            stubInfo.seen = true;
+    }
 
     return JSValue::encode(result);
 }
@@ -645,13 +655,18 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue value = JSValue::decode(encodedValue);
     JSValue baseValue(base);
     PutPropertySlot slot(true);
     
     baseValue.put(exec, *propertyName, value, slot);
     
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return;
+    
     if (stubInfo.seen)
         dfgRepatchPutByID(exec, baseValue, *propertyName, slot, stubInfo, NotDirect);
     else
@@ -664,13 +679,18 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue value = JSValue::decode(encodedValue);
     JSValue baseValue(base);
     PutPropertySlot slot(false);
     
     baseValue.put(exec, *propertyName, value, slot);
     
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return;
+    
     if (stubInfo.seen)
         dfgRepatchPutByID(exec, baseValue, *propertyName, slot, stubInfo, NotDirect);
     else
@@ -683,13 +703,18 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue value = JSValue::decode(encodedValue);
     PutPropertySlot slot(true);
     
     ASSERT(base->isObject());
     asObject(base)->putDirect(exec->globalData(), *propertyName, value, slot);
     
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return;
+    
     if (stubInfo.seen)
         dfgRepatchPutByID(exec, base, *propertyName, slot, stubInfo, Direct);
     else
@@ -702,13 +727,18 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue value = JSValue::decode(encodedValue);
     PutPropertySlot slot(false);
     
     ASSERT(base->isObject());
     asObject(base)->putDirect(exec->globalData(), *propertyName, value, slot);
     
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return;
+    
     if (stubInfo.seen)
         dfgRepatchPutByID(exec, base, *propertyName, slot, stubInfo, Direct);
     else
@@ -721,13 +751,18 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue value = JSValue::decode(encodedValue);
     JSValue baseValue(base);
     PutPropertySlot slot(true);
     
     baseValue.put(exec, *propertyName, value, slot);
     
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return;
+    
     dfgBuildPutByIdList(exec, baseValue, *propertyName, slot, stubInfo, NotDirect);
 }
 
@@ -737,13 +772,18 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue value = JSValue::decode(encodedValue);
     JSValue baseValue(base);
     PutPropertySlot slot(false);
     
     baseValue.put(exec, *propertyName, value, slot);
     
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return;
+    
     dfgBuildPutByIdList(exec, baseValue, *propertyName, slot, stubInfo, NotDirect);
 }
 
@@ -753,13 +793,18 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+    
     JSValue value = JSValue::decode(encodedValue);
     PutPropertySlot slot(true);
     
     ASSERT(base->isObject());
     asObject(base)->putDirect(exec->globalData(), *propertyName, value, slot);
     
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return;
+    
     dfgBuildPutByIdList(exec, base, *propertyName, slot, stubInfo, Direct);
 }
 
@@ -769,13 +814,18 @@
     JSGlobalData* globalData = &exec->globalData();
     NativeCallFrameTracer tracer(globalData, exec);
     
+    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue value = JSValue::decode(encodedValue);
     PutPropertySlot slot(false);
     
     ASSERT(base->isObject());
     asObject(base)->putDirect(exec->globalData(), *propertyName, value, slot);
     
-    StructureStubInfo& stubInfo = exec->codeBlock()->getStubInfo(returnAddress);
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return;
+    
     dfgBuildPutByIdList(exec, base, *propertyName, slot, stubInfo, Direct);
 }

Modified: trunk/Source/_javascript_Core/jit/JITStubs.cpp (124677 => 124678)


--- trunk/Source/_javascript_Core/jit/JITStubs.cpp	2012-08-04 00:40:01 UTC (rev 124677)
+++ trunk/Source/_javascript_Core/jit/JITStubs.cpp	2012-08-04 01:01:27 UTC (rev 124678)
@@ -1430,15 +1430,19 @@
     CallFrame* callFrame = stackFrame.callFrame;
     Identifier& ident = stackFrame.args[1].identifier();
     
+    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
+    StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
+    AccessType accessType = static_cast<AccessType>(stubInfo->accessType);
+
     PutPropertySlot slot(callFrame->codeBlock()->isStrictMode());
     stackFrame.args[0].jsValue().put(callFrame, ident, stackFrame.args[2].jsValue(), slot);
     
-    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
-    StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
-    if (!stubInfo->seenOnce())
-        stubInfo->setSeen();
-    else
-        JITThunks::tryCachePutByID(callFrame, codeBlock, STUB_RETURN_ADDRESS, stackFrame.args[0].jsValue(), slot, stubInfo, false);
+    if (accessType == static_cast<AccessType>(stubInfo->accessType)) {
+        if (!stubInfo->seenOnce())
+            stubInfo->setSeen();
+        else
+            JITThunks::tryCachePutByID(callFrame, codeBlock, STUB_RETURN_ADDRESS, stackFrame.args[0].jsValue(), slot, stubInfo, false);
+    }
     
     CHECK_FOR_EXCEPTION_AT_END();
 }
@@ -1449,18 +1453,22 @@
     CallFrame* callFrame = stackFrame.callFrame;
     Identifier& ident = stackFrame.args[1].identifier();
     
+    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
+    StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
+    AccessType accessType = static_cast<AccessType>(stubInfo->accessType);
+
     PutPropertySlot slot(callFrame->codeBlock()->isStrictMode());
     JSValue baseValue = stackFrame.args[0].jsValue();
     ASSERT(baseValue.isObject());
     
     asObject(baseValue)->putDirect(callFrame->globalData(), ident, stackFrame.args[2].jsValue(), slot);
     
-    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
-    StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
-    if (!stubInfo->seenOnce())
-        stubInfo->setSeen();
-    else
-        JITThunks::tryCachePutByID(callFrame, codeBlock, STUB_RETURN_ADDRESS, stackFrame.args[0].jsValue(), slot, stubInfo, true);
+    if (accessType == static_cast<AccessType>(stubInfo->accessType)) {
+        if (!stubInfo->seenOnce())
+            stubInfo->setSeen();
+        else
+            JITThunks::tryCachePutByID(callFrame, codeBlock, STUB_RETURN_ADDRESS, stackFrame.args[0].jsValue(), slot, stubInfo, true);
+    }
     
     CHECK_FOR_EXCEPTION_AT_END();
 }
@@ -1521,15 +1529,19 @@
     CallFrame* callFrame = stackFrame.callFrame;
     Identifier& ident = stackFrame.args[1].identifier();
 
+    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
+    MethodCallLinkInfo& methodCallLinkInfo = codeBlock->getMethodCallLinkInfo(STUB_RETURN_ADDRESS);
+    StructureStubInfo& stubInfo = codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue baseValue = stackFrame.args[0].jsValue();
     PropertySlot slot(baseValue);
     JSValue result = baseValue.get(callFrame, ident, slot);
     CHECK_FOR_EXCEPTION();
+    
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return JSValue::encode(result);
 
-    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
-    MethodCallLinkInfo& methodCallLinkInfo = codeBlock->getMethodCallLinkInfo(STUB_RETURN_ADDRESS);
-    StructureStubInfo& stubInfo = codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
-
     if (!methodCallLinkInfo.seenOnce()) {
         methodCallLinkInfo.setSeen();
         return JSValue::encode(result);
@@ -1595,15 +1607,19 @@
     CallFrame* callFrame = stackFrame.callFrame;
     Identifier& ident = stackFrame.args[1].identifier();
 
+    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
+    MethodCallLinkInfo& methodCallLinkInfo = codeBlock->getMethodCallLinkInfo(STUB_RETURN_ADDRESS);
+    StructureStubInfo& stubInfo = codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
+    AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
+
     JSValue baseValue = stackFrame.args[0].jsValue();
     PropertySlot slot(baseValue);
     JSValue result = baseValue.get(callFrame, ident, slot);
     CHECK_FOR_EXCEPTION();
+    
+    if (accessType != static_cast<AccessType>(stubInfo.accessType))
+        return JSValue::encode(result);
 
-    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
-    MethodCallLinkInfo& methodCallLinkInfo = codeBlock->getMethodCallLinkInfo(STUB_RETURN_ADDRESS);
-    StructureStubInfo& stubInfo = codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
-
     ASSERT(methodCallLinkInfo.seenOnce());
 
     // If we successfully got something, then the base from which it is being accessed must
@@ -1684,12 +1700,17 @@
     CallFrame* callFrame = stackFrame.callFrame;
     Identifier& ident = stackFrame.args[1].identifier();
 
+    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
+    StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
+    AccessType accessType = static_cast<AccessType>(stubInfo->accessType);
+
     JSValue baseValue = stackFrame.args[0].jsValue();
     PropertySlot slot(baseValue);
     JSValue result = baseValue.get(callFrame, ident, slot);
+    
+    if (accessType != static_cast<AccessType>(stubInfo->accessType))
+        return JSValue::encode(result);
 
-    CodeBlock* codeBlock = stackFrame.callFrame->codeBlock();
-    StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
     if (!stubInfo->seenOnce())
         stubInfo->setSeen();
     else
@@ -1706,9 +1727,16 @@
     CallFrame* callFrame = stackFrame.callFrame;
     Identifier& ident = stackFrame.args[1].identifier();
 
+    CodeBlock* codeBlock = callFrame->codeBlock();
+    StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
+    AccessType accessType = static_cast<AccessType>(stubInfo->accessType);
+
     JSValue baseValue = stackFrame.args[0].jsValue();
     PropertySlot slot(baseValue);
     JSValue result = baseValue.get(callFrame, ident, slot);
+    
+    if (accessType != static_cast<AccessType>(stubInfo->accessType))
+        return JSValue::encode(result);
 
     CHECK_FOR_EXCEPTION();
 
@@ -1717,9 +1745,6 @@
         && !baseValue.asCell()->structure()->isUncacheableDictionary()
         && slot.slotBase() == baseValue) {
 
-        CodeBlock* codeBlock = callFrame->codeBlock();
-        StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
-
         ASSERT(slot.slotBase().isObject());
 
         PolymorphicAccessStructureList* polymorphicStructureList;
@@ -1813,20 +1838,26 @@
     CallFrame* callFrame = stackFrame.callFrame;
     const Identifier& propertyName = stackFrame.args[1].identifier();
 
+    CodeBlock* codeBlock = callFrame->codeBlock();
+    StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
+    AccessType accessType = static_cast<AccessType>(stubInfo->accessType);
+
     JSValue baseValue = stackFrame.args[0].jsValue();
     PropertySlot slot(baseValue);
     JSValue result = baseValue.get(callFrame, propertyName, slot);
 
     CHECK_FOR_EXCEPTION();
 
-    if (!baseValue.isCell() || !slot.isCacheable() || baseValue.asCell()->structure()->isDictionary() || baseValue.asCell()->structure()->typeInfo().prohibitsPropertyCaching()) {
+    if (accessType != static_cast<AccessType>(stubInfo->accessType)
+        || !baseValue.isCell()
+        || !slot.isCacheable()
+        || baseValue.asCell()->structure()->isDictionary()
+        || baseValue.asCell()->structure()->typeInfo().prohibitsPropertyCaching()) {
         ctiPatchCallByReturnAddress(callFrame->codeBlock(), STUB_RETURN_ADDRESS, FunctionPtr(cti_op_get_by_id_proto_fail));
         return JSValue::encode(result);
     }
 
     Structure* structure = baseValue.asCell()->structure();
-    CodeBlock* codeBlock = callFrame->codeBlock();
-    StructureStubInfo* stubInfo = &codeBlock->getStubInfo(STUB_RETURN_ADDRESS);
 
     ASSERT(slot.slotBase().isObject());
     JSObject* slotBaseObject = asObject(slot.slotBase());

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to