IonMonkey: Crash [@ js::HeapPtr<js::ion::IonCode, unsigned long>::operator js::ion::IonCode*] or [@ checkInvalidation] or [@ js::ion::FrameRecovery::FromIterator]




JavaScript Engine
5 years ago
4 years ago


(Reporter: gkw, Assigned: nbp)


(Blocks: 1 bug, {crash, testcase})

Other Branch
crash, testcase
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)


(crash signature)


(4 attachments, 1 obsolete attachment)



5 years ago
Created attachment 610026 [details]

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++) {

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

5 years ago
Created attachment 610027 [details]
Valgrind stack


5 years ago
Blocks: 732847

Comment 2

5 years ago
We decided to look at it for a few more minutes (again!):

function List(l) {
    this.l = l;
function f(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++)
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.
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.
Attachment #610476 - Flags: review?(dvander)
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.
Attachment #610476 - Flags: review?(dvander) → review+
part 1:
part 2 & 3 (?) still have to be fixed.
Created attachment 610702 [details] [diff] [review]
[part 2] Do not inline uncompilable functions

Check if inline candidates are disabled or not.
Attachment #610702 - Flags: review?(sstangl)
Comment on attachment 610702 [details] [diff] [review]
[part 2] Do not inline uncompilable functions

This patch has moved to its own bug (Bug 740622)
Attachment #610702 - Flags: review?(sstangl)
Version: Trunk → Other Branch
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.
Attachment #610702 - Attachment is obsolete: true
Attachment #611515 - Flags: review?(dvander)
Attachment #611515 - Flags: review?(dvander) → review+
Last Resolved: 5 years ago
Resolution: --- → FIXED
Automatically extracted testcase for this bug was committed:
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.