Closed
Bug 380833
Opened 17 years ago
Closed 17 years ago
Crash during GC after uneval (involves E4X, mysterious sharp variable)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: jruderman, Assigned: igor)
References
Details
(4 keywords, Whiteboard: [sg:critical] regression from 308429)
Attachments
(2 files)
944 bytes,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
2.03 KB,
text/plain
|
Details |
js> www = <x><y/></x>; print(uneval(this) + "\n"); gc(); ({www:#1=<x> <y/> </x>}) Exception: EXC_BAD_ACCESS (0x0001) Codes: KERN_INVALID_ADDRESS (0x0001) at 0xdddddc00 Thread 0 Crashed: 0 js 0x00053264 js_GetGCThingFlags + 26 (jsgc.c:512) 1 js 0x00055663 JS_CallTracer + 495 (jsgc.c:2146) 2 js 0x0008696b js_TraceObject + 811 (jsobj.c:4898) 3 js 0x000548ec JS_TraceChildren + 140 (jsgc.c:1812) 4 js 0x0005577d JS_CallTracer + 777 (jsgc.c:2172) 5 js 0x0008696b js_TraceObject + 811 (jsobj.c:4898) 6 js 0x000548ec JS_TraceChildren + 140 (jsgc.c:1812) 7 js 0x0005577d JS_CallTracer + 777 (jsgc.c:2172) 8 js 0x00055ccb js_TraceStackFrame + 355 (jsgc.c:2333) 9 js 0x0005639a js_TraceContext + 124 (jsgc.c:2445) 10 js 0x000568d5 js_TraceRuntime + 179 (jsgc.c:2519) 11 js 0x00056b3b js_GC + 530 (jsgc.c:2747) 12 js 0x000161a7 JS_GC + 96 (jsapi.c:2372) 13 js 0x000037f6 GC + 45 (js.c:742) 14 js 0x00059941 js_Invoke + 2945 (jsinterp.c:1332) 15 js 0x0006a376 js_Interpret + 62065 (jsinterp.c:4025) 16 js 0x0005a295 js_Execute + 715 (jsinterp.c:1591) 17 js 0x0001b02f JS_ExecuteScript + 54 (jsapi.c:4692) 18 js 0x00002692 Process + 912 (js.c:268) 19 js 0x0000309b ProcessArgs + 2045 (js.c:519) 20 js 0x00007fb0 main + 612 (js.c:3256) 21 js 0x000021a6 _start + 216 22 js 0x000020cd start + 41
Reporter | ||
Comment 1•17 years ago
|
||
With less contamination: js> o = {}; o.a = <x><y/></x>; print(uneval(o)); gc(); ({a:#1=<x> <y/> </x>}) Segmentation fault
Assignee | ||
Updated•17 years ago
|
Assignee: general → igor
Assignee | ||
Comment 2•17 years ago
|
||
The problem is caused by the fact that during uneval(obj) the runtime will call JS_Enumerate of the xml object and that eventually will invoke xml_lookupProperty with id == 0 corresponding to the first child of the xml. That in turn calls js_AddNativeProperty and that locks the object and calls js_AddScopeProperty. In this particular case js_AddScopeProperty will allocate a new slot via calling js_AllocSlot. That slot is explicitly left uninitialized under the assumption that somebody will eventually set it before unlocking the scope. But this is never done in js_AddNativeProperty leaving the junk value (or explicit 0xddddddd8 marker with a debug build) in the slot and eventually crashing during tracing the object graph during GC. So I guess js_AddNativeProperty has to be patched to initialize the slot to something, but I am not sure under which conditions.
Assignee | ||
Comment 3•17 years ago
|
||
Here is almost trivial case to expose the same problem with uninitialized slot after js_AddNativeProperty: js> var x = <x><y/></x>; 0 in x; gc(); Segmentation fault
Reporter | ||
Comment 4•17 years ago
|
||
Is the "mysterious" (seemingly unnecessary) use of a sharp variable here related to that problem, or is it a separate bug?
Reporter | ||
Comment 5•17 years ago
|
||
Igor, do you know which patch from the last few days led to this regression?
Assignee | ||
Comment 6•17 years ago
|
||
(In reply to comment #5) > Igor, do you know which patch from the last few days led to this regression? I think this is an old problem, will test it later today.
Assignee | ||
Comment 7•17 years ago
|
||
(In reply to comment #4) > Is the "mysterious" (seemingly unnecessary) use of a sharp variable here > related to that problem, or is it a separate bug? This is a separated and unrelated bug.
Assignee | ||
Comment 8•17 years ago
|
||
This is a recent regression, see bug 308429. I was wrong to advise to set the slot to 0xddddddd8 there. If for some reason under way-too-much GC it was necessary to set the slot to void in AllocSlot, then there was bug somewhere else. At that point the free slot must be void as either ReallocSlot or js_Clear/js_FreeSlot sets the available slots to void.
Blocks: 308429
Assignee | ||
Comment 9•17 years ago
|
||
Attachment #265031 -
Flags: review?(brendan)
Comment 10•17 years ago
|
||
Comment on attachment 265031 [details] [diff] [review] fix v1 Whew, glad this was caught quickly. /be
Attachment #265031 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 11•17 years ago
|
||
I committed the patch from comment 9 to the trunk: Checking in jsobj.c; /cvsroot/mozilla/js/src/jsobj.c,v <-- jsobj.c new revision: 3.337; previous revision: 3.336 done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 12•17 years ago
|
||
I filed bug 380946 on the unnecessary sharping.
Updated•17 years ago
|
Flags: wanted1.8.1.x-
Flags: wanted1.8.0.x-
Whiteboard: post 1.8-branch
Comment 13•17 years ago
|
||
Updated•17 years ago
|
Flags: in-testsuite+
Updated•17 years ago
|
Whiteboard: post 1.8-branch → [sg:critical] post 1.8-branch
Updated•17 years ago
|
Group: security
Comment 14•17 years ago
|
||
verified fixed 1.9.0 linux/mac*/windows. /cvsroot/mozilla/js/tests/e4x/Regress/regress-380833.js,v <-- regress-380833.js initial revision: 1.1
Status: RESOLVED → VERIFIED
Comment 15•16 years ago
|
||
Wanted on the 1.8 branch if we take bug 308429
Flags: wanted1.8.1.x-
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.15?
Whiteboard: [sg:critical] post 1.8-branch → [sg:critical] regression from 308429
Updated•16 years ago
|
Flags: blocking1.8.1.15? → blocking1.8.1.15+
Updated•16 years ago
|
Keywords: fixed1.8.1.15
Comment 16•16 years ago
|
||
verified fixed 1.8.1.15 linux/mac/win
Keywords: fixed1.8.1.15 → verified1.8.1.15
You need to log in
before you can comment on or make changes to this bug.
Description
•