Very easy to hang with sticky global regexp

VERIFIED FIXED in Firefox 23

Status

()

defect
--
critical
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: jruderman, Assigned: sstangl)

Tracking

({hang, testcase})

Trunk
mozilla24
x86_64
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox21 wontfix, firefox22 wontfix, firefox23 fixed, firefox24 verified)

Details

Attachments

(2 attachments)

This hangs the shell:

var r = /(\1)/gy;
var s = "Z";
s.match(r);

So does this:

var r = /((?=a))+/gy;
var s = "aa";
s.match(r);
Most likely a regression from Bug 876976.
Assignee: general → sstangl
Depends on: 876976
This is a longstanding bug from the MatchOnly refactoring. IIRC this affects release -- it won't loop forever, but results for sticky regexps in MatchOnly scenarios will be nonsense. But only when compilation fails and we fall back to the YARR interpreter.

The problem is that the "result" value, representing the start offset, did not have the sticky displacement applied. This caused empty matches to appear to have a positive length.

This patch also fixed calling displace() for all of the matches, when only displacing the first match is necessary.
Attachment #756277 - Flags: review?(kvijayan)
Comment on attachment 756277 [details] [diff] [review]
Correctly apply displament to MatchOnly results in sticky mode

Review of attachment 756277 [details] [diff] [review]:
-----------------------------------------------------------------

I don't know this code that well, but based on the description of the bug, this looks like a reasonable course of action.
Attachment #756277 - Flags: review?(kvijayan) → review+
Thanks! I'll put up a minimalistic patch to fix aurora (maybe beta) once this gets onto m-c.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d1e0a7011fd
Same fix, without removing the unnecessary O(n) patching.
Attachment #758272 - Flags: review?(mrosenberg)
Attachment #758272 - Flags: review?(mrosenberg) → review+
https://hg.mozilla.org/mozilla-central/rev/1d1e0a7011fd
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Comment on attachment 758272 [details] [diff] [review]
Minimal patch for beta and aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 808245
User impact if declined: Executing certain regexps causes the browser to enter indefinite busy loops.
Testing completed (on m-c, etc.): m-c, local
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none
Attachment #758272 - Flags: approval-mozilla-beta?
Attachment #758272 - Flags: approval-mozilla-aurora?
It might be good to add some correctness tests for these kinds of regexps.
Flags: in-testsuite?
Comment on attachment 758272 [details] [diff] [review]
Minimal patch for beta and aurora

this has been around since FF20 so let's take it on Aurora and let it ride the trains to beta to avoid unnecessary risk in the last betas.
Attachment #758272 - Flags: approval-mozilla-beta?
Attachment #758272 - Flags: approval-mozilla-beta-
Attachment #758272 - Flags: approval-mozilla-aurora?
Attachment #758272 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/70f5ae9b46ff

I forgot to set the author before pushing :(. Please make sure your attached patches have it in the future.
Reproduced in nightly 2013-05-30 using the testcases in comment 0.
No hang in jshell-mac FF 24b6, Mac OS X 10.7.5.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.