Bug 975182 - OdinMonkey: unprotect code while cloning (r=benj)

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);

js/src/jit/AsmJSModule.cpp

@@ -833,9 +833,40 @@ AsmJSModule::deserialize(ExclusiveContext *cx, const uint8_t *cursor)
     return cursor;
 }
 
-bool
-AsmJSModule::clone(ExclusiveContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleOut) const
+// 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(JSContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleOut) const
+{
+    AutoUnprotectCodeForClone cloneGuard(cx, *this);
+
     *moduleOut = cx->new_<AsmJSModule>(scriptSource_, charsBegin_);
     if (!*moduleOut)
         return false;
@@ -871,6 +902,48 @@ AsmJSModule::clone(ExclusiveContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleO
     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)
 {

js/src/jit/AsmJSModule.h

@@ -416,6 +416,11 @@ class AsmJSModule
 
     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();
@@ -806,7 +811,13 @@ class AsmJSModule
     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.

js/src/jit/AsmJSSignalHandlers.cpp

@@ -458,9 +458,7 @@ HandleException(PEXCEPTION_POINTERS exception)
     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;
     }
 
@@ -645,7 +643,7 @@ HandleMachException(JSRuntime *rt, const ExceptionRequest &request)
 
     const AsmJSModule &module = activation->module();
     if (HandleSimulatorInterrupt(rt, activation, faultingAddress)) {
-        mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC);
+        module.unprotectCode(rt);
         return true;
     }
 
@@ -660,7 +658,7 @@ HandleMachException(JSRuntime *rt, const ExceptionRequest &request)
     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);
@@ -892,7 +890,7 @@ HandleSignal(int signum, siginfo_t *info, void *ctx)
 
     const AsmJSModule &module = activation->module();
     if (HandleSimulatorInterrupt(rt, activation, faultingAddress)) {
-        mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC);
+        module.unprotectCode(rt);
         return true;
     }
 
@@ -907,7 +905,7 @@ HandleSignal(int signum, siginfo_t *info, void *ctx)
     if (module.containsPC(faultingAddress)) {
         activation->setResumePC(pc);
         *ppc = module.operationCallbackExit();
-        mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC);
+        module.unprotectCode(rt);
         return true;
     }
 
@@ -1027,16 +1025,7 @@ js::TriggerOperationCallbackForAsmJSCode(JSRuntime *rt)
     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)