Backed out changeset 11a4906850b2 (bug 1322554) for frequent crashes. a=jcristau
authorRyan VanderMeulen <ryanvm@gmail.com>
Wed, 12 Apr 2017 16:44:02 -0400
changeset 375886 e82a6f21f043
parent 375885 5c05900360e7
child 375887 a4b7a2a843e5
push id11051
push userryanvm@gmail.com
push dateWed, 12 Apr 2017 20:50:10 +0000
treeherdermozilla-aurora@cef7b5c139ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjcristau
bugs1322554
milestone54.0a2
backs out11a4906850b2
Backed out changeset 11a4906850b2 (bug 1322554) for frequent crashes. a=jcristau
mozglue/build/WindowsDllBlocklist.cpp
toolkit/xre/test/win/TestDllInterceptor.cpp
xpcom/build/nsWindowsDllInterceptor.h
--- a/mozglue/build/WindowsDllBlocklist.cpp
+++ b/mozglue/build/WindowsDllBlocklist.cpp
@@ -281,20 +281,16 @@ printf_stderr(const char *fmt, ...)
   vfprintf(fp, fmt, args);
   va_end(args);
 
   fclose(fp);
 }
 
 namespace {
 
-typedef void (__fastcall* BaseThreadInitThunk_func)(BOOL aIsInitialThread, void* aStartAddress, void* aThreadParam);
-
-static BaseThreadInitThunk_func stub_BaseThreadInitThunk = nullptr;
-
 typedef NTSTATUS (NTAPI *LdrLoadDll_func) (PWCHAR filePath, PULONG flags, PUNICODE_STRING moduleFileName, PHANDLE handle);
 
 static LdrLoadDll_func stub_LdrLoadDll = 0;
 
 template <class T>
 struct RVAMap {
   RVAMap(HANDLE map, DWORD offset) {
     SYSTEM_INFO info;
@@ -704,53 +700,17 @@ patched_LdrLoadDll (PWCHAR filePath, PUL
 continue_loading:
 #ifdef DEBUG_very_verbose
   printf_stderr("LdrLoadDll: continuing load... ('%S')\n", moduleFileName->Buffer);
 #endif
 
   return stub_LdrLoadDll(filePath, flags, moduleFileName, handle);
 }
 
-static bool
-ShouldBlockThread(void* aStartAddress)
-{
-  // Allows crashfirefox.exe to continue to work. Also if your threadproc is null, this crash is intentional.
-  if (aStartAddress == 0)
-    return false;
-
-  bool shouldBlock = false;
-  MEMORY_BASIC_INFORMATION startAddressInfo = {0};
-  if (VirtualQuery(aStartAddress, &startAddressInfo, sizeof(startAddressInfo))) {
-    shouldBlock |= startAddressInfo.State != MEM_COMMIT;
-    shouldBlock |= startAddressInfo.Protect != PAGE_EXECUTE_READ;
-  }
-
-  return shouldBlock;
-}
-
-// Allows blocked threads to still run normally through BaseThreadInitThunk, in case there's any magic there that we shouldn't skip.
-static DWORD WINAPI
-NopThreadProc(void* /* aThreadParam */)
-{
-  return 0;
-}
-
-static MOZ_NORETURN void __fastcall
-patched_BaseThreadInitThunk(BOOL aIsInitialThread, void* aStartAddress,
-                            void* aThreadParam)
-{
-  if (ShouldBlockThread(aStartAddress)) {
-    aStartAddress = NopThreadProc;
-  }
-
-  stub_BaseThreadInitThunk(aIsInitialThread, aStartAddress, aThreadParam);
-}
-
 WindowsDllInterceptor NtDllIntercept;
-WindowsDllInterceptor Kernel32DllIntercept;
 
 } // namespace
 
 MFBT_API void
 DllBlocklist_Initialize()
 {
   if (sBlocklistInitAttempted) {
     return;
@@ -776,26 +736,16 @@ DllBlocklist_Initialize()
   bool ok = NtDllIntercept.AddDetour("LdrLoadDll", reinterpret_cast<intptr_t>(patched_LdrLoadDll), (void**) &stub_LdrLoadDll);
 
   if (!ok) {
     sBlocklistInitFailed = true;
 #ifdef DEBUG
     printf_stderr("LdrLoadDll hook failed, no dll blocklisting active\n");
 #endif
   }
-
-  Kernel32DllIntercept.Init("kernel32.dll");
-  ok = Kernel32DllIntercept.AddHook("BaseThreadInitThunk",
-                                    reinterpret_cast<intptr_t>(patched_BaseThreadInitThunk),
-                                    (void**) &stub_BaseThreadInitThunk);
-  if (!ok) {
-#ifdef DEBUG
-    printf_stderr("BaseThreadInitThunk hook failed\n");
-#endif
-  }
 }
 
 MFBT_API void
 DllBlocklist_WriteNotes(HANDLE file)
 {
   DWORD nBytes;
 
   WriteFile(file, kBlockedDllsParameter, kBlockedDllsParameterLen, &nBytes, nullptr);
--- a/toolkit/xre/test/win/TestDllInterceptor.cpp
+++ b/toolkit/xre/test/win/TestDllInterceptor.cpp
@@ -463,16 +463,15 @@ int main()
       TestHook(TestGetOpenFileNameW, "comdlg32.dll", "GetOpenFileNameW") &&
 #ifdef _M_X64
       TestHook(TestGetKeyState, "user32.dll", "GetKeyState") &&    // see Bug 1316415
 #endif
       MaybeTestHook(ShouldTestTipTsf(), TestProcessCaretEvents, "tiptsf.dll", "ProcessCaretEvents") &&
 #ifdef _M_IX86
       TestHook(TestSendMessageTimeoutW, "user32.dll", "SendMessageTimeoutW") &&
 #endif
-      TestDetour("kernel32.dll", "BaseThreadInitThunk") &&
       TestDetour("ntdll.dll", "LdrLoadDll")) {
     printf("TEST-PASS | WindowsDllInterceptor | all checks passed\n");
     return 0;
   }
 
   return 1;
 }
--- a/xpcom/build/nsWindowsDllInterceptor.h
+++ b/xpcom/build/nsWindowsDllInterceptor.h
@@ -572,17 +572,16 @@ protected:
     return numBytes;
   }
 
 #if defined(_M_X64)
   // To patch for JMP and JE
 
   enum JumpType {
    Je,
-   Jne,
    Jmp,
    Call
   };
 
   struct JumpPatch {
     JumpPatch()
       : mHookOffset(0), mJumpAddress(0), mType(JumpType::Jmp)
     {
@@ -596,21 +595,16 @@ protected:
     size_t GenerateJump(uint8_t* aCode)
     {
       size_t offset = mHookOffset;
       if (mType == JumpType::Je) {
         // JNE RIP+14
         aCode[offset]     = 0x75;
         aCode[offset + 1] = 14;
         offset += 2;
-      } else if (mType == JumpType::Jne) {
-        // JE RIP+14
-        aCode[offset]     = 0x74;
-        aCode[offset + 1] = 14;
-        offset += 2;
       }
 
       // Near call/jmp, absolute indirect, address given in r/m32
       if (mType == JumpType::Call) {
         // CALL [RIP+0]
         aCode[offset] = 0xff;
         aCode[offset + 1] = 0x15;
         // The offset to jump destination -- ie it is placed 2 bytes after the offset.
@@ -884,18 +878,18 @@ protected:
 
         if (origBytes[nOrigBytes] == 0x33) {
           // xor r32, r32
           COPY_CODES(2);
         } else {
           MOZ_ASSERT_UNREACHABLE("Unrecognized opcode sequence");
           return;
         }
-      } else if ((origBytes[nOrigBytes] & 0xfa) == 0x48) {
-        // REX.W | REX.WR | REX.WRB | REX.WB
+      } else if ((origBytes[nOrigBytes] & 0xfb) == 0x48) {
+        // REX.W | REX.WR
         COPY_CODES(1);
 
         if (origBytes[nOrigBytes] == 0x81 &&
             (origBytes[nOrigBytes + 1] & 0xf8) == 0xe8) {
           // sub r, dword
           COPY_CODES(6);
         } else if (origBytes[nOrigBytes] == 0x83 &&
                    (origBytes[nOrigBytes + 1] & 0xf8) == 0xe8) {
@@ -1064,19 +1058,16 @@ protected:
         BYTE subOpcode = 0;
         int nModRmSibBytes = CountModRmSib(&origBytes[nOrigBytes + 1], &subOpcode);
         if (nModRmSibBytes < 0 || subOpcode != 0) {
           // Unsupported
           MOZ_ASSERT_UNREACHABLE("Unrecognized opcode sequence");
           return;
         }
         COPY_CODES(2 + nModRmSibBytes);
-      } else if (origBytes[nOrigBytes] == 0x85) {
-        // test r/m32, r32
-        COPY_CODES(2);
       } else if (origBytes[nOrigBytes] == 0xd1 &&
                   (origBytes[nOrigBytes+1] & kMaskMod) == kModReg) {
         // bit shifts/rotates : (SA|SH|RO|RC)(R|L) r32
         // (e.g. 0xd1 0xe0 is SAL, 0xd1 0xc8 is ROR)
         COPY_CODES(2);
       } else if (origBytes[nOrigBytes] == 0xc3) {
         // ret
         COPY_CODES(1);
@@ -1088,40 +1079,30 @@ protected:
         // CALL (0xe8) or JMP (0xe9) 32bit offset
         foundJmp = origBytes[nOrigBytes] == 0xe9;
         JumpPatch jump(nTrampBytes,
                        (intptr_t)(origBytes + nOrigBytes + 5 +
                                   *(reinterpret_cast<int32_t*>(origBytes + nOrigBytes + 1))),
                        origBytes[nOrigBytes] == 0xe8 ? JumpType::Call : JumpType::Jmp);
         nTrampBytes = jump.GenerateJump(tramp);
         nOrigBytes += 5;
-      } else if (origBytes[nOrigBytes] == 0x75) {
-        // jne rel8
-        char offset = origBytes[nOrigBytes + 1];
-        JumpPatch jump(nTrampBytes, (intptr_t)(origBytes + nOrigBytes + 2 +
-                       offset), JumpType::Jne);
-        nTrampBytes = jump.GenerateJump(tramp);
-        nOrigBytes += 2;
       } else if (origBytes[nOrigBytes] == 0xff) {
         COPY_CODES(1);
         if ((origBytes[nOrigBytes] & (kMaskMod|kMaskReg)) == 0xf0) {
           // push r64
           COPY_CODES(1);
         } else if (origBytes[nOrigBytes] == 0x25) {
           // jmp absolute indirect m32
           foundJmp = true;
           int32_t offset = *(reinterpret_cast<int32_t*>(origBytes + nOrigBytes + 1));
           int64_t* ptrToJmpDest = reinterpret_cast<int64_t*>(origBytes + nOrigBytes + 5 + offset);
           intptr_t jmpDest = static_cast<intptr_t>(*ptrToJmpDest);
           JumpPatch jump(nTrampBytes, jmpDest, JumpType::Jmp);
           nTrampBytes = jump.GenerateJump(tramp);
           nOrigBytes += 5;
-        } else if ((origBytes[nOrigBytes] & (kMaskMod|kMaskReg)) == BuildModRmByte(kModReg, 2, 0)) {// (rm = xx010xxx) | (mod = 11xxxxxx)
-          // CALL reg (ff nn)
-          COPY_CODES(1);
         } else {
           MOZ_ASSERT_UNREACHABLE("Unrecognized opcode sequence");
           return;
         }
       } else {
         MOZ_ASSERT_UNREACHABLE("Unrecognized opcode sequence");
         return;
       }