Closed Bug 9141 Opened 25 years ago Closed 25 years ago

Regular expression crashes system

Categories

(Core :: JavaScript Engine, defect, P3)

Product:
Core
Component:
JavaScript Engine
Platform:
PowerPC
Mac System 8.6
Type:
defect

Tracking

()

Status:
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 ago → 25 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 ago → 25 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.