787848 - IonMonkey: Crash [@ ExpressionDecompiler::decompilePC] or "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth),"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

+++ This bug was initially created as a clone of Bug #787309 +++

+++ This bug was initially created as a clone of Bug #781660 +++

try {
    i
}
catch (x if (function() {})()) {}
catch (d) {
    this.z.z
}

=====

try {
  t
} catch (d if true.b) {}
catch (x if new(print)) {}

=====

options('strict')
try {
  w
} catch (e if 2[1]) {}
catch (N) {
  eval.m
}

=====

try {
  e
} catch (e if "") {}
catch (x if (function() {})) {
  new print
}

=====

try {
  x
} catch (b if print()) {}
catch (z) {
  let(a = print()()) {}
}

=====

These testcases assert js debug shell on IonMonkey changeset f9ff9c554d4b without any CLI arguments, even with the patch in bug 781660 comment 10 (which fixes the testcases spun off in bug 787309) applied, at Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth),

options('strict');
try {
  w
} catch (a if (function() {})()) {}
catch (a if ({})) {
  NaN.m
}

crashes js opt shell on IonMonkey changeset f9ff9c554d4b without any CLI arguments at ExpressionDecompiler::decompilePC


These testcases are not fixed by the patches in bug 781660. Also assigning to Nicolas.

Comment 1

[Nicolas B. Pierron [:nbp] {away}]

Attachment: Handle JSOP_THROWING opcode in the decompiler.

If we are not following the JSOP_GOTO, we might evaluate a hidden JSOP_THROWING which expect 2 extra stack slots, as documented in the bytecode emitter code.  Fake the 2 extra stack slots when we do not follow gotos.

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 660435 [details] [diff] [review]
Handle JSOP_THROWING opcode in the decompiler.

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

::: js/src/jsopcode.cpp
@@ +6466,3 @@
>              }
> +
> +            // see BytecodeEmitter.cpp

BytecodeEmitter.cpp is a large file; it would be much nicer to write a nice little paragraph to the effect of your comment 1.  Also, let's not mix // and /* comments in the same function.

Comment 3

[Nicolas B. Pierron [:nbp] {away}]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0401e945059d

Comment 4

[Nicolas B. Pierron [:nbp] {away}]

I did not put all the test cases reported in this bug because they were all having the same problem.

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

This went to Try previously:

https://tbpl.mozilla.org/?tree=Try&rev=b1ba09d0a184

Comment 6

[Ed Morley [:emorley]]

Push backed out for make check failures:
https://tbpl.mozilla.org/php/getParsedLog.php?id=15645062&tree=Mozilla-Inbound

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b903d1d1b861

Comment 7

[Nicolas B. Pierron [:nbp] {away}]

Try server:
https://tbpl.mozilla.org/?tree=Try&rev=2e9b651cf28a

Minor fix to original patch:
https://hg.mozilla.org/integration/mozilla-inbound/rev/57e4febd2775

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/57e4febd2775

Comment 9

[Nicolas B. Pierron [:nbp] {away}]

decoder, how can it be reconfirmed, knowing that the test case is landed and that tbpl is green?

Comment 10

[Christian Holler (:decoder)]

(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #9)
> decoder, how can it be reconfirmed, knowing that the test case is landed and
> that tbpl is green?

I think I marked the wrong bug here, thanks :)

Comment 11

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.