Bug 21514: Restore W^X JIT implementation removed from ESR45.

The JIT W^X implemetation in question was removed by https://hg.mozilla.org/releases/mozilla-esr45/rev/347c10e4d6d1 That patch also coalesced the three JIT allocator implementations into a single, cross-platform version. So in this patch I added back the new cross-platform ReprotectRegion function as seen in https://dxr.mozilla.org/mozilla-central/rev/e150eaff1f83e4e4a97d1e30c57d233859efe9cb/js/src/jit/ProcessExecutableMemory.cpp#636 which has equivalent functionality to the old platform-specific "reprotectRegion" functions in three ExecutableAlloctor*.cpp files. I also updated the LIRGenerator::visitInterruptCheck function to match the old behavior when nonWritableJitCode=true. And I updated the ProtectionSettingToFlags functions so they provide the correct flags for the different ProtectionSettings states. This patch appears to restore all behavior controlled by the old nonWritableJitCode flag.
arthuredelstein · Mar 2, 2017 · e2af5f2f127257ff9f792258b19aec5b68bfb421 · e2af5f2
1 parent c01caea
commit e2af5f2f127257ff9f792258b19aec5b68bfb421
Showing with 51 additions and 13 deletions.

+2 −0 js/src/jit/ExecutableAllocator.h

+1 −11 js/src/jit/Lowering.cpp

+46 −2 js/src/jit/ProcessExecutableMemory.cpp

+2 −0 js/src/jit/ProcessExecutableMemory.h
diff --git a/js/src/jit/ExecutableAllocator.h b/js/src/jit/ExecutableAllocator.h
@@ -372,10 +372,12 @@ class ExecutableAllocator
 
     static void makeWritable(void* start, size_t size)
     {
+        ReprotectRegion(start, size, ProtectionSetting::Writable);
     }
 
     static void makeExecutable(void* start, size_t size)
     {
+        ReprotectRegion(start, size, ProtectionSetting::Executable);
     }
 
 #if defined(JS_CODEGEN_X86) || defined(JS_CODEGEN_X64)

diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp
@@ -2304,17 +2304,7 @@ LIRGenerator::visitFunctionEnvironment(MFunctionEnvironment* ins)
 void
 LIRGenerator::visitInterruptCheck(MInterruptCheck* ins)
 {
-    // Implicit interrupt checks require asm.js signal handlers to be installed.
-    // They also require writable JIT code: reprotecting in patchIonBackedges
-    // would be expensive and using AutoWritableJitCode in the signal handler
-    // is complicated because there could be another AutoWritableJitCode on the
-    // stack.
-    LInstructionHelper<0, 0, 0>* lir;
-    if (GetJitContext()->runtime->canUseSignalHandlers()) {
-        lir = new(alloc()) LInterruptCheckImplicit();
-    } else {
-        lir = new(alloc()) LInterruptCheck();
-    }
+    LInstructionHelper<0, 0, 0>* lir = new(alloc()) LInterruptCheck();
     add(lir, ins);
     assignSafepoint(lir, ins);
 }

diff --git a/js/src/jit/ProcessExecutableMemory.cpp b/js/src/jit/ProcessExecutableMemory.cpp
@@ -219,7 +219,12 @@ DeallocateProcessExecutableMemory(void* addr, size_t bytes)
 static DWORD
 ProtectionSettingToFlags(ProtectionSetting protection)
 {
-    return PAGE_EXECUTE_READWRITE;
+    switch (protection) {
+      case ProtectionSetting::Protected:  return PAGE_NOACCESS;
+      case ProtectionSetting::Writable:   return PAGE_READWRITE;
+      case ProtectionSetting::Executable: return PAGE_EXECUTE_READ;
+    }
+    MOZ_CRASH();
 }
 
 static void
@@ -283,7 +288,12 @@ DeallocateProcessExecutableMemory(void* addr, size_t bytes)
 static unsigned
 ProtectionSettingToFlags(ProtectionSetting protection)
 {
-    return PROT_READ | PROT_WRITE | PROT_EXEC;
+    switch (protection) {
+      case ProtectionSetting::Protected:  return PROT_NONE;
+      case ProtectionSetting::Writable:   return PROT_READ | PROT_WRITE;
+      case ProtectionSetting::Executable: return PROT_READ | PROT_EXEC;
+    }
+    MOZ_CRASH();
 }
 
 static void
@@ -605,3 +615,37 @@ js::jit::CanLikelyAllocateMoreExecutableMemory()
 
     return execMemory.bytesAllocated() + BufferSize <= MaxCodeBytesPerProcess;
 }
+
+bool
+js::jit::ReprotectRegion(void* start, size_t size, ProtectionSetting protection)
+{
+    // Calculate the start of the page containing this region,
+    // and account for this extra memory within size.
+    size_t pageSize = gc::SystemPageSize();
+    intptr_t startPtr = reinterpret_cast<intptr_t>(start);
+    intptr_t pageStartPtr = startPtr & ~(pageSize - 1);
+    void* pageStart = reinterpret_cast<void*>(pageStartPtr);
+    size += (startPtr - pageStartPtr);
+
+    // Round size up
+    size += (pageSize - 1);
+    size &= ~(pageSize - 1);
+
+    MOZ_ASSERT((uintptr_t(pageStart) % pageSize) == 0);
+
+    execMemory.assertValidAddress(pageStart, size);
+
+    #ifdef XP_WIN
+    DWORD oldProtect;
+    DWORD flags = ProtectionSettingToFlags(protection);
+    if (!VirtualProtect(pageStart, size, flags, &oldProtect))
+        return false;
+    #else
+    unsigned flags = ProtectionSettingToFlags(protection);
+    if (mprotect(pageStart, size, flags))
+        return false;
+    #endif
+
+    execMemory.assertValidAddress(pageStart, size);
+    return true;
+}
diff --git a/js/src/jit/ProcessExecutableMemory.h b/js/src/jit/ProcessExecutableMemory.h
@@ -23,6 +23,8 @@ enum class ProtectionSetting {
     Executable,
 };
 
+extern MOZ_MUST_USE bool ReprotectRegion(void* start, size_t size, ProtectionSetting protection);
+
 // Functions called at process start-up/shutdown to initialize/release the
 // executable memory region.
 extern MOZ_MUST_USE bool InitProcessExecutableMemory();