Created attachment 66801 [details] repro When firstline is true the renderers style() method may return null and then be derefed. I hav been hitting this a lot with a fuzzer I am working on and will submit a patch shortly. the attached repro will trigger this.
Created attachment 66812 [details] Patch
+jamesr James can you review this when you get a chance. Thanks!
Comment on attachment 66812 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=66812&action=prettypatch > WebCore/rendering/InlineBox.cpp:99 > + if (!renderer()->style(m_firstLine)) > + return 0; > if (renderer()->isText()) > return m_isText ? renderer()->style(m_firstLine)->font().height() : 0; > if (renderer()->isBox() && parent()) There's one very slight behavior change here - if renderer()->isBox() && parent() == true, old code wouldn't call renderer()->style(m_firstLine) at all whereas this patch will. Can you reoranize this so that's still true? I think moving the isBox() && ... branch above the NULL check will suffice.
Created attachment 66917 [details] Patch
Good catch. Thank you!
Comment on attachment 66917 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=66917&action=prettypatch > WebCore/rendering/InlineBox.cpp:-97 > + if (!renderer()->style(m_firstLine)) > + return 0; > if (renderer()->isText()) > return m_isText ? renderer()->style(m_firstLine)->font().height() : 0; > - if (renderer()->isBox() && parent()) I believe style(m_firstLine) is non-trivial. Shouldn't we cache it in a local variable? RenderStyle* firstLineStyle() const { return document()->usesFirstLineRules() ? firstLineStyleSlowCase() : style(); } RenderStyle* style(bool firstLine) const { return firstLine ? firstLineStyle() : style(); }
Does this still happen with the fix for bug 45574 (r67255 or later)? style() should never return null and if it is, then that should be fixed, not papered over.
The repro now causes a crash at the line 96 of InlineBox.cpp: if (renderer()->isText()) return m_isText ? renderer()->style(m_firstLine)->font().height() : 0; font() is 0 and crashes inside RefPtr.
(In reply to comment #8) > The repro now causes a crash at the line 96 of InlineBox.cpp: > > if (renderer()->isText()) > return m_isText ? renderer()->style(m_firstLine)->font().height() : 0; > > font() is 0 and crashes inside RefPtr. Seems like style(true) is still null.
I think the issue is that RenderObject::firstLineStyleSlowCase() calls setHasPseudoStyle() on the wrong RenderStyle, namely the RenderText’s instead of its parent’s.
<rdar://problem/8478160>
Created attachment 68814 [details] Set the pseudo style bit on the RenderStyle which is going to supply the first-line style
Fixed in <http://trac.webkit.org/projects/webkit/changeset/68335>.