Closed Bug 620637 Opened 11 years ago Closed 11 years ago

Assertion failure: result->isImmI(jsint(r))

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- .x+

People

(Reporter: decoder, Assigned: luke)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: fixed-in-tracemonkey)

Attachments

(3 files, 1 obsolete file)

The following code:

options('tracejit');
var actual = ''
function f() {
    for (var a = 0; a < 3; ++a) {
        (function () {
            for (var b = 0; b < 2; ++b) {
                (function () {
                    for (a = 0, b = 0; b < 15; b++, actual = actual + "7") {
                    }
                })();
            }
        })();
    }
}
f();

causes Assertion failure: result->isImmI(jsint(r)), at jstracer.cpp:8390

verified on mozilla-central b8 and trunk.
This is the culprit:

changeset:   53418:8721b595e7ab
user:        Luke Wagner <lw@mozilla.com>
date:        Mon Aug 09 22:43:33 2010 -0700
summary:     Bug 539144 - Make formal args a jit-time const offset from fp; rm argv/argc/thisv/script/callobj (r=brendan,dvander)
Blocks: 539144
blocking2.0: --- → ?
Assignee: general → lw
This bug stems from a faulty assumption about how inner trees can mutate outer trees' upvars that predates bug 539144 (as demonstrated by the attached testcase).  The hole in the reasoning here (http://hg.mozilla.org/tracemonkey/file/0be4f01086ea/js/src/jstracer.cpp#l5257) is that 4 is not strong enough to imply 5 since we do not check scopeChain identity when entering trace.  Rather than fixing this by remembering and checking the scopeChain, I'd rather just remove the whole optimization and just clear everything when calling an inner tree.

This can be done quite simplfy by factoring clearCurrentFrameSlotsFromTracker into a visitor that reuses VisitFrameSlots.
blocking2.0: ? → .x
Attached patch fix this bug, expose another (obsolete) — Splinter Review
This does what comment 3 says and doesn't assert for the test-case in comment 0 (which iloops).  Unfortunately, this exposes another invalid assumption (which, coincidentally, hits the test case I just posted in attachment 499407 [details]): the elements of importStackSlots that get left as JSVAL_TYPE_UNINITIALIZED by emitTreeCall() are now being observed (and thus, asserting) since the corresponding entries in the tracker are now getting cleared.  The fix is to add an extra bit of logic to emitTreeCall to fill these elements with valid types.  On the bright side, this should allow us kill JSVAL_TYPE_UNINITIALIZED :)
Attached patch better fixSplinter Review
The last patch had a bug in that it only cleared up to 'sp' instead of 'slots + script->nslots'.
Attachment #499479 - Attachment is obsolete: true
Attachment #499485 - Flags: review?(dvander)
So it turns out that attemptTreeCall was already doing the necessary preparation of importTypeMap so, IIUC, all the JSVAL_TYPE_UNINITIALIZED-filling wasn't necessary.
Attachment #499486 - Flags: review?(dvander)
Oops, assume the two test cases above are included (and un-iloop-ed) in those patches.
Comment on attachment 499485 [details] [diff] [review]
better fix

>+TraceRecorder::clearReturningFrameFromNativeveTracker()

Looks "Native" got some extra letters.
Attachment #499485 - Flags: review?(dvander) → review+
Attachment #499486 - Flags: review?(dvander) → review+
http://hg.mozilla.org/tracemonkey/rev/974015054395
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/974015054395
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.