Changeset 251178 in webkit

Timestamp:

Oct 15, 2019, 9:01:18 PM (6 years ago)

Author:

mark.lam@apple.com

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/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:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings.js (added)
• JSTests/stress/operationSwitchCharWithUnknownKeyType-should-avoid-resolving-rope-strings2.js (added)
• JSTests/stress/switch-on-char-llint-rope.js (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (modified) (2 diffs)
• Source/JavaScriptCore/jit/JITOperations.cpp (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+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>
 

trunk/JSTests/stress/switch-on-char-llint-rope.js

@@ -15 +15 @@
 noInline(foo);
 
-let str = 'a' + constStr();
 for (let i = 0; i < 10000; ++i) {
+    let str = 'a' + constStr();
     let result = foo(str);
     if (result !== 2)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -10720 +10720 @@
     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(
@@ -10727 +10728 @@
             TrustedImm32(1)),
         data->fallThrough.block);
-    
-    addSlowPathGenerator(slowPathCall(isRope, this, operationResolveRope, scratch, value));
-    
+
     m_jit.loadPtr(MacroAssembler::Address(scratch, StringImpl::dataOffset()), value);
     

trunk/Source/JavaScriptCore/jit/JITOperations.cpp

@@ -2304 +2304 @@
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(vm, exec);
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
     JSValue key = JSValue::decode(encodedKey);
     CodeBlock* codeBlock = exec->codeBlock();
@@ -2311 +2312 @@
 
     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();
+        }
     }