Bug 139170 - Crash when setting 'column-span' CSS property to 'calc(2 * 3)'
Summary: Crash when setting 'column-span' CSS property to 'calc(2 * 3)'
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks: 138778
  Show dependency tree / graph
 
Reported: 2014-12-01 19:08 PST by Chris Dumez
Modified: 2014-12-02 10:52 PST (History)
5 users (show)

See Also:

Attachments
Patch (9.11 KB, patch)
2014-12-01 21:10 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2014-12-01 19:08:07 PST 
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
Comment 1 Chris Dumez 2014-12-01 21:10:07 PST 
Created attachment 242381 [details]
Patch
Comment 2 Darin Adler 2014-12-02 09:04:28 PST 
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.
Comment 3 Chris Dumez 2014-12-02 10:12:21 PST 
(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.
Comment 4 WebKit Commit Bot 2014-12-02 10:52:50 PST 
Comment on attachment 242381 [details]
Patch

Clearing flags on attachment: 242381

Committed r176671: <http://trac.webkit.org/changeset/176671>
Comment 5 WebKit Commit Bot 2014-12-02 10:52:54 PST 
All reviewed patches have been landed.  Closing bug.