Bug 154350 – SES selftest page crashes on nightly r196694

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark S. Miller]

Created attachment 271577 [details]
Screenshots of Nightly before and after bug

Visit https://rawgit.com/tvcutsem/es-lab/master/src/ses/contract.html
in the latest Webkit.

As shown on the attachment, it worked fine on Nightly r196074, so the
problem happened since then.

[Keith Miller]

This looks like it's probably because of http://trac.webkit.org/changeset/196220. That's the commit that has modified the crashing function, getOwnPropertyDescriptor, this year.

[Radar WebKit Bug Importer]

<rdar://problem/24704334>

[Radar WebKit Bug Importer]

<rdar://problem/24704338>

[Chris Dumez]

Likely to be a regression from:
http://trac.webkit.org/changeset/196001
or
http://trac.webkit.org/changeset/196145

[Chris Dumez]

I am working on this.

[Chris Dumez]

Created attachment 271592 [details]
Patch

[Mark Lam]

Comment on attachment 271592 [details]
Patch

r=me

[Chris Dumez]

Comment on attachment 271592 [details]
Patch

Clearing flags on attachment: 271592

Committed r196723: <http://trac.webkit.org/changeset/196723>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Mark S. Miller]

As of r196733 I am now seeing "A problem occurred with this webpage so it was reloaded." most times when I visit https://rawgit.com/tvcutsem/es-lab/master/src/ses/contract.html . It doesn't happen every time. But if I bring up the web inspector, set a breakpoint, and then reload, then it does happen every time closing the web inspector in the process, preventing me from catching ses at a breakpoint.

Will attach a screenshot momentarily.

Should I reopen this bug or file a fresh one?

[Mark S. Miller]

Created attachment 271617 [details]
SES selftest page now: A problem occurred with this webpage so it was reloaded

[Chris Dumez]

(In reply to comment #11)
> Created attachment 271617 [details]
> SES selftest page now: A problem occurred with this webpage so it was
> reloaded

Ok, I will take another look and see if I can reproduce. Thank you for verifying the fix.

[Chris Dumez]

(In reply to comment #11)
> Created attachment 271617 [details]
> SES selftest page now: A problem occurred with this webpage so it was
> reloaded

Also, a backtrace is more useful than a screenshot :) You can get the backtrace the .crash file for the com.apple.WebKit.WebContent process from the "console" utility.

[Chris Dumez]

Looks like a can reproduce the crash but I have to open Web Inspector and reload the page. I filed https://bugs.webkit.org/show_bug.cgi?id=154378 to track it.

[Alexey Proskuryakov]

*** Bug 154349 has been marked as a duplicate of this bug. ***

[Darin Adler]

Comment on attachment 271592 [details]
Patch

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

> Source/JavaScriptCore/runtime/JSObject.h:1231
> -            if ((attributes & Accessor) != (currentAttributes & Accessor)) {
> +            if ((attributes & Accessor) != (currentAttributes & Accessor) || (attributes & CustomAccessor) != (currentAttributes & CustomAccessor)) {

Here’s how I’d write it:

    auto accessAttributes = Accessor | CustomAccessor;
    if ((attributes & accessAttributes) != (currentAttributes & accessAttributes)) {

Better than the || I think.