Closed Bug 509143 Opened 14 years ago Closed 14 years ago

js_CloneRegExp is missing a call to js_SetLastIndex

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: Waldo, Assigned: igor)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(2 files, 4 obsolete files)

crash stack
3.82 KB, text/plain
Details
v4
10.57 KB, patch
igor
: review+
Details | Diff | Splinter Review 
Fallout from bug 493457.
Attached patch v1 (obsolete) — — Splinter Review 
Besides adding the missed SetLastIndex call the patch cleanups lastIndex access to access fixed slots directly.
Attachment #393632 - Flags: review?(jwalden+bmo)
Attached patch v2 (obsolete) — — Splinter Review 
The updated patch inlines js_ClearRegExpLastIndex.
Attachment #393632 - Attachment is obsolete: true
Attachment #394301 - Flags: review?(jwalden+bmo)
Attachment #393632 - Flags: review?(jwalden+bmo)
Attachment #394301 - Flags: review?(jwalden+bmo) → review?(mrbkap)
Comment on attachment 394301 [details] [diff] [review]
v2

>@@ -5323,17 +5352,17 @@ js_XDRRegExpObject(JSXDRState *xdr, JSOb
>         if (!obj)
>             return JS_FALSE;
>         STOBJ_CLEAR_PARENT(obj);
>         STOBJ_CLEAR_PROTO(obj);
>         re = js_NewRegExp(xdr->cx, NULL, source, (uint8)flagsword, JS_FALSE);
>         if (!re)
>             return JS_FALSE;
>         obj->setPrivate(re);
>-        if (!js_SetLastIndex(xdr->cx, obj, 0))
>+        js_ClearRegExpLastIndex(0);
>             return JS_FALSE;

You need to nuke the return here.

r=mrbkap with that.
Attachment #394301 - Flags: review?(mrbkap) → review+
Attached patch v3 (obsolete) — — Splinter Review 
The new patch fixes the issue from the comment 3.
Attachment #394301 - Attachment is obsolete: true
Attachment #394856 - Flags: review+
Attached patch v3 for real (obsolete) — — Splinter Review 
The previous attachment has a wrong patch.
Attachment #394856 - Attachment is obsolete: true
Attachment #394865 - Flags: review+
http://hg.mozilla.org/tracemonkey/rev/5819b38a8686

I may not be available for longer today, so if any troubles, please backout this from Tracemonkey.
Whiteboard: fixed-in-tracemonkey
Attached file crash stack — 
backed out, crash attached
Whiteboard: fixed-in-tracemonkey
Attached patch v4 — — Splinter Review 
The one-liner fix for the crash above. In the patch I have used js_ClearRegExpLastIndex(0), not js_ClearRegExpLastIndex(obj). Sorry for wasted time with the backout.
Attachment #394865 - Attachment is obsolete: true
Attachment #395624 - Flags: review+
relanded - http://hg.mozilla.org/tracemonkey/rev/8e8a71e91c81
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/8e8a71e91c81
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.