672438 - Crash [@ nsINode::CompareDocPosition(nsINode*) ] with too many attr children

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Bob Clary [:bc] (inactive)]

Attachment: aurora stack

1. http://dopey.de/flash/
2. Crash Aurora/7, Nightly/8 on Mac.

rapidly issues oodles of warnings then crash with Aurora. Nightly takes a bit longer. Haven't checked Beta/6.

aurora bp-8698ca1b-054a-41c5-8d01-4f6572110718

WARNING: NS_ENSURE_TRUE(childCount < ATTRCHILD_ARRAY_MAX_CHILD_COUNT) failed: file /work/mozilla/builds/aurora/mozilla/content/base/src/nsAttrAndChildArray.cpp, line 170
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /work/mozilla/builds/aurora/mozilla/content/base/src/nsGenericElement.cpp, line 3473

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x054254eb in nsINode::CompareDocPosition (this=0x0, aOtherNode=0x26151260) at /work/mozilla/builds/aurora/mozilla/content/base/src/nsGenericElement.cpp:774
774	  if (node2->IsNodeOfType(nsINode::eATTRIBUTE)) {

fwiw, this is another of these sites with broken php scripts that eat memory and crash the browser.

Automation did see a different stack with aurora:
NS_DebugBreak_P | nsCOMPtr<nsIParser>::operator-> | nsScriptElement::MaybeProcessScript nsHTMLScriptElement::BindToTree nsINode::doInsertChildAt nsGenericElement::InsertChildAt nsINode::AppendChildTo  

The original Socorro signature was mozalloc_abort(char const* const) | mozalloc_handle_oom() | nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) | nsTArray<nsHtml5TreeOperation, nsTArrayDefaultAllocator>::MoveElementsFrom<nsHtml5TreeOperation, nsTArrayD...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Jonas, can you take a look?

Comment 2

[Mats Palmgren (inactive)]

PostPendingAppendNotification is called before AppendChildTo:
http://mxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5TreeOperation.cpp#213
which results in a new nsHtml5PendingNotification object:
http://mxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5PendingNotification.h#50
with mChildCount = aParent->GetChildCount().

The bug occurs when the child count has reached the maximum value,
then AppendChildTo() fails and mChildCount has a value that isn't
a valid index so the mParent->GetChildAt(mChildCount) at line 60
returns null leading to the null-pointer crash.

Comment 3

[Mats Palmgren (inactive)]

Attachment: Like so?

Notifying only if AppendChildTo succeeds seems like the way to go.

Comment 4

[Henri Sivonen (:hsivonen)]

Comment on attachment 547002 [details] [diff] [review]
Like so?

Looks reasonable in order to fix the crash.

I wonder if we should try, as a more elaborate follow-up, to halt the parser on unload the document upon failures that leave the DOM in a state that doesn't correspond to what the parser saw.

Comment 5

[Mats Palmgren (inactive)]

Filed bug 672928 to follow-up Henri's review comment.

http://hg.mozilla.org/integration/mozilla-inbound/rev/8d56702130fb

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

http://hg.mozilla.org/mozilla-central/rev/8d56702130fb