[JSC] Do not check isValid() in op_new_regexp

https://bugs.webkit.org/show_bug.cgi?id=180970

Reviewed by Saam Barati.

JSTests:

* stress/regexp-syntax-error-invalid-flags.js: Added.
(shouldThrow):

Source/JavaScriptCore:

We should not check `isValid()` inside op_new_regexp.
This simplifies the semantics of NewRegexp node in DFG.

* bytecompiler/NodesCodegen.cpp:
(JSC::RegExpNode::emitBytecode):
* dfg/DFGMayExit.cpp:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileNewRegexp):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNewRegexp):
* jit/JITOperations.cpp:
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@226209 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 1883e39..00f0e36 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,13 @@
+2017-12-19  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Do not check isValid() in op_new_regexp
+        https://bugs.webkit.org/show_bug.cgi?id=180970
+
+        Reviewed by Saam Barati.
+
+        * stress/regexp-syntax-error-invalid-flags.js: Added.
+        (shouldThrow):
+
 2017-12-18  Guillaume Emont  <guijemont@igalia.com>
 
         Skip stress/call-apply-exponential-bytecode-size.js unless x86-64 or arm64

diff --git a/JSTests/stress/regexp-syntax-error-invalid-flags.js b/JSTests/stress/regexp-syntax-error-invalid-flags.js
new file mode 100644 (file)
index 0000000..a4961ba
--- /dev/null
+++ b/JSTests/stress/regexp-syntax-error-invalid-flags.js
@@ -0,0 +1,23 @@
+function shouldThrow(func, errorMessage) {
+    var errorThrown = false;
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        errorThrown = true;
+        error = e;
+    }
+    if (!errorThrown)
+        throw new Error('not thrown');
+    if (String(error) !== errorMessage)
+        throw new Error(`bad error: ${String(error)}`);
+}
+
+function test()
+{
+    return /Hello/cocoa;
+}
+noInline(test);
+
+for (var i = 0; i < 1e4; ++i)
+    shouldThrow(test, `SyntaxError: Invalid regular expression: invalid flags`);

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index dd6668f..55e305b 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,24 @@
+2017-12-19  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Do not check isValid() in op_new_regexp
+        https://bugs.webkit.org/show_bug.cgi?id=180970
+
+        Reviewed by Saam Barati.
+
+        We should not check `isValid()` inside op_new_regexp.
+        This simplifies the semantics of NewRegexp node in DFG.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::RegExpNode::emitBytecode):
+        * dfg/DFGMayExit.cpp:
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileNewRegexp):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileNewRegexp):
+        * jit/JITOperations.cpp:
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+
 2017-12-20  Saam Barati  <sbarati@apple.com>
 
         GetPropertyEnumerator in DFG/FTL should not unconditionally speculate cell

diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
index 7f3dc54..fc5ad7b 100644 (file)
--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
@@ -141,8 +141,14 @@ RegisterID* NumberNode::emitBytecode(BytecodeGenerator& generator, RegisterID* d
 RegisterID* RegExpNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 {
     if (dst == generator.ignoredResult())
-        return 0;
-    return generator.emitNewRegExp(generator.finalDestination(dst), RegExp::create(*generator.vm(), m_pattern.string(), regExpFlags(m_flags.string())));
+        return nullptr;
+    RegExp* regExp = RegExp::create(*generator.vm(), m_pattern.string(), regExpFlags(m_flags.string()));
+    if (regExp->isValid())
+        return generator.emitNewRegExp(generator.finalDestination(dst), regExp);
+    const char* messageCharacters = regExp->errorMessage();
+    const Identifier& message = generator.parserArena().identifierArena().makeIdentifier(generator.vm(), bitwise_cast<const LChar*>(messageCharacters), strlen(messageCharacters));
+    generator.emitThrowStaticError(ErrorType::SyntaxError, message);
+    return generator.emitLoad(generator.finalDestination(dst), jsUndefined());
 }
 
 // ------------------------------ ThisNode -------------------------------------

diff --git a/Source/JavaScriptCore/dfg/DFGMayExit.cpp b/Source/JavaScriptCore/dfg/DFGMayExit.cpp
index 6e08736..3127bde 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGMayExit.cpp
+++ b/Source/JavaScriptCore/dfg/DFGMayExit.cpp
@@ -116,6 +116,7 @@ ExitMode mayExitImpl(Graph& graph, Node* node, StateType& state)
     case NewAsyncFunction:
     case NewAsyncGeneratorFunction:
     case NewStringObject:
+    case NewRegexp:
     case ToNumber:
         result = ExitsForExceptions;
         break;

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index f378ac3..a6605f7 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -9127,19 +9127,7 @@ void SpeculativeJIT::compileNewTypedArray(Node* node)
 void SpeculativeJIT::compileNewRegexp(Node* node)
 {
     RegExp* regexp = node->castOperand<RegExp*>();
-
-    // FIXME: If the RegExp is invalid, we should emit a different bytecode.
-    // https://bugs.webkit.org/show_bug.cgi?id=180970
-    if (!regexp->isValid()) {
-        flushRegisters();
-        GPRFlushedCallResult result(this);
-
-        callOperation(operationNewRegexp, result.gpr(), regexp);
-        m_jit.exceptionCheck();
-
-        cellResult(result.gpr(), node);
-        return;
-    }
+    ASSERT(regexp->isValid());
 
     GPRTemporary result(this);
     GPRTemporary scratch1(this);

diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
index 9c0fa1f..f602c5f 100644 (file)
--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
@@ -10270,18 +10270,7 @@ private:
     {
         FrozenValue* regexp = m_node->cellOperand();
         ASSERT(regexp->cell()->inherits(vm(), RegExp::info()));
-
-        // FIXME: If the RegExp is invalid, we should emit a different bytecode.
-        // https://bugs.webkit.org/show_bug.cgi?id=180970
-        if (!m_node->castOperand<RegExp*>()->isValid()) {
-            LValue result = vmCall(
-                pointerType(),
-                m_out.operation(operationNewRegexp), m_callFrame,
-                frozenPointer(regexp));
-
-            setJSValue(result);
-            return;
-        }
+        ASSERT(m_node->castOperand<RegExp*>()->isValid());
 
         LBasicBlock slowCase = m_out.newBlock();
         LBasicBlock continuation = m_out.newBlock();

diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
index 67a011a..4e60b34 100644 (file)
--- a/Source/JavaScriptCore/jit/JITOperations.cpp
+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
@@ -1264,17 +1264,9 @@ JSCell* JIT_OPERATION operationNewRegexp(ExecState* exec, JSCell* regexpPtr)
     SuperSamplerScope superSamplerScope(false);
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
-    auto scope = DECLARE_THROW_SCOPE(vm);
 
     RegExp* regexp = static_cast<RegExp*>(regexpPtr);
-
-    // FIXME: If the RegExp is invalid, we should emit a different bytecode.
-    // https://bugs.webkit.org/show_bug.cgi?id=180970
-    if (!regexp->isValid()) {
-        throwException(exec, scope, createSyntaxError(exec, regexp->errorMessage()));
-        return nullptr;
-    }
-
+    ASSERT(regexp->isValid());
     return RegExpObject::create(vm, exec->lexicalGlobalObject()->regExpStructure(), regexp);
 }
 

diff --git a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
index 8264b3b..632bfdb 100644 (file)
--- a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
+++ b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
@@ -552,11 +552,7 @@ LLINT_SLOW_PATH_DECL(slow_path_new_regexp)
 {
     LLINT_BEGIN();
     RegExp* regExp = exec->codeBlock()->regexp(pc[2].u.operand);
-
-    // FIXME: If the RegExp is invalid, we should emit a different bytecode.
-    // https://bugs.webkit.org/show_bug.cgi?id=180970
-    if (!regExp->isValid())
-        LLINT_THROW(createSyntaxError(exec, regExp->errorMessage()));
+    ASSERT(regExp->isValid());
     LLINT_RETURN(RegExpObject::create(vm, exec->lexicalGlobalObject()->regExpStructure(), regExp));
 }