877912 - Very easy to hang with sticky global regexp

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

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);

Comment 1

[Sean Stangl [:sstangl]]

Most likely a regression from Bug 876976.

Comment 2

[Sean Stangl [:sstangl]]

Attachment: 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.

Comment 3

[Marty Rosenberg [:mjrosenb]]

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.

Comment 4

[Sean Stangl [:sstangl]]

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

Comment 5

[Sean Stangl [:sstangl]]

Attachment: Minimal patch for beta and aurora

Same fix, without removing the unnecessary O(n) patching.

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1d1e0a7011fd

Comment 7

[Sean Stangl [:sstangl]]

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

Comment 8

[Jesse Ruderman]

It might be good to add some correctness tests for these kinds of regexps.

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 11

[Paul Silaghi, QA [:pauly]]

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.