Last Comment Bug 870007 - Assertion failure: pn, at ion/AsmJS.cpp:1159 or Crash [@ ~ModuleCompiler]
: Assertion failure: pn, at ion/AsmJS.cpp:1159 or Crash [@ ~ModuleCompiler]
Status: VERIFIED FIXED
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- critical (vote)
: mozilla24
Assigned To: Benjamin Bouvier [:bbouvier]
: general
Mentors:
Depends on:
Blocks: langfuzz
  Show dependency treegraph
 
Reported: 2013-05-08 10:13 PDT by Christian Holler (:decoder)
Modified: 2013-08-23 07:39 PDT (History)
7 users (show)
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
verified


Attachments
[crash-signature] Machine-readable crash signature (1.00 KB, text/plain)
2013-05-08 10:15 PDT, Christian Holler (:decoder)
no flags Details
proposed fix (1.23 KB, patch)
2013-05-08 11:01 PDT, Benjamin Bouvier [:bbouvier]
luke: review+
Details | Diff | Review
proposed fix compatible with tip (1.21 KB, patch)
2013-05-08 13:33 PDT, Benjamin Bouvier [:bbouvier]
no flags Details | Diff | Review
[crash-signature] Machine-readable crash signature (945 bytes, text/plain)
2013-05-27 06:05 PDT, Christian Holler (:decoder)
no flags Details
Same fix with qref (1.27 KB, patch)
2013-05-27 19:22 PDT, Benjamin Bouvier [:bbouvier]
bbouvier: review+
bajaj.bhavana: approval‑mozilla‑aurora+
Details | Diff | Review

Description Christian Holler (:decoder) 2013-05-08 10:13:38 PDT
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) 2013-05-08 10:15:42 PDT
Created attachment 747014 [details]
[crash-signature] Machine-readable crash signature
Comment 2 Benjamin Bouvier [:bbouvier] 2013-05-08 11:01:16 PDT
Created attachment 747037 [details] [diff] [review]
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] 2013-05-08 11:27:19 PDT
Comment on attachment 747037 [details] [diff] [review]
proposed fix

Nice, thanks!
Comment 4 Benjamin Bouvier [:bbouvier] 2013-05-08 13:33:47 PDT
Created attachment 747098 [details] [diff] [review]
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) 2013-05-27 06:05:15 PDT
Created attachment 754428 [details]
[crash-signature] Machine-readable crash signature
Comment 6 Benjamin Bouvier [:bbouvier] 2013-05-27 19:22:32 PDT
Created attachment 754629 [details] [diff] [review]
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] 2013-05-29 17:40:11 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/50d9e3c1574f
Comment 8 Ryan VanderMeulen [:RyanVM] 2013-05-30 09:10:27 PDT
https://hg.mozilla.org/mozilla-central/rev/50d9e3c1574f
Comment 9 Ryan VanderMeulen [:RyanVM] 2013-06-03 06:11:29 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/1e5d085c9b12
Comment 10 Paul Silaghi, QA [:pauly] 2013-08-23 04:34:59 PDT
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) 2013-08-23 04:49:43 PDT
Yes, this is likely correct :)
Comment 12 Paul Silaghi, QA [:pauly] 2013-08-23 04:55:55 PDT
Thanks Christian. Verified based on comment 11
Comment 13 Paul Silaghi, QA [:pauly] 2013-08-23 07:39:26 PDT
Off-topic: Why do I get on Linux "No such file or directory" error when trying to run ./js from jsshell-linux-i686.zip ?

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