- Revision
- 251178
- Author
- mark....@apple.com
- Date
- 2019-10-15 21:01:18 -0700 (Tue, 15 Oct 2019)
Log Message
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/_javascript_Core:
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:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (251177 => 251178)
--- trunk/JSTests/ChangeLog 2019-10-16 02:32:05 UTC (rev 251177)
+++ trunk/JSTests/ChangeLog 2019-10-16 04:01:18 UTC (rev 251178)
@@ -1,3 +1,17 @@
+2019-10-15 Mark Lam <mark....@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 <ysuz...@apple.com>
[JSC] GetterSetter should be JSCell, not JSObject
Added: trunk/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings.js (0 => 251178)
--- trunk/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings.js (rev 0)
+++ trunk/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings.js 2019-10-16 04:01:18 UTC (rev 251178)
@@ -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();
+
Added: trunk/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings2.js (0 => 251178)
--- trunk/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings2.js (rev 0)
+++ trunk/JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings2.js 2019-10-16 04:01:18 UTC (rev 251178)
@@ -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);
+}
+
Modified: trunk/JSTests/stress/switch-on-char-llint-rope.js (251177 => 251178)
--- trunk/JSTests/stress/switch-on-char-llint-rope.js 2019-10-16 02:32:05 UTC (rev 251177)
+++ trunk/JSTests/stress/switch-on-char-llint-rope.js 2019-10-16 04:01:18 UTC (rev 251178)
@@ -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");
Modified: trunk/Source/_javascript_Core/ChangeLog (251177 => 251178)
--- trunk/Source/_javascript_Core/ChangeLog 2019-10-16 02:32:05 UTC (rev 251177)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-10-16 04:01:18 UTC (rev 251178)
@@ -1,3 +1,23 @@
+2019-10-15 Mark Lam <mark....@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.l...@apple.com>
[Picture-in-Picture Web API] Implement HTMLVideoElement.requestPictureInPicture() / Document.exitPictureInPicture()
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (251177 => 251178)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2019-10-16 02:32:05 UTC (rev 251177)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2019-10-16 04:01:18 UTC (rev 251178)
@@ -10719,7 +10719,8 @@
{
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,
@@ -10726,9 +10727,7 @@
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(
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (251177 => 251178)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2019-10-16 02:32:05 UTC (rev 251177)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2019-10-16 04:01:18 UTC (rev 251178)
@@ -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);