Closed
Bug 783315
Opened 12 years ago
Closed 12 years ago
Intermittent crash in jsreftest.html?test=js1_8_1/regress/regress-452498-117.js [@ js::gc::MarkInternal<js::PropertyName>]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: emorley, Assigned: luke)
References
Details
(Keywords: crash, intermittent-failure)
Crash Data
Attachments
(1 file)
11.74 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #783278 +++ Rev4 MacOSX Lion 10.7 mozilla-inbound debug test jsreftest on 2012-08-16 09:29:28 PDT for push 4392d5928cf1 slave: talos-r4-lion-020 https://tbpl.mozilla.org/php/getParsedLog.php?id=14440085&tree=Mozilla-Inbound { REFTEST TEST-START | file:///Users/cltbld/talos-slave/test/build/jsreftest/tests/jsreftest.html?test=js1_8_1/regress/regress-452498-117.js | 2925 / 3198 (91%) ++DOMWINDOW == 80 (0x146e7e320) [serial = 5488] [outer = 0x1111a1890] BUGNUMBER: 452498 STATUS: TM: upvar2 regression tests TEST-UNEXPECTED-FAIL | file:///Users/cltbld/talos-slave/test/build/jsreftest/tests/jsreftest.html?test=js1_8_1/regress/regress-452498-117.js | Exited with code 1 during test run INFO | automation.py | Application ran for: 0:12:56.183104 INFO | automation.py | Reading PID log: /var/folders/qd/srwd5f710sj0fcl9z464lkj00000gn/T/tmphS5nmlpidlog Downloading symbols from: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-macosx64-debug/1345132906/firefox-17.0a1.en-US.mac64.crashreporter-symbols.zip PROCESS-CRASH | file:///Users/cltbld/talos-slave/test/build/jsreftest/tests/jsreftest.html?test=js1_8_1/regress/regress-452498-117.js | application crashed (minidump found) Crash dump filename: /var/folders/qd/srwd5f710sj0fcl9z464lkj00000gn/T/tmp3cVUcc/minidumps/0768A338-BEF1-43F6-B029-646A2C2F1CD6.dmp Operating system: Mac OS X 10.7.2 11C74 CPU: amd64 family 6 model 23 stepping 10 2 CPUs Crash reason: EXC_BAD_ACCESS / 0x0000000d Crash address: 0x0 Thread 0 (crashed) 0 XUL!js::gc::MarkInternal<js::PropertyName> [Heap.h : 1010 + 0x0] rbx = 0x05c15268 r12 = 0x031690b1 r13 = 0xcdcdc000 r14 = 0xcdcdcdc8 r15 = 0x5fbf7240 rip = 0x02e326cb rsp = 0x5fbf7200 rbp = 0x5fbf7230 Found by: given as instruction pointer in context 1 XUL!JSScript::markChildren [jsscript.cpp : 232 + 0xe] rip = 0x02d12b3a rsp = 0x5fbf7240 Found by: stack scanning 2 XUL!js::gc::PushArenaTyped<JSScript> [Marking.cpp : 773 + 0xa] rip = 0x02e313a2 rsp = 0x5fbf7280 Found by: stack scanning 3 XUL!js::gc::PushArena [Marking.cpp : 929 + 0xa] rip = 0x02e2d17d rsp = 0x5fbf72f0 Found by: stack scanning }
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 2•12 years ago
|
||
If I'm reading the code correctly, I think Luke added this MarkInternal<PropertyName> thing in bug 775323.
Assignee | ||
Comment 3•12 years ago
|
||
You're right. I just reproduced in a debugger; I seem to have a JSScript where almost, but not all, of the fields have been nulled out. I wonder if it is one of these degenerate scripts which doesn't call the normal JSScript init functions...
Assignee | ||
Comment 4•12 years ago
|
||
This is a fun corner case: if there is an error after generating bindings but before a script is fully emitted, then Bindings can be left pointing into the LifoAlloc'd storage. This would be fine if the script wasn't reachable but apparently it was being found in this test via conservative stack scanning. With this patch, I can't reproduce the crash.
Comment 5•12 years ago
|
||
Bug 783278 has what is probably the same crash, if you haven't checked that out already.
Assignee | ||
Comment 6•12 years ago
|
||
Thanks, yes, looks like the same thing.
Comment on attachment 652542 [details] [diff] [review] fix Review of attachment 652542 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsscript.cpp @@ +75,5 @@ > return false; > } > > + JS_ASSERT(!(size_t(bindingArray) & TEMPORARY_STORAGE_BIT)); > + bindingArrayAndFlag_ = size_t(bindingArray) | TEMPORARY_STORAGE_BIT; size_t -> uintptr_t @@ +139,4 @@ > { > + JS_ASSERT(bindingArrayUsingTemporaryStorage()); > + PodCopy(newBindingArray, bindingArray(), count()); > + bindingArrayAndFlag_ = size_t(newBindingArray); uintptr_t. Also, you should probably check the low bit here if you check it above. Maybe have a private setter for this field? @@ +238,2 @@ > PropertyName *name = b->name(); > MarkStringUnbarriered(trc, &name, "bindingArray"); What keeps this stuff alive during parsing? gcKeepAtoms? If so, please comment about it. ::: js/src/jsscript.h @@ +129,5 @@ > friend class BindingIter; > friend class AliasedFormalIter; > > HeapPtr<Shape> callObjShape_; > + size_t bindingArrayAndFlag_; I think this should be a uintptr_t. @@ +143,5 @@ > + * error path, which is error-prone, the solution used it to not mark > + * bindingArray when it points to temporary storage. This is safe since > + * compilation already keeps atoms alive (hence no rooting is necessary). > + * The "temporary" status is tracked a bit stolen from the bindingArray > + * pointer. This comment seems too low-level to me. I think something like this would be more helpful (unless this info is available elsewhere): During parsing, bindings are allocated in a temporary LifoAlloc arena. After parsing, a JSScript object is created and the bindings are permanently transferred to it. On some error paths, the JSScript object may end up with bindings that still point to the (now dead) LIFO memory. To avoid tracing these bindings during GC, we keep track of whether the bindings are temporary or permanent in the low bit of bindingArrayAndFlag_.
Attachment #652542 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to Bill McCloskey (:billm) from comment #7) Thanks! https://hg.mozilla.org/integration/mozilla-inbound/rev/c068429832ea
Comment 9•12 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=14455738&tree=Ionmonkey (in js1_8_1/extensions/regress-437288-01.js)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Comment 11•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c068429832ea
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Updated•12 years ago
|
Keywords: intermittent-failure
Updated•12 years ago
|
Whiteboard: [orange]
You need to log in
before you can comment on or make changes to this bug.
Description
•