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)
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)
(deleted),
patch
|
igor
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Updated•18 years ago
|
Assignee: general → igor
Reporter | ||
Updated•18 years ago
|
Whiteboard: [sg:critical?]
Assignee | ||
Comment 1•18 years ago
|
||
The bug happens just with executing '<x>':
~/m/trunk/mozilla/js/src $ ./Linux_All_DBG.OBJ/js -e '<x/>'
segmentation fault
Assignee | ||
Comment 2•18 years ago
|
||
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
Assignee | ||
Updated•18 years ago
|
Keywords: regression
Assignee | ||
Comment 3•18 years ago
|
||
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 4•18 years ago
|
||
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+
Assignee | ||
Comment 5•18 years ago
|
||
(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.
Assignee | ||
Comment 6•18 years ago
|
||
The patch to commit with
js_CallGCThingAsValueTracer -> js_CallValueTracerIfGCThing
rename.
Attachment #262684 -
Attachment is obsolete: true
Attachment #262695 -
Flags: review+
Assignee | ||
Comment 7•18 years ago
|
||
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
Comment 8•18 years ago
|
||
/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+
Updated•17 years ago
|
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.
Description
•