Closed
Bug 1228400
Opened 9 years ago
Closed 8 years ago
crash in mozilla::dom::PBrowserChild::SendPDocAccessibleConstructor
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: davidb, Assigned: tbsaunde)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
1.15 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-414ed0b4-d069-41b0-885a-c4ada2151124. ============================================================= More here: https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Adom%3A%3APBrowserChild%3A%3ASendPDocAccessibleConstructor
Updated•9 years ago
|
Flags: needinfo?(bobowen.code)
Comment 1•9 years ago
|
||
All the reports I spot checked are crashing on a line which dereferences the actor (first argument to SendPDocAccessibleConstructor). That doesn't make any sense as it's been null checked just above. The line calls Register, which dereferences the mManager (which will be the PContentChild). So, could this be getting called during some sort of tear down where the PContentChild has already gone away? Has anyone been able to find steps to reproduce this?
Flags: needinfo?(bobowen.code) → needinfo?(tbsaunde+mozbugs)
Assignee | ||
Comment 2•9 years ago
|
||
(In reply to Bob Owen (:bobowen) from comment #1) > All the reports I spot checked are crashing on a line which dereferences the > actor (first argument to SendPDocAccessibleConstructor). > > That doesn't make any sense as it's been null checked just above. > The line calls Register, which dereferences the mManager (which will be the > PContentChild). can you post disassembly for one of the crashes? I'm curious which of the actor or mManager is actually null. It certainly doesn't make sense its the actor we are creating since we just created that with new and yeah its also null checked. > So, could this be getting called during some sort of tear down where the > PContentChild has already gone away? I would guess not looking at the first stack I saw it was while handling a normal async message from the parent about vsync nothing shutdown related. > Has anyone been able to find steps to reproduce this? I haven't really tried to reproduce, so I suspect not.
Flags: needinfo?(tbsaunde+mozbugs)
Comment 3•9 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #2) > (In reply to Bob Owen (:bobowen) from comment #1) > > All the reports I spot checked are crashing on a line which dereferences the > > actor (first argument to SendPDocAccessibleConstructor). > > > > That doesn't make any sense as it's been null checked just above. > > The line calls Register, which dereferences the mManager (which will be the > > PContentChild). > > can you post disassembly for one of the crashes? I'm curious which of the > actor or mManager is actually null. Here's the c++ source and disassembly from the start of SendPDocAccessibleConstructor: auto PBrowserChild::SendPDocAccessibleConstructor( PDocAccessibleChild* actor, PDocAccessibleChild* aParentDoc, const uint64_t& aParentAcc) -> PDocAccessibleChild* { 00007FFD6D6D8D30 push rbx 00007FFD6D6D8D32 push rbp 00007FFD6D6D8D33 push rsi 00007FFD6D6D8D34 push rdi 00007FFD6D6D8D35 push r14 00007FFD6D6D8D37 sub rsp,50h 00007FFD6D6D8D3B mov rax,qword ptr [__security_cookie (07FFD6F6221D0h)] 00007FFD6D6D8D42 xor rax,rsp 00007FFD6D6D8D45 mov qword ptr [rsp+40h],rax 00007FFD6D6D8D4A mov r14,r9 00007FFD6D6D8D4D mov rbp,r8 00007FFD6D6D8D50 mov rdi,rdx 00007FFD6D6D8D53 mov rsi,rcx if ((!(actor))) { 00007FFD6D6D8D56 test rdx,rdx 00007FFD6D6D8D59 jne mozilla::dom::PBrowserChild::SendPDocAccessibleConstructor+32h (07FFD6D6D8D62h) NS_WARNING("Error constructing actor PDocAccessibleChild"); return nullptr; 00007FFD6D6D8D5B xor eax,eax 00007FFD6D6D8D5D jmp mozilla::dom::PBrowserChild::SendPDocAccessibleConstructor+166h (07FFD6D6D8E96h) } (actor)->mId = Register(actor); 00007FFD6D6D8D62 lea rbx,[rcx+10h] 00007FFD6D6D8D66 mov rcx,rbx 00007FFD6D6D8D69 mov rax,qword ptr [rbx] 00007FFD6D6D8D6C call qword ptr [rax] End of disassembly. It's failing on the second to last instruction, because rbx contains 0x68 and so is an invalid location. So it seems that "this" is corrupt (0x58) presumably from the static cast in NotificationController::WillRefresh. That do_GetInterface into tabChild must be returning nullptr, which when static cast to TabChild* gives 0x58. So, we need a null check on tabChild.
Comment 4•9 years ago
|
||
(In reply to Bob Owen (:bobowen) from comment #3) > That do_GetInterface into tabChild must be returning nullptr, which when > static cast to TabChild* gives 0x58. Erm, I meant when cast to TabChild* and implicitly converted to a PBrowserChild* for the call to SendPDocAccessibleConstructor gives 0x58.
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8698034 -
Flags: review?(dbolter)
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8698034 [details] [diff] [review] null check tabChild before notifying the parent process about new child documents Review of attachment 8698034 [details] [diff] [review]: ----------------------------------------------------------------- Thanks both!
Attachment #8698034 -
Flags: review?(dbolter) → review+
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → tbsaunde+mozbugs
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a5356e384c04
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•