329747 - Coverity thinks js_AddScopeProperty might be called w/ setter=JS_PropertyStub and attrs & JSPROP_SETTER

Status: VERIFIED INVALID

(Core :: JavaScript Engine, defect)

Description

[timeless]

I'm really not sure about this, but so far, I think it's the closest thing I've seen to a real almost maybe bug.

Assuming js_AddScopeProperty is called with setter=JS_PropertyStub and attrs & JSPROP_SETTER and there is a watch point for with js_FindWatchPoint returns true, then http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsdbgapi.c&rev=3.61&mark=406,413,415,424#405 would crash according to my reading of coverity's report.

Personally, i'm having a hard enough time understanding how to cast from JSPropertyOp to JSObject*.

It's late, so it's quite likely i'm missing a dozen obvious things and that i shouldn't try to read coverity reports at this hour, but oh well....

Comment 1

[Brendan Eich [:brendan]]

Timeless, JSPROP_SETTER means the setter is a JSObject *, not a JSPropertyOp.  Also note that js_AddScopeProperty does

    /*
     * Normalize stub getter and setter values for faster is-stub testing in
     * the SPROP_CALL_[GS]ETTER macros.
     */
    if (getter == JS_PropertyStub)
        getter = NULL;
    if (setter == JS_PropertyStub)
        setter = NULL;

/be

Comment 2

[timeless]

oh, believe me, i noted that, that's precisely why coverity complained to begin with. i guess the problem is that coverity can't know that JSPROP_SETTER and setter=JS_PropertyStub are mutually exclusive. I suppose if i were trying to silence coverity, i'd change the if (setter == JS_PropertyStub) to  if (!(attrs & JSPROP_SETTER) && setter == JS_PropertyStub)

but that's pretty silly and I know you wouldn't want it.