commit 076919bcd96dcced7020e6003c25425a15ff0cb7 Author: Jan de Mooij <jdemooij@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))