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

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

Attachment: stack obtained during reproduction/reduction

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?

Comment 2

[Luke Wagner [:luke]]

Attachment: fix and test

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.

Comment 3

[Luke Wagner [:luke]]

Not s-s since this is a safe-but-spurious crash.

Comment 4

[Brian Hackett [Laid off!]]

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

Comment 5

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6a3870adc1de

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6a3870adc1de