Bug 154378

Summary: Crash on SES selftest page when loading the page while WebInspector is open
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: JavaScriptCoreAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, commit-queue, erights, ggaren, joepeck, keith_miller, mark.lam, msaboff, sbarati, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://rawgit.com/tvcutsem/es-lab/master/src/ses/contract.html
See Also: https://bugs.webkit.org/show_bug.cgi?id=154350
https://bugs.webkit.org/show_bug.cgi?id=154102
https://bugs.webkit.org/show_bug.cgi?id=200560
Attachments:
Description Flags
Patch none

Description Chris Dumez 2016-02-17 19:51:51 PST 
Crash on SES selftest page when loading the page while WebInspector is open:
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000010a901f81 JSC::JSObject::getOwnPropertyDescriptor(JSC::ExecState*, JSC::PropertyName, JSC::PropertyDescriptor&) + 449
1   com.apple.JavaScriptCore      	0x000000010a9bee88 JSC::objectConstructorGetOwnPropertyDescriptor(JSC::ExecState*, JSC::JSObject*, JSC::Identifier const&) + 72
2   com.apple.JavaScriptCore      	0x000000010a9bd336 JSC::objectConstructorGetOwnPropertyDescriptor(JSC::ExecState*) + 550
3   ???                           	0x00003a5d9d601028 0 + 64173746688040
4   com.apple.JavaScriptCore      	0x000000010a99baeb llint_entry + 23561
5   com.apple.JavaScriptCore      	0x000000010a99bb5d llint_entry + 23675
6   com.apple.JavaScriptCore      	0x000000010a99baeb llint_entry + 23561
7   com.apple.JavaScriptCore      	0x000000010a99baeb llint_entry + 23561
8   com.apple.JavaScriptCore      	0x000000010a995cff vmEntryToJavaScript + 299
9   com.apple.JavaScriptCore      	0x000000010a88389e JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 158
Comment 1 Radar WebKit Bug Importer 2016-02-17 19:53:47 PST 
<rdar://problem/24713422>
Comment 2 Chris Dumez 2016-02-17 19:57:54 PST 
Looks like there were 2 checks like this in putDirectInternal:
 if ((attributes & Accessor) != (currentAttributes & Accessor))

And I only updated one of them :/
Comment 3 Chris Dumez 2016-02-17 20:58:33 PST 
This time it seems we hit the following assertion in getOwnPropertyDescriptor():
ASSERT(maybeGetterSetter);

So we have a slot with CustomAccessor attribute but getDirect() returns no value somehow.

|this| is a DebuggerScope and the propertyName is “document”.
Comment 4 Chris Dumez 2016-02-17 22:24:12 PST 
I think the issue is that DebuggerScope::getOwnPropertySlot() does not only return *own* properties. It searches the prototype chain, like JSDOMWindow used to do before r196676. We used to have a check at the top of GetOwnPropertyDescriptor() to return early if getOwnPropertySlot() returned a non-own property but Gavin dropped it in r 196676, assuming the workaround was only needed for JSDOMWindow...

We probably need to add the following check back:
    if (slot.slotBase() != this && slot.slotBase()) {
        if (!proxy || proxy->target() != slot.slotBase())
            return false;
    }

I will verify.
Comment 5 Chris Dumez 2016-02-18 08:47:26 PST 
Created attachment 271663 [details]
Patch
Comment 6 Mark Lam 2016-02-18 08:56:26 PST 
Comment on attachment 271663 [details]
Patch

r=me
Comment 7 Chris Dumez 2016-02-18 09:19:30 PST 
Comment on attachment 271663 [details]
Patch

Clearing flags on attachment: 271663

Committed r196760: <http://trac.webkit.org/changeset/196760>
Comment 8 Chris Dumez 2016-02-18 09:19:35 PST 
All reviewed patches have been landed.  Closing bug.