If we have block A that terminates with a non-returning function, followed by block B, the register allocator will assume control flow falls into block B. Normally this is not an issue, since the client producing LIR would not make the same assumption, otherwise they'd have incorrect code and block B will have some other incoming edge. But the allocator incorrectly assumes there is an incoming edge from A and this currently leads to spurious non-reachable code being generated.

(In reply to comment #0) > But the allocator incorrectly assumes there is an incoming edge from A and this > currently leads to spurious non-reachable code being generated. Is this spurious nonreachable code causing the checkForResourceLeaks() assert in bug 607110? One would think that the assert would happen, or not, regardless of whether the function can return, or not, since the assembler doesn't know the function can't return. Can you explain with a simple code sample why the assert is wrong and why teaching the assembler about nonreturning functions will fix it? I'm just not understanding the root problem.

(In reply to comment #1) > (In reply to comment #0) > > > But the allocator incorrectly assumes there is an incoming edge from A and this > > currently leads to spurious non-reachable code being generated. > > Is this spurious nonreachable code causing the checkForResourceLeaks() assert > in bug 607110? yes. > Can you explain with a simple code sample why the assert is wrong and why > teaching the assembler about nonreturning functions will fix it? Will work on putting a lirasm based test cases that covers this situation, since our test suite doesn't catch it and it will illustrate clearly what is going on .

Here's an example (lots of add clutter to force the register allocator to use caller saved regs on x86). This is a re-creation of a CodegenLIR production, wherein the call is to a non-returning function. This code will properly assert in debug builds, since l3 control is assumed to fall into l4. Placing a 'j l5' after the call fixes the issue, but another solution would be to place this flow-control knowledge into LIR, as proposed in the bug. ptr = allocp 32 a = immi 1 b = immi 2 c = immi 3 d = immi 4 s = immi 65 l1: sti s ptr 0 j l3 l2: oa = addi a a ob = addi b b oc = addi c c od = addi d d j l4 l3: calli puts cdecl ptr l4: n1 = addi s oa n2 = addi s ob n3 = addi s oc n4 = addi s od sti n1 ptr 0 sti n2 ptr 1 sti n3 ptr 2 sti n4 ptr 3 l5: rn = immi 0 reti rn

Bug 614510 highlights another example of this issue.

Running the brightspot swfs with -Dverifyonly: $ avmshell_sd -Dverifyonly avmglue.abc <brightspot tests>.swf there were 97 failed assertions out of 33k swfs. The assertion was: Assertion failure: frame entry 4 wasn't freed|: _entries[i] == __null (/Users/build/buildbot/tamarin-redux/mac-intel-10_5/repo/nanojit/Assembler.cpp:2335) For example as3/as3_231110/as3/dump2/as3/6120.swf triggers the assertion. In bug https://bugzilla.mozilla.org/show_bug.cgi?id=625458 I am working on running debug-debugger builds with brightspot.

see bug 620860 for another potential example.

Rick, this appears to be a long-standing bug. Is there a reason for making this a P1 fix for Wasabi? Appears to be an issue that we want to address but isn't a must-fix for Wasabi. Pls advise.

Its important and we didn't want to lose track of it until we completely understood the issue, as the asserts are merely a symptom of something deeper. We are converging on a solution, but I'm not sure this warrants a P1 for wasabi.

Fixed directly in tamarin (see bug 607816). Did not modify nanojit.