511835 - Crash [@ QuoteString] or [@ js_HashString] or "Assertion failure: (*flagp & GCF_FINAL) == 0, at ../jsgc.cpp"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

Testcase 1 (pass in as a file, i.e. ./js testcase.js ):

try {
    (function () {
        for (;;)( * )
    })()
} catch(e) {} 
(function () {
    with(0) {
        gc()
    }
})()
try {
    (function () {
        ({
            x:*
        })()
    })()
} catch(e) {}

crashes QuoteString at 0x002ae000 in dbg without -j on TM branch and crashes QuoteString at 0x001e9bec in opt.

=====

Testcase 2:

gczeal(2);*.l

asserts js dbg shell without -j on TM branch at Assertion failure: (*flagp & GCF_FINAL) == 0, at ../jsgc.cpp:2657

=====

Testcase 3:

__defineGetter__("x",gc)
x(false.*)

crashes js dbg and opt shells without -j on TM branch at js_HashString at a weird address.

Setting security-sensitive because the above testcases all seem to involve gc or gczeal.

autoBisect shows the above testcases are probably related to bug 511567:

The first bad revision is:
changeset:   31637:3d98dade7db3
user:        Andreas Gal <[e-mail]>
date:        Wed Aug 19 19:37:59 2009 -0700
summary:     Atomize unit strings at birth (511567, r=shaver).

Comment 1

[Andreas Gal :gal]

Attachment: patch

Don't GC unit strings since they are atoms and we don't always properly mark atoms we expect to stick around.

Comment 2

[Andreas Gal :gal]

We broke this very recently. I will open up the bug and we will fix this in m-c asap. This isn't on any branch.

Comment 3

[Brendan Eich [:brendan]]

(In reply to comment #1)
> Created an attachment (id=395955) [details]
> patch
> 
> Don't GC unit strings since they are atoms

Atoms are marked. What's stopping the unit strings from being marked too?

> we don't always properly mark atoms we expect to stick around.

What's this mean?

/be

Comment 4

[Andreas Gal :gal]

Unit strings are not in the atom table. I still don't understand why this hazard occurs. Whoever holds the atom should have had it traced.

Comment 5

[Brendan Eich [:brendan]]

GC_KEEP_ATOMS relies on the atom table holding refs to mark. Taking unit strings out of the table means they won't be marked so will be collected. I thought this was a latent bug in js_GetAnyName (jsxml.cpp) but it's really pervasive: last ditch GC must not collect atoms.

/be

Comment 6

[Brendan Eich [:brendan]]

Attachment: fix

Comment 7

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/6af141306c8e

/be

Comment 8

[Brendan Eich [:brendan]]

http://hg.mozilla.org/mozilla-central/rev/6af141306c8e

/be

Comment 9

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7bc822155dcf