Bug 93096

Summary: regression(r124510) webintents/web-intents-obj-constructor.html is crashing
Product: WebKit Reporter: Sudarsana Nagineni (babu) <naginenis>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, fpizlo, gyuyoung.kim, haraken, lucas.de.marchi, oliver, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 93031    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
haraken: review+, haraken: commit-queue-
Patch for landing none

Sudarsana Nagineni (babu)
Reported 2012-08-03 05:28:52 PDT
After r124510, webintents/web-intents-obj-constructor.html begun to crash on EFL. http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug/r124510%20%283782%29/webintents/web-intents-obj-constructor-crash-log.txt
Attachments
Patch (2.87 KB, patch)
2012-08-12 07:15 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (2.87 KB, patch)
2012-08-12 07:17 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (2.83 KB, patch)
2012-08-13 08:27 PDT, Chris Dumez 		haraken: review+
haraken: commit-queue-
DetailsFormatted DiffDiff
Patch for landing (4.44 KB, patch)
2012-08-13 23:27 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2012-08-12 05:29:29 PDT
crash log for DumpRenderTree (pid 23071): STDOUT: <empty> STDERR: 1 0x7fbddbcd7cc3 STDERR: 2 0x7fbdd2efbcb0 STDERR: 3 0x494404 JSC::Register::jsValue() const STDERR: 4 0x4954f0 JSC::Register::scopeChain() const STDERR: 5 0x4944c2 JSC::ExecState::scopeChain() const STDERR: 6 0x495456 JSC::ExecState::globalData() const STDERR: 7 0x7fbdd7e5a629 WebCore::JSDictionary::JSDictionary(JSC::ExecState*, JSC::JSObject*) STDERR: 8 0x7fbdd7e5a2b6 WebCore::Dictionary::Dictionary() STDERR: 9 0x7fbdd71a9402 WebCore::Intent::create(JSC::ExecState*, WebCore::Dictionary const&, int&) STDERR: 10 0x7fbdd7f159b0 WebCore::JSIntentConstructor::constructJSIntent(JSC::ExecState*) STDERR: 11 0x7fbddbb8100d STDERR: 12 0x7fbddbb7c004 STDERR: 13 0x7fff68fe9910 STDERR: LEAK: 2 JSLazyEventListener STDERR: LEAK: 21 RenderObject STDERR: LEAK: 1 Page STDERR: LEAK: 1 Frame STDERR: LEAK: 5 CachedResource STDERR: LEAK: 1 SubresourceLoader STDERR: LEAK: 215 WebCoreNode
Chris Dumez
Comment 2 2012-08-12 05:42:12 PDT
Due to r124510, we can no longer pass 0 for JSC::ExecState* argument of: WebCore::JSDictionary::JSDictionary(JSC::ExecState*, JSC::JSObject*) This is because, the exec argument is now used in the constructor without NULL check. We pass 0 JSC::ExecState* argument, when using the Dictionary default constructor, e.g.: Dictionary d; // Will call WebCore::JSDictionary::JSDictionary(0, 0) internally
Chris Dumez
Comment 3 2012-08-12 07:15:35 PDT
Created attachment 157898 [details] Patch
Chris Dumez
Comment 4 2012-08-12 07:17:19 PDT
Created attachment 157899 [details] Patch Minor typo fix.
Chris Dumez
Comment 5 2012-08-13 08:27:51 PDT
Created attachment 158006 [details] Patch Update bug title
Kentaro Hara
Comment 6 2012-08-13 16:43:12 PDT
Comment on attachment 158006 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158006&action=review > Source/WebCore/ChangeLog:9 > + before using it. The exec may indeed be null, thus causing crashes. Who passes a null? I've expected that we always pass a valid exec to the JSDictionary constructor.
Kentaro Hara
Comment 7 2012-08-13 16:44:08 PDT
(In reply to comment #6) > Who passes a null? I've expected that we always pass a valid exec to the JSDictionary constructor. Sorry, now I read your comment #2:)
Kentaro Hara
Comment 8 2012-08-13 16:57:50 PDT
Comment on attachment 158006 [details] Patch Shall we insert ASSERT(isValid()) to JSDictionary::getWithUndefinedOrNullCheck() and JSDictionary::tryGetProperty() so that we can prevent a null exec and a null initializerObject from being used?
Chris Dumez
Comment 9 2012-08-13 22:35:19 PDT
(In reply to comment #8) > (From update of attachment 158006 [details]) > Shall we insert ASSERT(isValid()) to JSDictionary::getWithUndefinedOrNullCheck() and JSDictionary::tryGetProperty() so that we can prevent a null exec and a null initializerObject from being used? Agreed. I'll add those assertions before landing, thanks.
Chris Dumez
Comment 10 2012-08-13 23:27:25 PDT
Created attachment 158232 [details] Patch for landing Add assertions as advised by haraken.
WebKit Review Bot
Comment 11 2012-08-14 00:22:54 PDT
Comment on attachment 158232 [details] Patch for landing Clearing flags on attachment: 158232 Committed r125513: <http://trac.webkit.org/changeset/125513>
WebKit Review Bot
Comment 12 2012-08-14 00:23:00 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.