Bug 93096 – regression(r124510) webintents/web-intents-obj-constructor.html is crashing

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Sudarsana Nagineni (babu)]

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

[Chris Dumez]

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]

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]

Created attachment 157898 [details]
Patch

[Chris Dumez]

Created attachment 157899 [details]
Patch

Minor typo fix.

[Chris Dumez]

Created attachment 158006 [details]
Patch

Update bug title

[Kentaro Hara]

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]

(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 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]

(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]

Created attachment 158232 [details]
Patch for landing

Add assertions as advised by haraken.

[WebKit Review Bot]

Comment on attachment 158232 [details]
Patch for landing

Clearing flags on attachment: 158232

Committed r125513: <http://trac.webkit.org/changeset/125513>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.