Last Comment Bug 732847 - IonMonkey: Crash on Heap near [@ js::ObjectImpl::getClass]
: IonMonkey: Crash on Heap near [@ js::ObjectImpl::getClass]
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]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 739901
Blocks: langfuzz IonFuzz
  Show dependency treegraph
 
Reported: 2012-03-04 17:46 PST by Christian Holler (:decoder)
Modified: 2013-01-14 08:15 PST (History)
7 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase for shell (1.01 KB, text/javascript)
2012-03-04 17:46 PST, Christian Holler (:decoder)
no flags Details
Monitor result of interpreted functions. (15.98 KB, patch)
2012-03-26 16:59 PDT, Nicolas B. Pierron [:nbp]
no flags Details | Diff | Splinter Review
Monitor interpreted functions and Guard on compile info. (20.28 KB, patch)
2012-03-26 18:42 PDT, Nicolas B. Pierron [:nbp]
jdemooij: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-03-04 17:46:04 PST
Created attachment 602779 [details]
Testcase for shell

The attached testcase crashes on ionmonkey revision 1fd6c40d3852 (run with --ion -n -m), tested on 64 bit.
Comment 1 Christian Holler (:decoder) 2012-03-04 17:47:29 PST
Crash trace:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7f4090f in ?? ()
(gdb) bt
#0  0x00007ffff7f4090f in ?? ()
#1  0x00007fffffffc4b0 in ?? ()
#2  0xfff9000000000000 in ?? ()
#3  0x00000000004153a4 in js::ObjectImpl::getClass (this=0x2) at ../../vm/ObjectImpl-inl.h:31
#4  0x0000000000761708 in EnterIon (cx=0xcd5d00, fp=0x7ffff0beb1e8, target=..., jitcode=0x7ffff7f406d8, osr=false)
    at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Ion.cpp:941
#5  0x000000000076194a in js::ion::Cannon (cx=0xcd5d00, fp=0x7ffff0beb1e8) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Ion.cpp:970
#6  0x000000000050d28a in js::Interpret (cx=0xcd5d00, entryFrame=0x7ffff0beb030, interpMode=js::JSINTERP_NORMAL)
    at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:2793
Comment 2 Nicolas B. Pierron [:nbp] 2012-03-24 01:29:17 PDT
The following test case highlight the issue with --ion -n:


---
var callStack = [];
function currentFunc() {
    return callStack[0];
}
function reportFailure () {
    var funcName = currentFunc();
    // play with the result to cause a SEGV.
    var prefix = (funcName) ? funcName : "";
    // Use OSR to compile the function at the end of the first run.
    for (var i=0; i < 50; i++) ;
}

callStack[0] = 'test';
// Run and compile with a string as result of currentFunc.
reportFailure();
callStack[0] = undefined;
// Use previously compiled code with the string assumption.
reportFailure();
---



The problem is that Type Inference does not set the monitoredTypesReturn flag to the call of currentFunc.  The output of the types at the moment of the MIR creation looks like:



(gdb) call cx->compartment->types.print(cx, true)
Function #2 _build/bug732847.js (line 7):
locals:
    return: void
    this: void
#2:00000:   8  callgname "currentFunc"
  typeset 0: object[1] <0x7ffff0a10800>
  type 0: object[1] <0x7ffff0a10800>
#2:00005:   8  undefined
  type 0: void
#2:00006:   8  notearg
#2:00007:   8  call 0
  typeset 1: string
  type 0: string
#2:00010:   8  setlocal 0
  type 0: string
#2:00013:   8  pop
#2:00014:  10  getlocal 0
  type 0: string
#2:00017:  10  ifeq 30 (+13)
#2:00022:  10  getlocal 0
  type 0: string
#2:00025:  10  goto 35 (+10)
#2:00030:  10  string ""
  type 0: string
#2:00035:  10  setlocal 1
  type 0: string
#2:00038:  10  pop
#2:00039:  12  zero
  type 0: int
#2:00040:  12  setlocal 2
  type 0: int
#2:00043:  12  pop
#2:00044:  12  goto 54 (+10)
#2:00049:  12  loophead
#2:00050:  12  localinc 2
  type 0: int
#2:00053:  12  pop
#2:00054:  12  loopentry
#2:00055:  12  getlocal 2
  type 0: int
#2:00058:  12  int8 50
  type 0: int
#2:00060:  12  lt
  type 0: bool
#2:00061:  12  ifne 49 (-12)
#2:00066:  12  stop

[0x7ffff0a00040] : (null) unknown {}
int : (null) {}
[0x7ffff0a000a0] : <0x7ffff0a05020> unknown {}
int : <0x7ffff0a05020> {}
[0x7ffff0a00100] : <0x7ffff0a06040> unknown {}
int : <0x7ffff0a06040> {}
<0x7ffff0a03060> : <0x7ffff0a05020> {
    currentFunc: [own] object[1] <0x7ffff0a10800>
}
<0x7ffff0a05020> : (null) {
    currentFunc: missing
}
[0x7ffff0a001c0] : <0x7ffff0a05060> unknown {}
[0x7ffff0a001f0] : <0x7ffff0a05060> packed dense {
    (index): [own] string
}
Counts: 14/0/0/0 (0 over)
Recompilations: 0



This does not change, even after the second execution of currentFunc.



(gdb) call js_DumpScript(cx, (JSScript*)0x7ffff0a07180)
loc   line  op
----- ----  --
main:
00000:   3  getgname "callStack"
00005:   3  zero
00006:   3  getelem
00007:   3  return
00008:   3  stop



IonMonkey does not produce any type barrier because the enalysis at the call-site (#2 @ 7) does not report any need for a monitoredTypesReturn.



(gdb) p ((JSScript * const) 0x7ffff0a07268)->analysis()->getCode((jsbytecode *) 0xd13757)
$11 = (js::analyze::Bytecode &) @0x7ffff7f5deb0: {
  jumpTarget = false,
  fallthrough = true,
  jumpFallthrough = false, 
  switchTarget = false,
  unconditional = true,
  analyzed = true,
  exceptionEntry = false,
  inTryBlock = false,
  inLoop = false, 
  safePoint = false,
  monitoredTypes = false,
  monitoredTypesReturn = false,
  arrayWriteHole = false,
  getStringElement = false, 
  accessGetter = false,
  stackDepth = 2,
  {observedTypes = 0xd15008, loop = 0xd15008},
  allocation = 0x0, 
  poppedValues = 0x7ffff7f5e730,
  pushedUses = 0x7ffff7f5e780,
  {newValues = 0x0, pendingValues = 0x0},
  pushedTypes = 0x7ffff7f5ee40, 
  typeBarriers = 0x0
}
Comment 3 Nicolas B. Pierron [:nbp] 2012-03-24 02:50:00 PDT
The current implementation in JM uses a TypeMonitor on the result if the function call succeeded.  This can be implemented with a MMonitorType which does the same as a TypeBarrier and only create a difference at Bailout time.


// UncachedInlineCall in InvokHelpers.h
    bool ok = !!Interpret(cx, cx->fp());
    f.cx->stack.popInlineFrame(regs);

    if (ok)
        types::TypeScript::Monitor(f.cx, f.script(), f.pc(), args.rval());


(Thanks jandem)
Comment 4 Nicolas B. Pierron [:nbp] 2012-03-26 16:59:30 PDT
Created attachment 609541 [details] [diff] [review]
Monitor result of interpreted functions.

This patch fix the issue reported in this bug when ionmonkey is executed with --ion -n.  It also reveal a bug with --ion -n --ion-eager on the same test-case.

I am will investigate this second issue and discard this patch once I have the --ion-eager fix.
Comment 5 Nicolas B. Pierron [:nbp] 2012-03-26 18:42:57 PDT
Created attachment 609576 [details] [diff] [review]
Monitor interpreted functions and Guard on compile info.

Add monitoring of the returned values of interpreted function and ensure that type compile info are accurate by adding a watchdog.
Comment 6 Jan de Mooij [:jandem] 2012-03-27 06:17:35 PDT
Comment on attachment 609576 [details] [diff] [review]
Monitor interpreted functions and Guard on compile info.

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

::: js/src/ion/TypeOracle.cpp
@@ +419,5 @@
> +    if (script->analysis()->usesScopeChain())
> +        return false;
> +
> +    if (types::TypeSet::HasObjectFlags(cx, target->getType(cx), types::OBJECT_FLAG_UNINLINEABLE))
> +        return false;

Nice catch!
Comment 7 Nicolas B. Pierron [:nbp] 2012-03-29 03:51:33 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/4e9eeb5ae686
Comment 8 Christian Holler (:decoder) 2013-01-14 08:15:40 PST
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug732847.js.

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