Backed out 2 changesets (bug 1495512) for failing Win MinGW builds
authorAndreea Pavel <apavel@mozilla.com>
Sat, 10 Nov 2018 04:06:10 +0200
changeset 504720 bb6f1073c5ce8beb459b69df723ee812ab94a7e8
parent 504719 ef627706b4ab4ba77e18f6fb738af56257d5747d
child 504721 af173815a3756b93d0830bacc85510ee9b7b9ff5
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1495512
milestone65.0a1
backs out78154ca1e2ac4b5f07276ab9969883d517940d9d
17212e7dfe2971e3da25648df48565324e2589e8
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
Backed out 2 changesets (bug 1495512) for failing Win MinGW builds Backed out changeset 78154ca1e2ac (bug 1495512) Backed out changeset 17212e7dfe29 (bug 1495512)
mozglue/misc/interceptor/MMPolicies.h
mozglue/misc/interceptor/PatcherDetour.h
mozglue/misc/interceptor/TargetFunction.h
mozglue/misc/interceptor/VMSharingPolicies.h
mozglue/misc/nsWindowsDllInterceptor.h
mozglue/tests/interceptor/TestDllInterceptor.cpp
--- a/mozglue/misc/interceptor/MMPolicies.h
+++ b/mozglue/misc/interceptor/MMPolicies.h
@@ -3,38 +3,24 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_interceptor_MMPolicies_h
 #define mozilla_interceptor_MMPolicies_h
 
 #include "mozilla/Assertions.h"
-#include "mozilla/TypedEnumBits.h"
 #include "mozilla/Types.h"
 #include "mozilla/WindowsMapRemoteView.h"
 
 #include <windows.h>
 
-// _CRT_RAND_S is not defined everywhere, but we need it.
-#if !defined(_CRT_RAND_S)
-extern "C" errno_t rand_s(unsigned int* randomValue);
-#endif // !defined(_CRT_RAND_S)
-
 namespace mozilla {
 namespace interceptor {
 
-enum class ReservationFlags : uint32_t
-{
-  eDefault = 0,
-  eForceFirst2GB = 1,
-};
-
-MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ReservationFlags)
-
 class MMPolicyBase
 {
 public:
   static DWORD ComputeAllocationSize(const uint32_t aRequestedSize)
   {
     MOZ_ASSERT(aRequestedSize);
     DWORD result = aRequestedSize;
 
@@ -64,110 +50,16 @@ public:
     static const DWORD kPageSize = []() -> DWORD {
       SYSTEM_INFO sysInfo;
       ::GetSystemInfo(&sysInfo);
       return sysInfo.dwPageSize;
     }();
 
     return kPageSize;
   }
-
-#if defined(_M_X64)
-
-  /**
-   * This function locates a virtual memory region of |aDesiredBytesLen| that
-   * resides in the lowest 2GB of address space. We do this by scanning the
-   * virtual memory space for a block of unallocated memory that is sufficiently
-   * large. We must stay below the 2GB mark because a 10-byte patch uses movsxd
-   * (ie, sign extension) to expand the pointer to 64-bits, so bit 31 of the
-   * found region must be 0.
-   */
-  static PVOID FindLowRegion(HANDLE aProcess, const size_t aDesiredBytesLen)
-  {
-    const DWORD granularity = GetAllocGranularity();
-
-    MOZ_ASSERT(aDesiredBytesLen / granularity > 0);
-    if (!aDesiredBytesLen) {
-      return nullptr;
-    }
-
-    // Generate a randomized base address that falls within the interval
-    // [1MiB, 2GiB - aDesiredBytesLen]
-    unsigned int rnd = 0;
-    rand_s(&rnd);
-
-    // Reduce rnd to a value that falls within the acceptable range
-    const uint64_t kMinAddress = 0x0000000000100000ULL;
-    const uint64_t kMaxAddress = 0x0000000080000000ULL;
-    uint64_t maxOffset = (kMaxAddress - kMinAddress - aDesiredBytesLen) /
-                         granularity;
-    uint64_t offset = (uint64_t(rnd) % maxOffset) * granularity;
-
-    // Start searching at this address
-    char* address = reinterpret_cast<char*>(kMinAddress) + offset;
-    // The max address needs to incorporate the desired length
-    char* const kMaxPtr = reinterpret_cast<char*>(kMaxAddress) -
-                          aDesiredBytesLen;
-
-    MOZ_DIAGNOSTIC_ASSERT(address <= kMaxPtr);
-
-    MEMORY_BASIC_INFORMATION mbi = {};
-    SIZE_T len = sizeof(mbi);
-
-    // Scan the range for a free chunk that is at least as large as
-    // aDesiredBytesLen
-    while (address <= kMaxPtr &&
-           ::VirtualQueryEx(aProcess, address, &mbi, len)) {
-      if (mbi.State == MEM_FREE && mbi.RegionSize >= aDesiredBytesLen) {
-        return mbi.BaseAddress;
-      }
-
-      address = reinterpret_cast<char*>(mbi.BaseAddress) + mbi.RegionSize;
-    }
-
-    return nullptr;
-  }
-
-#endif // defined(_M_X64)
-
-  template <typename ReserveFnT>
-  static PVOID Reserve(HANDLE aProcess, const uint32_t aSize,
-                       const ReserveFnT& aReserveFn,
-                       const ReservationFlags aFlags)
-  {
-#if defined(_M_X64)
-    if (aFlags & ReservationFlags::eForceFirst2GB) {
-      size_t curAttempt = 0;
-      const size_t kMaxAttempts = 8;
-
-      // We loop here because |FindLowRegion| may return a base address that
-      // is reserved elsewhere before we have had a chance to reserve it
-      // ourselves.
-      while (curAttempt < kMaxAttempts) {
-        PVOID base = FindLowRegion(aProcess, aSize);
-        if (!base) {
-          return nullptr;
-        }
-
-        PVOID result = aReserveFn(aProcess, base, aSize);
-        if (result) {
-          return result;
-        }
-
-        ++curAttempt;
-      }
-
-      // If we run out of attempts, we fall through to the default case where
-      // the system chooses any base address it wants. In that case, the hook
-      // will be set on a best-effort basis.
-    }
-#endif // defined(_M_X64)
-
-    return aReserveFn(aProcess, nullptr, aSize);
-  }
 };
 
 class MMPolicyInProcess : public MMPolicyBase
 {
 public:
   typedef MMPolicyInProcess MMPolicyT;
 
   explicit MMPolicyInProcess()
@@ -270,65 +162,48 @@ public:
     return !!::FlushInstructionCache(::GetCurrentProcess(), nullptr, 0);
   }
 
   static DWORD GetTrampWriteProtFlags()
   {
     return PAGE_EXECUTE_READWRITE;
   }
 
-#if defined(_M_X64)
-  bool IsTrampolineSpaceInLowest2GB() const
-  {
-    return (mBase + mReservationSize) <=
-           reinterpret_cast<uint8_t*>(0x0000000080000000ULL);
-  }
-#endif // defined(_M_X64)
-
 protected:
   uint8_t* GetLocalView() const
   {
     return mBase;
   }
 
   uintptr_t GetRemoteView() const
   {
     // Same as local view for in-process
     return reinterpret_cast<uintptr_t>(mBase);
   }
 
   /**
    * @return the effective number of bytes reserved, or 0 on failure
    */
-  uint32_t Reserve(const uint32_t aSize,
-                   const ReservationFlags aFlags)
+  uint32_t Reserve(const uint32_t aSize)
   {
     if (!aSize) {
       return 0;
     }
 
     if (mBase) {
       MOZ_ASSERT(mReservationSize >= aSize);
       return mReservationSize;
     }
 
     mReservationSize = ComputeAllocationSize(aSize);
-
-    auto reserveFn = [](HANDLE aProcess, PVOID aBase, uint32_t aSize) -> PVOID {
-      return ::VirtualAlloc(aBase, aSize, MEM_RESERVE, PAGE_NOACCESS);
-    };
-
-    mBase = static_cast<uint8_t*>(
-      MMPolicyBase::Reserve(::GetCurrentProcess(), mReservationSize, reserveFn,
-                            aFlags));
-
+    mBase = static_cast<uint8_t*>(::VirtualAlloc(nullptr, mReservationSize,
+                                                 MEM_RESERVE, PAGE_NOACCESS));
     if (!mBase) {
       return 0;
     }
-
     return mReservationSize;
   }
 
   bool MaybeCommitNextPage(const uint32_t aRequestedOffset,
                            const uint32_t aRequestedLength)
   {
     if (!(*this)) {
       return false;
@@ -508,39 +383,31 @@ public:
     return !!::FlushInstructionCache(mProcess, nullptr, 0);
   }
 
   static DWORD GetTrampWriteProtFlags()
   {
     return PAGE_READWRITE;
   }
 
-#if defined(_M_X64)
-  bool IsTrampolineSpaceInLowest2GB() const
-  {
-    return (GetRemoteView() + mReservationSize) <= 0x0000000080000000ULL;
-  }
-#endif // defined(_M_X64)
-
 protected:
   uint8_t* GetLocalView() const
   {
     return mLocalView;
   }
 
   uintptr_t GetRemoteView() const
   {
     return reinterpret_cast<uintptr_t>(mRemoteView);
   }
 
   /**
    * @return the effective number of bytes reserved, or 0 on failure
    */
-  uint32_t Reserve(const uint32_t aSize,
-                   const ReservationFlags aFlags)
+  uint32_t Reserve(const uint32_t aSize)
   {
     if (!aSize || !mProcess) {
       return 0;
     }
 
     if (mRemoteView) {
       MOZ_ASSERT(mReservationSize >= aSize);
       return mReservationSize;
@@ -556,24 +423,18 @@ protected:
     }
 
     mLocalView = static_cast<uint8_t*>(
                    ::MapViewOfFile(mMapping, FILE_MAP_WRITE, 0, 0, 0));
     if (!mLocalView) {
       return 0;
     }
 
-    auto reserveFn = [mapping = mMapping](HANDLE aProcess, PVOID aBase,
-                                          uint32_t aSize) -> PVOID {
-      return mozilla::MapRemoteViewOfFile(mapping, aProcess, 0ULL, aBase, 0, 0,
-                                          PAGE_EXECUTE_READ);
-    };
-
-    mRemoteView = MMPolicyBase::Reserve(mProcess, mReservationSize, reserveFn,
-                                        aFlags);
+    mRemoteView = MapRemoteViewOfFile(mMapping, mProcess, 0ULL,
+                                      nullptr, 0, 0, PAGE_EXECUTE_READ);
     if (!mRemoteView) {
       return 0;
     }
 
     return mReservationSize;
   }
 
   bool MaybeCommitNextPage(const uint32_t aRequestedOffset,
--- a/mozglue/misc/interceptor/PatcherDetour.h
+++ b/mozglue/misc/interceptor/PatcherDetour.h
@@ -6,46 +6,34 @@
 
 #ifndef mozilla_interceptor_PatcherDetour_h
 #define mozilla_interceptor_PatcherDetour_h
 
 #include "mozilla/interceptor/PatcherBase.h"
 #include "mozilla/interceptor/Trampoline.h"
 
 #include "mozilla/ScopeExit.h"
-#include "mozilla/TypedEnumBits.h"
 
 #define COPY_CODES(NBYTES)  do {    \
   tramp.CopyFrom(origBytes.GetAddress(), NBYTES); \
   origBytes += NBYTES;              \
 } while (0)
 
 namespace mozilla {
 namespace interceptor {
 
-enum class DetourFlags : uint32_t
-{
-  eDefault = 0,
-  eEnable10BytePatch = 1, // Allow 10-byte patches when conditions allow
-  eTestOnlyForce10BytePatch = 3, // Force 10-byte patches at all times (testing only)
-};
-
-MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(DetourFlags)
-
 template <typename VMPolicy>
 class WindowsDllDetourPatcher final : public WindowsDllPatcherBase<VMPolicy>
 {
   typedef typename VMPolicy::MMPolicyT MMPolicyT;
-  DetourFlags mFlags;
 
 public:
   template <typename... Args>
   explicit WindowsDllDetourPatcher(Args... aArgs)
     : WindowsDllPatcherBase<VMPolicy>(std::forward<Args>(aArgs)...)
-    , mFlags(DetourFlags::eDefault)
   {
   }
 
   ~WindowsDllDetourPatcher()
   {
     Clear();
   }
 
@@ -123,128 +111,63 @@ public:
 
       intptr_t startOfTrampInstructions =
         static_cast<intptr_t>(tramp.GetCurrentRemoteAddress());
 
       origBytes.WriteDisp32(startOfTrampInstructions);
       if (!origBytes) {
         continue;
       }
+#elif defined(_M_X64)
+      // Ensure the MOV R11 from CreateTrampoline is where we expect it to be.
+      MOZ_ASSERT(opcode1 == 0x49);
+      if (opcode1 != 0x49) {
+        continue;
+      }
 
-      origBytes.Commit();
-#elif defined(_M_X64)
-      if (opcode1 == 0x49) {
-        if (!Clear13BytePatch(origBytes, tramp.GetCurrentRemoteAddress())) {
-          continue;
-        }
-      } else if (opcode1 == 0xB8) {
-        if (!Clear10BytePatch(origBytes)) {
-          continue;
-        }
-      } else {
-        MOZ_ASSERT_UNREACHABLE("Unrecognized patch!");
+      Maybe<uint8_t> maybeOpcode2 = origBytes.ReadByte();
+      if (!maybeOpcode2) {
+        continue;
+      }
+
+      uint8_t opcode2 = maybeOpcode2.value();
+      if (opcode2 != 0xBB) {
+        continue;
+      }
+
+      origBytes.WritePointer(tramp.GetCurrentRemoteAddress());
+      if (!origBytes) {
         continue;
       }
 #elif defined(_M_ARM64)
       MOZ_RELEASE_ASSERT(false, "Shouldn't get here");
 #else
 #error "Unknown processor type"
 #endif
+
+      origBytes.Commit();
     }
 
     this->mVMPolicy.Clear();
   }
 
-  bool Clear13BytePatch(WritableTargetFunction<MMPolicyT>& aOrigBytes,
-                        const uintptr_t aResetToAddress)
-  {
-    Maybe<uint8_t> maybeOpcode2 = aOrigBytes.ReadByte();
-    if (!maybeOpcode2) {
-      return false;
-    }
-
-    uint8_t opcode2 = maybeOpcode2.value();
-    if (opcode2 != 0xBB) {
-      return false;
-    }
-
-    aOrigBytes.WritePointer(aResetToAddress);
-    if (!aOrigBytes) {
-      return false;
-    }
-
-    return aOrigBytes.Commit();
-  }
-
-  bool Clear10BytePatch(WritableTargetFunction<MMPolicyT>& aOrigBytes)
-  {
-    Maybe<uint32_t> maybePtr32 = aOrigBytes.ReadLong();
-    if (!maybePtr32) {
-      return false;
-    }
-
-    uint32_t ptr32 = maybePtr32.value();
-    // We expect the high bit to be clear
-    if (ptr32 & 0x80000000) {
-      return false;
-    }
-
-    uintptr_t trampPtr = ptr32;
-
-    // trampPtr points to an intermediate trampoline that contains a 13-byte
-    // patch. We back up by sizeof(uintptr_t) so that we can access the pointer
-    // to the stub trampoline.
-    WritableTargetFunction<MMPolicyT> writableIntermediate(this->mVMPolicy,
-        trampPtr - sizeof(uintptr_t), 13 + sizeof(uintptr_t));
-    if (!writableIntermediate) {
-      return false;
-    }
-
-    Maybe<uintptr_t> stubTramp = writableIntermediate.ReadEncodedPtr();
-    if (!stubTramp || !stubTramp.value()) {
-      return false;
-    }
-
-    Maybe<uint8_t> maybeOpcode1 = writableIntermediate.ReadByte();
-    if (!maybeOpcode1) {
-      return false;
-    }
-
-    // We expect this opcode to be the beginning of our normal mov r11, ptr
-    // patch sequence.
-    uint8_t opcode1 = maybeOpcode1.value();
-    if (opcode1 != 0x49) {
-      return false;
-    }
-
-    // Now we can just delegate the rest to our normal 13-byte patch clearing.
-    return Clear13BytePatch(writableIntermediate, stubTramp.value());
-  }
-
-  void Init(DetourFlags aFlags = DetourFlags::eDefault, int aNumHooks = 0)
+  void Init(int aNumHooks = 0)
   {
     if (Initialized()) {
       return;
     }
 
-    mFlags = aFlags;
-
     if (aNumHooks == 0) {
       // Win32 allocates VM addresses at a 64KiB granularity, so by default we
       // might as well utilize that entire 64KiB reservation instead of
       // artifically constraining ourselves to the page size.
       aNumHooks = this->mVMPolicy.GetAllocGranularity() / kHookSize;
     }
 
-    ReservationFlags resFlags = ReservationFlags::eDefault;
-    if (aFlags & DetourFlags::eEnable10BytePatch) {
-      resFlags |= ReservationFlags::eForceFirst2GB;
-    }
-
-    this->mVMPolicy.Reserve(aNumHooks, resFlags);
+    this->mVMPolicy.Reserve(aNumHooks);
   }
 
   bool Initialized() const
   {
     return !!this->mVMPolicy;
   }
 
   bool AddHook(FARPROC aTargetFn, intptr_t aHookDest, void** aOrigFunc)
@@ -617,47 +540,31 @@ protected:
     // The trampoline is a copy of the instructions that we just traced,
     // followed by a jump that we add below.
     tramp.CopyFrom(origBytes.GetBaseAddress(), origBytes.GetOffset());
     if (!tramp) {
       return;
     }
 #elif defined(_M_X64)
     bool foundJmp = false;
-    // |use10BytePatch| should always default to |false| in production. It is
-    // not set to true unless we detect that a 10-byte patch is necessary.
-    // OTOH, for testing purposes, if we want to force a 10-byte patch, we
-    // always initialize |use10BytePatch| to |true|.
-    bool use10BytePatch = (mFlags & DetourFlags::eTestOnlyForce10BytePatch) ==
-                          DetourFlags::eTestOnlyForce10BytePatch;
-    const uint32_t bytesRequired = use10BytePatch ? 10 : 13;
 
-    while (origBytes.GetOffset() < bytesRequired) {
+    while (origBytes.GetOffset() < 13) {
       // If we found JMP 32bit offset, we require that the next bytes must
       // be NOP or INT3.  There is no reason to copy them.
       // TODO: This used to trigger for Je as well.  Now that I allow
       // instructions after CALL and JE, I don't think I need that.
       // The only real value of this condition is that if code follows a JMP
       // then its _probably_ the target of a JMP somewhere else and we
       // will be overwriting it, which would be tragic.  This seems
       // highly unlikely.
       if (foundJmp) {
         if (*origBytes == 0x90 || *origBytes == 0xcc) {
           ++origBytes;
           continue;
         }
-
-        // If our trampoline space is located in the lowest 2GB, we can do a ten
-        // byte patch instead of a thirteen byte patch.
-        if (this->mVMPolicy.IsTrampolineSpaceInLowest2GB() &&
-            origBytes.GetOffset() >= 10) {
-          use10BytePatch = true;
-          break;
-        }
-
         MOZ_ASSERT_UNREACHABLE("Opcode sequence includes commands after JMP");
         return;
       }
       if (*origBytes == 0x0f) {
         COPY_CODES(1);
         if (*origBytes == 0x1f) {
           // nop (multibyte)
           COPY_CODES(1);
@@ -1038,28 +945,16 @@ protected:
           uintptr_t jmpDest = origBytes.ChasePointerFromDisp();
 
           if (!GenerateJump(tramp, jmpDest, JumpType::Jmp)) {
             return;
           }
         } else if ((origBytes[1] & (kMaskMod|kMaskReg)) == BuildModRmByte(kModReg, 2, 0)) {
           // CALL reg (ff nn)
           COPY_CODES(2);
-        } else if (((origBytes[1] & kMaskReg) >> kRegFieldShift) == 4) {
-          // JMP r/m
-          int len = CountModRmSib(origBytes + 1);
-          if (len < 0) {
-            // RIP-relative not yet supported
-            MOZ_ASSERT_UNREACHABLE("Unrecognized opcode sequence");
-            return;
-          }
-
-          COPY_CODES(len + 1);
-
-          foundJmp = true;
         } else {
           MOZ_ASSERT_UNREACHABLE("Unrecognized opcode sequence");
           return;
         }
       } else if (*origBytes == 0x83 &&
                  (origBytes[1] & 0xf8) == 0x60) {
         // and [r+d], imm8
         COPY_CODES(5);
@@ -1105,94 +1000,43 @@ protected:
     if (!foundJmp) {
       if (!GenerateJump(tramp, origBytes.GetAddress(), JumpType::Jmp)) {
         return;
       }
     }
 #endif
 
     // The trampoline is now complete.
-    void* trampPtr = tramp.EndExecutableCode();
-    if (!trampPtr) {
+    *aOutTramp = tramp.EndExecutableCode();
+    if (!(*aOutTramp)) {
       return;
     }
 
     WritableTargetFunction<MMPolicyT> target(origBytes.Promote());
     if (!target) {
       return;
     }
 
 #if defined(_M_IX86)
     // now modify the original bytes
     target.WriteByte(0xe9); //jmp
     target.WriteDisp32(aDest); // hook displacement
 #elif defined(_M_X64)
-    if (use10BytePatch) {
-      // Okay, now we can write the actual tramp.
-      // Note: Even if the target function is also below 2GB, we still use an
-      // intermediary trampoline so that we consistently have a 64-bit pointer
-      // that we can use to reset the trampoline upon interceptor shutdown.
-      Trampoline<MMPolicyT> callTramp(this->mVMPolicy.GetNextTrampoline());
-      if (!callTramp) {
-        return;
-      }
-
-      // Write a null instance so that Clear() does not consider this tramp to
-      // be a normal tramp to be torn down.
-      callTramp.WriteEncodedPointer(nullptr);
-      // Use the second pointer slot to store a pointer to the primary tramp
-      callTramp.WriteEncodedPointer(trampPtr);
-      callTramp.StartExecutableCode();
-
-      // mov r11, address
-      callTramp.WriteByte(0x49);
-      callTramp.WriteByte(0xbb);
-      callTramp.WritePointer(aDest);
-
-      // jmp r11
-      callTramp.WriteByte(0x41);
-      callTramp.WriteByte(0xff);
-      callTramp.WriteByte(0xe3);
+    // mov r11, address
+    target.WriteByte(0x49);
+    target.WriteByte(0xbb);
+    target.WritePointer(aDest);
 
-      void* callTrampStart = callTramp.EndExecutableCode();
-      if (!callTrampStart) {
-        return;
-      }
-
-      target.WriteByte(0xB8); // MOV EAX, IMM32
-
-      // Assert that the topmost 33 bits are 0
-      MOZ_ASSERT(!(reinterpret_cast<uintptr_t>(callTrampStart) & (~0x7FFFFFFFULL)));
-
-      target.WriteLong(static_cast<uint32_t>(reinterpret_cast<uintptr_t>(callTrampStart) & 0x7FFFFFFFU));
-      target.WriteByte(0x48); // REX.W
-      target.WriteByte(0x63); // MOVSXD r64, r/m32
-      // dest: rax, src: eax
-      target.WriteByte(BuildModRmByte(kModReg, kRegAx, kRegAx));
-      target.WriteByte(0xFF); // JMP /4
-      target.WriteByte(BuildModRmByte(kModReg, 4, kRegAx)); // rax
-    } else {
-      // mov r11, address
-      target.WriteByte(0x49);
-      target.WriteByte(0xbb);
-      target.WritePointer(aDest);
-
-      // jmp r11
-      target.WriteByte(0x41);
-      target.WriteByte(0xff);
-      target.WriteByte(0xe3);
-    }
+    // jmp r11
+    target.WriteByte(0x41);
+    target.WriteByte(0xff);
+    target.WriteByte(0xe3);
 #endif
 
-    if (!target.Commit()) {
-      return;
-    }
-
-    // Output the trampoline, thus signalling that this call was a success
-    *aOutTramp = trampPtr;
+    target.Commit();
   }
 };
 
 } // namespace interceptor
 } // namespace mozilla
 
 #endif // mozilla_interceptor_PatcherDetour_h
 
--- a/mozglue/misc/interceptor/TargetFunction.h
+++ b/mozglue/misc/interceptor/TargetFunction.h
@@ -237,58 +237,16 @@ public:
       return Nothing();
     }
 
     mOffset += sizeof(uint8_t);
     mStartWriteOffset += sizeof(uint8_t);
     return Some(value);
   }
 
-  Maybe<uintptr_t> ReadEncodedPtr()
-  {
-    // Reading is only permitted prior to any writing
-    MOZ_ASSERT(mOffset == mStartWriteOffset);
-    if (mOffset > mStartWriteOffset) {
-      mAccumulatedStatus = false;
-      return Nothing();
-    }
-
-    uintptr_t value;
-    if (!mMMPolicy.Read(&value, reinterpret_cast<const void*>(mFunc + mOffset),
-                        sizeof(uintptr_t))) {
-      mAccumulatedStatus = false;
-      return Nothing();
-    }
-
-    mOffset += sizeof(uintptr_t);
-    mStartWriteOffset += sizeof(uintptr_t);
-    return Some(ReadOnlyTargetFunction<MMPolicy>::DecodePtr(value));
-  }
-
-  Maybe<uint32_t> ReadLong()
-  {
-    // Reading is only permitted prior to any writing
-    MOZ_ASSERT(mOffset == mStartWriteOffset);
-    if (mOffset > mStartWriteOffset) {
-      mAccumulatedStatus = false;
-      return Nothing();
-    }
-
-    uint32_t value;
-    if (!mMMPolicy.Read(&value, reinterpret_cast<const void*>(mFunc + mOffset),
-                        sizeof(uint32_t))) {
-      mAccumulatedStatus = false;
-      return Nothing();
-    }
-
-    mOffset += sizeof(uint32_t);
-    mStartWriteOffset += sizeof(uint32_t);
-    return Some(value);
-  }
-
   void WriteShort(const uint16_t& aValue)
   {
     if (!mLocalBytes.append(reinterpret_cast<const uint8_t*>(&aValue),
                             sizeof(uint16_t))) {
       mAccumulatedStatus = false;
       return;
     }
 
@@ -340,29 +298,16 @@ public:
     if (!mLocalBytes.append(reinterpret_cast<uint8_t*>(&disp), sizeof(int32_t))) {
       mAccumulatedStatus = false;
       return;
     }
 
     mOffset += sizeof(int32_t);
   }
 
-#if defined(_M_X64)
-  void WriteLong(const uint32_t aValue)
-  {
-    if (!mLocalBytes.append(reinterpret_cast<const uint8_t*>(&aValue),
-                            sizeof(uint32_t))) {
-      mAccumulatedStatus = false;
-      return;
-    }
-
-    mOffset += sizeof(uint32_t);
-  }
-#endif // defined(_M_X64)
-
   void WritePointer(const uintptr_t aAbsTarget)
   {
     if (!mLocalBytes.append(reinterpret_cast<const uint8_t*>(&aAbsTarget),
                             sizeof(uintptr_t))) {
       mAccumulatedStatus = false;
       return;
     }
 
--- a/mozglue/misc/interceptor/VMSharingPolicies.h
+++ b/mozglue/misc/interceptor/VMSharingPolicies.h
@@ -19,20 +19,20 @@ class VMSharingPolicyUnique : public MMP
 public:
   template <typename... Args>
   explicit VMSharingPolicyUnique(Args... aArgs)
     : MMPolicy(std::forward<Args>(aArgs)...)
     , mNextChunkIndex(0)
   {
   }
 
-  bool Reserve(uint32_t aCount, const ReservationFlags aFlags)
+  bool Reserve(uint32_t aCount)
   {
     MOZ_ASSERT(aCount);
-    uint32_t bytesReserved = MMPolicy::Reserve(aCount * kChunkSize, aFlags);
+    uint32_t bytesReserved = MMPolicy::Reserve(aCount * kChunkSize);
     return !!bytesReserved;
   }
 
   Trampoline<MMPolicy> GetNextTrampoline()
   {
     uint32_t offset = mNextChunkIndex * kChunkSize;
     if (!this->MaybeCommitNextPage(offset, kChunkSize)) {
       return nullptr;
@@ -122,36 +122,28 @@ public:
   }
 
   bool ShouldUnhookUponDestruction() const
   {
     AutoCriticalSection lock(&sCS);
     return sUniqueVM.ShouldUnhookUponDestruction();
   }
 
-  bool Reserve(uint32_t aCount, const ReservationFlags aFlags)
+  bool Reserve(uint32_t aCount)
   {
     AutoCriticalSection lock(&sCS);
-    return sUniqueVM.Reserve(aCount, aFlags);
+    return sUniqueVM.Reserve(aCount);
   }
 
   bool IsPageAccessible(void* aVAddress) const
   {
     AutoCriticalSection lock(&sCS);
     return sUniqueVM.IsPageAccessible(aVAddress);
   }
 
-#if defined(_M_X64)
-  bool IsTrampolineSpaceInLowest2GB() const
-  {
-    AutoCriticalSection lock(&sCS);
-    return sUniqueVM.IsTrampolineSpaceInLowest2GB();
-  }
-#endif // defined(_M_X64)
-
   Trampoline<MMPolicyInProcess> GetNextTrampoline()
   {
     AutoCriticalSection lock(&sCS);
     return sUniqueVM.GetNextTrampoline();
   }
 
   TrampolineCollection<MMPolicyInProcess> Items() const
   {
--- a/mozglue/misc/nsWindowsDllInterceptor.h
+++ b/mozglue/misc/nsWindowsDllInterceptor.h
@@ -366,28 +366,16 @@ public:
     if (mModule) {
       return;
     }
 
     mModule = ::LoadLibraryW(aModuleName);
     mNHooks = aNumHooks;
   }
 
-  /** Force a specific configuration for testing purposes. NOT to be used in
-      production code! **/
-  void TestOnlyDetourInit(const wchar_t* aModuleName, DetourFlags aFlags,
-                          int aNumHooks = 0)
-  {
-    Init(aModuleName, aNumHooks);
-
-    if (!mDetourPatcher.Initialized()) {
-      mDetourPatcher.Init(aFlags, mNHooks);
-    }
-  }
-
   void Clear()
   {
     if (!mModule) {
       return;
     }
 
 #if defined(_M_IX86)
     mNopSpacePatcher.Clear();
@@ -462,26 +450,17 @@ private:
 
 #if defined(_M_ARM64)
     // XXX: this is just to get things compiling; we'll have to add real
     // support at some future point.
     return false;
 #endif
 
     if (!mDetourPatcher.Initialized()) {
-      DetourFlags flags = DetourFlags::eDefault;
-#if defined(_M_X64)
-      if (mModule == ::GetModuleHandleW(L"ntdll.dll")) {
-        // NTDLL hooks should attempt to use a 10-byte patch because some
-        // injected DLLs do the same and interfere with our stuff.
-        flags |= DetourFlags::eEnable10BytePatch;
-      }
-#endif // defined(_M_X64)
-
-      mDetourPatcher.Init(flags, mNHooks);
+      mDetourPatcher.Init(mNHooks);
     }
 
     return mDetourPatcher.AddHook(aProc, aHookDest, aOrigFunc);
   }
 
 private:
   template <typename InterceptorT, typename FuncPtrT>
   friend class FuncHook;
--- a/mozglue/tests/interceptor/TestDllInterceptor.cpp
+++ b/mozglue/tests/interceptor/TestDllInterceptor.cpp
@@ -27,30 +27,16 @@ NTSTATUS NTAPI NtWriteFile(HANDLE, HANDL
                            PIO_STATUS_BLOCK, PVOID, ULONG,
                            PLARGE_INTEGER, PULONG);
 NTSTATUS NTAPI NtWriteFileGather(HANDLE, HANDLE, PIO_APC_ROUTINE, PVOID,
                                  PIO_STATUS_BLOCK, PFILE_SEGMENT_ELEMENT, ULONG,
                                  PLARGE_INTEGER, PULONG);
 NTSTATUS NTAPI NtQueryFullAttributesFile(POBJECT_ATTRIBUTES, PVOID);
 NTSTATUS NTAPI LdrLoadDll(PWCHAR filePath, PULONG flags, PUNICODE_STRING moduleFileName, PHANDLE handle);
 NTSTATUS NTAPI LdrUnloadDll(HMODULE);
-
-enum SECTION_INHERIT
-{
-  ViewShare = 1,
-  ViewUnmap = 2
-};
-
-NTSTATUS NTAPI
-NtMapViewOfSection(HANDLE aSection, HANDLE aProcess, PVOID* aBaseAddress,
-                   ULONG_PTR aZeroBits, SIZE_T aCommitSize,
-                   PLARGE_INTEGER aSectionOffset, PSIZE_T aViewSize,
-                   SECTION_INHERIT aInheritDisposition, ULONG aAllocationType,
-                   ULONG aProtectionFlags);
-
 // These pointers are disguised as PVOID to avoid pulling in obscure headers
 PVOID NTAPI LdrResolveDelayLoadedAPI(PVOID, PVOID, PVOID, PVOID, PVOID, ULONG);
 void CALLBACK ProcessCaretEvents(HWINEVENTHOOK, DWORD, HWND, LONG, LONG, DWORD, DWORD);
 void __fastcall BaseThreadInitThunk(BOOL aIsInitialThread, void* aStartAddress, void* aThreadParam);
 
 using namespace mozilla;
 
 struct payload {
@@ -91,33 +77,33 @@ decltype(auto) Apply(CallableT&& aFunc, 
 {
   return std::forward<CallableT>(aFunc)(Get<Indices>(std::forward<ArgTuple>(aArgs))...);
 }
 
 template <typename CallableT>
 bool TestFunction(CallableT aFunc);
 
 #define DEFINE_TEST_FUNCTION(calling_convention) \
-  template <template <typename I, typename F> class FuncHookT, typename InterceptorT, typename R, typename... Args, typename... TestArgs> \
-  bool TestFunction(FuncHookT<InterceptorT, R (calling_convention *)(Args...)>&& aFunc, \
+  template <typename R, typename... Args, typename... TestArgs> \
+  bool TestFunction(WindowsDllInterceptor::FuncHookType<R (calling_convention *)(Args...)>&& aFunc, \
                     bool (* aPred)(R), TestArgs... aArgs) \
   { \
-    using FuncHookType = FuncHookT<InterceptorT, R (calling_convention *)(Args...)>; \
+    using FuncHookType = WindowsDllInterceptor::FuncHookType<R (calling_convention *)(Args...)>; \
     using ArgTuple = Tuple<Args...>; \
     using Indices = std::index_sequence_for<Args...>; \
     ArgTuple fakeArgs{ std::forward<TestArgs>(aArgs)... }; \
     return aPred(Apply(std::forward<FuncHookType>(aFunc), std::forward<ArgTuple>(fakeArgs), Indices())); \
   } \
   \
   /* Specialization for functions returning void */ \
-  template <template <typename I, typename F> class FuncHookT, typename InterceptorT, typename... Args, typename PredicateT, typename... TestArgs> \
-  bool TestFunction(FuncHookT<InterceptorT, void (calling_convention *)(Args...)>&& aFunc, \
+  template <typename... Args, typename PredicateT, typename... TestArgs> \
+  bool TestFunction(WindowsDllInterceptor::FuncHookType<void (calling_convention *)(Args...)>&& aFunc, \
                     PredicateT&& aPred, TestArgs... aArgs) \
   { \
-    using FuncHookType = FuncHookT<InterceptorT, void (calling_convention *)(Args...)>; \
+    using FuncHookType = WindowsDllInterceptor::FuncHookType<void (calling_convention *)(Args...)>; \
     using ArgTuple = Tuple<Args...>; \
     using Indices = std::index_sequence_for<Args...>; \
     ArgTuple fakeArgs{ std::forward<TestArgs>(aArgs)... }; \
     Apply(std::forward<FuncHookType>(aFunc), std::forward<ArgTuple>(fakeArgs), Indices()); \
     return true; \
   }
 
 // C++11 allows empty arguments to macros. clang works just fine. MSVC does the
@@ -130,21 +116,21 @@ DEFINE_TEST_FUNCTION()
 
 #ifdef _M_IX86
 DEFINE_TEST_FUNCTION(__stdcall)
 DEFINE_TEST_FUNCTION(__fastcall)
 #endif // _M_IX86
 
 // Test the hooked function against the supplied predicate
 template <typename OrigFuncT, typename PredicateT, typename... Args>
-bool CheckHook(OrigFuncT &aOrigFunc,
+bool CheckHook(WindowsDllInterceptor::FuncHookType<OrigFuncT> &aOrigFunc,
                const char* aDllName, const char* aFuncName, PredicateT&& aPred,
                Args... aArgs)
 {
-  if (TestFunction(std::forward<OrigFuncT>(aOrigFunc), std::forward<PredicateT>(aPred), std::forward<Args>(aArgs)...)) {
+  if (TestFunction(std::forward<WindowsDllInterceptor::FuncHookType<OrigFuncT>>(aOrigFunc), std::forward<PredicateT>(aPred), std::forward<Args>(aArgs)...)) {
     printf("TEST-PASS | WindowsDllInterceptor | "
            "Executed hooked function %s from %s\n", aFuncName, aDllName);
     return true;
   }
   printf("TEST-FAILED | WindowsDllInterceptor | "
          "Failed to execute hooked function %s from %s\n", aFuncName, aDllName);
   return false;
 }
@@ -408,80 +394,16 @@ bool ShouldTestTipTsf()
   if (!LoadLibraryW(fullPath)) {
     return false;
   }
 
   // Leak the module so that it's loaded for the interceptor test
   return true;
 }
 
-#if defined(_M_X64)
-
-// Use VMSharingPolicyUnique for the TenByteInterceptor, as it needs to
-// reserve its trampoline memory in a special location.
-using TenByteInterceptor =
-  mozilla::interceptor::WindowsDllInterceptor<
-    mozilla::interceptor::VMSharingPolicyUnique<
-      mozilla::interceptor::MMPolicyInProcess,
-      mozilla::interceptor::kDefaultTrampolineSize>>;
-
-static TenByteInterceptor::FuncHookType<decltype(&::NtMapViewOfSection)>
-  orig_NtMapViewOfSection;
-
-#endif // defined(_M_X64)
-
-bool
-TestTenByteDetour()
-{
-#if defined(_M_X64)
-  auto pNtMapViewOfSection =
-    reinterpret_cast<decltype(&::NtMapViewOfSection)>(
-      ::GetProcAddress(::GetModuleHandleW(L"ntdll.dll"), "NtMapViewOfSection"));
-  if (!pNtMapViewOfSection) {
-    printf("TEST-FAILED | WindowsDllInterceptor | "
-           "Failed to resolve ntdll!NtMapViewOfSection\n");
-    return false;
-  }
-
-  { // Scope for tenByteInterceptor
-    TenByteInterceptor tenByteInterceptor;
-    tenByteInterceptor.TestOnlyDetourInit(L"ntdll.dll",
-      mozilla::interceptor::DetourFlags::eTestOnlyForce10BytePatch);
-    if (!orig_NtMapViewOfSection.SetDetour(tenByteInterceptor,
-                                           "NtMapViewOfSection", nullptr)) {
-      printf("TEST-FAILED | WindowsDllInterceptor | "
-             "Failed to hook ntdll!NtMapViewOfSection via 10-byte patch\n");
-      return false;
-    }
-
-    auto pred = &Predicates<decltype(&::NtMapViewOfSection)>::Ignore<((NTSTATUS)0)>;
-
-    if (!CheckHook(orig_NtMapViewOfSection, "ntdll.dll", "NtMapViewOfSection", pred)) {
-      // CheckHook has already printed the error message for us
-      return false;
-    }
-  }
-
-  // Now ensure that our hook cleanup worked
-  NTSTATUS status = pNtMapViewOfSection(nullptr, nullptr, nullptr, 0, 0, nullptr,
-                                        nullptr, ((SECTION_INHERIT)0), 0, 0);
-  if (NT_SUCCESS(status)) {
-    printf("TEST-FAILED | WindowsDllInterceptor | "
-           "Unexpected successful call to ntdll!NtMapViewOfSection after removing 10-byte hook\n");
-    return false;
-  }
-
-  printf("TEST-PASS | WindowsDllInterceptor | "
-         "Successfully unhooked ntdll!NtMapViewOfSection via 10-byte patch\n");
-  return true;
-#else
-  return true;
-#endif
-}
-
 extern "C"
 int wmain(int argc, wchar_t* argv[])
 {
   LARGE_INTEGER start;
   QueryPerformanceCounter(&start);
 
   // We disable this part of the test because the code coverage instrumentation
   // injects code in rotatePayload in a way that WindowsDllInterceptor doesn't
@@ -610,18 +532,17 @@ int wmain(int argc, wchar_t* argv[])
       TEST_HOOK(wininet.dll, HttpEndRequestA, Equals, FALSE) &&
       TEST_HOOK(wininet.dll, InternetQueryOptionA, Equals, FALSE) &&
 
       TEST_HOOK(sspicli.dll, AcquireCredentialsHandleA, NotEquals, SEC_E_OK) &&
       TEST_HOOK(sspicli.dll, QueryCredentialsAttributesA, NotEquals, SEC_E_OK) &&
       TEST_HOOK(sspicli.dll, FreeCredentialsHandle, NotEquals, SEC_E_OK) &&
 
       TEST_DETOUR_SKIP_EXEC(kernel32.dll, BaseThreadInitThunk) &&
-      TEST_DETOUR_SKIP_EXEC(ntdll.dll, LdrLoadDll) &&
-      TestTenByteDetour()) {
+      TEST_DETOUR_SKIP_EXEC(ntdll.dll, LdrLoadDll)) {
     printf("TEST-PASS | WindowsDllInterceptor | all checks passed\n");
 
     LARGE_INTEGER end, freq;
     QueryPerformanceCounter(&end);
 
     QueryPerformanceFrequency(&freq);
 
     LARGE_INTEGER result;