455464 - TM: "Assertion failure: !TRACE_RECORDER(cx) ^ (jumpTable == recordingJumpTable)" with gc getter

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

$ ~/tracemonkey/js/src/Darwin_DBG.OBJ/js -j 
js> a=b=c=d=0; this.__defineGetter__('g', gc); [1 for each (y in this)];

Assertion failure: !TRACE_RECORDER(cx) ^ (jumpTable == recordingJumpTable), at jsinterp.cpp:3289

Some variants trigger this instead:

Assertion failure: ngslots == tm->globalTypeMap->length(), at jstracer.cpp:2452

Comment 1

[Jesse Ruderman]

Oops.  The array comprehension isn't necessary:

js> a=b=c=d=0; this.__defineGetter__('g', gc); for each (y in this);

Assertion failure: !TRACE_RECORDER(cx) ^ (jumpTable == recordingJumpTable), at jsinterp.cpp:3289

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

IIRC we hacked something like this on your machine, but possibly it was still buggy (returning with gcRunning set? Something like that). This works for me in shell, will test browser when able.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: fix, without patch for bug 453411 mixed in

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 338830 [details] [diff] [review]
fix, without patch for bug 453411 mixed in

Well, we put the initial version above the set slot request stuff, which caused problems (obviously in hindsight).

I wonder if we want to put the out label after the restart stuff to be clear that we will never actually restart if we go to that label.

Comment 5

[Brendan Eich [:brendan]]

(In reply to comment #4)
> I wonder if we want to put the out label after the restart stuff to be clear
> that we will never actually restart if we go to that label.

Not safe, another thread could come in and try to GC, bumping gcLevel. We need to keep book properly and fully, even if it means another trip 'round the block. We're guaranteed to terminate.

/be

Comment 6

[Brendan Eich [:brendan]]

Fixed on tm:

http://hg.mozilla.org/tracemonkey/rev/f59912630f60

/be

Comment 7

[Bob Clary [:bc] (inactive)]

Note I couldn't reproduce the original test in the suite although I could by simply running the original test in the shell. -04 was able to reproduce the other assertion ngslots == tm->globalTypeMap->length() by adding a gczeal 2 however.

/cvsroot/mozilla/js/tests/js1_6/extensions/regress-455464-01.js,v  <--  regress-455464-01.js
initial revision: 1.1

/cvsroot/mozilla/js/tests/js1_6/extensions/regress-455464-02.js,v  <--  regress-455464-02.js
initial revision: 1.1

/cvsroot/mozilla/js/tests/js1_6/extensions/regress-455464-03.js,v  <--  regress-455464-03.js
initial revision: 1.1

/cvsroot/mozilla/js/tests/js1_6/extensions/regress-455464-04.js,v  <--  regress-455464-04.js
initial revision: 1.1
done

http://hg.mozilla.org/mozilla-central/rev/eb923c50f120

Comment 8

[Bob Clary [:bc] (inactive)]

update test to remove extraneous statements
http://hg.mozilla.org/mozilla-central/rev/e321f7b3c513

Comment 9

[Bob Clary [:bc] (inactive)]

v 1.9.1, 1.9.2. -04 times out.