Skip to content
Permalink
Browse files
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.


Canonical link: https://commits.webkit.org/132447@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@147816 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Mark Hahnenberg committed Apr 5, 2013
1 parent 599fdd7 commit 3c467ff88ad4aa65e9f28b2f1a7f469e6f9d22aa
Showing 6 changed files with 277 additions and 2 deletions.
@@ -1,3 +1,19 @@
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>

AX: Make SVG Group containers accessible elements
@@ -0,0 +1,208 @@
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

@@ -0,0 +1,10 @@
<!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>
@@ -0,0 +1,20 @@
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");
}

@@ -1,3 +1,19 @@
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>

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
// Cache hit: Specialize instruction and ref Structures.
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,6 +1594,9 @@ DEFINE_STUB_FUNCTION(EncodedJSValue, op_get_by_id_self_fail)
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) {
ASSERT(!stubInfo->stubRoutine);
polymorphicStructureList = new PolymorphicAccessStructureList(callFrame->globalData(), codeBlock->ownerExecutable(), 0, stubInfo->u.getByIdSelf.baseObjectStructure.get(), true);

0 comments on commit 3c467ff

Please sign in to comment.