Bug 112322

Summary: Crash at RenderStyle::inheritFrom reported by fuzzer
Product: WebKit Reporter: Takashi Sakamoto <tasak>
Component: CSSAssignee: Takashi Sakamoto <tasak>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, esprehn+autocc, macpherson, menard, ojan.autocc, webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 97279    
Attachments:
Description Flags
Patch
none
Patch none

Takashi Sakamoto
Reported 2013-03-13 23:54:08 PDT
Fuzzer Inferno_twister Job Type Linux_asan_chrome_mp Crash type UNKNOWN Crash address 0x000000000030 Crash state - crash stack - WebCore::RenderStyle::inheritFrom WebCore::StyleResolver::pseudoStyleForElement WebCore::RenderObject::getUncachedPseudoStyle Redzone 32 bytes
Attachments
Patch (5.00 KB, patch)
2013-03-14 00:22 PDT, Takashi Sakamoto 		no flags
DetailsFormatted DiffDiff
Patch (7.29 KB, patch)
2013-03-15 00:39 PDT, Takashi Sakamoto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Takashi Sakamoto
Comment 1 2013-03-13 23:54:32 PDT
https://cluster-fuzz.appspot.com/testcase?key=159273524
Takashi Sakamoto
Comment 2 2013-03-14 00:22:24 PDT
Created attachment 193080 [details] Patch
Mike West
Comment 3 2013-03-14 02:42:56 PDT
Comment on attachment 193080 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193080&action=review > Source/WebCore/css/StyleResolver.cpp:1180 > + RefPtr<RenderStyle> cloneForParent; Nit: You're only using this in the else block below. It would likely be possible to define it there rather than here in the outer scope. Actually, I'm not sure it's necessary at all, as you could call `state.setParentStyle(RenderStyle::clone(state.style()))` directly with little loss in clarity.
Hajime Morrita
Comment 4 2013-03-14 02:51:04 PDT
Comment on attachment 193080 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193080&action=review > Source/WebCore/css/StyleResolver.cpp:1188 > + state.setParentStyle(cloneForParent.get()); Why is this safe? It looks cloneForParent is gone after this function. How about just make State::m_parentStyle RefPtr?
Takashi Sakamoto
Comment 5 2013-03-15 00:39:52 PDT
Created attachment 193251 [details] Patch
Takashi Sakamoto
Comment 6 2013-03-15 00:41:53 PDT
Comment on attachment 193080 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193080&action=review Thank you for reviewing. >> Source/WebCore/css/StyleResolver.cpp:1180 >> + RefPtr<RenderStyle> cloneForParent; > > Nit: You're only using this in the else block below. It would likely be possible to define it there rather than here in the outer scope. Actually, I'm not sure it's necessary at all, as you could call `state.setParentStyle(RenderStyle::clone(state.style()))` directly with little loss in clarity. I see. Done. >> Source/WebCore/css/StyleResolver.cpp:1188 >> + state.setParentStyle(cloneForParent.get()); > > Why is this safe? It looks cloneForParent is gone after this function. > How about just make State::m_parentStyle RefPtr? Done.
WebKit Review Bot
Comment 7 2013-03-15 01:26:37 PDT
Comment on attachment 193251 [details] Patch Clearing flags on attachment: 193251 Committed r145885: <http://trac.webkit.org/changeset/145885>
WebKit Review Bot
Comment 8 2013-03-15 01:26:40 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.