[tbb-commits] [tor-browser/tor-browser-45.2.0esr-6.5-1] Bug 1234246 - Don't reprotect JIT code more than once when linking. r=nbp

gk at torproject.org gk at torproject.org
Fri Jun 3 22:11:13 UTC 2016


commit 076919bcd96dcced7020e6003c25425a15ff0cb7
Author: Jan de Mooij <jdemooij at mozilla.com>
Date:   Tue Dec 22 10:56:36 2015 +0100

    Bug 1234246 - Don't reprotect JIT code more than once when linking. r=nbp
---
 js/src/irregexp/NativeRegExpMacroAssembler.cpp |  2 -
 js/src/jit/BaselineCompiler.cpp                |  4 +-
 js/src/jit/BaselineJIT.cpp                     |  7 +-
 js/src/jit/BaselineJIT.h                       |  4 +-
 js/src/jit/CodeGenerator.cpp                   | 93 +++++++++++++-------------
 js/src/jit/Ion.cpp                             | 21 +++---
 js/src/jit/IonCaches.cpp                       | 54 +++++++--------
 js/src/jit/IonCaches.h                         |  7 +-
 js/src/jit/IonCode.h                           |  4 +-
 js/src/jit/IonTypes.h                          |  2 +
 js/src/jit/JitCompartment.h                    |  2 -
 js/src/jit/Linker.h                            |  3 +-
 js/src/jit/SharedIC.cpp                        |  2 +-
 13 files changed, 104 insertions(+), 101 deletions(-)

diff --git a/js/src/irregexp/NativeRegExpMacroAssembler.cpp b/js/src/irregexp/NativeRegExpMacroAssembler.cpp
index a699f5b..c154c79 100644
--- a/js/src/irregexp/NativeRegExpMacroAssembler.cpp
+++ b/js/src/irregexp/NativeRegExpMacroAssembler.cpp
@@ -465,8 +465,6 @@ NativeRegExpMacroAssembler::GenerateCode(JSContext* cx, bool match_only)
     writePerfSpewerJitCodeProfile(code, "RegExp");
 #endif
 
-    AutoWritableJitCode awjc(code);
-
     for (size_t i = 0; i < labelPatches.length(); i++) {
         LabelPatch& v = labelPatches[i];
         MOZ_ASSERT(!v.label);
diff --git a/js/src/jit/BaselineCompiler.cpp b/js/src/jit/BaselineCompiler.cpp
index 8a03334..3b88801 100644
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -238,14 +238,12 @@ BaselineCompiler::compile()
 
     // All barriers are emitted off-by-default, toggle them on if needed.
     if (cx->zone()->needsIncrementalBarrier())
-        baselineScript->toggleBarriers(true);
+        baselineScript->toggleBarriers(true, DontReprotect);
 
     // If profiler instrumentation is enabled, toggle instrumentation on.
     if (cx->runtime()->jitRuntime()->isProfilerInstrumentationEnabled(cx->runtime()))
         baselineScript->toggleProfilerInstrumentation(true);
 
-    AutoWritableJitCode awjc(code);
-
     // Patch IC loads using IC entries.
     for (size_t i = 0; i < icLoadLabels_.length(); i++) {
         CodeOffset label = icLoadLabels_[i].label;
diff --git a/js/src/jit/BaselineJIT.cpp b/js/src/jit/BaselineJIT.cpp
index f0dc265..6ae1d55 100644
--- a/js/src/jit/BaselineJIT.cpp
+++ b/js/src/jit/BaselineJIT.cpp
@@ -1023,8 +1023,6 @@ BaselineScript::toggleProfilerInstrumentation(bool enable)
     JitSpew(JitSpew_BaselineIC, "  toggling profiling %s for BaselineScript %p",
             enable ? "on" : "off", this);
 
-    AutoWritableJitCode awjc(method());
-
     // Toggle the jump
     CodeLocationLabel enterToggleLocation(method_, CodeOffset(profilerEnterToggleOffset_));
     CodeLocationLabel exitToggleLocation(method_, CodeOffset(profilerExitToggleOffset_));
@@ -1136,11 +1134,16 @@ jit::AddSizeOfBaselineData(JSScript* script, mozilla::MallocSizeOf mallocSizeOf,
 void
 jit::ToggleBaselineProfiling(JSRuntime* runtime, bool enable)
 {
+    JitRuntime* jrt = runtime->jitRuntime();
+    if (!jrt)
+        return;
+
     for (ZonesIter zone(runtime, SkipAtoms); !zone.done(); zone.next()) {
         for (gc::ZoneCellIter i(zone, gc::AllocKind::SCRIPT); !i.done(); i.next()) {
             JSScript* script = i.get<JSScript>();
             if (!script->hasBaselineScript())
                 continue;
+            AutoWritableJitCode awjc(script->baselineScript()->method());
             script->baselineScript()->toggleProfilerInstrumentation(enable);
         }
     }
diff --git a/js/src/jit/BaselineJIT.h b/js/src/jit/BaselineJIT.h
index 1b35e6e..08ff74e 100644
--- a/js/src/jit/BaselineJIT.h
+++ b/js/src/jit/BaselineJIT.h
@@ -356,8 +356,8 @@ struct BaselineScript
         templateScope_ = templateScope;
     }
 
-    void toggleBarriers(bool enabled) {
-        method()->togglePreBarriers(enabled);
+    void toggleBarriers(bool enabled, ReprotectCode reprotect = Reprotect) {
+        method()->togglePreBarriers(enabled, reprotect);
     }
 
     bool containsCodeAddress(uint8_t* addr) const {
diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp
index d921f1b..c6959f0 100644
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -1446,7 +1446,7 @@ JitCompartment::generateRegExpExecStub(JSContext* cx)
 #endif
 
     if (cx->zone()->needsIncrementalBarrier())
-        code->togglePreBarriers(true);
+        code->togglePreBarriers(true, DontReprotect);
 
     return code;
 }
@@ -1579,7 +1579,7 @@ JitCompartment::generateRegExpTestStub(JSContext* cx)
 #endif
 
     if (cx->zone()->needsIncrementalBarrier())
-        code->togglePreBarriers(true);
+        code->togglePreBarriers(true, DontReprotect);
 
     return code;
 }
@@ -8259,61 +8259,58 @@ CodeGenerator::link(JSContext* cx, CompilerConstraintList* constraints)
     // Adopt fallback shared stubs from the compiler into the ion script.
     ionScript->adoptFallbackStubs(&stubSpace_);
 
-    {
-        AutoWritableJitCode awjc(code);
-        Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, invalidateEpilogueData_),
+    Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, invalidateEpilogueData_),
+                                       ImmPtr(ionScript),
+                                       ImmPtr((void*)-1));
+
+    for (size_t i = 0; i < ionScriptLabels_.length(); i++) {
+        Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, ionScriptLabels_[i]),
                                            ImmPtr(ionScript),
                                            ImmPtr((void*)-1));
-
-        for (size_t i = 0; i < ionScriptLabels_.length(); i++) {
-            Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, ionScriptLabels_[i]),
-                                               ImmPtr(ionScript),
-                                               ImmPtr((void*)-1));
-        }
+    }
 
 #ifdef JS_TRACE_LOGGING
-        TraceLoggerThread* logger = TraceLoggerForMainThread(cx->runtime());
-        for (uint32_t i = 0; i < patchableTraceLoggers_.length(); i++) {
-            Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, patchableTraceLoggers_[i]),
-                                               ImmPtr(logger),
-                                               ImmPtr(nullptr));
-        }
-
-        if (patchableTLScripts_.length() > 0) {
-            MOZ_ASSERT(TraceLogTextIdEnabled(TraceLogger_Scripts));
-            TraceLoggerEvent event(logger, TraceLogger_Scripts, script);
-            ionScript->setTraceLoggerEvent(event);
-            uint32_t textId = event.payload()->textId();
-            for (uint32_t i = 0; i < patchableTLScripts_.length(); i++) {
-                Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, patchableTLScripts_[i]),
-                                                   ImmPtr((void*) uintptr_t(textId)),
-                                                   ImmPtr((void*)0));
-            }
+    TraceLoggerThread* logger = TraceLoggerForMainThread(cx->runtime());
+    for (uint32_t i = 0; i < patchableTraceLoggers_.length(); i++) {
+        Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, patchableTraceLoggers_[i]),
+                                           ImmPtr(logger),
+                                           ImmPtr(nullptr));
+    }
+
+    if (patchableTLScripts_.length() > 0) {
+        MOZ_ASSERT(TraceLogTextIdEnabled(TraceLogger_Scripts));
+        TraceLoggerEvent event(logger, TraceLogger_Scripts, script);
+        ionScript->setTraceLoggerEvent(event);
+        uint32_t textId = event.payload()->textId();
+        for (uint32_t i = 0; i < patchableTLScripts_.length(); i++) {
+            Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, patchableTLScripts_[i]),
+                                               ImmPtr((void*) uintptr_t(textId)),
+                                               ImmPtr((void*)0));
         }
+    }
 #endif
-        // Patch shared stub IC loads using IC entries
-        for (size_t i = 0; i < sharedStubs_.length(); i++) {
-            CodeOffset label = sharedStubs_[i].label;
-
-            IonICEntry& entry = ionScript->sharedStubList()[i];
-            entry = sharedStubs_[i].entry;
-            Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, label),
-                                               ImmPtr(&entry),
-                                               ImmPtr((void*)-1));
-
-            MOZ_ASSERT(entry.hasStub());
-            MOZ_ASSERT(entry.firstStub()->isFallback());
+    // Patch shared stub IC loads using IC entries
+    for (size_t i = 0; i < sharedStubs_.length(); i++) {
+        CodeOffset label = sharedStubs_[i].label;
+
+        IonICEntry& entry = ionScript->sharedStubList()[i];
+        entry = sharedStubs_[i].entry;
+        Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, label),
+                                           ImmPtr(&entry),
+                                           ImmPtr((void*)-1));
 
-            entry.firstStub()->toFallbackStub()->fixupICEntry(&entry);
-        }
+        MOZ_ASSERT(entry.hasStub());
+        MOZ_ASSERT(entry.firstStub()->isFallback());
 
-        // for generating inline caches during the execution.
-        if (runtimeData_.length())
-            ionScript->copyRuntimeData(&runtimeData_[0]);
-        if (cacheList_.length())
-            ionScript->copyCacheEntries(&cacheList_[0], masm);
+        entry.firstStub()->toFallbackStub()->fixupICEntry(&entry);
     }
 
+    // for generating inline caches during the execution.
+    if (runtimeData_.length())
+        ionScript->copyRuntimeData(&runtimeData_[0]);
+    if (cacheList_.length())
+        ionScript->copyCacheEntries(&cacheList_[0], masm);
+
     JitSpew(JitSpew_Codegen, "Created IonScript %p (raw %p)",
             (void*) ionScript, (void*) code->raw());
 
@@ -8363,7 +8360,7 @@ CodeGenerator::link(JSContext* cx, CompilerConstraintList* constraints)
     // since a GC can occur during code generation. All barriers are emitted
     // off-by-default, and are toggled on here if necessary.
     if (cx->zone()->needsIncrementalBarrier())
-        ionScript->toggleBarriers(true);
+        ionScript->toggleBarriers(true, DontReprotect);
 
     // Attach any generated script counts to the script.
     if (IonScriptCounts* counts = extractScriptCounts())
diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp
index 8e7063a..21baaf8 100644
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -743,14 +743,14 @@ JitCompartment::toggleBarriers(bool enabled)
 {
     // Toggle barriers in compartment wide stubs that have patchable pre barriers.
     if (regExpExecStub_)
-        regExpExecStub_->togglePreBarriers(enabled);
+        regExpExecStub_->togglePreBarriers(enabled, Reprotect);
     if (regExpTestStub_)
-        regExpTestStub_->togglePreBarriers(enabled);
+        regExpTestStub_->togglePreBarriers(enabled, Reprotect);
 
     // Toggle barriers in baseline IC stubs.
     for (ICStubCodeMap::Enum e(*stubCodes_); !e.empty(); e.popFront()) {
         JitCode* code = *e.front().value().unsafeGet();
-        code->togglePreBarriers(enabled);
+        code->togglePreBarriers(enabled, Reprotect);
     }
 }
 
@@ -878,20 +878,23 @@ JitCode::finalize(FreeOp* fop)
 }
 
 void
-JitCode::togglePreBarriers(bool enabled)
+JitCode::togglePreBarriers(bool enabled, ReprotectCode reprotect)
 {
-    AutoWritableJitCode awjc(this);
     uint8_t* start = code_ + preBarrierTableOffset();
     CompactBufferReader reader(start, start + preBarrierTableBytes_);
 
-    while (reader.more()) {
+    if (!reader.more())
+        return;
+
+    MaybeAutoWritableJitCode awjc(this, reprotect);
+    do {
         size_t offset = reader.readUnsigned();
         CodeLocationLabel loc(this, CodeOffset(offset));
         if (enabled)
             Assembler::ToggleToCmp(loc);
         else
             Assembler::ToggleToJmp(loc);
-    }
+    } while (reader.more());
 }
 
 IonScript::IonScript()
@@ -1269,9 +1272,9 @@ IonScript::Destroy(FreeOp* fop, IonScript* script)
 }
 
 void
-IonScript::toggleBarriers(bool enabled)
+IonScript::toggleBarriers(bool enabled, ReprotectCode reprotect)
 {
-    method()->togglePreBarriers(enabled);
+    method()->togglePreBarriers(enabled, reprotect);
 }
 
 void
diff --git a/js/src/jit/IonCaches.cpp b/js/src/jit/IonCaches.cpp
index b5fe6de..36c2358 100644
--- a/js/src/jit/IonCaches.cpp
+++ b/js/src/jit/IonCaches.cpp
@@ -99,20 +99,6 @@ IonCache::CacheName(IonCache::Kind kind)
     return names[kind];
 }
 
-IonCache::LinkStatus
-IonCache::linkCode(JSContext* cx, MacroAssembler& masm, IonScript* ion, JitCode** code)
-{
-    Linker linker(masm);
-    *code = linker.newCode<CanGC>(cx, ION_CODE);
-    if (!*code)
-        return LINK_ERROR;
-
-    if (ion->invalidated())
-        return CACHE_FLUSHED;
-
-    return LINK_GOOD;
-}
-
 const size_t IonCache::MAX_STUBS = 16;
 
 // Helper class which encapsulates logic to attach a stub to an IC by hooking
@@ -239,27 +225,20 @@ class IonCache::StubAttacher
     void patchRejoinJump(MacroAssembler& masm, JitCode* code) {
         rejoinOffset_.fixup(&masm);
         CodeLocationJump rejoinJump(code, rejoinOffset_);
-        AutoWritableJitCode awjc(code);
         PatchJump(rejoinJump, rejoinLabel_);
     }
 
     void patchStubCodePointer(JitCode* code) {
         if (hasStubCodePatchOffset_) {
-            AutoWritableJitCode awjc(code);
             Assembler::PatchDataWithValueCheck(CodeLocationLabel(code, stubCodePatchOffset_),
                                                ImmPtr(code), STUB_ADDR);
         }
     }
 
     void patchNextStubJump(MacroAssembler& masm, JitCode* code) {
-        // Patch the previous nextStubJump of the last stub, or the jump from the
-        // codeGen, to jump into the newly allocated code.
-        PatchJump(cache_.lastJump_, CodeLocationLabel(code), Reprotect);
-
         // If this path is not taken, we are producing an entry which can no
         // longer go back into the update function.
         if (hasNextStubOffset_) {
-            AutoWritableJitCode awjc(code);
             nextStubOffset_.fixup(&masm);
             CodeLocationJump nextStubJump(code, nextStubOffset_);
             PatchJump(nextStubJump, cache_.fallbackLabel_);
@@ -285,21 +264,41 @@ IonCache::emitInitialJump(MacroAssembler& masm, RepatchLabel& entry)
 }
 
 void
-IonCache::attachStub(MacroAssembler& masm, StubAttacher& attacher, Handle<JitCode*> code)
+IonCache::attachStub(MacroAssembler& masm, StubAttacher& attacher, CodeLocationJump lastJump,
+                     Handle<JitCode*> code)
 {
     MOZ_ASSERT(canAttachStub());
     incrementStubCount();
 
+    // Patch the previous nextStubJump of the last stub, or the jump from the
+    // codeGen, to jump into the newly allocated code.
+    PatchJump(lastJump, CodeLocationLabel(code), Reprotect);
+}
+
+IonCache::LinkStatus
+IonCache::linkCode(JSContext* cx, MacroAssembler& masm, StubAttacher& attacher, IonScript* ion,
+                   JitCode** code)
+{
+    Linker linker(masm);
+    *code = linker.newCode<CanGC>(cx, ION_CODE);
+    if (!*code)
+        return LINK_ERROR;
+
+    if (ion->invalidated())
+        return CACHE_FLUSHED;
+
     // Update the success path to continue after the IC initial jump.
-    attacher.patchRejoinJump(masm, code);
+    attacher.patchRejoinJump(masm, *code);
 
     // Replace the STUB_ADDR constant by the address of the generated stub, such
     // as it can be kept alive even if the cache is flushed (see
     // MarkJitExitFrame).
-    attacher.patchStubCodePointer(code);
+    attacher.patchStubCodePointer(*code);
 
     // Update the failure path.
-    attacher.patchNextStubJump(masm, code);
+    attacher.patchNextStubJump(masm, *code);
+
+    return LINK_GOOD;
 }
 
 bool
@@ -307,12 +306,13 @@ IonCache::linkAndAttachStub(JSContext* cx, MacroAssembler& masm, StubAttacher& a
                             IonScript* ion, const char* attachKind,
                             JS::TrackedOutcome trackedOutcome)
 {
+    CodeLocationJump lastJumpBefore = lastJump_;
     Rooted<JitCode*> code(cx);
     {
         // Need to exit the AutoFlushICache context to flush the cache
         // before attaching the stub below.
         AutoFlushICache afc("IonCache");
-        LinkStatus status = linkCode(cx, masm, ion, code.address());
+        LinkStatus status = linkCode(cx, masm, attacher, ion, code.address());
         if (status != LINK_GOOD)
             return status != LINK_ERROR;
     }
@@ -330,7 +330,7 @@ IonCache::linkAndAttachStub(JSContext* cx, MacroAssembler& masm, StubAttacher& a
     writePerfSpewerJitCodeProfile(code, "IonCache");
 #endif
 
-    attachStub(masm, attacher, code);
+    attachStub(masm, attacher, lastJumpBefore, code);
 
     // Add entry to native => bytecode mapping for this stub if needed.
     if (cx->runtime()->jitRuntime()->isProfilerInstrumentationEnabled(cx->runtime())) {
diff --git a/js/src/jit/IonCaches.h b/js/src/jit/IonCaches.h
index f99dd2b..e3c3e35 100644
--- a/js/src/jit/IonCaches.h
+++ b/js/src/jit/IonCaches.h
@@ -293,10 +293,13 @@ class IonCache
     // monitoring/allocation caused an invalidation of the running ion script,
     // this function returns CACHE_FLUSHED. In case of allocation issue this
     // function returns LINK_ERROR.
-    LinkStatus linkCode(JSContext* cx, MacroAssembler& masm, IonScript* ion, JitCode** code);
+    LinkStatus linkCode(JSContext* cx, MacroAssembler& masm, StubAttacher& attacher, IonScript* ion,
+                        JitCode** code);
+
     // Fixup variables and update jumps in the list of stubs.  Increment the
     // number of attached stubs accordingly.
-    void attachStub(MacroAssembler& masm, StubAttacher& attacher, Handle<JitCode*> code);
+    void attachStub(MacroAssembler& masm, StubAttacher& attacher, CodeLocationJump lastJump,
+                    Handle<JitCode*> code);
 
     // Combine both linkStub and attachStub into one function. In addition, it
     // produces a spew augmented with the attachKind string.
diff --git a/js/src/jit/IonCode.h b/js/src/jit/IonCode.h
index 085a907..63cdd64 100644
--- a/js/src/jit/IonCode.h
+++ b/js/src/jit/IonCode.h
@@ -123,7 +123,7 @@ class JitCode : public gc::TenuredCell
         hasBytecodeMap_ = true;
     }
 
-    void togglePreBarriers(bool enabled);
+    void togglePreBarriers(bool enabled, ReprotectCode reprotect);
 
     // If this JitCode object has been, effectively, corrupted due to
     // invalidation patching, then we have to remember this so we don't try and
@@ -512,7 +512,7 @@ struct IonScript
         MOZ_ASSERT(locIndex < runtimeSize_);
         return (CacheLocation*) &runtimeData()[locIndex];
     }
-    void toggleBarriers(bool enabled);
+    void toggleBarriers(bool enabled, ReprotectCode reprotect = Reprotect);
     void purgeCaches();
     void unlinkFromRuntime(FreeOp* fop);
     void copySnapshots(const SnapshotWriter* writer);
diff --git a/js/src/jit/IonTypes.h b/js/src/jit/IonTypes.h
index 8030f16..471cc42 100644
--- a/js/src/jit/IonTypes.h
+++ b/js/src/jit/IonTypes.h
@@ -776,6 +776,8 @@ enum class BarrierKind : uint32_t {
     TypeSet
 };
 
+enum ReprotectCode { Reprotect = true, DontReprotect = false };
+
 } // namespace jit
 } // namespace js
 
diff --git a/js/src/jit/JitCompartment.h b/js/src/jit/JitCompartment.h
index a5364cd..5f17664 100644
--- a/js/src/jit/JitCompartment.h
+++ b/js/src/jit/JitCompartment.h
@@ -518,8 +518,6 @@ class MOZ_STACK_CLASS AutoWritableJitCode
     }
 };
 
-enum ReprotectCode { Reprotect = true, DontReprotect = false };
-
 class MOZ_STACK_CLASS MaybeAutoWritableJitCode
 {
     mozilla::Maybe<AutoWritableJitCode> awjc_;
diff --git a/js/src/jit/Linker.h b/js/src/jit/Linker.h
index 4575d2e3..a0fd487 100644
--- a/js/src/jit/Linker.h
+++ b/js/src/jit/Linker.h
@@ -22,6 +22,7 @@ namespace jit {
 class Linker
 {
     MacroAssembler& masm;
+    mozilla::Maybe<AutoWritableJitCode> awjc;
 
     JitCode* fail(JSContext* cx) {
         ReportOutOfMemory(cx);
@@ -68,7 +69,7 @@ class Linker
             return nullptr;
         if (masm.oom())
             return fail(cx);
-        AutoWritableJitCode awjc(result, bytesNeeded);
+        awjc.emplace(result, bytesNeeded);
         code->copyFrom(masm);
         masm.link(code);
         if (masm.embedsNurseryPointers())
diff --git a/js/src/jit/SharedIC.cpp b/js/src/jit/SharedIC.cpp
index 2867776..03f49a5 100644
--- a/js/src/jit/SharedIC.cpp
+++ b/js/src/jit/SharedIC.cpp
@@ -748,7 +748,7 @@ ICStubCompiler::getStubCode()
 
     // All barriers are emitted off-by-default, enable them if needed.
     if (cx->zone()->needsIncrementalBarrier())
-        newStubCode->togglePreBarriers(true);
+        newStubCode->togglePreBarriers(true, DontReprotect);
 
     // Cache newly compiled stubcode.
     if (!comp->putStubCode(cx, stubKey, newStubCode))





More information about the tbb-commits mailing list