Bug 1079826 - OdinMonkey: disallow changing heap inside an interrupt callback (r=bbouvier)
☠☠ backed out by 7199ca292bee ☠ ☠
authorLuke Wagner <luke@mozilla.com>
Thu, 09 Oct 2014 08:14:25 -0500
changeset 209573 61b3f3d9bffe987e2098c37df5447db3dd883747
parent 209572 d3e3fc74801b9f9fbbc29c971e6a0a3e4334f5b7
child 209574 af8dc8209353ffcdb208225717c645ba1225b6c6
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbbouvier
bugs1079826
milestone35.0a1
Bug 1079826 - OdinMonkey: disallow changing heap inside an interrupt callback (r=bbouvier)
js/src/asmjs/AsmJSModule.cpp
js/src/asmjs/AsmJSModule.h
js/src/jit-test/tests/asm.js/testTimeout7-nosignals.js
js/src/jit-test/tests/asm.js/testTimeout7.js
--- a/js/src/asmjs/AsmJSModule.cpp
+++ b/js/src/asmjs/AsmJSModule.cpp
@@ -76,30 +76,33 @@ AsmJSModule::AsmJSModule(ScriptSource *s
     globalArgumentName_(nullptr),
     importArgumentName_(nullptr),
     bufferArgumentName_(nullptr),
     code_(nullptr),
     interruptExit_(nullptr),
     dynamicallyLinked_(false),
     loadedFromCache_(false),
     profilingEnabled_(false),
+    interrupted_(false),
     codeIsProtected_(false)
 {
     mozilla::PodZero(&pod);
     pod.funcPtrTableAndExitBytes_ = SIZE_MAX;
     pod.functionBytes_ = UINT32_MAX;
     pod.minHeapLength_ = RoundUpToNextValidAsmJSHeapLength(0);
     pod.strict_ = strict;
     pod.usesSignalHandlers_ = canUseSignalHandlers;
 
     scriptSource_->incref();
 }
 
 AsmJSModule::~AsmJSModule()
 {
+    MOZ_ASSERT(!interrupted_);
+
     scriptSource_->decref();
 
     if (code_) {
         for (unsigned i = 0; i < numExits(); i++) {
             AsmJSModule::ExitDatum &exitDatum = exitIndexToGlobalDatum(i);
             if (!exitDatum.ionScript)
                 continue;
 
@@ -441,18 +444,21 @@ AsmJSReportOverRecursed()
 {
     JSContext *cx = PerThreadData::innermostAsmJSActivation()->cx();
     js_ReportOverRecursed(cx);
 }
 
 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
 CoerceInPlace_ToInt32(MutableHandleValue val)
 {
     JSContext *cx = PerThreadData::innermostAsmJSActivation()->cx();
 
     int32_t i32;
@@ -1541,16 +1547,23 @@ AsmJSModule::clone(JSContext *cx, Scoped
 
     out.restoreToInitialState(maybeHeap_, code_, cx);
     return true;
 }
 
 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;
 
     MOZ_ASSERT(IsValidAsmJSHeapLength(heapLength));
     MOZ_ASSERT(!IsDeprecatedAsmJSHeapLength(heapLength));
 
     restoreHeapToInitialState(maybeHeap_);
--- a/js/src/asmjs/AsmJSModule.h
+++ b/js/src/asmjs/AsmJSModule.h
@@ -816,16 +816,17 @@ class AsmJSModule
     PropertyName *                        bufferArgumentName_;
     uint8_t *                             code_;
     uint8_t *                             interruptExit_;
     StaticLinkData                        staticLinkData_;
     HeapPtrArrayBufferObjectMaybeShared   maybeHeap_;
     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.
     mutable bool                          codeIsProtected_;
 
   public:
     explicit AsmJSModule(ScriptSource *scriptSource, uint32_t srcStart, uint32_t srcBodyStart,
                          bool strict, bool canUseSignalHandlers);
@@ -1473,16 +1474,20 @@ class AsmJSModule
         MOZ_ASSERT(isDynamicallyLinked());
         return maybeHeap_ ? maybeHeap_->byteLength() : 0;
     }
     bool profilingEnabled() const {
         MOZ_ASSERT(isDynamicallyLinked());
         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.
     void protectCode(JSRuntime *rt) const;
     void unprotectCode(JSRuntime *rt) const;
     bool codeIsProtected(JSRuntime *rt) const;
 };
 
new file mode 100644
--- /dev/null
+++ b/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);
--- a/js/src/jit-test/tests/asm.js/testTimeout7.js
+++ b/js/src/jit-test/tests/asm.js/testTimeout7.js
@@ -1,8 +1,9 @@
+// |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();
 
 var byteLength =
@@ -17,25 +18,10 @@ new Int32Array(buf2)[0] = 42;
 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 });
+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);