Closed Bug 613399 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: JavaScript Engine, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: jruderman, Assigned: dmandelin)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: hardblocker, fixed-in-tracemonkey)

Attachments

(1 file, 1 obsolete file)

/((?=()+))?/.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.
blocking2.0: --- → ?
Assignee: general → cdleary
blocking2.0: ? → betaN+
Keywords: regression
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?
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.
Sounds like we could use an assertion that we see an OP_END!
Whiteboard: hardblocker
Attached patch WIP Patch (obsolete) — Splinter Review
Fixes the problem, passes jit-test and jstest. Now running on try.
Assignee: cdleary → dmandelin
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
Attachment #503001 - Attachment is obsolete: true
Attachment #503391 - Flags: review?(cdleary)
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.
Attachment #503391 - Flags: review?(cdleary) → review+
That's exactly my understanding too.

http://hg.mozilla.org/tracemonkey/rev/c8cb1ab7612a
Whiteboard: hardblocker → hardblocker, fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/c8cb1ab7612a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug613399.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.