613399 - "Assertion failure: size_t(limit) <= inputLength"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

/((?=()+))?/.exec("")

Assertion failure: size_t(limit) <= inputLength, at /Users/jruderman/tracemonkey/js/src/jsregexpinlines.h:232


This might be a morphed bug 576836, or it might have been introduced in rev 552bb56871e0 from bug 571355. I'm too confused to tell.

Comment 1

[Nicholas Nethercote [inactive]]

With Valgrind's help (especially the VALGRIND_CHECK_VALUE_IS_DEFINED macro) 
I determined that buf[1] is undefined in checkMatchPairs() just before the
assertion fails.

Working backwards, in RegExp::executeInternal(), buf[1] is defined before 
the call to JSC::Yarr::executeRegex(), but undefined afterwords.

In executeRegex(), we take the fallback case and call jsRegExpExecute().  In
jsRegExpExecute(), this line assigns buf[1] (which is called offsets[1] 
here) an undefined value:

    offsets[1] = matchBlock.endMatchPtr - matchBlock.startSubject;
            
It's undefined because matchBlock.endMatchPtr is undefined.  And that is 
because the call to match() inside jsRegExpExecute() is failing to set 
matchBlock.endMatchPtr.  And the only way that match() can set 
matchBlock.endMatchPtr is when the OP_END opcode is encountered.

So it appears that the compiled fallback JSRegExp is missing the OP_END 
opcode, which is presumably something that should appear at the end of every
compiled JSRegExp?

Comment 2

[Nicholas Nethercote [inactive]]

I stepped through pcre_exec.cpp:match() in the debugger, and confirmed that the OP_END is not hit.  I didn't understand what was happening, though, all this jumping to RECURSE and use of the RRETURN macro bamboozled me.

Maybe cdleary understands how match() works well enough to work out why OP_END wasn't hit and whether this is a problem.

Comment 3

[Jesse Ruderman]

Sounds like we could use an assertion that we see an OP_END!

Comment 4

[David Mandelin [:dmandelin]]

Attachment: WIP Patch

Fixes the problem, passes jit-test and jstest. Now running on try.

Comment 5

[David Mandelin [:dmandelin]]

Attachment: Patch

Comment 6

[Chris Leary [:cdleary] (not checking bugmail)]

Comment on attachment 503391 [details] [diff] [review]
Patch

Just to take some notes on what was found through debugging, the isMatch acts, in most instances, as a flag that indicates "done", and therefore is false most of the time. (The notable exception is zero-width assertions which indicate whether they successfully matched via this flag).

Because this was RRETURNing without setting the isMatch flag back to false, the "parent" opcode took this as a signal the pattern was finished and we returned all the way out to the point where the pattern matching was completed. Instead, by having isMatch of false, the "parent" opcode continues on executing the pattern's bytecodes, knowing it had successfully matched the necessary number of times in a previous iteration.

Dave, correct me if I'm wrong in any of that.

Comment 7

[David Mandelin [:dmandelin]]

That's exactly my understanding too.

http://hg.mozilla.org/tracemonkey/rev/c8cb1ab7612a

Comment 8

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/c8cb1ab7612a

Comment 9

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug613399.js.