870007 - Assertion failure: pn, at ion/AsmJS.cpp:1159 or Crash [@ ~ModuleCompiler]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 41ff3b67b692 (run with --ion-eager):


assertEq((function() {
  'use asm';
  function _main() {
    var $1=0, $2=0, $3=0, $4=0, $5=0, $6=0, $7=0, $8=0, $9=0, $10=0, label=0;
      switch (label | 0) {
       case 1:
       case (2147483647):
    }
  }
  return _main;
})()(), 1);

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: proposed fix

This crash would happen as in CheckSwitchRange, if the switch table is larger than 512Mb, fail would be called with stmt, which is NULL here (end condition of the for loop above).

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 747037 [details] [diff] [review]
proposed fix

Nice, thanks!

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: proposed fix compatible with tip

Oops, forgot to pull tip before making the patch.
This version is compatible with tip.

Comment 5

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Same fix with qref

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 870007
User impact if declined: low.
Risk to taking this patch (and alternatives if risky): no risk.

Same patch file as the one reviewed by Luke.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50d9e3c1574f

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/50d9e3c1574f

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1e5d085c9b12

Comment 10

[Paul Silaghi, QA [:pauly]]

Running jsshell-win32 from a build before the fix with the testcase I got:
"Assertion failure: pn, at e:/builds/moz2_slave/m-cen-w32-d-000000000000000000/bu
ild/js/src/ion/AsmJS.cpp:1175"

After the fix (FF 24b5):
"test.js:6:7 warning: asm.js type error: all switch statements generate tables; t
his table would be too big
test.js:11:0 Error: Assertion failed: got (void 0), expected 1"

Is it ok ?

Comment 11

[Christian Holler (:decoder)]

Yes, this is likely correct :)

Comment 12

[Paul Silaghi, QA [:pauly]]

Thanks Christian. Verified based on comment 11

Comment 13

[Paul Silaghi, QA [:pauly]]

Off-topic: Why do I get on Linux "No such file or directory" error when trying to run ./js from jsshell-linux-i686.zip ?