Bug 199613 – REGRESSION(r244906): Crash in WebCore::positionOffsetValue

• Component: CSS
• Status: RESOLVED FIXED

[Antti Koivisto]

Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000030)
[  0] 0x00007fff3f970006 WebCore`WebCore::positionOffsetValue(WebCore::RenderStyle const&, WebCore::CSSPropertyID, WebCore::RenderObject*) [inlined] WebCore::RenderObject::RenderObjectBitfields::horizontalWritingMode() const at RenderObject.h:935:9
       931 	        ADD_BOOLEAN_BITFIELD(isBox, IsBox);
       932 	        ADD_BOOLEAN_BITFIELD(isInline, IsInline);
       933 	        ADD_BOOLEAN_BITFIELD(isReplaced, IsReplaced);
       934 	        ADD_BOOLEAN_BITFIELD(isLineBreak, IsLineBreak);
    -> 935 	        ADD_BOOLEAN_BITFIELD(horizontalWritingMode, HorizontalWritingMode);
       936 	
       937 	        ADD_BOOLEAN_BITFIELD(hasLayer, HasLayer);
       938 	        ADD_BOOLEAN_BITFIELD(hasOverflowClip, HasOverflowClip); // Set in the case of overflow:auto/scroll/hidden
       939 	        ADD_BOOLEAN_BITFIELD(hasTransformRelatedProperty, HasTransformRelatedProperty);
    

     0x00007fff3f96fffb:       je 0x1166006            ; <+534> [inlined] WebCore::RenderObject::RenderObjectBitfields::horizontalWritingMode() const at RenderObject.h:443
     0x00007fff3f96fffd:    testb $0x2, 0x32(%rax)
     0x00007fff3f970001:       je 0x1165ff0            ; <+512> [inlined] WebCore::RenderBox::enclosingScrollportBox() const + 25 at CSSComputedStyleDeclaration.cpp:794
     0x00007fff3f970003:     movq %rax, %rbx
 ->  0x00007fff3f970006:     movl 0x30(%rbx), %ecx
     0x00007fff3f970009:     shrl $0xf, %ecx
     0x00007fff3f97000c:     andl $0x1, %ecx
     0x00007fff3f97000f:   movzwl 0x8c(%rbx), %eax
     0x00007fff3f970016:     cmpl %ecx, %r15d

[  0] 0x00007fff3f970006 WebCore`WebCore::positionOffsetValue(WebCore::RenderStyle const&, WebCore::CSSPropertyID, WebCore::RenderObject*) [inlined] WebCore::RenderObject::isHorizontalWritingMode() const at RenderObject.h:443
       439 	    bool isBox() const { return m_bitfields.isBox(); }
       440 	    bool isRenderView() const  { return m_bitfields.isBox() && m_bitfields.isTextOrRenderView(); }
       441 	    bool isInline() const { return m_bitfields.isInline(); } // inline object
       442 	    bool isReplaced() const { return m_bitfields.isReplaced(); } // a "replaced" element (see CSS)
    -> 443 	    bool isHorizontalWritingMode() const { return m_bitfields.horizontalWritingMode(); }
       444 	
       445 	    bool isDragging() const { return m_bitfields.hasRareData() && rareData().isDragging(); }
       446 	    bool hasReflection() const { return m_bitfields.hasRareData() && rareData().hasReflection(); }
       447 	    bool isRenderFragmentedFlow() const { return m_bitfields.hasRareData() && rareData().isRenderFragmentedFlow(); }
    
[  0] 0x00007fff3f970006 WebCore`WebCore::positionOffsetValue(WebCore::RenderStyle const&, WebCore::CSSPropertyID, WebCore::RenderObject*) + 534 at CSSComputedStyleDeclaration.cpp:795
       791 	        LayoutUnit containingBlockSize;
       792 	        if (box.isStickilyPositioned()) {
       793 	            const RenderBox& enclosingScrollportBox =
       794 	                box.enclosingScrollportBox();
    -> 795 	            if (isVerticalProperty == enclosingScrollportBox.isHorizontalWritingMode())
       796 	                containingBlockSize = enclosingScrollportBox.contentLogicalHeight();
       797 	            else
       798 	                containingBlockSize = enclosingScrollportBox.contentLogicalWidth();
       799 	        } else {
    
[  1] 0x00007fff3f9629d2 WebCore`WebCore::ComputedStyleExtractor::valueForPropertyinStyle(WebCore::RenderStyle const&, WebCore::CSSPropertyID, WebCore::RenderElement*) + 24466 at CSSComputedStyleDeclaration.cpp:0
       1   	/*
       2   	 * Copyright (C) 2004 Zack Rusin <[e-mail]>
       3   	 * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
       4   	 * Copyright (C) 2007 Alexey Proskuryakov <[e-mail]>
       5   	 * Copyright (C) 2007 Nicholas Shanks <[e-mail]>
       6   	 * Copyright (C) 2011 Sencha, Inc. All rights reserved.
       7   	 * Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
       8   	 *
       9   	 * This library is free software; you can redistribute it and/or
    
[  2] 0x00007fff3f95b8d5 WebCore`WebCore::ComputedStyleExtractor::propertyValue(WebCore::CSSPropertyID, WebCore::EUpdateLayout) + 2165 at CSSComputedStyleDeclaration.cpp:2778:12
[  3] 0x00007fff3e8da604 WebCore`WebCore::CSSComputedStyleDeclaration::getPropertyValue(WebCore::CSSPropertyID) const [inlined] WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue(WebCore::CSSPropertyID, WebCore::EUpdateLayout) const + 40 at CSSComputedStyleDeclaration.cpp:2466:99
[  3] 0x00007fff3e8da5dc

[Antti Koivisto]

<rdar://problem/51518172>

[Antti Koivisto]

Created attachment 373708 [details]
patch

[alan]

Comment on attachment 373708 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=373708&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:794
> +            auto& enclosingClippingBox =
> +                box.enclosingClippingBoxForStickyPosition();

why 2 lines.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:448
> +    auto* clipLayer = layer()->enclosingOverflowClipLayer(ExcludeSelf);

I guess if sticky does not produce a layer, we have bigger problems than this null deref.

[Antti Koivisto]

> why 2 lines.

OOPS

> I guess if sticky does not produce a layer, we have bigger problems than
> this null deref.

Yep.

[Antti Koivisto]

Created attachment 373718 [details]
patch

[WebKit Commit Bot]

Comment on attachment 373718 [details]
patch

Clearing flags on attachment: 373718

Committed r247256: <https://trac.webkit.org/changeset/247256>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

Comment on attachment 373718 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=373718&action=review

> LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash.html:3
> +    position: -webkit-sticky;

No need to prefix any more.