Bug 139170 – Crash when setting 'column-span' CSS property to 'calc(2 * 3)'

• Component: CSS
• Status: RESOLVED FIXED

[Chris Dumez]

Crash when setting 'column-span' CSS property to 'calc(2 * 3)':
ASSERTION FAILED: !m_parsedCalculation
/Users/chris/WebKit/OpenSource/Source/WebCore/css/CSSParser.cpp(3115) : bool WebCore::CSSParser::parseValue(WebCore::CSSPropertyID, bool)
1   0x10fda70f0 WTFCrash
2   0x1118cf297 WebCore::CSSParser::parseValue(WebCore::CSSPropertyID, bool)
3   0x1118910a3 cssyyparse(WebCore::CSSParser*)
4   0x1118c666e WebCore::CSSParser::parseValue(WebCore::MutableStyleProperties*, WebCore::CSSPropertyID, WTF::String const&, bool, WebCore::StyleSheetContents*)
5   0x1118c58a7 WebCore::CSSParser::parseValue(WebCore::MutableStyleProperties*, WebCore::CSSPropertyID, WTF::String const&, bool, WebCore::CSSParserMode, WebCore::StyleSheetContents*)
6   0x11322c40f WebCore::MutableStyleProperties::setProperty(WebCore::CSSPropertyID, WTF::String const&, bool, WebCore::StyleSheetContents*)
7   0x112bced2b WebCore::PropertySetCSSStyleDeclaration::setPropertyInternal(WebCore::CSSPropertyID, WTF::String const&, bool, int&)
8   0x11239eec8 WebCore::JSCSSStyleDeclaration::putDelegate(JSC::ExecState*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&)
9   0x11239a079 WebCore::JSCSSStyleDeclaration::put(JSC::JSCell*, JSC::ExecState*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&)
10  0x10f720252 JSC::JSValue::put(JSC::ExecState*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&)
11  0x10f6f2cf6 void JSC::DFG::operationPutByValInternal<false, false>(JSC::ExecState*, long long, long long, long long)
12  0x10f6f2a3b operationPutByValNonStrict

[Chris Dumez]

Created attachment 242381 [details]
Patch

[Darin Adler]

Comment on attachment 242381 [details]
Patch

I’m really concerned about the fragility of the way parsedDouble is written. It seems super strange to pass in a value but separately have a side bit of global state for the calculated value. We need to fix this, because this code is way too easy to get wrong. Maybe this should all be global state, rather than passing a value pointer around. Or we could go the other way and pass the calculated value around.

[Chris Dumez]

(In reply to comment #2)
> Comment on attachment 242381 [details]
> Patch
> 
> I’m really concerned about the fragility of the way parsedDouble is written.
> It seems super strange to pass in a value but separately have a side bit of
> global state for the calculated value. We need to fix this, because this
> code is way too easy to get wrong. Maybe this should all be global state,
> rather than passing a value pointer around. Or we could go the other way and
> pass the calculated value around.

Yes, I agree this needs to be refactored. It is on my TODO list.

[WebKit Commit Bot]

Comment on attachment 242381 [details]
Patch

Clearing flags on attachment: 242381

Committed r176671: <http://trac.webkit.org/changeset/176671>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.