561936 - Crash [@ JSObject::isNative] with defineProperty on window.__proto__ (XOW?), iter

Status: RESOLVED DUPLICATE of bug 560796

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

cf bug 561309, where the property goes on the window object rather than its __proto__, and the crash happens during GC rather than during iteration.

Comment 1

[Jesse Ruderman]

Attachment: stack trace for the crash

Comment 2

[Andreas Gal :gal]

Ouch. From casual inspection this looks like a rooting bug in xpconnect. I will try to reproduce. ccing mrbkap.

Comment 3

[Andreas Gal :gal]

Weird. Setter is a raw function pointer, but we think it should be an obj. waldo?

(gdb) up
#4  0x0000000104140295 in JSScopeProperty::JSScopeProperty (this=0x7fff5fbfa6d0, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0) at jsscope.h:678
678	        JS_ASSERT_IF(setter && (attrs & JSPROP_SETTER),
(gdb) up
#5  0x000000010415f0e8 in JSScope::addPropertyHelper (this=0x11b50cdc0, cx=0x124f76a00, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0, spp=0x129583908) at ../../../js/src/jsscope.cpp:825
warning: Source file is more recent than executable.
825	        JSScopeProperty child(id, getter, setter, slot, attrs, flags, shortid);
(gdb) up
#6  0x000000010415f357 in JSScope::putProperty (this=0x11b50cdc0, cx=0x124f76a00, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0) at ../../../js/src/jsscope.cpp:883
883	        return addPropertyHelper(cx, id, getter, setter, slot, attrs, flags, shortid, spp);
(gdb) down
#5  0x000000010415f0e8 in JSScope::addPropertyHelper (this=0x11b50cdc0, cx=0x124f76a00, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0, spp=0x129583908) at ../../../js/src/jsscope.cpp:825
825	        JSScopeProperty child(id, getter, setter, slot, attrs, flags, shortid);
(gdb) down
#4  0x0000000104140295 in JSScopeProperty::JSScopeProperty (this=0x7fff5fbfa6d0, id=4808854276, getter=0x11e46b140, setter=0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>, slot=4294967295, attrs=113, flags=0, shortid=0) at jsscope.h:678
678	        JS_ASSERT_IF(setter && (attrs & JSPROP_SETTER),
(gdb) list
673	        : id(id), rawGetter(getter), rawSetter(setter), slot(slot), attrs(uint8(attrs)),
674	          flags(uint8(flags)), shortid(int16(shortid))
675	    {
676	        JS_ASSERT_IF(getter && (attrs & JSPROP_GETTER),
677	                     JSVAL_TO_OBJECT(getterValue())->isCallable());
678	        JS_ASSERT_IF(setter && (attrs & JSPROP_SETTER),
679	                     JSVAL_TO_OBJECT(setterValue())->isCallable());
680	    }
681	
682	    bool marked() const { return (flags & MARK) != 0; }
(gdb) p getter
$4 = (JSPropertyOp) 0x11e46b140
(gdb) p setter
$5 = (JSPropertyOp) 0x1000c26e3 <XPC_XOW_SetProperty(JSContext*, JSObject*, long, long*)>
(gdb) p (JSObject *)getter
$6 = (JSObject *) 0x11e46b140
(gdb) p *(JSObject *)getter
$7 = {
  map = 0x11b30a1a0, 
  classword = 4364931872, 
  fslots = {4795869264, 4802938752, 4795839696, 22, 22}, 
  dslots = 0x0, 
  static JSSLOT_PRIMITIVE_THIS = 2, 
  static JSSLOT_ARRAY_LENGTH = 2, 
  static JSSLOT_ARRAY_COUNT = 3, 
  static JSSLOT_ARRAY_UNUSED = 4, 
  static JSSLOT_ARGS_LENGTH = 3, 
  static JSSLOT_ARGS_CALLEE = 4, 
  static ARGS_FIXED_RESERVED_SLOTS = 2, 
  static JSSLOT_DATE_UTC_TIME = 2, 
  static JSSLOT_DATE_LOCAL_TIME = 3, 
  static DATE_FIXED_RESERVED_SLOTS = 2, 
  static JSSLOT_REGEXP_LAST_INDEX = 3, 
  static REGEXP_FIXED_RESERVED_SLOTS = 1
}
(gdb)

Comment 4

[Jeff Walden [:Waldo]]

The problem is that in GetPropertyAttributesById, we assign desc->setter = sprop->setter().  rawSetter is NULL, and then we (reasonably?) call JS_DefineProperty with desc->setter and desc->attrs.  desc->attrs sez we have a setter function, desc->setter is NULL but that gets converted to clasp->getProperty, and assertions complain loudly.

Not sure what exactly is the right fix here...

Comment 5

[Jesse Ruderman]

You get assertions?  I just get a crash.

Comment 6

[Andreas Gal :gal]

Yeah this is not a mere assertion. We start using the function pointer like a jsval, something thats unlikely to result in happy times. Waldo, this is really your part of town. Want to grab it? I will help if needed :)

Comment 7

[Jeff Walden [:Waldo]]

(gdb) p this->rawSetter
$2 = (JSPropertyOp) 0x7ffff5c5d69b <XPC_XOW_SetProperty(JSContext*, JSObject*, jsval, jsval*)>

Your XPC_XOW_SetProperty method is probably "aligned" as far as jsval tagging is concerned.

I have no problem grabbing this, I'm just not sure what the fix should be.  Making JSScopeProperty::{g,s}etter() censor to JS_PropertyStub maybe?

Comment 8

[Brendan Eich [:brendan]]

Don't write a raw JSPropertyOp but leave JSPROP_[SG]ETTER set in attrs!

/be

Comment 10

[Damon Sicore (:damons)]

Jeff, any progress on this?

Comment 11

[Damon Sicore (:damons)]

Jeff?

Comment 12

[Jeff Walden [:Waldo]]

Oops, I'd had some inkling that this might be fixed by bug 560796 but forgot to check back after that bug's patch got pushed -- and indeed it is.  Compare comment 4 to the change enacted there: NULL getter or setter with JSPROP_GETTER or JSPROP_SETTER in attrs passes through directly without being converted to the class {g,s}etProperty hook after that change, which directly addresses the problem noted in that comment.

Comment 13

[Christian Holler (:decoder)]

A testcase for this bug was already added in the original bug (bug 560796).