Closed Bug 464862 Opened 16 years ago Closed 16 years ago

"Assertion failed: ( int32_t(delta) == uint8_t(delta) )" on amberjack.org

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: graydon)

References

()

Details

(Keywords: assertion, testcase, verified1.9.1)

Attachments

(3 files)

On mozilla-central, http://amberjack.org/ triggers

Assertion failed: ( int32_t(delta) == uint8_t(delta) ) (/Users/jruderman/central/js/src/nanojit/LIR.cpp:241)

I'll work on a testcase.

This is not the same as bug 456607, which is 64-bit-specific (based on conversation with dvander)
Doesn't happen on tracemonkey branch :)
The JS code there is fairly straightforward in structure.
I'm not hitting this any more on amberjack.

How long does the ray tracer run before hitting the assertion?
I hit it after running the ray tracer for 10 or 30 seconds.
Also happens 70% of the time when loading http://developer.yahoo.com/ypatterns/.  I'll try to reduce this one.
Attached file partially reduced
If I try to make it any smaller, it drops way below 70% reproducibility :(  It dies with delta==256.

To reproduce, run Firefox with this file on the command line.  If you don't hit the bug, you'll have to do that again to try again (reloading won't do it).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Still happens on tracemonkey-branch tip.
Graydon, could you pls. take a look? Andreas thought this might be from the nanojit merge.

/be
Assignee: general → graydon
Hmm. The nanojit merge doesn't contain those lines, so seems unlikely. Indeed, as it *removes* those lines, and the problem does not happen on tracemonkey, it seems more likely that the nj merge fixes it rather than causes it. Or am I misunderstanding?
No, I'm the misunderstander here -- sounds like we need a tm to m-c merge soon to fix this bug.

/be
This problem *does* happen on the tracemonkey branch.
(In reply to comment #13)
> This problem *does* happen on the tracemonkey branch.

Hrm. Graydon knows all :-P.

/be
This assertion is still present (and indeed hits) on TraceMonkey.  Though I don't think the LIR encoding changes made it into our repository yet?
I am hitting this on walmart.com now (fix my apply fix). This urgently needs fixing.
Sorry for the confusion over presence/absence of the code; it's a macro call of course. I'll try to work out "logically" how that value gets to be 256, but meantime ... I'm unable to reproduce. Tried all the sites listed, all the datestamps listed by reporters. Can I get a specific revision ID that shows it?
Ah, nevermind. I was (absurdly) using a non-debug build and hoping to catch an assert. Duh. Yes it traps, looks like it's actually trying to reference something 1024 bytes away, it's just truncating to 256. Will try to fix. No need to help reproduce for now, thanks.
Attached patch Fix the bugSplinter Review
I was wrong, it was only failing with an offset of exactly 256. This was happening because the 'from' anchor-position, that the current code uses as a basis of the decision to trampoline (or not) each argument, is calculated as the *first* word of the LirCallIns structure, not the *last* (LIns-containing) word. So when referencing an argument that is exactly 256 words away from the *last* word, makeReachable would decide not to trampoline the argument since it thinks it's only 255 words away; but LIns::reference() fails because it's called as a member of a structure living 1 word later, and it calculates its reference delta using distance from 'this', getting 256.

Solution is to calculate the 'from' position correctly. I also put in an ASCII-art diagram comment, to help work out which serialized bits go where. This function is a mess; could use further cleanup.
Attachment #353895 - Flags: review?(danderson)
(Oh also, I'll not bother to refresh the patch here to reflect it, but the comment in there should refer to the last word as containing a LIns, not a LIns*, obviously)
Attachment #353895 - Flags: review?(danderson) → review+
http://hg.mozilla.org/tracemonkey/rev/173464ec399c
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
the attached testcases can be used to verify this bug but neither are appropriate for the jstests.
Flags: in-testsuite-
Flags: in-litmus-
actually, going ahead and adding jesse's.

http://hg.mozilla.org/mozilla-central/rev/9ea1807e6800
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-464862.js,v  <--  regress-464862.js
initial revision: 1.1
Flags: in-testsuite- → in-testsuite+
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: