Closed Bug 1117255 Opened 5 years ago Closed 5 years ago

Crash involving testcase with asm.js, does not occur with --no-asmjs

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: gkw, Assigned: luke)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase)

Attachments

(2 files)

The upcoming testcase asserts js debug shell on m-c changeset 13fe5ad0364d with  --fuzzing-safe --no-threads --no-baseline --no-ion

The shell was obtained from:

https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-02-mozilla-central-debug/

in particular:

https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-02-mozilla-central-debug/jsshell-win64-x86_64.zip

Setting s-s as a start. I cannot get a stack from the FTP builds as the builds from FTP do not have symbols, so I attached a stack from a build that initially spotted the issue. Further reduced testcases are extremely intermittent, even this one is fairly intermittent. (1 crash every 5-20 tries)

This seems to only happen on Windows 64-bit.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/7db30249d1d8
user:        Luke Wagner
date:        Tue Nov 11 08:36:52 2014 -0600
summary:     Bug 1091912 - stop using mprotect to halt Ion/asm.js execution (r=bhackett)

Luke, is bug 1091912 a likely regressor?
Flags: needinfo?(luke)
Attached patch fix and testSplinter Review
It looks like, between an instruction faulting and user-space handling the fault, SuspendThread can change pc to the interrupt stub and when user-space handling starts (after ResumeThread), the CONTEXT.pc AND EXCEPTION_RECORD.FaultingAddress are the updated pc, not the faulting pc.  This seems like a bug in Window's error handling (CONTEXT.pc is understandable but one would expect FaultingAddress to be correct).  Fortunately, this case is unique and easily detectable.
Assignee: nobody → luke
Status: NEW → ASSIGNED
Flags: needinfo?(luke)
Attachment #8544117 - Flags: review?(bhackett1024)
Not s-s since this is a safe-but-spurious crash.
Group: core-security
Comment on attachment 8544117 [details] [diff] [review]
fix and test

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

::: js/src/asmjs/AsmJSSignalHandlers.cpp
@@ +468,5 @@
> +    if (!module.containsFunctionPC(pc)) {
> +        // On Windows, it is possible for InterruptRunningCode to execute
> +        // between a faulting heap access and the handling of the fault due
> +        // to InterruptRunningCode's use of SuspendThread. In this case, when
> +        // the time the exception is handled, pc will be module.interruptExit

a bit garbled
Attachment #8544117 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/6a3870adc1de
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.