JSIL game "Soulcaster" fails to parse XML level description

VERIFIED FIXED in Firefox 27

Status

()

defect
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: cpeterson, Assigned: djvj)

Tracking

({regression})

unspecified
mozilla28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox26 unaffected, firefox27+ verified, firefox28 verified)

Details

()

Attachments

(1 attachment)

STR:
1. Load the JSIL game "Soulcaster": http://www.newgrounds.com/portal/view/606067
2. Select "New Game"
3. Select "Normal"

RESULT:
The game screen goes black and the game level does not load. The following JS error is logged:

JavaScript error: http://uploads.ungrounded.net/alternate/623000/623433_alternate_372.zip/17/Libraries/JSIL.XML.js, line 112: Error: Failed to parse XML document

This bug is a regression in Nightly 27 build 2013-10-01. I bisected the mozilla-inbound builds archived on TBPL and identified bug 921120 as the regression:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4184c8516ee5&tochange=2963a336e7ec

This XML error is thrown from JSIL's JS code here:

JSIL.XML.ReaderFromString = function (xml) {
    var parser = new DOMParser;
    var root = parser.parseFromString(xml, 'application/xml');
    if (root === null || root.documentElement.localName == 'parsererror') {
        throw new Error('Failed to parse XML document')
    }
    ...
"Bytown Lumberjack", another JSIL-compiled game, also fails to load a new game starting in Nightly 27 build 2013-10-01 with the following error message:

  Unhandled exception at line 0:
  uncaught exception: System.ArgumentOutOfRangeException: index

http://hildr.luminance.org/Lumberjack/Lumberjack.html
Component: JavaScript Engine → JavaScript Engine: JIT
Flags: needinfo?(kvijayan)
Looking at it.
Flags: needinfo?(kvijayan)
Assignee: nobody → kvijayan
Is there a known way to get a local install of Bytown Lumberjack so I can start mutating the JS to narrow down the issue.

I think the lumberjack game is a better candidate for tracking this issue than Soulcaster.

However the thing loads a bunch of static and dynamic files, and I tried a crude attempt at replicating the repository locally and failed.

I can't use the debugger on this since it's an Ion issue and Ion doesn't compile when the debugger turns on :)  So with debugging on the thing works perfectly (if slowly).

The other option is just for me to start instrumenting the JS engine directly and running it on the remote code, trying to identify where exceptions are thrown.
See comment 3.  Kael: Is there an easy way to get a local copy of Bytown Lumberjack to debug this issue?
Flags: needinfo?(kg)
I sent a build to your bugmail, Kannan.
Flags: needinfo?(kg)
Found the issue.  In strict mode, functions which have JSOP_SETARG are forced to create an ArgumentsObject to preserve the original arguments that were passed in.

When OSR-ing into functions with ArgumentsObjects present, we unconditionally took the arguments from the ArgsObj.  However, in strict mode, the actual "current" formals aren't present in the argsobj, but on the stack frame.  They should be retrieved from the stack frame instead.

The fix is a one-liner (and some commentary).
Attachment #827453 - Flags: review?(hv1989)
Comment on attachment 827453 [details] [diff] [review]
fix-ion-argsobj-strictmode-osr.patch

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

Good catch, but I definitely want a testcase for this in the tree.

I quickly looked to all other cases where we test needsArgsObj in IM and didn't found the same issue appearing there. So I think this is the only place it was forgotten.
Attachment #827453 - Flags: review?(hv1989) → review+
https://hg.mozilla.org/mozilla-central/rev/c904637b6e59
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Comment on attachment 827453 [details] [diff] [review]
fix-ion-argsobj-strictmode-osr.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
Bug 921120
 
User impact if declined: 
Incorrect behavior of strict-mode code that assigns to formal arguments, and contains loops.  This bug break JSIL and causes JSIL-run games not to work.

Testing completed (on m-c, etc.): 
Green for 4 days on m-c.

Risk to taking this patch (and alternatives if risky):
Very low.
 
String or IDL/UUID changes made by this patch:
N/A
Attachment #827453 - Flags: approval-mozilla-aurora?
Attachment #827453 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: verifyme
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0

Verified as fixed on latest Aurora 27.0a2 (buildID: 20131202004002).
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0

Verified as fixed on latest Aurora 28.0a2 (buildID: 20131212004008).
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.