Bug 1079826 - OdinMonkey: disallow changing heap inside an interrupt callback (r=bbouvier)

js/src/asmjs/AsmJSModule.cpp

@@ -81,6 +81,7 @@ AsmJSModule::AsmJSModule(ScriptSource *scriptSource, uint32_t srcStart, uint32_t
     dynamicallyLinked_(false),
     loadedFromCache_(false),
     profilingEnabled_(false),
+    interrupted_(false),
     codeIsProtected_(false)
 {
     mozilla::PodZero(&pod);
@@ -95,6 +96,8 @@ AsmJSModule::AsmJSModule(ScriptSource *scriptSource, uint32_t srcStart, uint32_t
 
 AsmJSModule::~AsmJSModule()
 {
+    MOZ_ASSERT(!interrupted_);
+
     scriptSource_->decref();
 
     if (code_) {
@@ -446,8 +449,11 @@ AsmJSReportOverRecursed()
 static bool
 AsmJSHandleExecutionInterrupt()
 {
-    JSContext *cx = PerThreadData::innermostAsmJSActivation()->cx();
-    return HandleExecutionInterrupt(cx);
+    AsmJSActivation *act = PerThreadData::innermostAsmJSActivation();
+    act->module().setInterrupted(true);
+    bool ret = HandleExecutionInterrupt(act->cx());
+    act->module().setInterrupted(false);
+    return ret;
 }
 
 static int32_t
@@ -1546,6 +1552,13 @@ AsmJSModule::clone(JSContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleOut) con
 bool
 AsmJSModule::changeHeap(Handle<ArrayBufferObject*> newBuffer, JSContext *cx)
 {
+    // Content JS should not be able to run (and change heap) from within an
+    // interrupt callback, but in case it does, fail to change heap. Otherwise,
+    // the heap can change at every single instruction which would prevent
+    // future optimizations like heap-base hoisting.
+    if (interrupted_)
+        return false;
+
     uint32_t heapLength = newBuffer->byteLength();
     if (heapLength & pod.heapLengthMask_ || heapLength < pod.minHeapLength_)
         return false;

js/src/asmjs/AsmJSModule.h

@@ -821,6 +821,7 @@ class AsmJSModule
     bool                                  dynamicallyLinked_;
     bool                                  loadedFromCache_;
     bool                                  profilingEnabled_;
+    bool                                  interrupted_;
 
     // This field is accessed concurrently when requesting an interrupt.
     // Access must be synchronized via the runtime's interrupt lock.
@@ -1478,6 +1479,10 @@ class AsmJSModule
         return profilingEnabled_;
     }
     void setProfilingEnabled(bool enabled, JSContext *cx);
+    void setInterrupted(bool interrupted) {
+        MOZ_ASSERT(isDynamicallyLinked());
+        interrupted_ = interrupted;
+    }
 
     // Additionally, these functions may only be called while holding the
     // runtime's interrupt lock.

js/src/jit-test/tests/asm.js/testTimeout7-nosignals.js

@@ -0,0 +1,29 @@
+// |jit-test| exitstatus: 6;
+load(libdir + "asm.js");
+
+// This test may iloop for valid reasons if not compiled with asm.js (namely,
+// inlining may allow the heap load to be hoisted out of the loop).
+if (!isAsmJSCompilationAvailable())
+    quit();
+
+setJitCompilerOption("signals.enable", 0);
+
+var byteLength =
+  Function.prototype.call.bind(Object.getOwnPropertyDescriptor(ArrayBuffer.prototype, 'byteLength').get);
+
+var buf1 = new ArrayBuffer(BUF_CHANGE_MIN);
+new Int32Array(buf1)[0] = 13;
+var buf2 = new ArrayBuffer(BUF_CHANGE_MIN);
+new Int32Array(buf2)[0] = 42;
+
+// Test changeHeap from interrupt (as if that could ever happen...)
+var m = asmCompile('glob', 'ffis', 'b', USE_ASM +
+                   `var I32=glob.Int32Array; var i32=new I32(b);
+                    var len=glob.byteLength;
+                    function changeHeap(b2) { if(len(b2) & 0xffffff || len(b2) <= 0xffffff) return false; i32=new I32(b2); b=b2; return true }
+                    function f() {}
+                    function loop(i) { i=i|0; while((i32[i>>2]|0) == 13) { f() } }
+                    return {loop:loop, changeHeap:changeHeap}`);
+var { loop, changeHeap } = asmLink(m, this, null, buf1);
+timeout(1, function() { assertEq(changeHeap(buf2), false); return false });
+loop(0);

js/src/jit-test/tests/asm.js/testTimeout7.js

@@ -1,3 +1,4 @@
+// |jit-test| exitstatus: 6;
 load(libdir + "asm.js");
 
 // This test may iloop for valid reasons if not compiled with asm.js (namely,
@@ -22,20 +23,5 @@ var m = asmCompile('glob', 'ffis', 'b', USE_ASM +
                     function loop(i) { i=i|0; while((i32[i>>2]|0) == 13) { f() } }
                     return {loop:loop, changeHeap:changeHeap}`);
 var { loop, changeHeap } = asmLink(m, this, null, buf1);
-timeout(1, function() { changeHeap(buf2); return true });
+timeout(1, function() { assertEq(changeHeap(buf2), false); return false });
 loop(0);
-timeout(-1);
-
-// Try again, but this time with signals disabled
-setJitCompilerOption("signals.enable", 0);
-var m = asmCompile('glob', 'ffis', 'b', USE_ASM +
-                   `var I32=glob.Int32Array; var i32=new I32(b);
-                    var len=glob.byteLength;
-                    function changeHeap(b2) { if(len(b2) & 0xffffff || len(b2) <= 0xffffff) return false; i32=new I32(b2); b=b2; return true }
-                    function f() {}
-                    function loop(i) { i=i|0; while((i32[i>>2]|0) == 13) { f() } }
-                    return {loop:loop, changeHeap:changeHeap}`);
-var { loop, changeHeap } = asmLink(m, this, null, buf1);
-timeout(1, function() { changeHeap(buf2); return true });
-loop(0);
-timeout(-1);