407720 - js_FindClassObject causes crashes with getter/setter

Status: VERIFIED FIXED

Description

[moz_bug_r_a4]

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/js/src/jsobj.c&rev=3.403&mark=2780-2781,2790-2792#2780

A found property can be a getter/setter, in which case sprop->slot is
0xffffffff.

Comment 1

[moz_bug_r_a4]

Created attachment 292388 [details]
testcase (crashes Firefox when loaded)

With this testcase, on trunk:
*vp = pobj->dslots[-7]

on 1.8/1.8.0 branches:
*vp = pobj->slots[-1] = 8

Comment 2

[Igor Bukanov]

Created attachment 292398 [details] [diff] [review]
fix

This is a targeted fix to make sure that the slot for constructor is accessed only when it exists and the return from the function is either non-primitive or void.

Comment 3

[Igor Bukanov]

I accidentally committed the patch from comment 1 when I committed the patch for bug 397215, see bug 397215 comment 81.

Comment 4

[Samuel Sidler (old account; do not CC)]

Igor, is this patch applicable on the 1.8 branch or do we need a new one? If this one works please ask for approval1.8.1.12 on it, or else please create the branch version of this patch.

Comment 5

[Igor Bukanov]

Comment on attachment 292398 [details] [diff] [review]
fix

The patch applies to 1.8.1 branch as-is.

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 292398 [details] [diff] [review]
fix

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 7

[Igor Bukanov]

I checked in the patch from comment 2 to MOZILLA_1_8_BRANCH:

Checking in jsobj.c;
/cvsroot/mozilla/js/src/jsobj.c,v  <--  jsobj.c
new revision: 3.208.2.56; previous revision: 3.208.2.55
done

Comment 8

[Al Billings [:abillings]]

Verified in branch with https://bugzilla.mozilla.org/attachment.cgi?id=292388. No crash with testcase (though it crashes nicely in 2.0.0.11).

Comment 9

[Alexander Sack]

distro patches block 1.8.0.15

Comment 10

[Alexander Sack]

Comment on attachment 292398 [details] [diff] [review]
fix

we ship an unmodified patch in distros. caillon, please sign off.

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 292398 [details] [diff] [review]
fix

a=caillon for 1.8.0

Comment 12

[Reed Loden [:reed] (use needinfo?)]

(In reply to comment #10)
> (From update of attachment 292398 [details] [diff] [review])
> we ship an unmodified patch in distros. caillon, please sign off.

You do? Not sure how, considering there isn't a js_FindClassObject() in js/src/jsobj.c on MOZILLA_1_8_0_BRANCH! ;)

igor, is this patch needed on 1.8.0? If so, do we need some other patch first?

Comment 13

[Alexander Sack]

Created attachment 311136 [details] [diff] [review]
real 1.8.0

indeed, thanks for catching this.

Comment 14

[Alexander Sack]

Comment on attachment 311136 [details] [diff] [review]
real 1.8.0

a=asac for 1.8.0.15

Comment 15

[Reed Loden [:reed] (use needinfo?)]

MOZILLA_1_8_0_BRANCH:

Checking in js/src/jsobj.c;
/cvsroot/mozilla/js/src/jsobj.c,v  <--  jsobj.c
new revision: 3.208.2.12.2.29; previous revision: 3.208.2.12.2.28
done

Comment 16

[Bob Clary [:bc:]]

verified linux|mac|windows

/cvsroot/mozilla/js/tests/js1_5/extensions/regress-407720.js,v  <--  regress-407720.js
initial revision: 1.1