501270 - (CVE-2009-2664) Assertion failure: !fp->fun || !(fp->fun->flags & JSFUN_HEAVYWEIGHT) || fp->callobj

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

browser only, js1_5/extensions/regress-361964.js on mac and nt at least. security sensitive until I can find the regressor.

Assertion failure: !fp->fun || !(fp->fun->flags & JSFUN_HEAVYWEIGHT) || fp->callobj, at /work/mozilla/builds/1.9.0/mozilla/js/src/jsinterp.c:663

Comment 1

[Bob Clary [:bc] (inactive)]

bug 460882 regressed this.

Comment 2

[Bob Clary [:bc] (inactive)]

ping?

Comment 3

[Bob Clary [:bc] (inactive)]

bz: i'm only seeing this on 1.9.0. Is there an underlying bug on 1.9.1/1.9.2?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Blake says no.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

As far as I can tell, the only reason this doesn't bite on trunk is that we're a lot smarter about what we call "heavyweight" and what forces a call object. Also, document.title is quickstubbed on trunk and 1.9.1, so we end up computing this with a frame pushed on top of the pseudo-frame created by the watchpoint code.

I think this patch is generally correct, so I'll attach a trunk version right away.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: For trunk

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 386149 [details] [diff] [review]
Proposed fix

Looks right.

/be

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 386154 [details] [diff] [review]
For trunk

I diff'ed the patches to see the changes from 1.9.0 -- looks good again.

/be

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 386149 [details] [diff] [review]
Proposed fix

This is needed to fix a regression from a security fix. It's preventing bc from running the JS testsuite in debug builds.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/02eca43038ef

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 386149 [details] [diff] [review]
Proposed fix

Approved for 1.9.0.12, a=dveditz for release-drivers

bc: please verify this after Blake lands.

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Checking in js/src/jsdbgapi.c;
/cvsroot/mozilla/js/src/jsdbgapi.c,v  <--  jsdbgapi.c
new revision: 3.153; previous revision: 3.152
done

Comment 13

[Bob Clary [:bc] (inactive)]

v 1.9.0.12 on mac browser. no assert on 1.9.1-tracemonkey either.

Comment 14

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/02eca43038ef

Comment 15

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

Fixed this in 1.9.0.12, so we should fix it in 1.9.1.1.

Comment 16

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

Blake: Can you request approval on a patch that applies?

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

mrbkap says that we can wait until 1.9.1.2 if that's coming in the next few weeks, which it is!

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

Blake: can you whip us up a mozilla-1.9.1 patch so we can get it landed insteda of rushing at the last minute to get it in?

Comment 19

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 386154 [details] [diff] [review]
For trunk

If this gets approval, please approve the patch in bug 502449 at the same time.

Comment 20

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

Comment on attachment 386154 [details] [diff] [review]
For trunk

Approved for 1.9.1.2. a=ss for release-drivers

Please land on mozilla-1.9.1 and use the ".2-fixed" option of the "status1.9.1" flag.

Comment 21

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7853bc76a1e3

Comment 22

[Bob Clary [:bc] (inactive)]

the referenced test never failed on 1.9.1/1.9.2. it still doesn't.

Comment 23

[u279076]

What is the best/simplest way for QA to verify this for 3.5.2?

Comment 24

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #23)
> What is the best/simplest way for QA to verify this for 3.5.2?

Compile a js shell and see if the testcase file in comment #0 still asserts.

Comment 25

[Bob Clary [:bc] (inactive)]

(In reply to comment #24)
> (In reply to comment #23)
> > What is the best/simplest way for QA to verify this for 3.5.2?
> 
> Compile a js shell and see if the testcase file in comment #0 still asserts.

it never did.

Comment 26

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #25)
> (In reply to comment #24)
> > (In reply to comment #23)
> > > What is the best/simplest way for QA to verify this for 3.5.2?
> > 
> > Compile a js shell and see if the testcase file in comment #0 still asserts.
> 
> it never did.

D'oh, sorry, never noticed "browser-only" in comment #0 till now, so I guess you have to compile a debug browser build.

Comment 27

[Bob Clary [:bc] (inactive)]

no, actually it never asserted on 1.9.1/1.9.2 only 1.9.0. The underlying bug wasn't exposed by the test case there.

Comment 28

[u279076]

bc: Do you think it safe to mark this bug verified1.9.1 based on comment 27?

Comment 29

[Bob Clary [:bc] (inactive)]

nope, otherwise I would have. I don't think it is a problem shipping like this, but what's the point in rubber stamping it other than to clean up a bug query?