Bug 1534840 part 3 - Prevent ARM from generating nops within jump tables. r=sstangl
authorNicolas B. Pierron <nicolas.b.pierron@nbp.name>
Tue, 16 Apr 2019 13:56:58 +0000
changeset 469672 e55ace0633daf9eb7bd5f260300fdbd770834bb3
parent 469671 e024cb135284b83edb799f6f8cf84a4f054d7e35
child 469673 945a3400303e20ca6a99b7d68961fc00c46055cb
push id35879
push usernerli@mozilla.com
push dateTue, 16 Apr 2019 22:01:48 +0000
treeherdermozilla-central@12a60898fdc1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssstangl
bugs1534840
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1534840 part 3 - Prevent ARM from generating nops within jump tables. r=sstangl Depends on D26522 Differential Revision: https://phabricator.services.mozilla.com/D26523
js/src/jit/CodeGenerator.cpp
js/src/jit/arm/Assembler-arm.cpp
js/src/jit/arm/Assembler-arm.h
js/src/jit/arm/CodeGenerator-arm.cpp
js/src/jit/arm/MacroAssembler-arm.cpp
js/src/jit/arm/Trampoline-arm.cpp
js/src/wasm/WasmBaselineCompile.cpp
js/src/wasm/WasmFrameIter.cpp
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -12119,17 +12119,17 @@ void CodeGenerator::visitLoadElementFrom
       new (alloc()) OutOfLineSwitch<SwitchTableType::OutOfLine>(alloc());
 #endif
 
   {
 #if defined(JS_CODEGEN_ARM)
     // Inhibit pools within the following sequence because we are indexing into
     // a pc relative table. The region will have one instruction for ma_ldr, one
     // for breakpoint, and each table case takes one word.
-    AutoForbidPools afp(&masm, 1 + 1 + array->numElements());
+    AutoForbidPoolsAndNops afp(&masm, 1 + 1 + array->numElements());
 #endif
     jumpTable->jumpToCodeEntries(masm, index, temp0);
 
     // Add table entries if the table is inlined.
     for (size_t i = 0, e = array->numElements(); i < e; i++) {
       jumpTable->addTableEntry(masm);
     }
   }
--- a/js/src/jit/arm/Assembler-arm.cpp
+++ b/js/src/jit/arm/Assembler-arm.cpp
@@ -765,16 +765,17 @@ void Assembler::copyDataRelocationTable(
 
 void Assembler::processCodeLabels(uint8_t* rawCode) {
   for (const CodeLabel& label : codeLabels_) {
     Bind(rawCode, label);
   }
 }
 
 void Assembler::writeCodePointer(CodeLabel* label) {
+  m_buffer.assertNoPoolAndNoNops();
   BufferOffset off = writeInst(-1);
   label->patchAt()->bind(off.getOffset());
 }
 
 void Assembler::Bind(uint8_t* rawCode, const CodeLabel& label) {
   size_t offset = label.patchAt().offset();
   size_t target = label.target().offset();
   *reinterpret_cast<const void**>(rawCode + offset) = rawCode + target;
--- a/js/src/jit/arm/Assembler-arm.h
+++ b/js/src/jit/arm/Assembler-arm.h
@@ -2295,41 +2295,40 @@ class DoubleEncoder {
         *ret = table[i].data;
         return true;
       }
     }
     return false;
   }
 };
 
-class AutoForbidPools {
-  Assembler* masm_;
-
- public:
-  // The maxInst argument is the maximum number of word sized instructions
-  // that will be allocated within this context. It is used to determine if
-  // the pool needs to be dumped before entering this content. The debug code
-  // checks that no more than maxInst instructions are actually allocated.
-  //
-  // Allocation of pool entries is not supported within this content so the
-  // code can not use large integers or float constants etc.
-  AutoForbidPools(Assembler* masm, size_t maxInst) : masm_(masm) {
-    masm_->enterNoPool(maxInst);
-  }
-
-  ~AutoForbidPools() { masm_->leaveNoPool(); }
-};
-
 // Forbids nop filling for testing purposes. Not nestable.
 class AutoForbidNops {
+ protected:
   Assembler* masm_;
 
  public:
   explicit AutoForbidNops(Assembler* masm) : masm_(masm) {
     masm_->enterNoNops();
   }
   ~AutoForbidNops() { masm_->leaveNoNops(); }
 };
 
+class AutoForbidPoolsAndNops : public AutoForbidNops {
+ public:
+  // The maxInst argument is the maximum number of word sized instructions
+  // that will be allocated within this context. It is used to determine if
+  // the pool needs to be dumped before entering this content. The debug code
+  // checks that no more than maxInst instructions are actually allocated.
+  //
+  // Allocation of pool entries is not supported within this content so the
+  // code can not use large integers or float constants etc.
+  AutoForbidPoolsAndNops(Assembler* masm, size_t maxInst) : AutoForbidNops(masm) {
+    masm_->enterNoPool(maxInst);
+  }
+
+  ~AutoForbidPoolsAndNops() { masm_->leaveNoPool(); }
+};
+
 }  // namespace jit
 }  // namespace js
 
 #endif /* jit_arm_Assembler_arm_h */
--- a/js/src/jit/arm/CodeGenerator-arm.cpp
+++ b/js/src/jit/arm/CodeGenerator-arm.cpp
@@ -1143,17 +1143,17 @@ void CodeGeneratorARM::emitTableSwitchDi
   int32_t cases = mir->numCases();
   // Lower value with low value.
   masm.ma_sub(index, Imm32(mir->low()), index, scratch, SetCC);
   masm.ma_rsb(index, Imm32(cases - 1), index, scratch, SetCC,
               Assembler::NotSigned);
   // Inhibit pools within the following sequence because we are indexing into
   // a pc relative table. The region will have one instruction for ma_ldr, one
   // for ma_b, and each table case takes one word.
-  AutoForbidPools afp(&masm, 1 + 1 + cases);
+  AutoForbidPoolsAndNops afp(&masm, 1 + 1 + cases);
   masm.ma_ldr(DTRAddr(pc, DtrRegImmShift(index, LSL, 2)), pc, Offset,
               Assembler::NotSigned);
   masm.ma_b(defaultcase);
 
   // To fill in the CodeLabels for the case entries, we need to first generate
   // the case entries (we don't yet know their offsets in the instruction
   // stream).
   OutOfLineTableSwitch* ool = new (alloc()) OutOfLineTableSwitch(alloc(), mir);
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -4256,17 +4256,17 @@ CodeOffset MacroAssembler::farJumpWithPa
   // The goal of the thunk is to be able to jump to any address without the
   // usual 32MiB branch range limitation. Additionally, to make the thunk
   // simple to use, the thunk does not use the constant pool or require
   // patching an absolute address. Instead, a relative offset is used which
   // can be patched during compilation.
 
   // Inhibit pools since these three words must be contiguous so that the offset
   // calculations below are valid.
-  AutoForbidPools afp(this, 3);
+  AutoForbidPoolsAndNops afp(this, 3);
 
   // When pc is used, the read value is the address of the instruction + 8.
   // This is exactly the address of the uint32 word we want to load.
   ScratchRegisterScope scratch(*this);
   ma_ldr(DTRAddr(pc, DtrOffImm(0)), scratch);
 
   // Branch by making pc the destination register.
   ma_add(pc, scratch, pc, LeaveCC, Always);
@@ -4287,17 +4287,17 @@ void MacroAssembler::patchFarJump(CodeOf
   MOZ_ASSERT(editSrc(BufferOffset(addOffset))->is<InstALU>());
 
   // When pc is read as the operand of the add, its value is the address of
   // the add instruction + 8.
   *u32 = (targetOffset - addOffset) - 8;
 }
 
 CodeOffset MacroAssembler::nopPatchableToCall(const wasm::CallSiteDesc& desc) {
-  AutoForbidPools afp(this, /* max number of instructions in scope = */ 1);
+  AutoForbidPoolsAndNops afp(this, /* max number of instructions in scope = */ 1);
   CodeOffset offset(currentOffset());
   ma_nop();
   append(desc, CodeOffset(currentOffset()));
   return offset;
 }
 
 void MacroAssembler::patchNopToCall(uint8_t* call, uint8_t* target) {
   uint8_t* inst = call - 4;
--- a/js/src/jit/arm/Trampoline-arm.cpp
+++ b/js/src/jit/arm/Trampoline-arm.cpp
@@ -231,17 +231,17 @@ void JitRuntime::generateEnterJIT(JSCont
     masm.load32(slot_numStackValues, numStackValues);
 
     // Write return address. On ARM, CodeLabel is only used for tableswitch,
     // so we can't use it here to get the return address. Instead, we use pc
     // + a fixed offset to a jump to returnLabel. The pc register holds pc +
     // 8, so we add the size of 2 instructions to skip the instructions
     // emitted by storePtr and jump(&skipJump).
     {
-      AutoForbidPools afp(&masm, 5);
+      AutoForbidPoolsAndNops afp(&masm, 5);
       Label skipJump;
       masm.mov(pc, scratch);
       masm.addPtr(Imm32(2 * sizeof(uint32_t)), scratch);
       masm.storePtr(scratch, Address(sp, 0));
       masm.jump(&skipJump);
       masm.jump(&returnLabel);
       masm.bind(&skipJump);
     }
@@ -680,17 +680,17 @@ static void GenerateBailoutThunk(MacroAs
 JitRuntime::BailoutTable JitRuntime::generateBailoutTable(MacroAssembler& masm,
                                                           Label* bailoutTail,
                                                           uint32_t frameClass) {
   uint32_t offset = startTrampolineCode(masm);
 
   {
     // Emit the table without any pools being inserted.
     Label bailout;
-    AutoForbidPools afp(&masm, BAILOUT_TABLE_SIZE);
+    AutoForbidPoolsAndNops afp(&masm, BAILOUT_TABLE_SIZE);
     for (size_t i = 0; i < BAILOUT_TABLE_SIZE; i++) {
       masm.ma_bl(&bailout);
     }
     masm.bind(&bailout);
   }
 
   GenerateBailoutThunk(masm, frameClass, bailoutTail);
 
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -4690,17 +4690,17 @@ class BaseCompiler final : public BaseCo
     tableCl.target()->bind(theTable->offset());
     masm.addCodeLabel(tableCl);
 
     masm.jmp(Operand(scratch, switchValue, ScalePointer));
 #elif defined(JS_CODEGEN_ARM)
     // Flush constant pools: offset must reflect the distance from the MOV
     // to the start of the table; as the address of the MOV is given by the
     // label, nothing must come between the bind() and the ma_mov().
-    AutoForbidPools afp(&masm, /* number of instructions in scope = */ 5);
+    AutoForbidPoolsAndNops afp(&masm, /* number of instructions in scope = */ 5);
 
     ScratchI32 scratch(*this);
 
     // Compute the offset from the ma_mov instruction to the jump table.
     Label here;
     masm.bind(&here);
     uint32_t offset = here.offset() - theTable->offset();
 
--- a/js/src/wasm/WasmFrameIter.cpp
+++ b/js/src/wasm/WasmFrameIter.cpp
@@ -412,17 +412,17 @@ void wasm::ClearExitFP(MacroAssembler& m
 }
 
 static void GenerateCallablePrologue(MacroAssembler& masm, uint32_t* entry) {
   masm.setFramePushed(0);
 
   // ProfilingFrameIterator needs to know the offsets of several key
   // instructions from entry. To save space, we make these offsets static
   // constants and assert that they match the actual codegen below. On ARM,
-  // this requires AutoForbidPools to prevent a constant pool from being
+  // this requires AutoForbidPoolsAndNops to prevent a constant pool from being
   // randomly inserted between two instructions.
 #if defined(JS_CODEGEN_MIPS32) || defined(JS_CODEGEN_MIPS64)
   {
     *entry = masm.currentOffset();
 
     masm.subFromStackPtr(Imm32(sizeof(Frame)));
     masm.storePtr(ra, Address(StackPointer, offsetof(Frame, returnAddress)));
     MOZ_ASSERT_IF(!masm.oom(), PushedRetAddr == masm.currentOffset() - *entry);
@@ -453,17 +453,17 @@ static void GenerateCallablePrologue(Mac
              MemOperand(sp, offsetof(Frame, callerFP)));
     MOZ_ASSERT_IF(!masm.oom(), PushedFP == masm.currentOffset() - *entry);
     masm.Mov(ARMRegister(FramePointer, 64), sp);
     MOZ_ASSERT_IF(!masm.oom(), SetFP == masm.currentOffset() - *entry);
   }
 #else
   {
 #  if defined(JS_CODEGEN_ARM)
-    AutoForbidPools afp(&masm, /* number of instructions in scope = */ 7);
+    AutoForbidPoolsAndNops afp(&masm, /* number of instructions in scope = */ 7);
 
     *entry = masm.currentOffset();
 
     MOZ_ASSERT(BeforePushRetAddr == 0);
     masm.push(lr);
 #  else
     *entry = masm.currentOffset();
     // The x86/x64 call instruction pushes the return address.
@@ -523,17 +523,17 @@ static void GenerateCallableEpilogue(Mac
   *ret = masm.currentOffset();
 
   masm.Add(sp, sp, sizeof(Frame));
   masm.Ret(ARMRegister(lr, 64));
 
 #else
   // Forbid pools for the same reason as described in GenerateCallablePrologue.
 #  if defined(JS_CODEGEN_ARM)
-  AutoForbidPools afp(&masm, /* number of instructions in scope = */ 7);
+  AutoForbidPoolsAndNops afp(&masm, /* number of instructions in scope = */ 7);
 #  endif
 
   // There is an important ordering constraint here: fp must be repointed to
   // the caller's frame before any field of the frame currently pointed to by
   // fp is popped: asynchronous signal handlers (which use stack space
   // starting at sp) could otherwise clobber these fields while they are still
   // accessible via fp (fp fields are read during frame iteration which is
   // *also* done asynchronously).
@@ -692,25 +692,25 @@ void wasm::GenerateJitExitEpilogue(Macro
   MOZ_ASSERT(masm.framePushed() == 0);
 }
 
 void wasm::GenerateJitEntryPrologue(MacroAssembler& masm, Offsets* offsets) {
   masm.haltingAlign(CodeAlignment);
 
   {
 #if defined(JS_CODEGEN_ARM)
-    AutoForbidPools afp(&masm, /* number of instructions in scope = */ 2);
+    AutoForbidPoolsAndNops afp(&masm, /* number of instructions in scope = */ 2);
     offsets->begin = masm.currentOffset();
     MOZ_ASSERT(BeforePushRetAddr == 0);
     masm.push(lr);
 #elif defined(JS_CODEGEN_MIPS32) || defined(JS_CODEGEN_MIPS64)
     offsets->begin = masm.currentOffset();
     masm.push(ra);
 #elif defined(JS_CODEGEN_ARM64)
-    AutoForbidPools afp(&masm, /* number of instructions in scope = */ 3);
+    AutoForbidPoolsAndNops afp(&masm, /* number of instructions in scope = */ 3);
     offsets->begin = masm.currentOffset();
     MOZ_ASSERT(BeforePushRetAddr == 0);
     // Subtract from SP first as SP must be aligned before offsetting.
     masm.Sub(sp, sp, 8);
     masm.storePtr(lr, Address(masm.getStackPointer(), 0));
     masm.adjustFrame(8);
 #else
     // The x86/x64 call instruction pushes the return address.