464413 - "Assertion failed: _stats.freePages == _stats.pages"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Jesse Ruderman]

Attachment: testcase

During js_FinishJIT, this testcase triggers:

Assertion failed: _stats.freePages == _stats.pages
(../nanojit/Fragmento.cpp:99)

This is a regression from bug 464138.  I initially reported it in the wrong place: bug 456511 comment 37.  jsfunfuzz hits this bug every time I run it.

Comment 1

[Andreas Gal :gal]

Attachment: Don't lose the lirbuf if we already allocated one.

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 347727 [details] [diff] [review]
Don't lose the lirbuf if we already allocated one.

Thanks,

/be

Comment 3

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/038d7a5a32d8

Comment 4

[Andreas Gal :gal]

Comment on attachment 347727 [details] [diff] [review]
Don't lose the lirbuf if we already allocated one.

This fixes the leak on mc. The patch is green on the TM build/test infrastructure.

Comment 5

[Andreas Gal :gal]

Not a security bug, but a serious leak (every String.replace leaks 4k memory).

Comment 6

[Andreas Gal :gal]

Actually slight correction. We leak 40 bytes or so for the lirbuf descriptor. The 4k will be reclaimed when the code cache resets.

Comment 7

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

Comment on attachment 347727 [details] [diff] [review]
Don't lose the lirbuf if we already allocated one.

(now blocking, doesn't need explicit approval)

Comment 8

[Karl Tomlinson (:karlt)]

Would it cause hassle for tm developers if bugs were not marked fixed until they land on m-c?

Or do we need a fixed-mozilla-central or similar keyword?

Comment 9

[Andreas Gal :gal]

We are not closing externally reported bugs. We are currently closing bugs that were filed against the TM tree as we fix them there. If thats still troublesome, we can further relax the rules. The main problem is that bugs have to be closed once we merge, which we keep forgetting. Feel free to open this bug again for tracking purposes if you want.

Comment 10

[Brendan Eich [:brendan]]

I've been closing tm-only bugs. That is, bugs that I know are not in m-c. Otherwise it's too likely I'll hide a bug from an m-c-based tester who is trying to avoid filing a dup.

/be

Comment 11

[Justin Dolske [:Dolske]]

Pushed by request to m-c for beta 2: http://hg.mozilla.org/mozilla-central/rev/af15e29d4748

Comment 12

[Bob Clary [:bc] (inactive)]

this assertion was covered by 10 different tests.

Comment 13

[Bob Clary [:bc] (inactive)]

11.

ecma_2/String/match-001.js
ecma_3/String/15.5.4.11.js
ecma_3/String/regress-189898.js
ecma_3/String/regress-83293.js
js1_5/Regress/regress-167658.js
js1_5/Regress/regress-179524.js
js1_6/extensions/regress-312385-01.js
js1_8_1/trace/trace-test.js
js1_8/genexps/regress-380237-01.js
js1_8/regress/regress-384412.js

Comment 14

[Bob Clary [:bc] (inactive)]

v 1.9.1, 1.9.2