[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