Bug 17936

Summary: "ASSERTION FAILED: false" (GetterSetterImp::toObject is called)
Product: WebKit Reporter: Jesse Ruderman <jruderman>
Component: JavaScriptCoreAssignee: Cameron Zwarich (cpst) <zwarich>
Status: RESOLVED FIXED    
Severity: Normal CC: gavin.sharp, zwarich
Priority: P2 Keywords: HasReduction, InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 13638    
Attachments:
Description Flags
Proposed patch adding layout tests oliver: review+

Description Jesse Ruderman 2008-03-18 22:17:46 PDT
testkjs doesn't like this script.

b = 3;
this.__defineSetter__("a", function() {});
this.__defineSetter__("b", function() {});
delete a;
b.c;


A Debug build of testkjs exits, saying

ASSERTION FAILED: false
(/Users/jruderman/WebKit/JavaScriptCore/kjs/internal.cpp:191 virtual KJS::JSObject* KJS::GetterSetterImp::toObject(KJS::ExecState*) const)

A Release build of testjks says "TypeError: Null value" when perhaps it should say "TypeError: Undefined value", but nothing really bad happens.
Comment 1 Mark Rowe (bdash) 2008-03-19 14:46:01 PDT
<rdar://problem/5320744>
Comment 2 Mark Rowe (bdash) 2008-03-19 14:48:58 PDT
JSObject::defineSetter calls putDirect(propertyName, gs, GetterSetter), where "GetterSetter" is an attribute.  If a property exists with the name, this ends up inside PropertyMap::put at the following code:

            // Put a new value in an existing hash table entry.
            m_u.table->entries()[entryIndex - 1].value = value;
            // Attributes are intentionally not updated.

The problem here is the fact that we do not update the attribute of the property, which will lead to the PropertyMap returning an incorrect value for containsGettersOrSetters() when it is called after the setter for "a" in the test case is deleted.
Comment 3 Cameron Zwarich (cpst) 2008-09-01 22:02:52 PDT
It seems that this was fixed by the refactoring of the getter and setter code in r36016, the revision that introduced polymorphic inline caching. I'll write a test so we can close this bug.
Comment 4 Cameron Zwarich (cpst) 2008-09-02 18:35:33 PDT
Created attachment 23134 [details]
Proposed patch adding layout tests
Comment 5 Cameron Zwarich (cpst) 2008-09-02 19:47:39 PDT
Landed in r36035.