Closed Bug 877912 Opened 9 years ago Closed 9 years ago

Very easy to hang with sticky global regexp


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- fixed
firefox24 --- verified


(Reporter: jruderman, Assigned: sstangl)



(Keywords: hang, testcase)


(2 files)

This hangs the shell:

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

So does this:

var r = /((?=a))+/gy;
var s = "aa";
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.
Same fix, without removing the unnecessary O(n) patching.
Attachment #758272 - Flags: review?(mrosenberg)
Attachment #758272 - Flags: review?(mrosenberg) → review+
Closed: 9 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+

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.
You need to log in before you can comment on or make changes to this bug.