Last Comment Bug 770231 - IonMonkey: Assertion failure: !lastIns_, at ion/MIRGraph.cpp:484
: IonMonkey: Assertion failure: !lastIns_, at ion/MIRGraph.cpp:484
Status: RESOLVED FIXED
[jsbugmon:update][ion:p2:fx18]
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86 Linux
: -- major (vote)
: ---
Assigned To: general
:
Mentors:
Depends on:
Blocks: langfuzz IonFuzz
  Show dependency treegraph
 
Reported: 2012-07-02 09:57 PDT by Christian Holler (:decoder)
Modified: 2012-08-01 13:25 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (865 bytes, patch)
2012-08-01 12:52 PDT, Kannan Vijayan [:djvj]
marty.rosenberg: review+
Details | Diff | Review

Description Christian Holler (:decoder) 2012-07-02 09:57:35 PDT
The following testcase asserts on ionmonkey revision 6688ede89a36 (run with --ion -n -m --ion-eager):


function reportCompare (expected, actual, description) {
  var testcase = new TestCase("unknown-test-name", description, expected, actual);
}
function writeTestCaseResult( expect, actual, string ) {
  var passed = getTestCaseResult( expect, actual );
  if (typeof document != "object" || !document.location.href.match(/jsreftest.html/)) {}
  return passed;
  for ( var i = 0; i < gTestcases.length; i++ ) {}
}
try {
var summary = 'Do not assert with JIT: !TRACE_RECORDER(cx) ^ (jumpTable == recordingJumpTable)';
var actual = 'No Crash';
var expect = 'No Crash';
test();
} catch(exc1) {}
function test() {
  for each ((TestCase) in this);
  reportCompare(expect, actual, summary);
}
var UBound = 0;
var statprefix = 'Status: ';
var statusitems = [ ];
var actualvalues = [ ];
var expectedvalues = [ ];
  expectedvalues[UBound] = expect.toString();
  UBound++;
  for (var i = 0; i < UBound; i++)
    reportCompare(expectedvalues[i], actualvalues[i], getStatus(i));
function getStatus(i) {
  return statprefix + statusitems[i];
}
Comment 1 David Anderson [:dvander] 2012-07-17 17:50:43 PDT
(In reply to Christian Holler (:decoder) from comment #0)
> The following testcase asserts on ionmonkey revision 6688ede89a36 (run with

Christian, I can't reproduce this on the given cset. Does it still happen on tip? If so, could you show the stack?
Comment 2 Christian Holler (:decoder) 2012-07-31 12:13:07 PDT
Stack for b46621aba6fd:

Program received signal SIGSEGV, Segmentation fault.
0x084be0f6 in js::ion::MBasicBlock::add (this=0x86cbec0, ins=0x86cfe88) at js/src/ion/MIRGraph.cpp:455
455         JS_ASSERT(!lastIns_);
Missing separate debuginfos, use: debuginfo-install zlib-1.2.3-27.el6.i686
(gdb) bt
#0  0x084be0f6 in js::ion::MBasicBlock::add (this=0x86cbec0, ins=0x86cfe88) at js/src/ion/MIRGraph.cpp:455
#1  0x0845933a in js::ion::IonBuilder::jsop_call_inline (this=0xffffbe04, callee=..., argc=4, constructing=true, constFun=0x86cbc68, bottom=0x86cbb30, retvalDefns=...)
    at js/src/ion/IonBuilder.cpp:2843
#2  0x08459928 in js::ion::IonBuilder::inlineScriptedCall (this=0xffffbe04, targets=..., argc=4, constructing=true) at js/src/ion/IonBuilder.cpp:2958
#3  0x0845b0a1 in js::ion::IonBuilder::jsop_call_fun_barrier (this=0xffffbe04, targets=..., numTargets=1, argc=4, constructing=true, types=0xf79b0bf0, barrier=0x0)
    at js/src/ion/IonBuilder.cpp:3341
#4  0x0845b238 in js::ion::IonBuilder::jsop_call (this=0xffffbe04, argc=4, constructing=true) at js/src/ion/IonBuilder.cpp:3356
#5  0x08453ee2 in js::ion::IonBuilder::inspectOpcode (this=0xffffbe04, op=JSOP_NEW) at js/src/ion/IonBuilder.cpp:910
#6  0x08452f3b in js::ion::IonBuilder::traverseBytecode (this=0xffffbe04) at js/src/ion/IonBuilder.cpp:645
#7  0x084520d0 in js::ion::IonBuilder::build (this=0xffffbe04) at js/src/ion/IonBuilder.cpp:331
#8  0x084347b0 in js::ion::BuildMIR (builder=..., graph=...) at js/src/ion/Ion.cpp:689
#9  0x08435047 in js::ion::TestCompiler (builder=..., graph=...) at js/src/ion/Ion.cpp:836
#10 0x0843bea5 in js::ion::IonCompile<js::ion::TestCompiler> (cx=0x86beb10, script=0xf7707100, fun=0xf770be40, osrPc=0x0, constructing=false) at js/src/ion/Ion.cpp:873
#11 0x084393f7 in js::ion::Compile<js::ion::TestCompiler> (cx=0x86beb10, script=0xf7707100, fun=0xf770be40, osrPc=0x0, constructing=false) at js/src/ion/Ion.cpp:989
#12 0x084356d2 in js::ion::CanEnter (cx=0x86beb10, script=0xf7707100, fp=0xf79cd088, newType=false) at js/src/ion/Ion.cpp:1079
#13 0x08162abf in js::Interpret (cx=0x86beb10, entryFrame=0xf79cd020, interpMode=js::JSINTERP_NORMAL) at js/src/jsinterp.cpp:2488
Comment 3 Kannan Vijayan [:djvj] 2012-08-01 12:52:55 PDT
Created attachment 648045 [details] [diff] [review]
Patch

Decoder is having a hard time reproing this, but the stack trace he posted points out what's happening in a pretty obvious way.  See attached patch.

Basically, when adding the return-object filter to an inlined function (which takes a return value from a constructor and substitutes it with the |this| if it happens to be a primitive), the code chooses to add it to the same block as the definition which is returned.  However, that block isn't always the exit block for the inlined function.  The code unterminates the exit block so it can add instructions, but doesn't unterminate any other blocks.  So this assert gets raised in situations where an inlined function returns a definition that is defined in an earlier block.

Simple fix: add the filter object definition to the exit block explicitly.
Comment 4 Marty Rosenberg [:mjrosenb] 2012-08-01 12:57:18 PDT
Comment on attachment 648045 [details] [diff] [review]
Patch

Review of attachment 648045 [details] [diff] [review]:
-----------------------------------------------------------------

Just checking, there isn't a reason that we explicitly put it in a block other than the exit block, right?
Comment 5 Kannan Vijayan [:djvj] 2012-08-01 13:25:21 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/e2fb5c449803

Note You need to log in before you can comment on or make changes to this bug.