Bug 12782 – Reproducible crash in BidiContext::deref

• Component: New Bugs
• Status: RESOLVED FIXED

[Krzysztof Kowalczyk]

Happens quite often when navigating between google properties (google.com/video.google.com/news.google.com)

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1243199824 (LWP 22133)]
WebCore::BidiContext::deref (this=0xc) at ../../rendering/bidi.cpp:291
291         count--;
(gdb) bt
#0  WebCore::BidiContext::deref (this=0xc) at ../../rendering/bidi.cpp:291
#1  0xb75d5b7a in WebCore::RootInlineBox::setLineBreakInfo (this=0x839e61c, obj=0x0, breakPos=0, status=0x0, context=0x0)
    at ../../../JavaScriptCore/wtf/RefPtr.h:106
#2  0xb75d6052 in WebCore::RootInlineBox::childRemoved (this=0x839e61c, box=0x839e674) at ../../rendering/RootInlineBox.cpp:169
#3  0xb75378ba in WebCore::InlineFlowBox::removeChild (this=0x839f21c, child=0x839e674) at ../../rendering/InlineFlowBox.cpp:118
#4  0xb7537058 in WebCore::InlineBox::remove (this=0x839e61c) at ../../rendering/InlineBox.cpp:41
#5  0xb75bb3a8 in WebCore::RenderText::destroy (this=0x8293b4c) at ../../rendering/RenderText.cpp:111
#6  0xb72a7293 in WebCore::Node::detach (this=0x82e3ae0) at ../../dom/Node.cpp:824
#7  0xb725b9bb in WebCore::ContainerNode::detach (this=0x83a3e88) at ../../dom/ContainerNode.cpp:617
#8  0xb728b74a in WebCore::Element::detach (this=0x83a3e88) at ../../dom/Element.cpp:576
#9  0xb725b9bb in WebCore::ContainerNode::detach (this=0x83a3fa0) at ../../dom/ContainerNode.cpp:617
#10 0xb728b74a in WebCore::Element::detach (this=0x83a3fa0) at ../../dom/Element.cpp:576
#11 0xb725b9bb in WebCore::ContainerNode::detach (this=0x83a40a8) at ../../dom/ContainerNode.cpp:617
#12 0xb728b74a in WebCore::Element::detach (this=0x83a40a8) at ../../dom/Element.cpp:576
#13 0xb725b9bb in WebCore::ContainerNode::detach (this=0x83afce0) at ../../dom/ContainerNode.cpp:617
#14 0xb728b74a in WebCore::Element::detach (this=0x83afce0) at ../../dom/Element.cpp:576
#15 0xb725b9bb in WebCore::ContainerNode::detach (this=0x83bb478) at ../../dom/ContainerNode.cpp:617
#16 0xb728b74a in WebCore::Element::detach (this=0x83bb478) at ../../dom/Element.cpp:576
#17 0xb728fc37 in WebCore::Element::recalcStyle (this=0x83bb478, change=WebCore::Node::Force) at ../../dom/Element.cpp:590
#18 0xb728f902 in WebCore::Element::recalcStyle (this=0x83d6060, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#19 0xb728f902 in WebCore::Element::recalcStyle (this=0x84da758, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#20 0xb728f902 in WebCore::Element::recalcStyle (this=0x84d9870, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#21 0xb728f902 in WebCore::Element::recalcStyle (this=0x84d9ab0, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#22 0xb728f902 in WebCore::Element::recalcStyle (this=0x84db910, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#23 0xb728f902 in WebCore::Element::recalcStyle (this=0x84db7e8, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#24 0xb728f902 in WebCore::Element::recalcStyle (this=0x84dbc60, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#25 0xb728f902 in WebCore::Element::recalcStyle (this=0x84dbd70, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#26 0xb728f902 in WebCore::Element::recalcStyle (this=0x84dcb48, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#27 0xb728f902 in WebCore::Element::recalcStyle (this=0x84dcc68, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#28 0xb728f902 in WebCore::Element::recalcStyle (this=0x827f4b0, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#29 0xb728f902 in WebCore::Element::recalcStyle (this=0x82f5b58, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#30 0xb728f902 in WebCore::Element::recalcStyle (this=0x813c100, change=WebCore::Node::Force) at ../../dom/Element.cpp:626
#31 0xb7277dc9 in WebCore::Document::recalcStyle (this=0x815c0c0, change=WebCore::Node::Force) at ../../dom/Document.cpp:1004
#32 0xb7276ee4 in WebCore::Document::updateStyleSelector (this=0x815c0c0) at ../../dom/Document.cpp:1898
#33 0xb7276fa0 in WebCore::Document::stylesheetLoaded (this=0x839e61c) at ../../dom/Document.cpp:1877
#34 0xb73a5b12 in WebCore::HTMLLinkElement::setCSSStyleSheet (this=0x8251930, url=@0xbf92fff4, charset=@0xbf92fff0, sheetStr=@0x82c0fd0)
    at ../../html/HTMLLinkElement.cpp:230
#35 0xb7458313 in WebCore::CachedCSSStyleSheet::checkNotify (this=0x82c0ee8) at ../../loader/CachedCSSStyleSheet.cpp:89
#36 0xb74589dd in WebCore::CachedCSSStyleSheet::data (this=0x82c0ee8, data=@0x8499e20, allDataReceived=true)
    at ../../loader/CachedCSSStyleSheet.cpp:79
#37 0xb74a32b6 in WebCore::Loader::didFinishLoading (this=0xb781cf98, loader=0x812d188) at ../../loader/loader.cpp:107
#38 0xb749cdee in WebCore::SubresourceLoader::didFinishLoading (this=0x812d188) at ../../loader/SubresourceLoader.cpp:189
#39 0xb74978a1 in WebCore::ResourceLoader::didFinishLoading (this=0x812d188) at ../../loader/ResourceLoader.cpp:323
#40 0xb762a9c6 in WebCore::ResourceHandleManager::remove (this=0x8071b78, job=0x84993c0) at ../../platform/network/gdk/ResourceHandleManager.cpp:175
#41 0xb762b02e in WebCore::ResourceHandleManager::downloadTimerCallback (this=0x8071b78, timer=0x8071b80)
    at ../../platform/network/gdk/ResourceHandleManager.cpp:144
#42 0xb762b56b in WebCore::Timer<WebCore::ResourceHandleManager>::fired (this=0x8071b80) at ../../platform/Timer.h:96
#43 0xb7510bb4 in WebCore::TimerBase::fireTimers (fireTime=1171588046.3096969, firingTimers=@0xbf9303cc) at ../../platform/Timer.cpp:336
#44 0xb7510caf in WebCore::TimerBase::sharedTimerFired () at ../../platform/Timer.cpp:353
#45 0xb762530e in timeout_cb () at ../../platform/gdk/SharedTimerLinux.cpp:48

[Krzysztof Kowalczyk]

Created attachment 13193 [details]
Fix the crash

Honestly, I don't see how this could crash because     template <typename T> inline RefPtr<T>& RefPtr<T>::operator=(T* optr) has a check for NULL, but it does happen.

Even if it's a gcc miscompilation, I believe it's worth putting the work-around since this was compiled with gcc 4.1.2 that ships on Ubuntu 6.10, which is one of the most popular distros.

[Oliver Hunt]

Comment on attachment 13193 [details]
Fix the crash

Without any justification just putting a null check won't fly -- why can this crash gdk but not everything else?

if it can crash other platforms we need a specific bug

[Adam Roben (:aroben)]

We'll also need a layout test before this gets landed.

[Andrew Wellington]

Created attachment 13604 [details]
Crash log from Mac build

I've just seen this crash on a Mac build: nightly build r20136.

[Andrew Wellington]

Reproduction instructions in <rdar://problem/5058791>

[David Kilzer (:ddkilzer)]

(In reply to comment #5)
> Reproduction instructions in <rdar://problem/5058791>

Is there a reason why these steps can't be published in Bugzilla?

Reproducible crashers are P1.

[Andrew Wellington]

Created attachment 13610 [details]
Crashlog with line numbers

The reproduction instructions can't be posted here as they involve information that is under a NDA.

This crash log contains line number information. The crash only occurs with Release builds, not Debug builds.

[David Kilzer (:ddkilzer)]

*** Bug 13055 has been marked as a duplicate of this bug. ***

Created attachment 13615 [details]
Reduced test case (will crash)

Created attachment 13616 [details]
Add an assert to InlineBox::root()

The test case fails this assertion. The illegal case leads to the crash down the road. I think this assertion will be good to have in the code. For one, it can help make a reliable regression test for this bug.

Created attachment 13618 [details]
Reduction

This fails the ASSERT in InlineBox::root(), but doesn't crash.

Created attachment 13620 [details]
Patch without test and change log

[Darin Adler]

Comment on attachment 13620 [details]
Patch without test and change log

Looks good to me, but this is clearly a Hyatt-review patch.

[Mark Rowe (bdash)]

*** Bug 13063 has been marked as a duplicate of this bug. ***

[Dave Hyatt]

Comment on attachment 13620 [details]
Patch without test and change log

r=me but get a changelog and test etc.

Created attachment 13628 [details]
Adopt line boxes of anonymous blocks being destroyed

[Darin Adler]

Comment on attachment 13628 [details]
Adopt line boxes of anonymous blocks being destroyed

Nice test, nice change log, patch looks good and was reviewed by Hyatt. I give it thumbs up.

[Sam Weinig]

Landed in r20188.