If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Very easy to hang with sticky global regexp

VERIFIED FIXED in Firefox 23

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: sstangl)

Tracking

({hang, testcase})

Trunk
mozilla24
x86_64
Mac OS X
hang, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

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

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
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);
(Assignee)

Comment 1

4 years ago
Most likely a regression from Bug 876976.
Assignee: general → sstangl
Depends on: 876976
(Assignee)

Comment 2

4 years ago
Created attachment 756277 [details] [diff] [review]
Correctly apply displament to MatchOnly results in sticky mode

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)
(Assignee)

Updated

4 years ago
status-firefox21: --- → affected
status-firefox22: --- → affected
status-firefox23: --- → affected
status-firefox24: --- → affected
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+
(Assignee)

Comment 4

4 years ago
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
(Assignee)

Comment 5

4 years ago
Created attachment 758272 [details] [diff] [review]
Minimal patch for beta and aurora

Same fix, without removing the unnecessary O(n) patching.
Attachment #758272 - Flags: review?(mrosenberg)
Attachment #758272 - Flags: review?(mrosenberg) → review+

Comment 6

4 years ago
https://hg.mozilla.org/mozilla-central/rev/1d1e0a7011fd
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
(Assignee)

Comment 7

4 years ago
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?
(Reporter)

Comment 8

4 years ago
It might be good to add some correctness tests for these kinds of regexps.
Flags: in-testsuite?

Updated

4 years ago
status-firefox24: affected → fixed
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.
status-firefox21: affected → wontfix
status-firefox22: affected → wontfix
status-firefox23: affected → fixed
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
status-firefox24: fixed → verified
You need to log in before you can comment on or make changes to this bug.