Bug 1317033 - Baldr: hoist mprotect out of MacroAssembler::patchCall/FarJump (r=sunfish)
authorLuke Wagner <luke@mozilla.com>
Sun, 13 Nov 2016 13:21:49 -0600
changeset 349066 71825cbd0e2549d813d8279dcdb19fb357f8ab3f
parent 349065 8c158c99496d1f9dd0d7d2c434429b1831db3155
child 349067 1196bf3032e1bce1fb07a01fd9082a767426c5fb
child 350469 5d9a785a37c4cb6468ef5b3b610b8b84c4a337b7
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssunfish
bugs1317033
milestone52.0a1
Bug 1317033 - Baldr: hoist mprotect out of MacroAssembler::patchCall/FarJump (r=sunfish) MozReview-Commit-ID: Nejpro1fxc
js/src/ds/PageProtectingVector.h
js/src/jit/arm/MacroAssembler-arm.h
js/src/jit/arm64/MacroAssembler-arm64.h
js/src/jit/mips-shared/MacroAssembler-mips-shared.h
js/src/jit/x86-shared/Assembler-x86-shared.h
js/src/wasm/WasmGenerator.cpp
js/src/wasm/WasmGenerator.h
--- a/js/src/ds/PageProtectingVector.h
+++ b/js/src/ds/PageProtectingVector.h
@@ -73,30 +73,28 @@ class PageProtectingVector final
         unprotectedBytes += offsetToPage;
         offsetToPage = (pageSize - (uintptr_t(vector.begin()) & pageMask)) & pageMask;
         unprotectedBytes -= offsetToPage;
         protectionEnabled = vector.capacity() >= protectionLowerBound &&
                             vector.capacity() >= pageSize + offsetToPage;
     }
 
     void protect() {
-        MOZ_ASSERT(!regionUnprotected);
-        if (protectionEnabled && unprotectedBytes >= intptr_t(pageSize)) {
+        if (!regionUnprotected && protectionEnabled && unprotectedBytes >= intptr_t(pageSize)) {
             size_t toProtect = size_t(unprotectedBytes) & ~pageMask;
             uintptr_t addr = uintptr_t(vector.begin()) + offsetToPage + protectedBytes;
             gc::MakePagesReadOnly(reinterpret_cast<void*>(addr), toProtect);
             unprotectedBytes -= toProtect;
             protectedBytes += toProtect;
         }
     }
 
     void unprotect() {
-        MOZ_ASSERT(!regionUnprotected);
         MOZ_ASSERT_IF(!protectionEnabled, !protectedBytes);
-        if (protectedBytes) {
+        if (!regionUnprotected && protectedBytes) {
             uintptr_t addr = uintptr_t(vector.begin()) + offsetToPage;
             gc::UnprotectPages(reinterpret_cast<void*>(addr), protectedBytes);
             unprotectedBytes += protectedBytes;
             protectedBytes = 0;
         }
     }
 
     void protectNewBuffer() {
--- a/js/src/jit/arm/MacroAssembler-arm.h
+++ b/js/src/jit/arm/MacroAssembler-arm.h
@@ -1531,16 +1531,20 @@ class MacroAssemblerARMCompat : public M
         ma_ldr(Address(WasmTlsReg, offsetof(wasm::TlsData, memoryBase)), HeapReg, scratch);
         ma_ldr(Address(WasmTlsReg, offsetof(wasm::TlsData, globalData)), GlobalReg, scratch);
         ma_add(Imm32(WasmGlobalRegBias), GlobalReg, scratch);
     }
 
     // Instrumentation for entering and leaving the profiler.
     void profilerEnterFrame(Register framePtr, Register scratch);
     void profilerExitFrame();
+
+    struct AutoPrepareForPatching {
+        explicit AutoPrepareForPatching(MacroAssemblerARMCompat&) {}
+    };
 };
 
 typedef MacroAssemblerARMCompat MacroAssemblerSpecific;
 
 } // namespace jit
 } // namespace js
 
 #endif /* jit_arm_MacroAssembler_arm_h */
--- a/js/src/jit/arm64/MacroAssembler-arm64.h
+++ b/js/src/jit/arm64/MacroAssembler-arm64.h
@@ -2311,16 +2311,20 @@ class MacroAssemblerCompat : public vixl
     }
 
     // FIXME: Should be in Assembler?
     // FIXME: Should be const?
     uint32_t currentOffset() const {
         return nextOffset().getOffset();
     }
 
+    struct AutoPrepareForPatching {
+        explicit AutoPrepareForPatching(MacroAssemblerCompat&) {}
+    };
+
   protected:
     bool buildOOLFakeExitFrame(void* fakeReturnAddr) {
         uint32_t descriptor = MakeFrameDescriptor(framePushed(), JitFrame_IonJS,
                                                   ExitFrameLayout::Size());
         Push(Imm32(descriptor));
         Push(ImmPtr(fakeReturnAddr));
         return true;
     }
--- a/js/src/jit/mips-shared/MacroAssembler-mips-shared.h
+++ b/js/src/jit/mips-shared/MacroAssembler-mips-shared.h
@@ -244,14 +244,19 @@ class MacroAssemblerMIPSShared : public 
                          Register output);
 
     void atomicExchange(int nbytes, bool signExtend, const Address& address, Register value,
                         Register valueTemp, Register offsetTemp, Register maskTemp,
                         Register output);
     void atomicExchange(int nbytes, bool signExtend, const BaseIndex& address, Register value,
                         Register valueTemp, Register offsetTemp, Register maskTemp,
                         Register output);
+
+  public:
+    struct AutoPrepareForPatching {
+        explicit AutoPrepareForPatching(MacroAssemblerMIPSShared&) {}
+    };
 };
 
 } // namespace jit
 } // namespace js
 
 #endif /* jit_mips_shared_MacroAssembler_mips_shared_h */
--- a/js/src/jit/x86-shared/Assembler-x86-shared.h
+++ b/js/src/jit/x86-shared/Assembler-x86-shared.h
@@ -1052,27 +1052,34 @@ class AssemblerX86Shared : public Assemb
           default:
             MOZ_CRASH("unexpected operand kind");
         }
     }
 
     CodeOffset callWithPatch() {
         return CodeOffset(masm.call().offset());
     }
+
+    struct AutoPrepareForPatching : X86Encoding::AutoUnprotectAssemblerBufferRegion {
+        explicit AutoPrepareForPatching(AssemblerX86Shared& masm)
+          : X86Encoding::AutoUnprotectAssemblerBufferRegion(masm.masm, 0, masm.size())
+        {}
+    };
+
     void patchCall(uint32_t callerOffset, uint32_t calleeOffset) {
+        // The caller uses AutoUnprotectBuffer.
         unsigned char* code = masm.data();
-        X86Encoding::AutoUnprotectAssemblerBufferRegion unprotect(masm, callerOffset - 4, 4);
         X86Encoding::SetRel32(code + callerOffset, code + calleeOffset);
     }
     CodeOffset farJumpWithPatch() {
         return CodeOffset(masm.jmp().offset());
     }
     void patchFarJump(CodeOffset farJump, uint32_t targetOffset) {
+        // The caller uses AutoUnprotectBuffer.
         unsigned char* code = masm.data();
-        X86Encoding::AutoUnprotectAssemblerBufferRegion unprotect(masm, farJump.offset() - 4, 4);
         X86Encoding::SetRel32(code + farJump.offset(), code + targetOffset);
     }
     static void repatchFarJump(uint8_t* code, uint32_t farJumpOffset, uint32_t targetOffset) {
         X86Encoding::SetRel32(code + farJumpOffset, code + targetOffset);
     }
 
     CodeOffset twoByteNop() {
         return CodeOffset(masm.twoByteNop().offset());
--- a/js/src/wasm/WasmGenerator.cpp
+++ b/js/src/wasm/WasmGenerator.cpp
@@ -245,16 +245,18 @@ JumpRange()
     return Min(JitOptions.jumpThreshold, JumpImmediateRange);
 }
 
 typedef HashMap<uint32_t, uint32_t, DefaultHasher<uint32_t>, SystemAllocPolicy> OffsetMap;
 
 bool
 ModuleGenerator::patchCallSites(TrapExitOffsetArray* maybeTrapExits)
 {
+    MacroAssembler::AutoPrepareForPatching patching(masm_);
+
     masm_.haltingAlign(CodeAlignment);
 
     // Create far jumps for calls that have relative offsets that may otherwise
     // go out of range. Far jumps are created for two cases: direct calls
     // between function definitions and calls to trap exits by trap out-of-line
     // paths. Far jump code is shared when possible to reduce bloat. This method
     // is called both between function bodies (at a frequency determined by the
     // ISA's jump range) and once at the very end of a module's codegen after
@@ -338,16 +340,34 @@ ModuleGenerator::patchCallSites(TrapExit
           }
         }
     }
 
     return true;
 }
 
 bool
+ModuleGenerator::patchFarJumps(const TrapExitOffsetArray& trapExits)
+{
+    MacroAssembler::AutoPrepareForPatching patching(masm_);
+
+    for (CallThunk& callThunk : metadata_->callThunks) {
+        uint32_t funcIndex = callThunk.u.funcIndex;
+        callThunk.u.codeRangeIndex = funcToCodeRange_[funcIndex];
+        CodeOffset farJump(callThunk.offset);
+        masm_.patchFarJump(farJump, funcCodeRange(funcIndex).funcNonProfilingEntry());
+    }
+
+    for (const TrapFarJump& farJump : masm_.trapFarJumps())
+        masm_.patchFarJump(farJump.jump, trapExits[farJump.trap].begin);
+
+    return true;
+}
+
+bool
 ModuleGenerator::finishTask(IonCompileTask* task)
 {
     const FuncBytes& func = task->func();
     FuncCompileResults& results = task->results();
 
     masm_.haltingAlign(CodeAlignment);
 
     // Before merging in the new function's code, if calls in a prior function
@@ -525,32 +545,25 @@ ModuleGenerator::finishCodegen()
     if (!metadata_->codeRanges.emplaceBack(CodeRange::Inline, throwStub))
         return false;
 
     // Fill in LinkData with the offsets of these stubs.
 
     linkData_.outOfBoundsOffset = outOfBoundsExit.begin;
     linkData_.interruptOffset = interruptExit.begin;
 
-    // Now that all other code has been emitted, patch all remaining callsites.
+    // Now that all other code has been emitted, patch all remaining callsites
+    // then far jumps. Patching callsites can generate far jumps so there is an
+    // ordering dependency.
 
     if (!patchCallSites(&trapExits))
         return false;
 
-    // Now that all code has been generated, patch far jumps to destinations.
-
-    for (CallThunk& callThunk : metadata_->callThunks) {
-        uint32_t funcIndex = callThunk.u.funcIndex;
-        callThunk.u.codeRangeIndex = funcToCodeRange_[funcIndex];
-        CodeOffset farJump(callThunk.offset);
-        masm_.patchFarJump(farJump, funcCodeRange(funcIndex).funcNonProfilingEntry());
-    }
-
-    for (const TrapFarJump& farJump : masm_.trapFarJumps())
-        masm_.patchFarJump(farJump.jump, trapExits[farJump.trap].begin);
+    if (!patchFarJumps(trapExits))
+        return false;
 
     // Code-generation is complete!
 
     masm_.finish();
     return !masm_.oom();
 }
 
 bool
--- a/js/src/wasm/WasmGenerator.h
+++ b/js/src/wasm/WasmGenerator.h
@@ -113,16 +113,17 @@ class MOZ_STACK_CLASS ModuleGenerator
     DebugOnly<FunctionGenerator*>   activeFuncDef_;
     DebugOnly<bool>                 startedFuncDefs_;
     DebugOnly<bool>                 finishedFuncDefs_;
     DebugOnly<uint32_t>             numFinishedFuncDefs_;
 
     bool funcIsCompiled(uint32_t funcIndex) const;
     const CodeRange& funcCodeRange(uint32_t funcIndex) const;
     MOZ_MUST_USE bool patchCallSites(TrapExitOffsetArray* maybeTrapExits = nullptr);
+    MOZ_MUST_USE bool patchFarJumps(const TrapExitOffsetArray& trapExits);
     MOZ_MUST_USE bool finishTask(IonCompileTask* task);
     MOZ_MUST_USE bool finishOutstandingTask();
     MOZ_MUST_USE bool finishFuncExports();
     MOZ_MUST_USE bool finishCodegen();
     MOZ_MUST_USE bool finishLinkData(Bytes& code);
     MOZ_MUST_USE bool addFuncImport(const Sig& sig, uint32_t globalDataOffset);
     MOZ_MUST_USE bool allocateGlobalBytes(uint32_t bytes, uint32_t align, uint32_t* globalDataOff);
     MOZ_MUST_USE bool allocateGlobal(GlobalDesc* global);