operationSwitchCharWithUnknownKeyType failed to handle OOME when resolving rope string.
https://bugs.webkit.org/show_bug.cgi?id=202312
<rdar://problem/55782280>
Reviewed by Yusuke Suzuki.
JSTests:
* stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings.js: Added.
* stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings2.js: Added.
* stress/switch-on-char-llint-rope.js:
- Changed this test to make a new rope string for each iterations. Otherwise,
the rope will get resolved, and subsequent tiers will not be testing with a rope.
Source/JavaScriptCore:
operationSwitchCharWithUnknownKeyType() can only dispatch to a case handler
if the key string is of length 1. All other cases should dispatch to the default
handler. This patch also adds the missing OOME check.
Also fixed a bug in SpeculativeJIT::emitSwitchCharStringJump() where the slow
path rope resolution was returning after the length check. It needs to return to
the point before the length check.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::emitSwitchCharStringJump):
* jit/JITOperations.cpp:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251178 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 7f844d1..a93d49f 100644
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,17 @@
+2019-10-15 Mark Lam <mark.lam@apple.com>
+
+ operationSwitchCharWithUnknownKeyType failed to handle OOME when resolving rope string.
+ https://bugs.webkit.org/show_bug.cgi?id=202312
+ <rdar://problem/55782280>
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings.js: Added.
+ * stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings2.js: Added.
+ * stress/switch-on-char-llint-rope.js:
+ - Changed this test to make a new rope string for each iterations. Otherwise,
+ the rope will get resolved, and subsequent tiers will not be testing with a rope.
+
2019-10-14 Yusuke Suzuki <ysuzuki@apple.com>
[JSC] GetterSetter should be JSCell, not JSObject
diff --git a/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings.js b/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings.js
new file mode 100644
index 0000000..2fcf8bf
--- /dev/null
+++ b/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings.js
@@ -0,0 +1,17 @@
+//@ if $memoryLimited then skip else runDefault("--useConcurrentJIT=false") end
+//@ slow!
+
+var o = (-1).toLocaleString().padEnd(2 ** 31 - 1, "a");
+
+function f() {
+ switch (o) {
+ case "t":
+ case "s":
+ case "u":
+ }
+}
+noInline(f);
+
+for (var i = 0; i < 10000; i++)
+ f();
+
diff --git a/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings2.js b/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings2.js
new file mode 100644
index 0000000..02480cd
--- /dev/null
+++ b/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings2.js
@@ -0,0 +1,24 @@
+//@ if $memoryLimited then skip else runDefault("--useConcurrentJIT=false") end
+//@ slow!
+
+function f(o) {
+ switch (o) {
+ case "t":
+ case "s":
+ case "u":
+ }
+}
+noInline(f);
+
+for (var i = 0; i < 10000; i++) {
+ let o;
+ // This test needs to allocate a large rope string, which is slow.
+ // The following is tweaked so that we only use this large string once each to
+ // exercise the llint, baseline, DFG, and FTL, so that the test doesn't run too slow.
+ if (i == 0 || i == 99 || i == 200 || i == 9999)
+ o = (-1).toLocaleString().padEnd(2 ** 31 - 1, "a");
+ else
+ o = (-1).toLocaleString().padEnd(2, "a");
+ f(o);
+}
+
diff --git a/JSTests/stress/switch-on-char-llint-rope.js b/JSTests/stress/switch-on-char-llint-rope.js
index aff44b0..099f4bf 100644
--- a/JSTests/stress/switch-on-char-llint-rope.js
+++ b/JSTests/stress/switch-on-char-llint-rope.js
@@ -14,8 +14,8 @@
}
noInline(foo);
-let str = 'a' + constStr();
for (let i = 0; i < 10000; ++i) {
+ let str = 'a' + constStr();
let result = foo(str);
if (result !== 2)
throw new Error("Bad result");
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 5f16042..9a0008c 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,23 @@
+2019-10-15 Mark Lam <mark.lam@apple.com>
+
+ operationSwitchCharWithUnknownKeyType failed to handle OOME when resolving rope string.
+ https://bugs.webkit.org/show_bug.cgi?id=202312
+ <rdar://problem/55782280>
+
+ Reviewed by Yusuke Suzuki.
+
+ operationSwitchCharWithUnknownKeyType() can only dispatch to a case handler
+ if the key string is of length 1. All other cases should dispatch to the default
+ handler. This patch also adds the missing OOME check.
+
+ Also fixed a bug in SpeculativeJIT::emitSwitchCharStringJump() where the slow
+ path rope resolution was returning after the length check. It needs to return to
+ the point before the length check.
+
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::emitSwitchCharStringJump):
+ * jit/JITOperations.cpp:
+
2019-10-15 Peng Liu <peng.liu6@apple.com>
[Picture-in-Picture Web API] Implement HTMLVideoElement.requestPictureInPicture() / Document.exitPictureInPicture()
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index 9d038b5..95a7cd6 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -10719,16 +10719,15 @@
{
m_jit.loadPtr(MacroAssembler::Address(value, JSString::offsetOfValue()), scratch);
auto isRope = m_jit.branchIfRopeStringImpl(scratch);
-
+ addSlowPathGenerator(slowPathCall(isRope, this, operationResolveRope, scratch, value));
+
addBranch(
m_jit.branch32(
MacroAssembler::NotEqual,
MacroAssembler::Address(scratch, StringImpl::lengthMemoryOffset()),
TrustedImm32(1)),
data->fallThrough.block);
-
- addSlowPathGenerator(slowPathCall(isRope, this, operationResolveRope, scratch, value));
-
+
m_jit.loadPtr(MacroAssembler::Address(scratch, StringImpl::dataOffset()), value);
JITCompiler::Jump is8Bit = m_jit.branchTest32(
diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
index 639463c..c3eec64 100644
--- a/Source/JavaScriptCore/jit/JITOperations.cpp
+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
@@ -2303,6 +2303,7 @@
{
VM& vm = exec->vm();
NativeCallFrameTracer tracer(vm, exec);
+ auto throwScope = DECLARE_THROW_SCOPE(vm);
JSValue key = JSValue::decode(encodedKey);
CodeBlock* codeBlock = exec->codeBlock();
@@ -2310,9 +2311,12 @@
void* result = jumpTable.ctiDefault.executableAddress();
if (key.isString()) {
- StringImpl* value = asString(key)->value(exec).impl();
- if (value->length() == 1)
- result = jumpTable.ctiForValue((*value)[0]).executableAddress();
+ JSString* string = asString(key);
+ if (string->length() == 1) {
+ String value = string->value(exec);
+ RETURN_IF_EXCEPTION(throwScope, nullptr);
+ result = jumpTable.ctiForValue(value[0]).executableAddress();
+ }
}
assertIsTaggedWith(result, JSSwitchPtrTag);
