Last Comment Bug 729899 - IonMonkey: boxed JSFunction of inlined frames should be stored as a constant value in snapshots. (not live in register)
: IonMonkey: boxed JSFunction of inlined frames should be stored as a constant ...
Status: RESOLVED FIXED
: crash, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86_64 Linux
: -- major (vote)
: ---
Assigned To: Nicolas B. Pierron [:nbp]
:
Mentors:
Depends on: 728045
Blocks: langfuzz IonFuzz
  Show dependency treegraph
 
Reported: 2012-02-23 05:58 PST by Christian Holler (:decoder)
Modified: 2013-01-14 07:38 PST (History)
7 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Mark inlined JSFunction as constant. (2.27 KB, patch)
2012-03-16 15:47 PDT, Nicolas B. Pierron [:nbp]
dvander: review+
Details | Diff | Review

Description Christian Holler (:decoder) 2012-02-23 05:58:24 PST
The following testcase crashes on ionmonkey revision 5a04fd69aa09 (run with --ion -n -m --ion-eager), tested on 64 bit:


var lfcode = new Array();
lfcode.push("function addThis() {}");
lfcode.push("\
var UBound = 0;\
var expectedvalues = [];\
addThis();\
function addThis() {\
  expectedvalues[UBound] = expect;\
  UBound++;\
}\
");
lfcode.push("\
  var expect = 'No Crash';\
  for (var i = 0; i < (2 << 16); i++) addThis();\
");
while (true) {
	var file = lfcode.shift(); if (file == undefined) { break; }
        try { evaluate(file); } catch(lfVare) {}
}
Comment 1 Christian Holler (:decoder) 2012-02-23 05:58:56 PST
Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000007d1d95 in js::ion::MachineState::readReg (this=0x7fffffffad40, reg=...) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/IonFrames.h:275
275             return regs_[reg.code()];
Missing separate debuginfos, use: debuginfo-install libgcc-4.4.6-3.el6.x86_64 libstdc++-4.4.6-3.el6.x86_64
(gdb) bt
#0  0x00000000007d1d95 in js::ion::MachineState::readReg (this=0x7fffffffad40, reg=...) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/IonFrames.h:275
#1  0x00000000007d07b5 in js::ion::SnapshotIterator::fromLocation (this=0x7fffffffacd0, loc=...) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Snapshots.cpp:343
#2  0x00000000007d0a43 in js::ion::SnapshotIterator::slotValue (this=0x7fffffffacd0, slot=...) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Snapshots.cpp:403
#3  0x0000000000793e1b in js::ion::SnapshotIterator::read (this=0x7fffffffacd0) at ../ion/Snapshots.h:278
#4  0x00000000007931a1 in js::ion::GetPcScript (cx=0xcc6db0, scriptRes=0x7fffffffae50, pcRes=0x7fffffffae48)
    at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/IonFrames.cpp:436
#5  0x000000000051ab97 in js::types::TypeScript::GetPcScript (cx=0xcc6db0, script=0x7fffffffae50, pc=0x7fffffffae48) at ../jsinferinlines.h:604
#6  0x00000000004ff322 in js::SetObjectElementOperation (cx=0xcc6db0, obj=0x7ffff0910820, id=..., value=...)
    at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterpinlines.h:805
#7  0x0000000000517e7f in js::SetObjectElement (cx=0xcc6db0, obj=0x7ffff0910820, index=..., value=...) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:4604
#8  0x00007ffff7f423ee in ?? ()
Comment 2 David Anderson [:dvander] 2012-02-24 16:36:27 PST
The bug here is that GetPcScript works by reading safepoint information - it extracts the inlined-most callee out of the stack. But, for out-of-line calls, this could be in a register, and we don't have a MachineRegs available here.

Getting a MachineRegs would not be much work, but bug 728045 will also just fix this, so maybe we should wait for that.
Comment 3 Paul Wright 2012-02-24 16:44:55 PST
That bug is a resolved duplicate of bug 725357, which has been fixed.
Comment 4 Nicolas B. Pierron [:nbp] 2012-03-12 17:15:10 PDT
The following testcase crashes on ionmonkey revision bf6acad353e0 (run with --ion -n --ion-eager), tested on 86/64 bits linux:

function f2() {
    __proto__ = null;
}

for (var j = 0; j < 50; j++)
    f2();


Identical signature.
Comment 5 Nicolas B. Pierron [:nbp] 2012-03-16 11:04:30 PDT
The problem here comes from the GetPcScript (now InlineFrameReverseIterator::operator++) mechanism used to recover the script of the next inlined frame.

The problem here is that the boxed JSFunction pointer of the inlined frame (which is known at compile time) is still live in a register.  As we know it at compile time, the JSFunction pointer should be a constant value which is part of the snapshot, and not live in a register.
Comment 6 Nicolas B. Pierron [:nbp] 2012-03-16 15:47:18 PDT
Created attachment 606769 [details] [diff] [review]
Mark inlined JSFunction as constant.

Fix and add the 2 test cases reported here.
Comment 7 Nicolas B. Pierron [:nbp] 2012-03-21 17:51:54 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/150159ee5c26
Comment 8 Nicolas B. Pierron [:nbp] 2012-03-21 18:12:33 PDT
Reverted.
Comment 9 Nicolas B. Pierron [:nbp] 2012-03-23 02:13:06 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/fed610aff637
Comment 10 Christian Holler (:decoder) 2013-01-14 07:38:37 PST
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug729899-1.js.

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