Closed Bug 378492 Opened 18 years ago Closed 18 years ago

namespace_trace/qname_trace does not check for null private

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: igor)

References

Details

(4 keywords, Whiteboard: [sg:critical?] post 1.8-branch)

Attachments

(1 file, 1 obsolete file)

In a WAY_TOO_MUCH_GC build of the JavaScript shell: js> x = <x/>; for each(x.t in x) { } Bus error Thread 0 Crashed: 0 js 0x0005271c js_GetGCThingFlags + 26 (jsgc.c:512) 1 js 0x0005564d JS_CallTracer + 457 (jsgc.c:2517) 2 js 0x000cbfa7 namespace_trace + 93 (jsxml.c:216) 3 js 0x00086246 js_TraceObject + 407 (jsobj.c:4845) 4 js 0x000548f8 JS_TraceChildren + 139 (jsgc.c:2184) 5 js 0x00055767 JS_CallTracer + 739 (jsgc.c:2543) 6 js 0x00055826 js_CallGCThingTracer + 113 (jsgc.c:2583) 7 js 0x000566e6 js_TraceContext + 1013 (jsgc.c:2842) 8 js 0x00056912 TraceRuntime + 179 (jsgc.c:2886) 9 js 0x00056b68 js_GC + 530 (jsgc.c:3117) 10 js 0x00053762 js_NewGCThing + 186 (jsgc.c:1485) 11 js 0x0007ff0c js_NewObject + 246 (jsobj.c:2433) 12 js 0x0005184a js_NewFunction + 139 (jsfun.c:2119) 13 js 0x00051ac5 js_DefineFunction + 61 (jsfun.c:2188) 14 js 0x00015dac JS_InitClass + 559 (jsapi.c:2237) 15 js 0x000e0b61 js_InitNamespaceClass + 102 (jsxml.c:7591) 16 js 0x000803cf js_GetClassObject + 381 (jsobj.c:2572) 17 js 0x0008065a js_FindClassObject + 331 (jsobj.c:2625) 18 js 0x0008097f js_ConstructObject + 196 (jsobj.c:2663) 19 js 0x000e13b0 js_GetDefaultXMLNamespace + 310 (jsxml.c:7796) 20 js 0x000d1416 ParseXMLSource + 35 (jsxml.c:1974) 21 js 0x000d1d65 ToXML + 515 (jsxml.c:2126) 22 js 0x000e26b5 js_ValueToXMLObject + 24 (jsxml.c:8251) 23 js 0x0007243e js_Interpret + 94988 (jsinterp.c:5611) 24 js 0x0005a2c2 js_Execute + 715 (jsinterp.c:1591) 25 js 0x0001a60a JS_ExecuteScript + 54 (jsapi.c:4241) 26 js 0x00002a72 Process + 912 (js.c:268) 27 js 0x0000347b ProcessArgs + 2045 (js.c:519) 28 js 0x000082f0 main + 612 (js.c:3230) 29 js 0x00002586 _start + 216 30 js 0x000024ad start + 41 (In a normal js shell, this causes an assertion after about 70 seconds instead of crashing immediately.)
Flags: blocking1.9?
Assignee: general → igor
Whiteboard: [sg:critical?]
The bug happens just with executing '<x>': ~/m/trunk/mozilla/js/src $ ./Linux_All_DBG.OBJ/js -e '<x/>' segmentation fault
This is a trunk-only regression from bug 375270. When implementing the trace patch I decided not to allow to pass null to JS_CallTracer since in many cases the check for null was already done. But that required to add the null checks to other trace calls and I missed that for namespace_trace/qname_trace. Thus when GC runs after constructing Namespace.prototype object but before its private thing is created, the GC will trace Namespace.prototype with private == null and calls GetGCThingFlags on null.
Blocks: 375270
Summary: Crash [@ js_GetGCThingFlags] with E4X, "for each" → namespace_trace/qname_trace does not check for null private
Keywords: regression
Attached patch Fix (obsolete) (deleted) — Splinter Review
After creating an initial fix, I have found another problem (and only so far) exposed by the same test case. I missed the fact that JSContext.lastInternalResult can be an arbitrary GC thing (read XML) tagged as object so I could not use JS_CALL_VALUE_TRACER without extra checks. Since the same checks are done in few other places, I consolidated them into js_CallGCThingAsValueTracer function. Hopefully the ugly long name of the function would remind that (jsval)nonObjectGCThing is a hack.
Attachment #262684 - Flags: review?(brendan)
Comment on attachment 262684 [details] [diff] [review] Fix js_CallGCThingAsValueTracer seems misnamed -- it should be js_CallValueTracerIfGCThing or some such, no? r=me with that fixed. /be
Attachment #262684 - Flags: review?(brendan) → review+
(In reply to comment #4) > (From update of attachment 262684 [details] [diff] [review]) > js_CallGCThingAsValueTracer seems misnamed -- it should be > js_CallValueTracerIfGCThing or some such, no? Yes! That is indeed much better.
Attached patch Fix v1b (deleted) — Splinter Review
The patch to commit with js_CallGCThingAsValueTracer -> js_CallValueTracerIfGCThing rename.
Attachment #262684 - Attachment is obsolete: true
Attachment #262695 - Flags: review+
I committed the patch from comments 6 to the trunk: Checking in jsapi.c; /cvsroot/mozilla/js/src/jsapi.c,v <-- jsapi.c new revision: 3.315; previous revision: 3.314 done Checking in jscntxt.c; /cvsroot/mozilla/js/src/jscntxt.c,v <-- jscntxt.c new revision: 3.108; previous revision: 3.107 done Checking in jsgc.c; /cvsroot/mozilla/js/src/jsgc.c,v <-- jsgc.c new revision: 3.214; previous revision: 3.213 done Checking in jsgc.h; /cvsroot/mozilla/js/src/jsgc.h,v <-- jsgc.h new revision: 3.67; previous revision: 3.66 done Checking in jsxml.c; /cvsroot/mozilla/js/src/jsxml.c,v <-- jsxml.c new revision: 3.156; previous revision: 3.155 done
Group: security
Status: NEW → RESOLVED
CC list accessible: false
Closed: 18 years ago
Not accessible to reporter
Resolution: --- → FIXED
/cvsroot/mozilla/js/tests/js1_6/Regress/regress-378492.js,v <-- regress-378492.js initial revision: 1.1 /cvsroot/mozilla/js/tests/e4x/Regress/regress-378492.js,v <-- regress-378492.js initial revision: 1.1 verified fixed 1.9.0 linux with js shell compiled with way too much gc.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Flags: wanted1.8.1.x-
Flags: wanted1.8.0.x-
Whiteboard: [sg:critical?] → [sg:critical?] post 1.8-branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: