627106 - Assertion failure: 0 (./nanojit/NativeX64.cpp:1334)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: Set of files required to run test case (see bug report)

I'm hitting the specified assertion on x86_64 with a test case that I couldn't minimize further so far because the test does not always reproduce the problem (I'd say 80-90% of the cases), and does not run in valgrind.

The test case is attached a .tgz, unpack it and run it like this:

js -f shell.js -f driver.js < run.log

The test takes roughly 5 seconds and consumes approximately 3 GB of RAM as far as I could see in ps.

Locking this by recommendation of dvander.

Comment 2

[David Mandelin [:dmandelin]]

Nick, how serious does this seem to you?

Comment 3

[Nicholas Nethercote [inactive]]

A stack trace would really help.  (I ask for that rather than doing it myself because it's Saturday here and I'm reading email but not doing "real work".)

Also, with current TM tip I see no assertion on line 1334, but there is one on line 1331, so I assume that's it.

Comment 4

[Christian Holler (:decoder)]

I just tried on TM tip and can't reproduce anymore, so I'll try a bisect. The problem with these type of testcases is their instability even across revisions :(

Comment 5

[Christian Holler (:decoder)]

This bisect might be unreliable, although multiple bisects gave me this result:

Changeset 60429:807f31e3eb27: good
The first good revision is:
changeset:   60429:807f31e3eb27
user:        Andreas Gal <[e-mail]>
date:        Mon Jan 17 14:21:03 2011 -0800
summary:     Allow entering a compartment with a pending exception (bug 626267, r=luke).


Full trace obtained from revision b90090c29571

Assertion failure: 0 (../nanojit/NativeX64.cpp:1334)

Program received signal SIGABRT, Aborted.
0x00007ffff7bd1ebb in raise () from /lib/libpthread.so.0
(gdb) bt
#0  0x00007ffff7bd1ebb in raise () from /lib/libpthread.so.0
#1  0x000000000062fe84 in avmplus::AvmAssertFail () at ../nanojit/avmplus.cpp:71
#2  0x000000000063e823 in nanojit::Assembler::asm_branch_ov (this=0xaed768, 
    target=0x7fff399bff2d "H\213]\320L\213e\340L\213m\350L\213u\360L\213}\370\270@\203D\001H\213\345\351\210") at ../nanojit/NativeX64.cpp:1334
#3  0x000000000061931b in nanojit::Assembler::gen (this=0xaed768, reader=0x144d2f8) at ../nanojit/Assembler.cpp:1961
#4  0x0000000000616f99 in nanojit::Assembler::assemble (this=0xaed768, frag=0x1104d18, reader=0x144d2f8) at ../nanojit/Assembler.cpp:1084
#5  0x0000000000616770 in nanojit::Assembler::compile (this=0xaed768, frag=0x1104d18, alloc=..., optimize=true, printer=0x1105e28) at ../nanojit/Assembler.cpp:983
#6  0x00000000005c8c51 in js::TraceRecorder::compile (this=0xd93170) at ../jstracer.cpp:4511
#7  0x00000000005c96f9 in js::TraceRecorder::closeLoop (this=0xd93170) at ../jstracer.cpp:4950
#8  0x00000000005cb939 in js::TraceRecorder::checkTraceEnd (this=0xd93170, pc=0xb3c162 "\b\377\343\305\260\270\200\003T\v\263\260\262\260\260\261\260c\003\\\243")
    at ../jstracer.cpp:5454
#9  0x00000000005daaff in js::TraceRecorder::relational (this=0xd93170, op=nanojit::LIR_ltd, tryBranchAfterCond=true) at ../jstracer.cpp:9285
#10 0x00000000005df828 in js::TraceRecorder::record_JSOP_LT (this=0xd93170) at ../jstracer.cpp:10696
#11 0x00000000005d0d9f in js::TraceRecorder::monitorRecording (this=0xd93170, op=JSOP_LT) at ../jsopcode.tbl:141
#12 0x00000000006d0ca0 in js::Interpret (cx=0xabcda0, entryFrame=0x7ffff6abf178, inlineCallCount=1, interpMode=JSINTERP_NORMAL) at ../jsinterp.cpp:2714
#13 0x00000000004ca661 in js::RunScript (cx=0xabcda0, script=0x10e9510, fp=0x7ffff6abf178) at ../jsinterp.cpp:657
#14 0x00000000004cbab6 in js::Execute (cx=0xabcda0, chain=0x7ffff6903048, script=0x10e9510, prev=0x0, flags=0, result=0x0) at ../jsinterp.cpp:1023
#15 0x000000000042e16b in JS_ExecuteScript (cx=0xabcda0, obj=0x7ffff6903048, script=0x10e9510, rval=0x0) at ../jsapi.cpp:4883
#16 0x00000000004069a2 in Load (cx=0xabcda0, argc=1, vp=0x7ffff6abf118) at ../../shell/js.cpp:1048
#17 0x00000000004ce698 in js::CallJSNative (cx=0xabcda0, native=0x406807 <Load>, argc=1, vp=0x7ffff6abf118) at ../jscntxtinlines.h:692
#18 0x00000000006e6f3a in js::Interpret (cx=0xabcda0, entryFrame=0x7ffff6abf048, inlineCallCount=1, interpMode=JSINTERP_NORMAL) at ../jsinterp.cpp:4782
#19 0x00000000004ca661 in js::RunScript (cx=0xabcda0, script=0xb0f120, fp=0x7ffff6abf048) at ../jsinterp.cpp:657
#20 0x00000000004cbab6 in js::Execute (cx=0xabcda0, chain=0x7ffff6903048, script=0xb0f120, prev=0x0, flags=0, result=0x0) at ../jsinterp.cpp:1023
#21 0x000000000042e16b in JS_ExecuteScript (cx=0xabcda0, obj=0x7ffff6903048, script=0xb0f120, rval=0x0) at ../jsapi.cpp:4883
#22 0x000000000040554b in Process (cx=0xabcda0, obj=0x7ffff6903048, filename=0x7fffffffdcf2 "driver.js", forceTTY=0) at ../../shell/js.cpp:453
#23 0x0000000000406324 in ProcessArgs (cx=0xabcda0, obj=0x7ffff6903048, argv=0x7fffffffd9a0, argc=4) at ../../shell/js.cpp:867
#24 0x000000000040f99c in Shell (cx=0xabcda0, argc=4, argv=0x7fffffffd9a0, envp=0x7fffffffd9c8) at ../../shell/js.cpp:5435
#25 0x000000000040fb62 in main (argc=4, argv=0x7fffffffd9a0, envp=0x7fffffffd9c8) at ../../shell/js.cpp:5543

Comment 6

[Johnny Stenback (:jst)]

Andreas, looks like this *might* be caused by one of your checkins (see previous comment). Any thoughts here?

Comment 7

[Christian Holler (:decoder)]

(In reply to comment #6)
> Andreas, looks like this *might* be caused by one of your checkins (see
> previous comment). Any thoughts here?

My bisect shows the fixing rev although I think it might only cause the testcase to behave different. I guess this needs investigation on an earlier rev (like the one I used for the trace.)

Comment 8

[Andreas Gal :gal]

Which rev do you recommend?

Comment 9

[Christian Holler (:decoder)]

(In reply to comment #8)
> Which rev do you recommend?

Try b90090c29571, it worked for me :)

Comment 10

[David Mandelin [:dmandelin]]

I cannot repro on b90090c29571. I tried a debug build on OS X 10.6 and didn't get an assertion in 5 trials. Feel free to renom/block if anyone who can debug this does manage to reproduce it.

Comment 11

[Nicholas Nethercote [inactive]]

(In reply to comment #10)
> I cannot repro on b90090c29571. I tried a debug build on OS X 10.6 and didn't
> get an assertion in 5 trials.

Likewise for me, but I tried on my Linux64 box.

So let's look at the code.  The failing assertion is this one:

    NIns* Assembler::asm_branch_ov(LOpcode, NIns* target) {
        if (target && !isTargetWithinS32(target)) {
            setError(ConditionalBranchTooFar);  
            NanoAssert(0);
        }       

The problem is that we have a conditional branch whose offset doesn't fit into an int32.  It should be safe, because the ConditionalBranchTooFar error is set.  I guess the NanoAssert(0) is just there to notify us if it ever happens.

However, bug 624439 removed this assertion.  (That explains why you need an old revision to trigger it;  bug 624439 landed 9 revisions after b90090c29571.)  An equivalent assertion was added to emitTarget32():

        if (!isS32(offset)) {
            setError(BranchTooFar);
            NanoAssert(0);  // assert because we'd like to know if this ever hap
pens
        }

Now, asm_branch_ov() indirectly calls emitTarget32(), so it's interesting that the assertion isn't occurring in its new location.  I guess the occurrence is sensitive enough that minor changes in NativeX64.cpp is enough for it not to happen.

Conclusion:  I'm going to mark this as fixed and remove the [sg:critical?], because the assertion was a "we'd like to know if this happens" case rather than a "this should never happen" case, and it should be handled correctly by the BranchTooFar error code.  Furthermore, it's no longer reproducing thanks to bug 624439.

If the assertion that's now in emitTarget32() ever hits, we can open a new bug and remove the assertion once we have concrete evidence that the error code setting works.

decoder, thanks again for a really nasty test case :)  It's great that you're stressing the JS engine like this.

Comment 12

[Daniel Veditz [:dveditz]]

any reason not to unhide the bug then?

Comment 13

[Nicholas Nethercote [inactive]]

Unhiding should be fine.