From d624501a427e90cf9c6448886cf7f9b47a4b5bc0 Mon Sep 17 00:00:00 2001 From: Luke Wagner Date: Thu, 9 Oct 2014 08:14:25 -0500 Subject: [PATCH] Bug 1079826 - OdinMonkey: disallow changing heap inside an interrupt callback (r=bbouvier) --HG-- extra : rebase_source : 352434536dc3954de8e329bad43880b95ec98405 --- js/src/asmjs/AsmJSModule.cpp | 17 +++++++++-- js/src/asmjs/AsmJSModule.h | 5 ++++ .../tests/asm.js/testTimeout7-nosignals.js | 29 +++++++++++++++++++ js/src/jit-test/tests/asm.js/testTimeout7.js | 20 ++----------- 4 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 js/src/jit-test/tests/asm.js/testTimeout7-nosignals.js diff --git a/js/src/asmjs/AsmJSModule.cpp b/js/src/asmjs/AsmJSModule.cpp index eb6c077a8f1..375aa2c9e68 100644 --- a/js/src/asmjs/AsmJSModule.cpp +++ b/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 *moduleOut) con bool AsmJSModule::changeHeap(Handle 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; diff --git a/js/src/asmjs/AsmJSModule.h b/js/src/asmjs/AsmJSModule.h index e256d416ee4..280b3751b86 100644 --- a/js/src/asmjs/AsmJSModule.h +++ b/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. diff --git a/js/src/jit-test/tests/asm.js/testTimeout7-nosignals.js b/js/src/jit-test/tests/asm.js/testTimeout7-nosignals.js new file mode 100644 index 00000000000..8d460685013 --- /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(6); + +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); diff --git a/js/src/jit-test/tests/asm.js/testTimeout7.js b/js/src/jit-test/tests/asm.js/testTimeout7.js index aa7411def74..ee4df451925 100644 --- a/js/src/jit-test/tests/asm.js/testTimeout7.js +++ b/js/src/jit-test/tests/asm.js/testTimeout7.js @@ -1,9 +1,10 @@ +// |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(); + quit(6); var byteLength = Function.prototype.call.bind(Object.getOwnPropertyDescriptor(ArrayBuffer.prototype, 'byteLength').get); @@ -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);