Bug 39143 – REGRESSION (r59385) crash destroying inline renderers

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

Looks very similar to https://bugs.webkit.org/show_bug.cgi?id=39042

[James Robinson]

Callstack:


Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000440
0x00000001017dc2d7 in WebCore::RenderInline::destroy (this=0x10a1c85c8) at /usr/local/home/jamesr/WebKit/WebCore/rendering/RenderInline.cpp:64
64	        m_continuation->destroy();
(gdb) bt
#0  0x00000001017dc2d7 in WebCore::RenderInline::destroy (this=0x10a1c85c8) at /usr/local/home/jamesr/WebKit/WebCore/rendering/RenderInline.cpp:64
#1  0x00000001017dc2e4 in WebCore::RenderInline::destroy (this=0x10a1c7838) at /usr/local/home/jamesr/WebKit/WebCore/rendering/RenderInline.cpp:64
#2  0x00000001016fd9f7 in WebCore::Node::detach (this=0x10a1c75b0) at /usr/local/home/jamesr/WebKit/WebCore/dom/Node.cpp:1226
#3  0x0000000101014ffe in WebCore::ContainerNode::detach (this=0x10a1c75b0) at /usr/local/home/jamesr/WebKit/WebCore/dom/ContainerNode.cpp:628
#4  0x0000000101237649 in WebCore::Element::detach (this=0x10a1c75b0) at /usr/local/home/jamesr/WebKit/WebCore/dom/Element.cpp:842
#5  0x0000000101014fd8 in WebCore::ContainerNode::detach (this=0x10a1c6c00) at /usr/local/home/jamesr/WebKit/WebCore/dom/ContainerNode.cpp:626
#6  0x0000000101237649 in WebCore::Element::detach (this=0x10a1c6c00) at /usr/local/home/jamesr/WebKit/WebCore/dom/Element.cpp:842
#7  0x0000000101016383 in WebCore::ContainerNode::removeChild (this=0x10a1c6860, oldChild=0x10a1c6c00, ec=@0x7fff5fbfdc24) at /usr/local/home/jamesr/WebKit/WebCore/dom/ContainerNode.cpp:377
#8  0x000000010136eb0e in WebCore::HTMLParser::handleResidualStyleCloseTagAcrossBlocks (this=0x10a1888b0, elem=0x10a1c68f0) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLParser.cpp:1258
#9  0x000000010136f1c6 in WebCore::HTMLParser::popBlock (this=0x10a1888b0, tagName=@0x10a8cfa48, reportErrors=true) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLParser.cpp:1445
#10 0x0000000101374683 in WebCore::HTMLParser::processCloseTag (this=0x10a1888b0, t=0x10a8cfa38) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLParser.cpp:1024
#11 0x0000000101374003 in WebCore::HTMLParser::parseToken (this=0x10a1888b0, t=0x10a8cfa38) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLParser.cpp:258
#12 0x0000000101387347 in WebCore::HTMLTokenizer::processToken (this=0x10a8cfa00) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLTokenizer.cpp:1941
#13 0x000000010138e8c6 in WebCore::HTMLTokenizer::parseTag (this=0x10a8cfa00, src=@0x10a8d04e0, state={static EntityShift = 4, m_bits = 0}) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLTokenizer.cpp:1513
#14 0x000000010138f6c7 in WebCore::HTMLTokenizer::write (this=0x10a8cfa00, str=@0x7fff5fbfe190, appendData=true) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLTokenizer.cpp:1764
#15 0x0000000101386efb in WebCore::HTMLTokenizer::timerFired (this=0x10a8cfa00) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLTokenizer.cpp:1848
#16 0x0000000101390ba3 in WebCore::Timer<WebCore::HTMLTokenizer>::fired (this=0x10a8cfc58) at Timer.h:98
#17 0x0000000101a3db7c in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x10a11dcf0) at /usr/local/home/jamesr/WebKit/WebCore/platform/ThreadTimers.cpp:112
#18 0x0000000101a3dd0b in WebCore::ThreadTimers::sharedTimerFired () at /usr/local/home/jamesr/WebKit/WebCore/platform/ThreadTimers.cpp:90
#19 0x0000000101915a46 in WebCore::timerFired () at /usr/local/home/jamesr/WebKit/WebCore/platform/mac/SharedTimerMac.mm:86
#20 0x00007fff877c4708 in __CFRunLoopRun ()
#21 0x00007fff877c28df in CFRunLoopRunSpecific ()
#22 0x00007fff87ee8ada in RunCurrentEventLoopInMode ()
#23 0x00007fff87ee88df in ReceiveNextEventCommon ()
#24 0x00007fff87ee8798 in BlockUntilNextEventMatchingListInMode ()
#25 0x00007fff86d3da4a in _DPSNextEvent ()
#26 0x00007fff86d3d399 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#27 0x000000010000b6cc in ?? ()
#28 0x00007fff86d0306f in -[NSApplication run] ()
#29 0x00007fff86cfbd8c in NSApplicationMain ()
#30 0x00000001000016f4 in ?? ()

[Enrica Casucci]

Created attachment 56300 [details]
patch

I have a fix, but I'm still trying to create a reduction in order to include a layout test.

[James Robinson]

Created attachment 56307 [details]
reduction

This is a reduction from the http://www.766900.com/ page.  It's probably not completely minimal but should be useful as a starting point.  The document.write() loop is a bit unfortunate, but it seems necessary to force the </font> tag to get parsed on a separate callstack from the rest of the page.

(In reply to comment #3)
> Created an attachment (id=56307) [details]
> reduction

Thanks for the reduction!

> The document.write() loop is a bit unfortunate, but it seems necessary to force the </font> tag to get parsed on a separate callstack from the rest of the page.

That’s not really necessary. Just <script> document.body.offsetTop </script> suffices, as it forces layout at that point. Then when parsing continues, the crash occurs.

[James Robinson]

Created attachment 56440 [details]
Patch

[James Robinson]

We're seeing this crash on more and more URLs in Chromium's reliability testing which it making it really hard to spot new regressions.  Thus, I've attached a patch to revert r59385 and add the reduction as a layout test to catch regressions.  Please consider landing this if you aren't able to fix the issue very quickly.  Thanks.

Enrica is very close to fixing this.

[Enrica Casucci]

I'll be posting my patch shortly.

[Adele Peterson]

Comment on attachment 56440 [details]
Patch

R- since we're actively working on a real fix.

[Enrica Casucci]

Created attachment 56505 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 56505 [details]
Patch

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 59776)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,17 @@
> [phone]  Enrica Casucci  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        REGRESSION (r59385) crash destroying inline renderers
> +        https://bugs.webkit.org/show_bug.cgi?id=39143
> +        <rdar://problem/8003662>
> +        
> +        Test: fast/inline-block/anonymous-block-crash.html
> +
> +        * rendering/RenderBlock.cpp:
> +        (WebCore::RenderBlock::layoutBlock): resetting the flag m_inlineChildren only for non anonymous blocks otherwise we incurr in
> +        a double deletion of the renderer that couses the crash.

It would be nice of the changelog explained why the crash occurs, and why not resetting the flag for anonymous blocks fixes it. i.e. dump some of your knowledge that you gained from your investigation into the changelog!

[Dave Hyatt]

Comment on attachment 56505 [details]
Patch

r=me

This should probably be isAnonymousBlock() and not just isAnonymous().  I doubt there's any problem with display: block generated content for example.

We could leave it this way just to be safe though.  There's no harm in being more conservative about the flipping of the childrenInline switch.

[Enrica Casucci]

Added some additional info in the ChangeLog.
Committed revision 59786.

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