diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog index c59463680b5cb..477439444ec48 100644 --- a/JSTests/ChangeLog +++ b/JSTests/ChangeLog @@ -1,3 +1,16 @@ +2021-09-29 Saam Barati + + Code inside strength reduction can incorrectly prove that we know what lastIndex is + https://bugs.webkit.org/show_bug.cgi?id=230802 + + + Reviewed by Mark Lam. + + * stress/dont-fold-regexp-exec-when-we-dont-know-last-index-and-regexp-is-constant.js: Added. + (assert): + (let.reg.RegExp.foo.g.doExec): + (noInline.doExec): + 2021-09-29 Saam Barati DoesGCCheck does not use enough bits for nodeIndex diff --git a/JSTests/stress/dont-fold-regexp-exec-when-we-dont-know-last-index-and-regexp-is-constant.js b/JSTests/stress/dont-fold-regexp-exec-when-we-dont-know-last-index-and-regexp-is-constant.js new file mode 100644 index 0000000000000..0740a33090b1d --- /dev/null +++ b/JSTests/stress/dont-fold-regexp-exec-when-we-dont-know-last-index-and-regexp-is-constant.js @@ -0,0 +1,19 @@ +function assert(b) { + if (!b) + throw new Error; +} + +let reg = RegExp(/foo/g) +function doExec() { + return reg.exec("-foo"); +} +noInline(doExec) + +for (let i = 0; i < 1000; ++i) { + let r = doExec(); + if ((i % 2) === 0) + assert(r[0] === "foo"); + else + assert(r === null); +} + diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index 249d4bc7c5ce3..1254ab2133a76 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,29 @@ +2021-09-29 Saam Barati + + Code inside strength reduction can incorrectly prove that we know what lastIndex is + https://bugs.webkit.org/show_bug.cgi?id=230802 + + + Reviewed by Mark Lam. + + The phase was searching backwards in the graph to see if it found the RegExp + node. However, the RegExp node might be a JSConstant. Hence, the program + didn't allocate it. So we can't assume that we know what the lastIndex is. + We were incorrectly assuming it was "0" in a program like this: + a: JSConstant(RegExp) + b: RegExpExec(@a) + + And we assumed we're invoking RegExpExec with lastIndex is 0, because we found + our RegExp in a backwards search. This is likely because we're also matching + NewRegExp nodes, in which case, it is valid to say lastIndex is 0. + + This caused us to return a constant value that would've been the exec + result had we invoked it with a NewRegExpNode. + + * dfg/DFGStrengthReductionPhase.cpp: + (JSC::DFG::StrengthReductionPhase::run): + (JSC::DFG::StrengthReductionPhase::handleNode): + 2021-09-29 Yusuke Suzuki [JSC] Use FixedVector in JITConstantPool diff --git a/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp b/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp index 47dd17184a1e7..250a3b4872992 100644 --- a/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp +++ b/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp @@ -491,11 +491,13 @@ class StrengthReductionPhase : public Phase { Node* regExpObjectNode = nullptr; RegExp* regExp = nullptr; + bool regExpObjectNodeIsConstant = false; if (m_node->op() == RegExpExec || m_node->op() == RegExpTest || m_node->op() == RegExpMatchFast) { regExpObjectNode = m_node->child2().node(); - if (RegExpObject* regExpObject = regExpObjectNode->dynamicCastConstant(vm())) + if (RegExpObject* regExpObject = regExpObjectNode->dynamicCastConstant(vm())) { regExp = regExpObject->regExp(); - else if (regExpObjectNode->op() == NewRegexp) + regExpObjectNodeIsConstant = true; + } else if (regExpObjectNode->op() == NewRegexp) regExp = regExpObjectNode->castOperand(); else { if (verbose) @@ -545,6 +547,8 @@ class StrengthReductionPhase : public Phase { for (unsigned otherNodeIndex = m_nodeIndex; otherNodeIndex--;) { Node* otherNode = m_block->at(otherNodeIndex); if (otherNode == regExpObjectNode) { + if (regExpObjectNodeIsConstant) + break; lastIndex = 0; break; }