558813 - [MIPS] running tamarin as3/Vector/ acceptance testcases produces an assert

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect)

Description

[Brent Baker]

Running the test/acceptance/as3/Vector/ tamarin-redux testcases produces an assert when running on MIPS.

This was produced using tr tip (rev 4359) + MIPS patch posted via bug 551165


# ./avmshell_debug_4359 -Djitordie as3/Vector/concat.abc
  Vector.concat()
Assertion failed: targ != __null (/home/build/hg/tamarin-redux/nanojit/NativeMIPS.cpp:1502)
Trace/breakpoint trap

Comment 1

[Brent Baker]

There are about 180 different tamarin test/acceptance files that produce the above error when running.

gdb output:
  Vector.concat()
Assertion failed: targ != __null (/home/build/hg/tamarin-redux/nanojit/NativeMIPS.cpp:1502)
[New Thread 0x730000 (LWP 24680)]

Program received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 0x730000 (LWP 24680)]
0x006488ac in raise ()
Current language:  auto; currently asm

Comment 2

[Brent Baker]

backtrace from gdb:

(gdb) bt
#0  0x006488ac in raise ()
#1  0x0040142c in VMPI_debugBreak ()
    at /home/build/hg/tamarin-redux/VMPI/UnixDebugUtils.cpp:52
#2  0x005ca98c in avmplus::AvmDebugMsg (p=0x696274 "", debugBreak=true)
    at /home/build/hg/tamarin-redux/core/AvmDebug.cpp:66
#3  0x0040111c in avmplus::AvmAssertFail (message=0x696274 "")
    at /home/build/hg/tamarin-redux/core/AvmDebug.h:65
#4  0x004fc5a0 in nanojit::NanoAssertFail ()
    at /home/build/hg/tamarin-redux/core/AvmDebug.h:90
#5  0x0060a0d0 in nanojit::Assembler::asm_j (this=0x2ac3c078, targ=0x0,
    bdelay=true) at /home/build/hg/tamarin-redux/nanojit/NativeMIPS.cpp:1502
#6  0x005f2da8 in nanojit::Assembler::gen (this=0x2ac3c078, reader=0x7f8d82c8)
    at /home/build/hg/tamarin-redux/nanojit/Assembler.cpp:1569
#7  0x005f3774 in nanojit::Assembler::assemble (this=0x2ac3c078,
    frag=0x2ac12030, reader=0x7f8d82c8)
    at /home/build/hg/tamarin-redux/nanojit/Assembler.cpp:1046
#8  0x004eb2a8 in avmplus::CodegenLIR::emitMD (this=0x7f8d8508)
    at /home/build/hg/tamarin-redux/core/CodegenLIR.cpp:6110
#9  0x0052a608 in avmplus::MethodInfo::verify (this=0x2aae4e48,
    toplevel=0x2ab33088, abc_env=0x2ab32020)
    at /home/build/hg/tamarin-redux/core/MethodInfo.cpp:363
#10 0x0052ab30 in avmplus::MethodInfo::verifyEnterGPR (env=0x2ab88400,
    argc=0, ap=0x7f8d86f8)
    at /home/build/hg/tamarin-redux/core/MethodInfo.cpp:191
#11 0x0051b944 in avmplus::MethodEnv::delegateInvoke (env=0x2ab88400, argc=0,
    ap=0x7f8d86f8) at /home/build/hg/tamarin-redux/core/MethodEnv.cpp:542
#12 0x2ab6f178 in ?? ()
warning: GDB can't find the start of the function at 0x2ab6f177.

Comment 3

[Edwin Smith]

I took tamarin-redux, applied the patch from bug 551165.

This is the assert, I think.  targ will be 0 (NULL) when the assembler needs to generate a branch and then patch it later.  TR will do this for loop edges, because the branch will be encountered before the target.  It also will do this for conditional branches, but from what i can tell, the mips backend already handles patchable conditional branches correctly.

    void Assembler::asm_j(NIns * const targ, bool bdelay)
    {
==> NanoAssert(targ != NULL);
        NanoAssert(SEG(targ) == SEG(_nIns));
        if (bdelay) {
            underrunProtect(2*4);    // j + delay
            NOP();
        }
        J(targ);
        TAG("asm_j(targ=%p) bdelay=%d", targ);
    }

I think its being called from here, in Assembler::gen(), case LIR_j:

                    else {
                        // backwards jump
                        handleLoopCarriedExprs(pending_lives);
                        if (!label) {
                            // save empty register state at loop header
                            _labels.add(to, 0, _allocator);
                        }
                        else {
                            intersectRegisterState(label->regs);
                        }
==>                JMP(0);
                        _patches.put(_nIns, to);
                    }

Comment 4

[Brent Baker]

Information on the MIPS hardware that Adobe has in house for testing/debugging
https://zerowing.corp.adobe.com/display/FlashPlayer/MIPS+-+Broadcom+97405+HowTo

Comment 5

[Chris Dearman]

Attachment: Handle NULL target in asm_j

Handle the case where asm_j is called with a NULL target by calling
asm_bxx which will generate the additional code required to handle the unknown destination.

Comment 6

[Brent Baker]

(In reply to comment #5)
> Created an attachment (id=440267) [details]
> Handle NULL target in asm_j
> 
> Handle the case where asm_j is called with a NULL target by calling
> asm_bxx which will generate the additional code required to handle the unknown
> destination.

Running patch (with patch from 551165) through an acceptance pass now. I have confirmed that the earlier failures that I was seeing are no longer happening. I will update later when it is complete.

Comment 7

[Brent Baker]

Comment on attachment 440267 [details] [diff] [review]
Handle NULL target in asm_j

Patches look good, ran an acceptance pass with release and debug shell. There are a couple of date handling failures that I will log separately (ecma3/Date/e15_9_5_(28|29|32|34)_1). Can we please get these patches (attachment #440267 [details] [diff] [review] and attachment #437408 [details] [diff] [review]) landed into nanojit and tamarin-redux asap. I really want to have this enabled in the tamarin build system so that we can ensure that it continues to work. Tracking adding MIPS to the build system via bug# 524129

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/5a5d667a6609

Comment 9

[Brent Baker]

http://hg.mozilla.org/projects/nanojit-central/rev/25449d23ba02
http://hg.mozilla.org/tamarin-redux/rev/cf37b4ce0c0b