Skip to content

Commit b19f56a

Browse files
jjgriegoguijemont
authored andcommitted
[32 bit] DFG graph generation: intrinsic getters are fallible
https://bugs.webkit.org/show_bug.cgi?id=260908 Reviewed by Yusuke Suzuki. On 32-bit, unlike 64-bit, some of the DFG intrinsic getters (really, the TypedArray ones) are _fallible_: if the SpeculatedType doesn't match our expecations (a non-strict subset of SpecInt32Only), we refuse to generate code. [1] However, DFG::ByteCodeParser::handleGetById doesn't appear to handle this case gracefully--if `handleIntrinsicGetter` fails, we attempt to generate a call to the getter, but in the case of TypedArray intrinsics, we won't have the necessary CallLinkStatus and while attempting to do so, we crash. To fix this, I've added a bit of code that handles the failure from handleIntrinsicGetter and emits an ordinary `GetById` node instead of trying to inline anything for this op. I've added a test that demonstrates the current behavior (a segfault) on armv7 and passes with tihs patch. [1] For what it's worth, maybe this shouldn't be the case: it does seem like we should still be able to generate code in these cases anyhow, but it's simpler to just cope with the failure. * JSTests/stress/typed-array-intrinsic-getter-with-conflicting-value-profile.js: Added. (foo): (i.null.foo.Object.create): (i.42.foo): * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::handleGetById): Canonical link: https://commits.webkit.org/267511@main
1 parent c888663 commit b19f56a

2 files changed

Lines changed: 48 additions & 12 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
function foo(x) {
2+
return x.byteLength
3+
}
4+
5+
var arr = new Uint8Array(42);
6+
7+
var badPrototype = {};
8+
var bad = Object.create(badPrototype);
9+
10+
for (var i = 0; i < 1e6; i++) {
11+
if (null != foo(bad)) {
12+
throw new Error();
13+
}
14+
}
15+
16+
badPrototype.byteLength = 42;
17+
18+
for (var i = 0; i < 1e6; i++) {
19+
if (42 != foo(arr)) {
20+
throw new Error();
21+
}
22+
}

Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4760,27 +4760,41 @@ void ByteCodeParser::handleGetById(
47604760
return;
47614761
}
47624762

4763+
ASSERT(type == AccessType::GetById || type == AccessType::GetByIdDirect || !variant.callLinkStatus());
4764+
4765+
if (variant.intrinsic() != NoIntrinsic) {
4766+
auto const addChecks = [&] {
4767+
Node* getter = addToGraph(GetGetter, loadedValue);
4768+
addToGraph(CheckIsConstant, OpInfo(m_graph.freeze(variant.intrinsicFunction())), getter);
4769+
};
4770+
4771+
if (handleIntrinsicGetter(destination, prediction, variant, base, addChecks)) {
4772+
if (UNLIKELY(m_graph.compilation()))
4773+
m_graph.compilation()->noticeInlinedGetById();
4774+
addToGraph(Phantom, base);
4775+
return;
4776+
}
4777+
4778+
// We couldn't handle this as an intrinsic and can't emit a direct call
4779+
// to the intrinsic function--bail and emit a regular GetById
4780+
if (!variant.callLinkStatus()) {
4781+
set(destination,
4782+
addToGraph(getById, OpInfo(identifier), OpInfo(prediction), base));
4783+
return;
4784+
}
4785+
}
4786+
47634787
if (UNLIKELY(m_graph.compilation()))
47644788
m_graph.compilation()->noticeInlinedGetById();
47654789

4766-
ASSERT(type == AccessType::GetById || type == AccessType::GetByIdDirect || !variant.callLinkStatus());
4767-
if (!variant.callLinkStatus() && variant.intrinsic() == NoIntrinsic) {
4790+
if (!variant.callLinkStatus()) {
47684791
set(destination, loadedValue);
47694792
return;
47704793
}
4771-
4772-
Node* getter = addToGraph(GetGetter, loadedValue);
4773-
4774-
if (handleIntrinsicGetter(destination, prediction, variant, base,
4775-
[&] () {
4776-
addToGraph(CheckIsConstant, OpInfo(m_graph.freeze(variant.intrinsicFunction())), getter);
4777-
})) {
4778-
addToGraph(Phantom, base);
4779-
return;
4780-
}
47814794

47824795
// Make a call. We don't try to get fancy with using the smallest operand number because
47834796
// the stack layout phase should compress the stack anyway.
4797+
Node* getter = addToGraph(GetGetter, loadedValue);
47844798

47854799
unsigned numberOfParameters = 0;
47864800
numberOfParameters++; // The 'this' argument.

0 commit comments

Comments
 (0)