[webkit-reviews] review denied: [Bug 125329] Define m_hasBadParent in InlineBox if defined(ADDRESS_SANITIZER) : [Attachment 218697] Patch v1 for initial comments, not landing

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 125329: Define m_hasBadParent in InlineBox if defined(ADDRESS_SANITIZER)
https://bugs.webkit.org/show_bug.cgi?id=125329

Attachment 218697: Patch v1 for initial comments, not landing
https://bugs.webkit.org/attachment.cgi?id=218697&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=218697&action=review


> Source/WebCore/rendering/InlineBox.cpp:56
> -    if (!m_hasBadParent && m_parent)
> +    if (m_hasBadParent && m_parent)
>	   m_parent->setHasBadChildList();

This change seems wrong. The point here is to tell the parent that this child
is gone, and so the parent’s child list is now bad. It’s not safe to do that if
the parent is bad. So we should do this only if the parent is *not* bad.
Removing the ! breaks the code.

> Source/WebCore/rendering/InlineFlowBox.cpp:58
> -    if (!m_hasBadChildList)
> +    if (m_hasBadChildList) {
>	   for (InlineBox* child = firstChild(); child; child =
child->nextOnLine())
>	       child->setHasBadParent();
> +    }

Same mistake here. The parent is going away and so any child that is still
around now has a bad parent. But if the child list is already bad, then it’s
not safe to walk the child list so we don’t even try in that case. Removing the
! breaks the code.