From 8239d880f4339a205e5e29e3030fd2101e6fe28f Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Tue, 8 Aug 2017 13:55:08 +0200 Subject: [PATCH] Bug 1387535 part 2 - Encapsulate Label fields better. r=bhackett --- js/src/irregexp/RegExpMacroAssembler.cpp | 10 ++++-- js/src/jit/Label.h | 19 +++-------- js/src/jit/arm/Assembler-arm.cpp | 27 +++++++--------- js/src/jit/arm64/Assembler-arm64.cpp | 3 +- js/src/jit/arm64/Assembler-arm64.h | 15 +-------- js/src/jit/shared/Assembler-shared.h | 26 --------------- js/src/jit/x86-shared/Assembler-x86-shared.h | 33 ++++++++++++++------ 7 files changed, 48 insertions(+), 85 deletions(-) diff --git a/js/src/irregexp/RegExpMacroAssembler.cpp b/js/src/irregexp/RegExpMacroAssembler.cpp index 2f8542f8ef7a..b39b0773c2fd 100644 --- a/js/src/irregexp/RegExpMacroAssembler.cpp +++ b/js/src/irregexp/RegExpMacroAssembler.cpp @@ -156,6 +156,8 @@ InterpretedRegExpMacroAssembler::Backtrack() Emit(BC_POP_BT, 0); } +static const int32_t INVALID_OFFSET = -1; + void InterpretedRegExpMacroAssembler::Bind(jit::Label* label) { @@ -163,11 +165,12 @@ InterpretedRegExpMacroAssembler::Bind(jit::Label* label) MOZ_ASSERT(!label->bound()); if (label->used()) { int pos = label->offset(); - while (pos != jit::Label::INVALID_OFFSET) { + MOZ_ASSERT(pos >= 0); + do { int fixup = pos; pos = *reinterpret_cast(buffer_ + fixup); *reinterpret_cast(buffer_ + fixup) = pc_; - } + } while (pos != INVALID_OFFSET); } label->bind(pc_); } @@ -516,7 +519,8 @@ InterpretedRegExpMacroAssembler::EmitOrLink(jit::Label* label) if (label->bound()) { Emit32(label->offset()); } else { - int pos = label->use(pc_); + int pos = label->used() ? label->offset() : INVALID_OFFSET; + label->use(pc_); Emit32(pos); } } diff --git a/js/src/jit/Label.h b/js/src/jit/Label.h index 8f2617cb3c15..1ecd7bf16b6b 100644 --- a/js/src/jit/Label.h +++ b/js/src/jit/Label.h @@ -14,7 +14,7 @@ namespace jit { struct LabelBase { - protected: + private: // offset_ >= 0 means that the label is either bound or has incoming // uses and needs to be bound. int32_t offset_ : 31; @@ -23,11 +23,11 @@ struct LabelBase // correctly. uint32_t bound_ : 1; - void operator =(const LabelBase& label) = delete; + void operator=(const LabelBase& label) = delete; - public: static const int32_t INVALID_OFFSET = -1; + public: LabelBase() : offset_(INVALID_OFFSET), bound_(false) { } @@ -40,13 +40,6 @@ struct LabelBase MOZ_ASSERT(bound() || used()); return offset_; } - void offsetBy(int32_t delta) { - MOZ_ASSERT(bound() || used()); - MOZ_ASSERT(offset() + delta >= offset(), "no overflow"); - mozilla::DebugOnly oldOffset(offset()); - offset_ += delta; - MOZ_ASSERT(offset_ == delta + oldOffset, "new offset fits in 31 bits"); - } // Returns whether the label is not bound, but has incoming uses. bool used() const { return !bound() && offset_ > INVALID_OFFSET; @@ -65,14 +58,10 @@ struct LabelBase } // Sets the label's latest used position, returning the old use position in // the process. - int32_t use(int32_t offset) { + void use(int32_t offset) { MOZ_ASSERT(!bound()); - - int32_t old = offset_; offset_ = offset; MOZ_ASSERT(offset_ == offset, "offset fits in 31 bits"); - - return old; } }; diff --git a/js/src/jit/arm/Assembler-arm.cpp b/js/src/jit/arm/Assembler-arm.cpp index d9b3d4538212..5175804fc56a 100644 --- a/js/src/jit/arm/Assembler-arm.cpp +++ b/js/src/jit/arm/Assembler-arm.cpp @@ -954,8 +954,9 @@ Assembler::processCodeLabels(uint8_t* rawCode) } void -Assembler::writeCodePointer(CodeOffset* label) { - BufferOffset off = writeInst(LabelBase::INVALID_OFFSET); +Assembler::writeCodePointer(CodeOffset* label) +{ + BufferOffset off = writeInst(-1); label->bind(off.getOffset()); } @@ -2388,10 +2389,9 @@ Assembler::as_b(Label* l, Condition c) if (oom()) return BufferOffset(); - int32_t old; BufferOffset ret; if (l->used()) { - old = l->offset(); + int32_t old = l->offset(); // This will currently throw an assertion if we couldn't actually // encode the offset of the branch. if (!BOffImm::IsInRange(old)) { @@ -2400,7 +2400,6 @@ Assembler::as_b(Label* l, Condition c) } ret = as_b(BOffImm(old), c, l); } else { - old = LabelBase::INVALID_OFFSET; BOffImm inv; ret = as_b(inv, c, l); } @@ -2408,8 +2407,7 @@ Assembler::as_b(Label* l, Condition c) if (oom()) return BufferOffset(); - DebugOnly check = l->use(ret.getOffset()); - MOZ_ASSERT(check == old); + l->use(ret.getOffset()); return ret; } @@ -2475,20 +2473,18 @@ Assembler::as_bl(Label* l, Condition c) if (oom()) return BufferOffset(); - int32_t old; BufferOffset ret; - // See if the list was empty :( + // See if the list was empty. if (l->used()) { // This will currently throw an assertion if we couldn't actually encode // the offset of the branch. - old = l->offset(); + int32_t old = l->offset(); if (!BOffImm::IsInRange(old)) { m_buffer.fail_bail(); return ret; } ret = as_bl(BOffImm(old), c, l); } else { - old = LabelBase::INVALID_OFFSET; BOffImm inv; ret = as_bl(inv, c, l); } @@ -2496,8 +2492,7 @@ Assembler::as_bl(Label* l, Condition c) if (oom()) return BufferOffset(); - DebugOnly check = l->use(ret.getOffset()); - MOZ_ASSERT(check == old); + l->use(ret.getOffset()); return ret; } @@ -2924,7 +2919,8 @@ Assembler::retarget(Label* label, Label* target) // use chain, prepending the entire use chain of target. Instruction branch = *editSrc(labelBranchOffset); Condition c = branch.extractCond(); - int32_t prev = target->use(label->offset()); + int32_t prev = target->offset(); + target->use(label->offset()); if (branch.is()) as_b(BOffImm(prev), c, labelBranchOffset); else if (branch.is()) @@ -2934,8 +2930,7 @@ Assembler::retarget(Label* label, Label* target) } else { // The target is unbound and unused. We can just take the head of // the list hanging off of label, and dump that into target. - DebugOnly prev = target->use(label->offset()); - MOZ_ASSERT((int32_t)prev == Label::INVALID_OFFSET); + target->use(label->offset()); } } label->reset(); diff --git a/js/src/jit/arm64/Assembler-arm64.cpp b/js/src/jit/arm64/Assembler-arm64.cpp index d84dfa700d86..014646822d9a 100644 --- a/js/src/jit/arm64/Assembler-arm64.cpp +++ b/js/src/jit/arm64/Assembler-arm64.cpp @@ -662,8 +662,7 @@ Assembler::retarget(Label* label, Label* target) } else { // The target is unbound and unused. We can just take the head of // the list hanging off of label, and dump that into target. - DebugOnly prev = target->use(label->offset()); - MOZ_ASSERT((int32_t)prev == Label::INVALID_OFFSET); + target->use(label->offset()); } } label->reset(); diff --git a/js/src/jit/arm64/Assembler-arm64.h b/js/src/jit/arm64/Assembler-arm64.h index 1dc21cbe5e8b..e20f5734bec9 100644 --- a/js/src/jit/arm64/Assembler-arm64.h +++ b/js/src/jit/arm64/Assembler-arm64.h @@ -368,21 +368,8 @@ class Assembler : public vixl::Assembler static const size_t OffsetOfJumpTableEntryPointer = 8; public: - void writeCodePointer(AbsoluteLabel* absoluteLabel) { - MOZ_ASSERT(!absoluteLabel->bound()); - uintptr_t x = LabelBase::INVALID_OFFSET; - BufferOffset off = EmitData(&x, sizeof(uintptr_t)); - - // The x86/x64 makes general use of AbsoluteLabel and weaves a linked list - // of uses of an AbsoluteLabel through the assembly. ARM only uses labels - // for the case statements of switch jump tables. Thus, for simplicity, we - // simply treat the AbsoluteLabel as a label and bind it to the offset of - // the jump table entry that needs to be patched. - LabelBase* label = absoluteLabel; - label->bind(off.getOffset()); - } void writeCodePointer(CodeOffset* label) { - uintptr_t x = LabelBase::INVALID_OFFSET; + uintptr_t x = uintptr_t(-1); BufferOffset off = EmitData(&x, sizeof(uintptr_t)); label->bind(off.getOffset()); } diff --git a/js/src/jit/shared/Assembler-shared.h b/js/src/jit/shared/Assembler-shared.h index adf1a3a16a11..50445ff7edee 100644 --- a/js/src/jit/shared/Assembler-shared.h +++ b/js/src/jit/shared/Assembler-shared.h @@ -399,32 +399,6 @@ class RepatchLabel } }; -// An absolute label is like a Label, except it represents an absolute -// reference rather than a relative one. Thus, it cannot be patched until after -// linking. -struct AbsoluteLabel : public LabelBase -{ - public: - AbsoluteLabel() - { } - AbsoluteLabel(const AbsoluteLabel& label) : LabelBase(label) - { } - int32_t prev() const { - MOZ_ASSERT(!bound()); - if (!used()) - return INVALID_OFFSET; - return offset(); - } - void setPrev(int32_t offset) { - use(offset); - } - void bind() { - bound_ = true; - - // These labels cannot be used after being bound. - offset_ = -1; - } -}; class CodeOffset { diff --git a/js/src/jit/x86-shared/Assembler-x86-shared.h b/js/src/jit/x86-shared/Assembler-x86-shared.h index 4006207f0923..e3e1a5399887 100644 --- a/js/src/jit/x86-shared/Assembler-x86-shared.h +++ b/js/src/jit/x86-shared/Assembler-x86-shared.h @@ -449,8 +449,8 @@ class AssemblerX86Shared : public AssemblerShared masm.nopAlign(alignment); } void writeCodePointer(CodeOffset* label) { - // A CodeOffset only has one use, bake in the "end of list" value. - masm.jumpTablePointer(LabelBase::INVALID_OFFSET); + // Use -1 as dummy value. This will be patched after codegen. + masm.jumpTablePointer(-1); label->bind(masm.size()); } void cmovz(const Operand& src, Register dest) { @@ -853,7 +853,10 @@ class AssemblerX86Shared : public AssemblerShared } else { // Thread the jump list through the unpatched jump targets. JmpSrc j = masm.jCC(static_cast(cond)); - JmpSrc prev = JmpSrc(label->use(j.offset())); + JmpSrc prev; + if (label->used()) + prev = JmpSrc(label->offset()); + label->use(j.offset()); masm.setNextJump(j, prev); } } @@ -864,7 +867,10 @@ class AssemblerX86Shared : public AssemblerShared } else { // Thread the jump list through the unpatched jump targets. JmpSrc j = masm.jmp(); - JmpSrc prev = JmpSrc(label->use(j.offset())); + JmpSrc prev; + if (label->used()) + prev = JmpSrc(label->offset()); + label->use(j.offset()); masm.setNextJump(j, prev); } } @@ -877,7 +883,10 @@ class AssemblerX86Shared : public AssemblerShared masm.linkJump(j, JmpDst(label->offset())); } else { // Thread the jump list through the unpatched jump targets. - JmpSrc prev = JmpSrc(label->use(j.offset())); + JmpSrc prev; + if (label->used()) + prev = JmpSrc(label->offset()); + label->use(j.offset()); masm.setNextJump(j, prev); } return j; @@ -992,7 +1001,10 @@ class AssemblerX86Shared : public AssemblerShared masm.linkJump(jmp, JmpDst(target->offset())); } else { // Thread the jump list through the unpatched jump targets. - JmpSrc prev(target->use(jmp.offset())); + JmpSrc prev; + if (target->used()) + prev = JmpSrc(target->offset()); + target->use(jmp.offset()); masm.setNextJump(jmp, prev); } jmp = JmpSrc(next.offset()); @@ -1020,11 +1032,14 @@ class AssemblerX86Shared : public AssemblerShared masm.ret_i(n.value - sizeof(void*)); } CodeOffset call(Label* label) { + JmpSrc j = masm.call(); if (label->bound()) { - masm.linkJump(masm.call(), JmpDst(label->offset())); + masm.linkJump(j, JmpDst(label->offset())); } else { - JmpSrc j = masm.call(); - JmpSrc prev = JmpSrc(label->use(j.offset())); + JmpSrc prev; + if (label->used()) + prev = JmpSrc(label->offset()); + label->use(j.offset()); masm.setNextJump(j, prev); } return CodeOffset(masm.currentOffset());