Last Comment Bug 739901 - IonMonkey: Crash [@ js::HeapPtr<js::ion::IonCode, unsigned long>::operator js::ion::IonCode*] or [@ checkInvalidation] or [@ js::ion::FrameRecovery::FromIterator]
: IonMonkey: Crash [@ js::HeapPtr<js::ion::IonCode, unsigned long>::operator js...
Status: RESOLVED FIXED
: crash, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86_64 Linux
: -- critical (vote)
: ---
Assigned To: Nicolas B. Pierron [:nbp]
:
Mentors:
Depends on:
Blocks: IonFuzz 732847
  Show dependency treegraph
 
Reported: 2012-03-27 23:13 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2013-02-07 05:13 PST (History)
5 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stacks (7.05 KB, text/plain)
2012-03-27 23:13 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
Valgrind stack (38.01 KB, text/plain)
2012-03-27 23:14 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
[part 1] checkInvalidation handle script which are no-longer compilable (1.42 KB, patch)
2012-03-29 00:56 PDT, Nicolas B. Pierron [:nbp]
jdemooij: review+
Details | Diff | Splinter Review
[part 2] Do not inline uncompilable functions (1.80 KB, patch)
2012-03-29 14:46 PDT, Nicolas B. Pierron [:nbp]
no flags Details | Diff | Splinter Review
[part 2] Fix StackIter iteration over empty Ion activations (1.68 KB, patch)
2012-04-02 10:41 PDT, Nicolas B. Pierron [:nbp]
dvander: review+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2012-03-27 23:13:44 PDT
Created attachment 610026 [details]
stacks

function List(l) {
  this.l = l;
}
function f(p) {
  return g(p.l);
};
function g(p) {
  return !(p instanceof List) ? null : f(p.l);
};
list =
  new List(new List(
    new List(new List(
      new List(new List(
        new List(new List(null))))))))
for (let i = 0; i < 99999; i++) {
  g(list);
}

crashes js debug shell on IonMonkey changeset 940078281bbf with --ion and -n at js::HeapPtr<js::ion::IonCode, unsigned long>::operator js::ion::IonCode* and crashes js opt shell at checkInvalidation with js::ion::FrameRecovery::FromIterator on the stack.

Nicolas had the original super-large 5,000+ line testcase and I helped him reduce it.
Comment 1 Gary Kwong [:gkw] [:nth10sd] 2012-03-27 23:14:11 PDT
Created attachment 610027 [details]
Valgrind stack
Comment 2 Gary Kwong [:gkw] [:nth10sd] 2012-03-27 23:37:24 PDT
We decided to look at it for a few more minutes (again!):

function List(l) {
    this.l = l;
}
function f(p) {
    g(p);
};
function g(p) {
    if (p instanceof List) f(p.l);
};
list = new List(new List(new List(new List(null))))
for (let i = 0; i < 2560; i++)
    g(list);
Comment 3 Nicolas B. Pierron [:nbp] 2012-03-29 00:33:29 PDT
What happens:
* We fail to compile g due to the lack of instanceof.
* We compile f.
* We hit recompilecheck bailout in f while having entered f twice before.
* We bailout, js_DumpBacktrace show 3 IonFrames before.
* we finish to bailout and ion frame have disappeared from js_DumpBacktrace output.
* we return to the interpreter where we compile f again.
* we try to inline g, and fail due to instanceof, and flag f as disabled.
* we return from the interpreter, and call the monitor function.
* the monitor function use GetPcScript and recover the bailout ionscript with checkInvalidation which segv.

This bug contains 3 errors:

1/ CheckInvalidation does not support when script->ion is set ION_DISABLED_SCRIPT.

2/ js_DumpBacktrace, which depends on checkInvalidation, is unable to display Ion frames (same as 1 ?)

3/ We should not allow to inline a script which have failed to compiled previouly



The first error fix is needed to land Bug 732847.
Comment 4 Nicolas B. Pierron [:nbp] 2012-03-29 00:56:48 PDT
Created attachment 610476 [details] [diff] [review]
[part 1] checkInvalidation handle script which are no-longer compilable

This patch fix the issue which prevent the landing of bug 732847 fix, which is blocking decoder fuzzing.

It makes sure we don't try to access DISABLE scripts properties when checking for invalidated code.
Comment 5 Jan de Mooij [:jandem] 2012-03-29 01:43:13 PDT
Comment on attachment 610476 [details] [diff] [review]
[part 1] checkInvalidation handle script which are no-longer compilable

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

Looks good to me.
Comment 6 Nicolas B. Pierron [:nbp] 2012-03-29 02:44:29 PDT
part 1: https://hg.mozilla.org/projects/ionmonkey/rev/9d13b69838c2
part 2 & 3 (?) still have to be fixed.
Comment 7 Nicolas B. Pierron [:nbp] 2012-03-29 14:46:22 PDT
Created attachment 610702 [details] [diff] [review]
[part 2] Do not inline uncompilable functions

Check if inline candidates are disabled or not.
Comment 8 Nicolas B. Pierron [:nbp] 2012-03-29 15:59:44 PDT
Comment on attachment 610702 [details] [diff] [review]
[part 2] Do not inline uncompilable functions

This patch has moved to its own bug (Bug 740622)
Comment 9 Nicolas B. Pierron [:nbp] 2012-04-02 10:41:09 PDT
Created attachment 611515 [details] [diff] [review]
[part 2] Fix StackIter iteration over empty Ion activations

DumpBacktrace (from GDB) and dumpStack (from the shell) was not counting the right amount of frame just after the bailout caused by recompilecheck.  The reason was that StackIter did not increment the IonActivation iterator when it encounter an empty activation (Entry | Exit) which is caused by the bailout of the first Ion frame of the activation.

The test case is a modified version of the bug reported here, such as it returns the length of the stack to check for the current bug.
Comment 10 Nicolas B. Pierron [:nbp] 2012-04-02 15:51:17 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/291ff6ed10b5
Comment 11 Christian Holler (:decoder) 2013-02-07 05:13:57 PST
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397

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