Changeset 147816 in webkit

Timestamp:

Apr 5, 2013 4:52:20 PM (11 years ago)

Author:

mhahnenberg@apple.com

Message:

tryCacheGetByID sets StructureStubInfo accessType to an incorrect value
​https://bugs.webkit.org/show_bug.cgi?id=114068

Reviewed by Geoffrey Garen.

In the case where we have a non-Value cacheable property, we set the StructureStubInfo accessType to
get_by_id_self, but then we don't patch self and instead patch in a get_by_id_self_fail. This leads to
incorrect profiling data so when the DFG compiles the function, it uses a GetByOffset rather than a GetById,
which leads to loading a GetterSetter directly out of an object.

Source/JavaScriptCore:

• jit/JITStubs.cpp:

(JSC::tryCacheGetByID):
(JSC::DEFINE_STUB_FUNCTION):

LayoutTests:

• fast/js/jit-set-profiling-access-type-only-for-get-by-id-self-expected.txt: Added.
• fast/js/jit-set-profiling-access-type-only-for-get-by-id-self.html: Added.
• fast/js/script-tests/jit-set-profiling-access-type-only-for-get-by-id-self.js: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/js/jit-set-profiling-access-type-only-for-get-by-id-self-expected.txt (added)
• LayoutTests/fast/js/jit-set-profiling-access-type-only-for-get-by-id-self.html (added)
• LayoutTests/fast/js/script-tests/jit-set-profiling-access-type-only-for-get-by-id-self.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/jit/JITStubs.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-04-05  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        tryCacheGetByID sets StructureStubInfo accessType to an incorrect value
+        https://bugs.webkit.org/show_bug.cgi?id=114068
+
+        Reviewed by Geoffrey Garen.
+
+        In the case where we have a non-Value cacheable property, we set the StructureStubInfo accessType to 
+        get_by_id_self, but then we don't patch self and instead patch in a get_by_id_self_fail. This leads to 
+        incorrect profiling data so when the DFG compiles the function, it uses a GetByOffset rather than a GetById, 
+        which leads to loading a GetterSetter directly out of an object.
+
+        * fast/js/jit-set-profiling-access-type-only-for-get-by-id-self-expected.txt: Added.
+        * fast/js/jit-set-profiling-access-type-only-for-get-by-id-self.html: Added.
+        * fast/js/script-tests/jit-set-profiling-access-type-only-for-get-by-id-self.js: Added.
+
 2013-04-05  Chris Fleizach  <cfleizach@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2013-04-05  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        tryCacheGetByID sets StructureStubInfo accessType to an incorrect value
+        https://bugs.webkit.org/show_bug.cgi?id=114068
+
+        Reviewed by Geoffrey Garen.
+
+        In the case where we have a non-Value cacheable property, we set the StructureStubInfo accessType to 
+        get_by_id_self, but then we don't patch self and instead patch in a get_by_id_self_fail. This leads to 
+        incorrect profiling data so when the DFG compiles the function, it uses a GetByOffset rather than a GetById, 
+        which leads to loading a GetterSetter directly out of an object.
+
+        * jit/JITStubs.cpp:
+        (JSC::tryCacheGetByID):
+        (JSC::DEFINE_STUB_FUNCTION):
+
 2013-04-05  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/jit/JITStubs.cpp

@@ -941 +941 @@
 
     if (slot.slotBase() == baseValue) {
-        stubInfo->initGetByIdSelf(callFrame->globalData(), codeBlock->ownerExecutable(), structure);
+        RELEASE_ASSERT(stubInfo->accessType == access_unset);
         if ((slot.cachedPropertyType() != PropertySlot::Value)
             || !MacroAssembler::isCompactPtrAlignedAddressOffset(maxOffsetRelativeToPatchedStorage(slot.cachedOffset())))
             ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_self_fail));
-        else
+        else {
             JIT::patchGetByIdSelf(codeBlock, stubInfo, structure, slot.cachedOffset(), returnAddress);
+            stubInfo->initGetByIdSelf(callFrame->globalData(), codeBlock->ownerExecutable(), structure);
+        }
         return;
     }
@@ -1592 +1594 @@
         PolymorphicAccessStructureList* polymorphicStructureList;
         int listIndex = 1;
+
+        if (stubInfo->accessType == access_unset)
+            stubInfo->initGetByIdSelf(callFrame->globalData(), codeBlock->ownerExecutable(), baseValue.asCell()->structure());
 
         if (stubInfo->accessType == access_get_by_id_self) {