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

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: decoder, Assigned: luke)

Tracking

(Blocks 1 bug, {assertion, regression, testcase})

Trunk
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 .x+)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

9 years ago
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

Comment 2

9 years ago
Assignee: general → lw
Assignee

Comment 3

9 years ago
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
Assignee

Comment 4

9 years ago
Posted 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 :)
Assignee

Comment 5

9 years ago
Posted 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)
Assignee

Comment 6

9 years ago
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)
Assignee

Comment 7

9 years ago
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+
Assignee

Comment 9

9 years ago
http://hg.mozilla.org/tracemonkey/rev/974015054395
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/974015054395
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Reporter

Updated

8 years ago
Blocks: 676763
Reporter

Comment 11

7 years ago
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.