Bug 96567 – [V8] ScriptController::compileAndRunScript() can crash

• Component: WebCore JavaScript
• Status: RESOLVED WONTFIX

[Kentaro Hara]

See chromium bug: http://code.google.com/p/chromium/issues/detail?id=146776

The root cause is the following code in ScriptSourceCode.h:

  CachedScript* cachedScript() const { return m_cachedScript.get(); }

Here m_cachedScript can be 0.

[Kentaro Hara]

Created attachment 163715 [details]
Patch

[Adam Barth]

Comment on attachment 163715 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163715&action=review

> Source/WebCore/bindings/v8/ScriptSourceCode.h:67
> -    CachedScript* cachedScript() const { return m_cachedScript.get(); }
> +    CachedScript* cachedScript() const { return m_cachedScript ? m_cachedScript.get() : 0; }

What does it mean for m_cachedScript to be 0?  It is held as a member, not as a pointer.

[Kentaro Hara]

Comment on attachment 163715 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163715&action=review

>> Source/WebCore/bindings/v8/ScriptSourceCode.h:67
>> +    CachedScript* cachedScript() const { return m_cachedScript ? m_cachedScript.get() : 0; }
> 
> What does it mean for m_cachedScript to be 0?  It is held as a member, not as a pointer.

Ah, good point... maybe I was confused. More investigation is needed.

[Kentaro Hara]

Created attachment 164640 [details]
Patch

[Adam Barth]

Comment on attachment 164640 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=164640&action=review

ok

> Source/WebCore/bindings/v8/ScriptSourceCode.cpp:53
> +    if (!scriptData.get())
> +        return nullptr;

There's no reason to call get() here.  OwnPtr has a bool operator.

[Kentaro Hara]

Created attachment 164805 [details]
patch for landing

[WebKit Review Bot]

Comment on attachment 164805 [details]
patch for landing

Clearing flags on attachment: 164805

Committed r129077: <http://trac.webkit.org/changeset/129077>

[Kentaro Hara]

Comment on attachment 164640 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=164640&action=review

>> Source/WebCore/bindings/v8/ScriptSourceCode.cpp:53
>> +        return nullptr;
> 
> There's no reason to call get() here.  OwnPtr has a bool operator.

Fixed. Thanks.

[Anders Carlsson]

V8 is gone.