Closed Bug 9141 Opened 25 years ago Closed 25 years ago

Regular expression crashes system

Categories

(Core :: JavaScript Engine, defect, P3)

PowerPC
Mac System 8.6
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: waldemar, Assigned: rogerl)

Details

(Keywords: crash, js1.5)

The following page crashes the system: <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" "http://www.w3.org/ TR/REC-html40/loose.dtd"> <HTML> <HEAD> </HEAD> <BODY> <SCRIPT type="text/javascript"> var s = "x"; for (var i = 0; i != 13; i++) s += s; var a = /(?:xx|x)*/(s); var b = /(xx|x)*/(s); document.write("Results = " + a.length + "," + b.length); </SCRIPT> </BODY>
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → WORKSFORME
This works o.k. for me, both pc & mac. The newer version of the regexp. engine got checked in recently, and likely contained this fix.
Status: RESOLVED → VERIFIED
test case is mozilla/js/tests/js1_2/regexp/regress-9141.js
Status: VERIFIED → REOPENED
Executing var s = "x"; for (var i = 0; i != 13; i++) s += s; var a = /(?:xx|x)*/(s); still crashes my JSRef, taken from today's tip build. Waldemar
i haven't been able to reproduce a crash using either test case on linux and winnt. haven't tried the mac yet.
Status: REOPENED → RESOLVED
Closed: 25 years ago25 years ago
Target Milestone: M9
If cool on the Mac...please mark Verified
Status: RESOLVED → REOPENED
Not cool on the Mac (JSRef).
Resolution: WORKSFORME → ---
Clearing WorksForMe resolution since this is reopened.
Moving out to M10 per choffman's request
Target Milestone: M10 → M11
Target Milestone: M11 → M15
Runs fine on NT/Linux/Solaris - I need to get me at a mac. Waldemar, can you make this happen and then point me at the macsbug ?
Severity: normal → critical
Blocks: 20203
I checked in a version of the greedy matcher that uses less stack - hopefully this will fix it?? Waldemar, could you try this version out and let me know? Also, is there any way of increasing the stack on the Mac build to see how close this crash is to the limit? if it's not close at all I'll have to re-write the recursion to use a heap-based stack or something.
Changing stack usage by a constant factor would not fix this bug; I could just increase the 13 to 14 in the script to make it reappear. The problem is our inability to detect when the stack gets too large. I think it would be fine to refuse to process a regular expression and report a JavaScript error if the regular expression stack gets too large (or greater than some large fixed amount). I don't think it's fine to crash.
Yes, I agree. But, a). I was simply curious to see if my latest changes (which I'd made for clean-up purposes, not as a fix for this bug) had any impact on the bug, since it's pure speculation on my part that it's a stack crash issue. b). The js engine doesn't catch stack failures in other circumstances - relying on the OS to do something 'sensible'. (I'm not defending this !)
Blocks: 20870
Keywords: js1.5
This bug is still happening. Beard and I hacked around in jsregexp.c last night, adding a call to check the mac StackSpace() function in greedyRecurse() (diff below). This fixes the crash, but the patch needs a bit of work before its Right, perhaps a "match failed" flag in the match state structure, or an extra "failed" param in the greedyRecurse() function. Marking js1.5 Index: jsregexp.c =================================================================== RCS file: /cvsroot/mozilla/js/src/jsregexp.c,v retrieving revision 3.19 diff -c -2 -r3.19 jsregexp.c *** jsregexp.c 1999/12/22 21:57:08 3.19 --- jsregexp.c 2000/01/12 02:24:53 *************** *** 57,60 **** --- 57,64 ---- #include "jsstr.h" + #ifdef XP_MAC + #include <MacMemory.h> + #endif + #if JS_HAS_REGEXPS *************** *** 1321,1324 **** --- 1325,1335 ---- const jschar *kidMatch; const jschar *match; + + #ifdef XP_MAC + if (StackSpace() < 16384) { + JS_ReportOutOfMemory (grState->state->context); + return NULL; + } + #endif kidMatch = matchRENodes(grState->state, grState->kid, grState->next, cp);
Target Milestone: M13 → M14
Not going to make M13 for these.
QA Contact: cbegle → rginda
Updating QA Contact.
Adding "crash" keyword to all known open crasher bugs.
Keywords: crash
Checked in patch + set & test early out in greedyRecurse() when state->ok goes false.
Status: REOPENED → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
Marking Verified.
Status: RESOLVED → VERIFIED
No longer blocks: 20203
No longer blocks: 20870
You need to log in before you can comment on or make changes to this bug.