Bug 1117255 - Work around a Windows bug where exceptions can be reported with the wrong faulting instruction (r=bhackett)
authorLuke Wagner <luke@mozilla.com>
Mon, 05 Jan 2015 18:07:25 -0600
changeset 222195 6a3870adc1deeaf1d6960eca551e6d629524e1bb
parent 222194 9d2bba85e8063e645639ed401228f8e8f9083c68
child 222196 ba352db713862b84a6be32e75b5221416e5666d0
push id28059
push userryanvm@gmail.com
push dateTue, 06 Jan 2015 15:53:01 +0000
treeherdermozilla-central@4d91c33b351c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1117255
milestone37.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1117255 - Work around a Windows bug where exceptions can be reported with the wrong faulting instruction (r=bhackett)
js/src/asmjs/AsmJSSignalHandlers.cpp
js/src/jit-test/tests/asm.js/testBug1117255.js
js/src/vm/Stack.h
--- a/js/src/asmjs/AsmJSSignalHandlers.cpp
+++ b/js/src/asmjs/AsmJSSignalHandlers.cpp
@@ -433,46 +433,62 @@ HandleFault(PEXCEPTION_POINTERS exceptio
     EXCEPTION_RECORD *record = exception->ExceptionRecord;
     CONTEXT *context = exception->ContextRecord;
 
     if (record->ExceptionCode != EXCEPTION_ACCESS_VIOLATION)
         return false;
 
     uint8_t **ppc = ContextToPC(context);
     uint8_t *pc = *ppc;
-    MOZ_ASSERT(pc == record->ExceptionAddress);
 
     if (record->NumberParameters < 2)
         return false;
 
     // Don't allow recursive handling of signals, see AutoSetHandlingSignal.
     JSRuntime *rt = RuntimeForCurrentThread();
     if (!rt || rt->handlingSignal)
         return false;
     AutoSetHandlingSignal handling(rt);
 
     AsmJSActivation *activation = rt->mainThread.asmJSActivationStack();
     if (!activation)
         return false;
 
+# if defined(JS_CODEGEN_X64)
     const AsmJSModule &module = activation->module();
-    if (!module.containsFunctionPC(pc))
-        return false;
 
-# if defined(JS_CODEGEN_X64)
     // These checks aren't necessary, but, since we can, check anyway to make
     // sure we aren't covering up a real bug.
     void *faultingAddress = (void*)record->ExceptionInformation[1];
     if (!module.maybeHeap() ||
         faultingAddress < module.maybeHeap() ||
         faultingAddress >= module.maybeHeap() + AsmJSMappedSize)
     {
         return false;
     }
 
+    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. When this happens,
+        // after ResumeThread, the exception handler is called with pc equal to
+        // module.interruptExit, which is logically wrong. The Right Thing would
+        // be for the OS to make fault-handling atomic (so that CONTEXT.pc was
+        // always the logically-faulting pc). Fortunately, we can detect this
+        // case and silence the exception ourselves (the exception will
+        // retrigger after the interrupt jumps back to resumePC).
+        if (pc == module.interruptExit() &&
+            module.containsFunctionPC(activation->resumePC()) &&
+            module.lookupHeapAccess(activation->resumePC()))
+        {
+            return true;
+        }
+        return false;
+    }
+
     const AsmJSHeapAccess *heapAccess = module.lookupHeapAccess(pc);
     if (!heapAccess)
         return false;
 
     // Also not necessary, but, since we can, do.
     if (heapAccess->isLoad() != !record->ExceptionInformation[0])
         return false;
 
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/asm.js/testBug1117255.js
@@ -0,0 +1,14 @@
+function f(stdlib, foreign, buffer) {
+    "use asm";
+    var i32 = new stdlib.Int32Array(buffer);
+    function g(i) {
+        i=i|0;
+        var j=0;
+        for (; (j>>>0) < 100000; j=(j+1)|0)
+            i32[i>>2] = j;
+    }
+    return g
+}
+var g = f(this, null, new ArrayBuffer(1<<16));
+timeout(.1, function cb() { return true });
+g(1<<16);
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1487,18 +1487,19 @@ class AsmJSActivation : public Activatio
     static unsigned offsetOfContext() { return offsetof(AsmJSActivation, cx_); }
     static unsigned offsetOfResumePC() { return offsetof(AsmJSActivation, resumePC_); }
 
     // Written by JIT code:
     static unsigned offsetOfEntrySP() { return offsetof(AsmJSActivation, entrySP_); }
     static unsigned offsetOfFP() { return offsetof(AsmJSActivation, fp_); }
     static unsigned offsetOfExitReason() { return offsetof(AsmJSActivation, exitReason_); }
 
-    // Set from SIGSEGV handler:
+    // Read/written from SIGSEGV handler:
     void setResumePC(void *pc) { resumePC_ = pc; }
+    void *resumePC() const { return resumePC_; }
 };
 
 // A FrameIter walks over the runtime's stack of JS script activations,
 // abstracting over whether the JS scripts were running in the interpreter or
 // different modes of compiled code.
 //
 // FrameIter is parameterized by what it includes in the stack iteration:
 //  - The SavedOption controls whether FrameIter stops when it finds an