Commit d0be74de authored by mhahnenberg@apple.com's avatar 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.

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.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@147816 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 94a61e69
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> 2013-04-05 Chris Fleizach <cfleizach@apple.com>
AX: Make SVG Group containers accessible elements AX: Make SVG Group containers accessible elements
Tests that we use the correct profiling accessType for the case we end up compiling/patching.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(new L_()) is 1365109051943.000000
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(new L_()) is 1365109051943.000000
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(new L_()) is 1365109051943.000000
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(new L_()) is 1365109051943.000000
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(new L_()) is 1365109051943.000000
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(new L_()) is 1365109051943.000000
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS f(129681120) is 129681120
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE html>
<html>
<head>
<script src="resources/js-test-pre.js"></script>
</head>
<body>
<script src="script-tests/jit-set-profiling-access-type-only-for-get-by-id-self.js"></script>
<script src="resources/js-test-post.js"></script>
</body>
</html>
description(
"Tests that we use the correct profiling accessType for the case we end up compiling/patching."
);
function L_() {
this.__defineGetter__("mg", function() { return 1365109051943.000000; });
};
function Q2() {};
Q2.mg = 42;
var f = function (Sk){ if(Sk instanceof L_){ return Sk.mg; }else if(Sk instanceof Q2){ return Sk.mg; }else{ return Number(Sk); } }
for (var i = 1; i < 200; i++) {
if (i % 30 == 0)
shouldBe("f(new L_())", "1365109051943.000000");
else
shouldBe("f(129681120)", "129681120");
}
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> 2013-04-05 Filip Pizlo <fpizlo@apple.com>
If CallFrame::trueCallFrame() knows that it's about to read garbage instead of a valid CodeOrigin/InlineCallFrame, then it should give up and return 0 and all callers should be robust against this If CallFrame::trueCallFrame() knows that it's about to read garbage instead of a valid CodeOrigin/InlineCallFrame, then it should give up and return 0 and all callers should be robust against this
...@@ -940,12 +940,14 @@ NEVER_INLINE static void tryCacheGetByID(CallFrame* callFrame, CodeBlock* codeBl ...@@ -940,12 +940,14 @@ NEVER_INLINE static void tryCacheGetByID(CallFrame* callFrame, CodeBlock* codeBl
// Cache hit: Specialize instruction and ref Structures. // Cache hit: Specialize instruction and ref Structures.
if (slot.slotBase() == baseValue) { if (slot.slotBase() == baseValue) {
stubInfo->initGetByIdSelf(callFrame->globalData(), codeBlock->ownerExecutable(), structure); RELEASE_ASSERT(stubInfo->accessType == access_unset);
if ((slot.cachedPropertyType() != PropertySlot::Value) if ((slot.cachedPropertyType() != PropertySlot::Value)
|| !MacroAssembler::isCompactPtrAlignedAddressOffset(maxOffsetRelativeToPatchedStorage(slot.cachedOffset()))) || !MacroAssembler::isCompactPtrAlignedAddressOffset(maxOffsetRelativeToPatchedStorage(slot.cachedOffset())))
ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_self_fail)); ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_self_fail));
else else {
JIT::patchGetByIdSelf(codeBlock, stubInfo, structure, slot.cachedOffset(), returnAddress); JIT::patchGetByIdSelf(codeBlock, stubInfo, structure, slot.cachedOffset(), returnAddress);
stubInfo->initGetByIdSelf(callFrame->globalData(), codeBlock->ownerExecutable(), structure);
}
return; return;
} }
...@@ -1592,6 +1594,9 @@ DEFINE_STUB_FUNCTION(EncodedJSValue, op_get_by_id_self_fail) ...@@ -1592,6 +1594,9 @@ DEFINE_STUB_FUNCTION(EncodedJSValue, op_get_by_id_self_fail)
PolymorphicAccessStructureList* polymorphicStructureList; PolymorphicAccessStructureList* polymorphicStructureList;
int listIndex = 1; 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) { if (stubInfo->accessType == access_get_by_id_self) {
ASSERT(!stubInfo->stubRoutine); ASSERT(!stubInfo->stubRoutine);
polymorphicStructureList = new PolymorphicAccessStructureList(callFrame->globalData(), codeBlock->ownerExecutable(), 0, stubInfo->u.getByIdSelf.baseObjectStructure.get(), true); polymorphicStructureList = new PolymorphicAccessStructureList(callFrame->globalData(), codeBlock->ownerExecutable(), 0, stubInfo->u.getByIdSelf.baseObjectStructure.get(), true);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment