Bug 69453 – save resource zeroing stacks in ScriptElement for debugging

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Gavin Peters]

save resource zeroing stacks in ScriptElement for debugging

[Gavin Peters]

Created attachment 109837 [details]
Patch

[Gavin Peters]

add more data for crashes in ScriptElement::notifyFinished double calls.

[Adam Barth]

Comment on attachment 109837 [details]
Patch

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

> Source/WebCore/dom/ScriptElement.h:120
> +      MaxBacktraceSize = 32

bad indent

[Gavin Peters]

Created attachment 109838 [details]
Patch

[Gavin Peters]

Comment on attachment 109838 [details]
Patch

Fixed indent.

[Nate Chapin]

Comment on attachment 109838 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        two notifyFinished() events in a ScriptElement, which can crash chrome (see chrome bug
> +        75604).  This patch will save a stack in the ScriptElement when this happens, so that if

Please provide a link to the chromium bug.

> Source/WebCore/dom/ScriptElement.h:122
> +
> +    // We grab a backtrace when we zero m_cachedScript, so that at later crashes
> +    // we'll have a debuggable stack.
> +    enum {
> +        MaxBacktraceSize = 32
> +    };
> +    int m_backtraceSize;

Does this value ever change?  Can we just make it a static const in ScriptElement.cpp?

[Gavin Peters]

Created attachment 109842 [details]
Patch

[Gavin Peters]

Comment on attachment 109838 [details]
Patch

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

>> Source/WebCore/ChangeLog:8
>> +        75604).  This patch will save a stack in the ScriptElement when this happens, so that if
> 
> Please provide a link to the chromium bug.

Done.

>> Source/WebCore/dom/ScriptElement.h:122
>> +    int m_backtraceSize;
> 
> Does this value ever change?  Can we just make it a static const in ScriptElement.cpp?

It does change, it contains the actual size of the stack in m_backtrace, it could be anything from 0 to 32.

[Gavin Peters]

Comment on attachment 109842 [details]
Patch

Now with links to the chromium bug database.  Boy, isn't the chromium bug database really unbearably slow?

[WebKit Review Bot]

Comment on attachment 109842 [details]
Patch

Clearing flags on attachment: 109842

Committed r96756: <http://trac.webkit.org/changeset/96756>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Roben (:aroben)]

This broke many builds due to WTFGetBacktrace not being exported from JavaScriptCore. Rolling it out in bug 69472.

[Gavin Peters]

I just added a new patch to bug 69018 which exports these functions, after that this patch should be good to go, possibly after rerunning ews.

[Gavin Peters]

Created attachment 109957 [details]
Patch

[Gavin Peters]

Comment on attachment 109957 [details]
Patch

This patch is unchanged from the version that got a rollout; however I've since updated the exports from JavaScriptCore so it should work now.  Let's wait on the windows ews bot in particular, then land it?

[Nate Chapin]

Comment on attachment 109957 [details]
Patch

Looks like it's passed the bots this time, so r+ again.

[Gavin Peters]

Committed r96819: <http://trac.webkit.org/changeset/96819>