Bug 1673391 - Generalize JmpSrc to allow patching not at the end. r=nbp

Some instructions, such as RIP-relative CMPPS and CMPPD, have a patch
field that is embedded in the instruction, not at the end of it.  To
patch this field, the information about the distance between the field
and the end of the instruction must be transmitted in the JmpSrc that
is used for the patching.  (Or we must not use JmpSrc.)

This patch generalizes JmpSrc slightly to carry information about the
patch field's distance from the end of the instruction.  The only
values allowed are 0 and 1 bytes, since the only case we care about so
far has a single byte trailing the patch field, and this restricts the
offset value in JmpSrc the least while keeping its size small.

Assertions are added to the patching code to guard against values
other than zero except where such values can be used safely.

Differential Revision: https://phabricator.services.mozilla.com/D97268

js/src/jit/x64/MacroAssembler-x64.cpp

@@ -35,7 +35,7 @@ void MacroAssemblerX64::loadConstantDouble(double d, FloatRegister dest) {
   // PC-relative addressing. Use "jump" label support code, because we need
   // the same PC-relative address patching that jumps use.
   JmpSrc j = masm.vmovsd_ripr(dest.encoding());
-  propagateOOM(dbl->uses.append(CodeOffset(j.offset())));
+  propagateOOM(dbl->uses.append(j));
 }
 
 void MacroAssemblerX64::loadConstantFloat32(float f, FloatRegister dest) {
@@ -48,7 +48,7 @@ void MacroAssemblerX64::loadConstantFloat32(float f, FloatRegister dest) {
   }
   // See comment in loadConstantDouble
   JmpSrc j = masm.vmovss_ripr(dest.encoding());
-  propagateOOM(flt->uses.append(CodeOffset(j.offset())));
+  propagateOOM(flt->uses.append(j));
 }
 
 void MacroAssemblerX64::vpRiprOpSimd128(
@@ -60,7 +60,7 @@ void MacroAssemblerX64::vpRiprOpSimd128(
     return;
   }
   JmpSrc j = (masm.*op)(reg.encoding());
-  propagateOOM(val->uses.append(CodeOffset(j.offset())));
+  propagateOOM(val->uses.append(j));
 }
 
 void MacroAssemblerX64::loadConstantSimd128Int(const SimdConstant& v,
@@ -351,10 +351,9 @@ void MacroAssemblerX64::vpcmpgtdSimd128(const SimdConstant& v,
 
 void MacroAssemblerX64::bindOffsets(
     const MacroAssemblerX86Shared::UsesVector& uses) {
-  for (CodeOffset use : uses) {
+  for (JmpSrc src : uses) {
     JmpDst dst(currentOffset());
-    JmpSrc src(use.offset());
+    // Using linkJump here is safe, as explained in the comment in
-    // Using linkJump here is safe, as explaind in the comment in
     // loadConstantDouble.
     masm.linkJump(src, dst);
   }

js/src/jit/x86-shared/BaseAssembler-x86-shared.h

@@ -4092,6 +4092,7 @@ class BaseAssembler : public GenericAssembler {
     }
 
     assertValidJmpSrc(from);
+    MOZ_ASSERT(from.trailing() == 0);
 
     const unsigned char* code = m_formatter.data();
     int32_t offset = GetInt32(code + from.offset());
@@ -4138,6 +4139,7 @@ class BaseAssembler : public GenericAssembler {
     }
 
     assertValidJmpSrc(from);
+    MOZ_ASSERT(from.trailing() == 0);
     MOZ_RELEASE_ASSERT(to.offset() == -1 || size_t(to.offset()) <= size());
 
     unsigned char* code = m_formatter.data();
@@ -4159,7 +4161,7 @@ class BaseAssembler : public GenericAssembler {
 
     spew(".set .Lfrom%d, .Llabel%d", from.offset(), to.offset());
     unsigned char* code = m_formatter.data();
-    SetRel32(code + from.offset(), code + to.offset());
+    SetRel32(code + from.offset(), code + to.offset(), from.trailing());
   }
 
   void executableCopy(void* dst) {

js/src/jit/x86-shared/MacroAssembler-x86-shared.h

@@ -27,7 +27,14 @@ class MacroAssemblerX86Shared : public Assembler {
   const MacroAssembler& asMasm() const;
 
  public:
-  typedef Vector<CodeOffset, 0, SystemAllocPolicy> UsesVector;
+#ifdef JS_CODEGEN_X64
+  typedef X86Encoding::JmpSrc UsesItem;
+#else
+  typedef CodeOffset UsesItem;
+#endif
+
+  typedef Vector<UsesItem, 0, SystemAllocPolicy> UsesVector;
+  static_assert(sizeof(UsesItem) == 4);
 
  protected:
   // For Double, Float and SimdData, make the move ctors explicit so that MSVC

js/src/jit/x86-shared/Patching-x86-shared.h

@@ -28,11 +28,11 @@ inline int32_t GetInt32(const void* where) {
   return res;
 }
 
-inline void SetInt32(void* where, int32_t value) {
+inline void SetInt32(void* where, int32_t value, uint32_t trailing = 0) {
-  memcpy((char*)where - sizeof(int32_t), &value, sizeof(int32_t));
+  memcpy((char*)where - trailing - sizeof(int32_t), &value, sizeof(int32_t));
 }
 
-inline void SetRel32(void* from, void* to) {
+inline void SetRel32(void* from, void* to, uint32_t trailing = 0) {
   intptr_t offset =
       reinterpret_cast<intptr_t>(to) - reinterpret_cast<intptr_t>(from);
   MOZ_ASSERT(offset == static_cast<int32_t>(offset),
@@ -41,7 +41,7 @@ inline void SetRel32(void* from, void* to) {
     MOZ_CRASH("offset is too great for a 32-bit relocation");
   }
 
-  SetInt32(from, offset);
+  SetInt32(from, offset, trailing);
 }
 
 inline void* GetRel32Target(void* where) {
@@ -49,14 +49,46 @@ inline void* GetRel32Target(void* where) {
   return (char*)where + rel;
 }
 
+// JmpSrc represents a positive offset within a code buffer, or an uninitialized
+// value.  Lots of code depends on uninitialized JmpSrc holding the value -1, on
+// -1 being a legal value of JmpSrc, and on being able to initialize a JmpSrc
+// with the value -1.
+//
+// The value of the `offset` is always positive and <= MaxCodeBytesPerProcess,
+// see ProcessExecutableMemory.h.  The latter quantity in turn must fit in an
+// i32.  But we further require that the value is not precisely INT32_MAX, so as
+// to allow the JmpSrc value -1 to mean "uninitialized" without ambiguity.
+//
+// The quantity `trailing` denotes the number of bytes of data that follow the
+// patch field in the instruction.  The offset points to the end of the
+// instruction as per normal.  The information about trailing bytes is needed
+// separately from the offset to correctly patch instructions that have
+// immediates trailing the patch field (eg CMPSS and CMPSD).  Currently the only
+// allowed values for `trailing` are 0 and 1.
+
+static_assert(MaxCodeBytesPerProcess < size_t(INT32_MAX), "Invariant");
+
 class JmpSrc {
  public:
-  JmpSrc() : offset_(-1) {}
+  JmpSrc() : offset_(INT32_MAX), trailing_(0) {}
-  explicit JmpSrc(int32_t offset) : offset_(offset) {}
+  explicit JmpSrc(int32_t offset) : offset_(offset), trailing_(0) {
-  int32_t offset() const { return offset_; }
+    // offset -1 is stored as INT32_MAX
+    MOZ_ASSERT(offset == -1 || (offset >= 0 && offset < INT32_MAX));
+  }
+  JmpSrc(int32_t offset, uint32_t trailing)
+      : offset_(offset), trailing_(trailing) {
+    // Disallow offset -1 in this situation, it does not apply.
+    MOZ_ASSERT(offset >= 0 && offset < INT32_MAX);
+    MOZ_ASSERT(trailing <= 1);
+  }
+  int32_t offset() const {
+    return offset_ == INT32_MAX ? -1 : int32_t(offset_);
+  }
+  uint32_t trailing() const { return trailing_; }
 
  private:
-  int32_t offset_;
+  uint32_t offset_ : 31;
+  uint32_t trailing_ : 1;
 };
 
 class JmpDst {