Bug 975182 - OdinMonkey: unprotect code while cloning (r=benj)
authorLuke Wagner <luke@mozilla.com>
Mon, 24 Feb 2014 12:20:04 -0600
changeset 170564 4e7eba19b5731b5d18cb2618719e6e966df7c7da
parent 170563 5609571991994bbc2c55e5430b53b23378b87def
child 170565 2945a260ef0005e2d17142fbeb71919835955106
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersbenj
bugs975182
milestone30.0a1
Bug 975182 - OdinMonkey: unprotect code while cloning (r=benj)
js/src/jit-test/tests/asm.js/testBug975182.js
js/src/jit/AsmJSModule.cpp
js/src/jit/AsmJSModule.h
js/src/jit/AsmJSSignalHandlers.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/asm.js/testBug975182.js
@@ -0,0 +1,18 @@
+Function("\
+    g = (function(t,foreign){\
+        \"use asm\";\
+        var ff = foreign.ff;\
+        function f() {\
+            +ff()\
+        }\
+        return f\
+    })(this, {\
+        ff: arguments.callee\
+    }, ArrayBuffer(4096))\
+")()
+function m(f) {
+    for (var j = 0; j < 6000; ++j) {
+        f();
+    }
+}
+m(g);
--- a/js/src/jit/AsmJSModule.cpp
+++ b/js/src/jit/AsmJSModule.cpp
@@ -828,19 +828,50 @@ AsmJSModule::deserialize(ExclusiveContex
     (cursor = DeserializeVector(cx, cursor, &exports_)) &&
     (cursor = DeserializePodVector(cx, cursor, &heapAccesses_)) &&
     (cursor = staticLinkData_.deserialize(cx, cursor));
 
     loadedFromCache_ = true;
     return cursor;
 }
 
+// When a module is cloned, we memcpy its executable code. If, right before or
+// during the clone, another thread calls AsmJSModule::protectCode() then the
+// executable code will become inaccessible. In theory, we could take away only
+// PROT_EXEC, but this seems to break emulators.
+class AutoUnprotectCodeForClone
+{
+    JSRuntime *rt_;
+    JSRuntime::AutoLockForOperationCallback lock_;
+    const AsmJSModule &module_;
+    const bool protectedBefore_;
+
+  public:
+    AutoUnprotectCodeForClone(JSContext *cx, const AsmJSModule &module)
+      : rt_(cx->runtime()),
+        lock_(rt_),
+        module_(module),
+        protectedBefore_(module_.codeIsProtected(rt_))
+    {
+        if (protectedBefore_)
+            module_.unprotectCode(rt_);
+    }
+
+    ~AutoUnprotectCodeForClone()
+    {
+        if (protectedBefore_)
+            module_.protectCode(rt_);
+    }
+};
+
 bool
-AsmJSModule::clone(ExclusiveContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleOut) const
+AsmJSModule::clone(JSContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleOut) const
 {
+    AutoUnprotectCodeForClone cloneGuard(cx, *this);
+
     *moduleOut = cx->new_<AsmJSModule>(scriptSource_, charsBegin_);
     if (!*moduleOut)
         return false;
 
     AsmJSModule &out = **moduleOut;
 
     // Mirror the order of serialize/deserialize in cloning:
 
@@ -866,16 +897,58 @@ AsmJSModule::clone(ExclusiveContext *cx,
     }
 
     out.loadedFromCache_ = loadedFromCache_;
 
     out.restoreToInitialState(maybeHeap_, cx);
     return true;
 }
 
+void
+AsmJSModule::protectCode(JSRuntime *rt) const
+{
+    JS_ASSERT(rt->currentThreadOwnsOperationCallbackLock());
+
+    // Technically, we should be able to only take away the execute permissions,
+    // however this seems to break our emulators which don't always check
+    // execute permissions while executing code.
+#if defined(XP_WIN)
+    DWORD oldProtect;
+    if (!VirtualProtect(codeBase(), functionBytes(), PAGE_NOACCESS, &oldProtect))
+        MOZ_CRASH();
+#else  // assume Unix
+    if (mprotect(codeBase(), functionBytes(), PROT_NONE))
+        MOZ_CRASH();
+#endif
+
+    codeIsProtected_ = true;
+}
+
+void
+AsmJSModule::unprotectCode(JSRuntime *rt) const
+{
+#if defined(XP_WIN)
+    DWORD oldProtect;
+    if (!VirtualProtect(codeBase(), functionBytes(), PAGE_EXECUTE_READWRITE, &oldProtect))
+        MOZ_CRASH();
+#else  // assume Unix
+    if (mprotect(codeBase(), functionBytes(), PROT_READ | PROT_WRITE | PROT_EXEC))
+        MOZ_CRASH();
+#endif
+
+    codeIsProtected_ = false;
+}
+
+bool
+AsmJSModule::codeIsProtected(JSRuntime *rt) const
+{
+    JS_ASSERT(rt->currentThreadOwnsOperationCallbackLock());
+    return codeIsProtected_;
+}
+
 static bool
 GetCPUID(uint32_t *cpuId)
 {
     enum Arch {
         X86 = 0x1,
         X64 = 0x2,
         ARM = 0x3,
         ARCH_BITS = 2
--- a/js/src/jit/AsmJSModule.h
+++ b/js/src/jit/AsmJSModule.h
@@ -411,16 +411,21 @@ class AsmJSModule
     bool                                  loadedFromCache_;
     HeapPtr<ArrayBufferObject>            maybeHeap_;
 
     uint32_t                              charsBegin_;
     ScriptSource *                        scriptSource_;
 
     FunctionCountsVector                  functionCounts_;
 
+    // This field is accessed concurrently when triggering the operation
+    // callback and access must be sychronized via the runtime's operation
+    // callback lock.
+    mutable bool                          codeIsProtected_;
+
   public:
     explicit AsmJSModule(ScriptSource *scriptSource, uint32_t charsBegin);
     ~AsmJSModule();
 
     void trace(JSTracer *trc) {
         for (unsigned i = 0; i < globals_.length(); i++)
             globals_[i].trace(trc);
         for (unsigned i = 0; i < exports_.length(); i++)
@@ -801,17 +806,23 @@ class AsmJSModule
     void addSizeOfMisc(mozilla::MallocSizeOf mallocSizeOf, size_t *asmJSModuleCode,
                        size_t *asmJSModuleData);
 
     size_t serializedSize() const;
     uint8_t *serialize(uint8_t *cursor) const;
     const uint8_t *deserialize(ExclusiveContext *cx, const uint8_t *cursor);
     bool loadedFromCache() const { return loadedFromCache_; }
 
-    bool clone(ExclusiveContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleOut) const;
+    bool clone(JSContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleOut) const;
+
+    // These methods may only be called while holding the Runtime's operation
+    // callback lock.
+    void protectCode(JSRuntime *rt) const;
+    void unprotectCode(JSRuntime *rt) const;
+    bool codeIsProtected(JSRuntime *rt) const;
 };
 
 // Store the just-parsed module in the cache using AsmJSCacheOps.
 extern bool
 StoreAsmJSModuleInCache(AsmJSParser &parser,
                         const AsmJSModule &module,
                         ExclusiveContext *cx);
 
--- a/js/src/jit/AsmJSSignalHandlers.cpp
+++ b/js/src/jit/AsmJSSignalHandlers.cpp
@@ -453,19 +453,17 @@ HandleException(PEXCEPTION_POINTERS exce
     // If we faulted trying to execute code in 'module', this must be an
     // operation callback (see TriggerOperationCallbackForAsmJSCode). Redirect
     // execution to a trampoline which will call js_HandleExecutionInterrupt.
     // The trampoline will jump to activation->resumePC if execution isn't
     // interrupted.
     if (module.containsPC(faultingAddress)) {
         activation->setResumePC(pc);
         *ppc = module.operationCallbackExit();
-        DWORD oldProtect;
-        if (!VirtualProtect(module.codeBase(), module.functionBytes(), PAGE_EXECUTE, &oldProtect))
-            MOZ_CRASH();
+        module.unprotectCode(rt);
         return true;
     }
 
 # 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.
     if (!module.maybeHeap() ||
         faultingAddress < module.maybeHeap() ||
@@ -640,32 +638,32 @@ HandleMachException(JSRuntime *rt, const
         return true;
 
     AsmJSActivation *activation = rt->mainThread.asmJSActivationStackFromAnyThread();
     if (!activation)
         return false;
 
     const AsmJSModule &module = activation->module();
     if (HandleSimulatorInterrupt(rt, activation, faultingAddress)) {
-        mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC);
+        module.unprotectCode(rt);
         return true;
     }
 
     if (!module.containsPC(pc))
         return false;
 
     // If we faulted trying to execute code in 'module', this must be an
     // operation callback (see TriggerOperationCallbackForAsmJSCode). Redirect
     // execution to a trampoline which will call js_HandleExecutionInterrupt.
     // The trampoline will jump to activation->resumePC if execution isn't
     // interrupted.
     if (module.containsPC(faultingAddress)) {
         activation->setResumePC(pc);
         *ppc = module.operationCallbackExit();
-        mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC);
+        module.unprotectCode(rt);
 
         // Update the thread state with the new pc.
         kret = thread_set_state(rtThread, x86_THREAD_STATE, (thread_state_t)&state, x86_THREAD_STATE_COUNT);
         return kret == KERN_SUCCESS;
     }
 
 # if defined(JS_CODEGEN_X64)
     // These checks aren't necessary, but, since we can, check anyway to make
@@ -887,32 +885,32 @@ HandleSignal(int signum, siginfo_t *info
         return true;
 
     AsmJSActivation *activation = InnermostAsmJSActivation();
     if (!activation)
         return false;
 
     const AsmJSModule &module = activation->module();
     if (HandleSimulatorInterrupt(rt, activation, faultingAddress)) {
-        mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC);
+        module.unprotectCode(rt);
         return true;
     }
 
     if (!module.containsPC(pc))
         return false;
 
     // If we faulted trying to execute code in 'module', this must be an
     // operation callback (see TriggerOperationCallbackForAsmJSCode). Redirect
     // execution to a trampoline which will call js_HandleExecutionInterrupt.
     // The trampoline will jump to activation->resumePC if execution isn't
     // interrupted.
     if (module.containsPC(faultingAddress)) {
         activation->setResumePC(pc);
         *ppc = module.operationCallbackExit();
-        mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC);
+        module.unprotectCode(rt);
         return true;
     }
 
 # 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.
     if (!module.maybeHeap() ||
         faultingAddress < module.maybeHeap() ||
@@ -1022,26 +1020,17 @@ void
 js::TriggerOperationCallbackForAsmJSCode(JSRuntime *rt)
 {
     JS_ASSERT(rt->currentThreadOwnsOperationCallbackLock());
 
     AsmJSActivation *activation = rt->mainThread.asmJSActivationStackFromAnyThread();
     if (!activation)
         return;
 
-    const AsmJSModule &module = activation->module();
-
-#if defined(XP_WIN)
-    DWORD oldProtect;
-    if (!VirtualProtect(module.codeBase(), module.functionBytes(), PAGE_NOACCESS, &oldProtect))
-        MOZ_CRASH();
-#else  // assume Unix
-    if (mprotect(module.codeBase(), module.functionBytes(), PROT_NONE))
-        MOZ_CRASH();
-#endif
+    activation->module().protectCode(rt);
 }
 
 #if defined(MOZ_ASAN) && defined(JS_STANDALONE)
 // Usually, this definition is found in mozglue (see mozglue/build/AsanOptions.cpp).
 // However, when doing standalone JS builds, mozglue is not used and we must ensure
 // that we still allow custom SIGSEGV handlers for asm.js and ion to work correctly.
 extern "C" MOZ_ASAN_BLACKLIST
 const char* __asan_default_options() {