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

RESOLVED FIXED

Status

()

RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: decoder, Assigned: luke)

Tracking

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

Trunk
x86_64
Linux
assertion, regression, testcase
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

8 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: --- → ?
Keywords: assertion, regression, testcase
(Assignee)

Comment 2

8 years ago
Created attachment 499407 [details]
test case that asserts before 8721b595e7ab
Assignee: general → lw
(Assignee)

Comment 3

8 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

8 years ago
Created attachment 499479 [details] [diff] [review]
fix this bug, expose another

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

8 years ago
Created attachment 499485 [details] [diff] [review]
better fix

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

8 years ago
Created attachment 499486 [details] [diff] [review]
don't be so pessimistic

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

8 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

8 years ago
http://hg.mozilla.org/tracemonkey/rev/974015054395
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/974015054395
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Updated

7 years ago
Blocks: 676763
(Reporter)

Comment 11

6 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.