Backed out changeset 6866be136e53 (bug 1592486) for cppunit failures on TestNativeNt.exe . CLOSED TREE
authorNarcis Beleuzu <nbeleuzu@mozilla.com>
Thu, 06 Feb 2020 11:53:45 +0200
changeset 512704 30d70d402f129680a93490db6898d933013b67b3
parent 512703 034b35b48e8d71b0cb157f500d05490ca0753e47
child 512705 182a8403e7d01fd6b18403102013ad3fd18f05ae
push id106572
push usernbeleuzu@mozilla.com
push dateThu, 06 Feb 2020 10:14:26 +0000
treeherderautoland@30d70d402f12 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1592486
milestone74.0a1
backs out6866be136e5361dd4adea3bbdd349443b086cbe8
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 changeset 6866be136e53 (bug 1592486) for cppunit failures on TestNativeNt.exe . CLOSED TREE
browser/app/winlauncher/freestanding/ModuleLoadFrame.cpp
browser/app/winlauncher/freestanding/ModuleLoadFrame.h
browser/app/winlauncher/freestanding/SafeThreadLocal.h
browser/app/winlauncher/test/TestSafeThreadLocal.cpp
browser/app/winlauncher/test/moz.build
mozglue/misc/NativeNt.h
mozglue/tests/TestNativeNt.cpp
testing/cppunittest.ini
--- a/browser/app/winlauncher/freestanding/ModuleLoadFrame.cpp
+++ b/browser/app/winlauncher/freestanding/ModuleLoadFrame.cpp
@@ -118,12 +118,12 @@ NTSTATUS ModuleLoadFrame::SetLoadStatus(
   if (!gLoaderPrivateAPI.SubstituteForLSP(mLoadInfo.mRequestedDllName,
                                           aOutHandle)) {
     return aNtStatus;
   }
 
   return STATUS_SUCCESS;
 }
 
-SafeThreadLocal<ModuleLoadFrame*> ModuleLoadFrame::sTopFrame;
+MOZ_THREAD_LOCAL(ModuleLoadFrame*) ModuleLoadFrame::sTopFrame;
 
 }  // namespace freestanding
 }  // namespace mozilla
--- a/browser/app/winlauncher/freestanding/ModuleLoadFrame.h
+++ b/browser/app/winlauncher/freestanding/ModuleLoadFrame.h
@@ -6,21 +6,25 @@
 
 #ifndef mozilla_freestanding_ModuleLoadFrame_h
 #define mozilla_freestanding_ModuleLoadFrame_h
 
 #include "mozilla/LoaderAPIInterfaces.h"
 #include "mozilla/NativeNt.h"
 #include "mozilla/ThreadLocal.h"
 
-#include "SafeThreadLocal.h"
-
 namespace mozilla {
 namespace freestanding {
 
+// We cannot fall back to the Tls* APIs because kernel32 might not have been
+// loaded yet.
+#if defined(__MINGW32__) && !defined(HAVE_THREAD_TLS_KEYWORD)
+#  error "This code requires the compiler to have native TLS support"
+#endif  // defined(__MINGW32__) && !defined(HAVE_THREAD_TLS_KEYWORD)
+
 /**
  * This class holds information about a DLL load at a particular frame in the
  * current thread's stack. Each instance adds itself to a thread-local linked
  * list of ModuleLoadFrames, enabling us to query information about the
  * previous module load on the stack.
  */
 class MOZ_RAII ModuleLoadFrame final {
  public:
@@ -76,15 +80,15 @@ class MOZ_RAII ModuleLoadFrame final {
   // Set to |true| when we need to block a WinSock LSP
   bool mLSPSubstitutionRequired;
   // NTSTATUS code from the |LdrLoadDll| call
   NTSTATUS mLoadNtStatus;
   // Telemetry information that will be forwarded to the nt::LoaderObserver
   ModuleLoadInfo mLoadInfo;
 
   // Head of the linked list
-  static SafeThreadLocal<ModuleLoadFrame*> sTopFrame;
+  static MOZ_THREAD_LOCAL(ModuleLoadFrame*) sTopFrame;
 };
 
 }  // namespace freestanding
 }  // namespace mozilla
 
 #endif  // mozilla_freestanding_ModuleLoadFrame_h
deleted file mode 100644
--- a/browser/app/winlauncher/freestanding/SafeThreadLocal.h
+++ /dev/null
@@ -1,95 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* 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_freestanding_SafeThreadLocal_h
-#define mozilla_freestanding_SafeThreadLocal_h
-
-#include "mozilla/NativeNt.h"
-#include "mozilla/ThreadLocal.h"
-#include "mozilla/TypeTraits.h"
-
-namespace mozilla {
-namespace freestanding {
-
-// We cannot fall back to the Tls* APIs because kernel32 might not have been
-// loaded yet.
-#if defined(__MINGW32__) && !defined(HAVE_THREAD_TLS_KEYWORD)
-#  error "This code requires the compiler to have native TLS support"
-#endif  // defined(__MINGW32__) && !defined(HAVE_THREAD_TLS_KEYWORD)
-
-/**
- * This class holds data as a thread-local variable, or as a global variable
- * if the thread local storage is not initialized yet.  It should be safe
- * because in that early stage we assume there is no more than a single thread.
- */
-template <typename T>
-class SafeThreadLocal final {
-  static MOZ_THREAD_LOCAL(T) sThreadLocal;
-  static T sGlobal;
-  static bool sIsTlsUsed;
-
-  // In normal cases, TLS is always available and the class uses sThreadLocal
-  // without changing sMainThreadId.  So sMainThreadId is likely to be 0.
-  //
-  // If TLS is not available, we use sGlobal instead and update sMainThreadId
-  // so that that thread keeps using sGlobal even after TLS is initialized
-  // later.
-  static DWORD sMainThreadId;
-
-  // Need non-inline accessors to prevent the compiler from generating code
-  // accessing sThreadLocal before checking a condition.
-  MOZ_NEVER_INLINE static void SetGlobalValue(T aValue) { sGlobal = aValue; }
-  MOZ_NEVER_INLINE static T GetGlobalValue() { return sGlobal; }
-
- public:
-  static void set(T aValue) {
-    static_assert(mozilla::IsPointer<T>::value,
-                  "SafeThreadLocal must be used with a pointer");
-
-    if (sMainThreadId == mozilla::nt::RtlGetCurrentThreadId()) {
-      SetGlobalValue(aValue);
-    } else if (sIsTlsUsed) {
-      MOZ_ASSERT(mozilla::nt::RtlGetThreadLocalStoragePointer(),
-                 "Once TLS is used, TLS should be available till the end.");
-      sThreadLocal.set(aValue);
-    } else if (mozilla::nt::RtlGetThreadLocalStoragePointer()) {
-      sIsTlsUsed = true;
-      sThreadLocal.set(aValue);
-    } else {
-      MOZ_ASSERT(sMainThreadId == 0,
-                 "A second thread cannot be created before TLS is available.");
-      sMainThreadId = mozilla::nt::RtlGetCurrentThreadId();
-      SetGlobalValue(aValue);
-    }
-  }
-
-  static T get() {
-    if (sMainThreadId == mozilla::nt::RtlGetCurrentThreadId()) {
-      return GetGlobalValue();
-    } else if (sIsTlsUsed) {
-      return sThreadLocal.get();
-    }
-    return GetGlobalValue();
-  }
-};
-
-template <typename T>
-MOZ_THREAD_LOCAL(T)
-SafeThreadLocal<T>::sThreadLocal;
-
-template <typename T>
-T SafeThreadLocal<T>::sGlobal = nullptr;
-
-template <typename T>
-bool SafeThreadLocal<T>::sIsTlsUsed = false;
-
-template <typename T>
-DWORD SafeThreadLocal<T>::sMainThreadId = 0;
-
-}  // namespace freestanding
-}  // namespace mozilla
-
-#endif  // mozilla_freestanding_SafeThreadLocal_h
deleted file mode 100644
--- a/browser/app/winlauncher/test/TestSafeThreadLocal.cpp
+++ /dev/null
@@ -1,64 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* 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/. */
-
-#include "freestanding/SafeThreadLocal.h"
-
-#include "mozilla/NativeNt.h"
-#include "nsWindowsHelpers.h"
-
-#include <process.h>
-#include <stdio.h>
-
-static mozilla::freestanding::SafeThreadLocal<int*> gTheStorage;
-
-static unsigned int __stdcall TestNonMainThread(void* aArg) {
-  for (int i = 0; i < 100; ++i) {
-    gTheStorage.set(&i);
-    if (gTheStorage.get() != &i) {
-      printf(
-          "TEST-FAILED | TestSafeThreadLocal | "
-          "A value is not correctly stored in the thread-local storage.\n");
-      return 1;
-    }
-  }
-  return 0;
-}
-
-extern "C" int wmain(int argc, wchar_t* argv[]) {
-  int dummy = 0x1234;
-
-  auto origHead = mozilla::nt::RtlGetThreadLocalStoragePointer();
-  mozilla::nt::RtlSetThreadLocalStoragePointerForTestingOnly(nullptr);
-  // Setting gTheStorage when TLS is null.
-  gTheStorage.set(&dummy);
-  mozilla::nt::RtlSetThreadLocalStoragePointerForTestingOnly(origHead);
-
-  nsAutoHandle handles[8];
-  for (auto& handle : handles) {
-    handle.own(reinterpret_cast<HANDLE>(
-        ::_beginthreadex(nullptr, 0, TestNonMainThread, nullptr, 0, nullptr)));
-  }
-
-  for (int i = 0; i < 100; ++i) {
-    if (gTheStorage.get() != &dummy) {
-      printf(
-          "TEST-FAILED | TestSafeThreadLocal | "
-          "A value is not correctly stored in the global scope.\n");
-      return 1;
-    }
-  }
-
-  for (auto& handle : handles) {
-    ::WaitForSingleObject(handle, INFINITE);
-
-    DWORD exitCode;
-    if (!::GetExitCodeThread(handle, &exitCode) || exitCode) {
-      return 1;
-    }
-  }
-
-  return 0;
-}
--- a/browser/app/winlauncher/test/moz.build
+++ b/browser/app/winlauncher/test/moz.build
@@ -3,17 +3,16 @@
 # 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 http://mozilla.org/MPL/2.0/.
 
 DisableStlWrapping()
 
 GeckoCppUnitTests(
     [
-        'TestSafeThreadLocal',
         'TestSameBinary',
     ],
     linkage=None
 )
 
 LOCAL_INCLUDES += [
     '/browser/app/winlauncher',
 ]
--- a/mozglue/misc/NativeNt.h
+++ b/mozglue/misc/NativeNt.h
@@ -931,24 +931,16 @@ class MOZ_RAII PEHeaders final {
 };
 
 inline HANDLE RtlGetProcessHeap() {
   PTEB teb = ::NtCurrentTeb();
   PPEB peb = teb->ProcessEnvironmentBlock;
   return peb->Reserved4[1];
 }
 
-inline PVOID RtlGetThreadLocalStoragePointer() {
-  return ::NtCurrentTeb()->Reserved1[11];
-}
-
-inline void RtlSetThreadLocalStoragePointerForTestingOnly(PVOID aNewValue) {
-  ::NtCurrentTeb()->Reserved1[11] = aNewValue;
-}
-
 inline DWORD RtlGetCurrentThreadId() {
   PTEB teb = ::NtCurrentTeb();
   CLIENT_ID* cid = reinterpret_cast<CLIENT_ID*>(&teb->Reserved1[8]);
   return static_cast<DWORD>(reinterpret_cast<uintptr_t>(cid->UniqueThread) &
                             0xFFFFFFFFUL);
 }
 
 inline LauncherResult<DWORD> GetParentProcessId() {
--- a/mozglue/tests/TestNativeNt.cpp
+++ b/mozglue/tests/TestNativeNt.cpp
@@ -1,34 +1,31 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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/. */
 
 #include "mozilla/NativeNt.h"
-#include "mozilla/ThreadLocal.h"
 #include "mozilla/UniquePtr.h"
 
 #include <stdio.h>
 #include <windows.h>
 
 const wchar_t kNormal[] = L"Foo.dll";
 const wchar_t kHex12[] = L"Foo.ABCDEF012345.dll";
 const wchar_t kHex15[] = L"ABCDEF012345678.dll";
 const wchar_t kHex16[] = L"ABCDEF0123456789.dll";
 const wchar_t kHex17[] = L"ABCDEF0123456789a.dll";
 const wchar_t kHex24[] = L"ABCDEF0123456789cdabef98.dll";
 const wchar_t kHex8[] = L"01234567.dll";
 const wchar_t kNonHex12[] = L"Foo.ABCDEFG12345.dll";
 const wchar_t kHex13[] = L"Foo.ABCDEF0123456.dll";
 const wchar_t kHex11[] = L"Foo.ABCDEF01234.dll";
 const wchar_t kPrefixedHex16[] = L"Pabcdef0123456789.dll";
-const uint32_t kTlsDataValue = 1234;
-static MOZ_THREAD_LOCAL(uint32_t) sTlsData;
 
 const char kFailFmt[] =
     "TEST-FAILED | NativeNt | %s(%s) should have returned %s but did not\n";
 
 #define RUN_TEST(fn, varName, expected)         \
   if (fn(varName) == !expected) {               \
     printf(kFailFmt, #fn, #varName, #expected); \
     return 1;                                   \
@@ -36,25 +33,16 @@ const char kFailFmt[] =
 
 #define EXPECT_FAIL(fn, varName) RUN_TEST(fn, varName, false)
 
 #define EXPECT_SUCCESS(fn, varName) RUN_TEST(fn, varName, true)
 
 using namespace mozilla;
 using namespace mozilla::nt;
 
-// Need a non-inline function to bypass compiler optimization that the thread
-// local storage pointer is cached in a register before accessing a thread-local
-// variable.
-MOZ_NEVER_INLINE PVOID SwapThreadLocalStoragePointer(PVOID aNewValue) {
-  auto oldValue = RtlGetThreadLocalStoragePointer();
-  RtlSetThreadLocalStoragePointerForTestingOnly(aNewValue);
-  return oldValue;
-}
-
 int main(int argc, char* argv[]) {
   UNICODE_STRING normal;
   ::RtlInitUnicodeString(&normal, kNormal);
 
   UNICODE_STRING hex12;
   ::RtlInitUnicodeString(&hex12, kHex12);
 
   UNICODE_STRING hex16;
@@ -100,41 +88,16 @@ int main(int argc, char* argv[]) {
   EXPECT_FAIL(IsFileNameAtLeast16HexDigits, hex15);
   EXPECT_FAIL(IsFileNameAtLeast16HexDigits, prefixedHex16);
 
   if (RtlGetProcessHeap() != ::GetProcessHeap()) {
     printf("TEST-FAILED | NativeNt | RtlGetProcessHeap() is broken\n");
     return 1;
   }
 
-#ifdef HAVE_SEH_EXCEPTIONS
-  PVOID origTlsHead = nullptr;
-  bool isExceptionThrown = false;
-  MOZ_SEH_TRY {
-    // Need to call SwapThreadLocalStoragePointer inside __try to make sure
-    // accessing sTlsData is caught by SEH.  This is due to clang's design.
-    // https://bugs.llvm.org/show_bug.cgi?id=44174.
-    origTlsHead = SwapThreadLocalStoragePointer(nullptr);
-    sTlsData.set(~kTlsDataValue);
-  }
-  MOZ_SEH_EXCEPT(GetExceptionCode() == EXCEPTION_ACCESS_VIOLATION
-                     ? EXCEPTION_EXECUTE_HANDLER
-                     : EXCEPTION_CONTINUE_SEARCH) {
-    isExceptionThrown = true;
-  }
-  SwapThreadLocalStoragePointer(origTlsHead);
-  sTlsData.set(kTlsDataValue);
-  if (!isExceptionThrown || sTlsData.get() != kTlsDataValue) {
-    printf(
-        "TEST-FAILED | NativeNt | RtlGetThreadLocalStoragePointer() is "
-        "broken\n");
-    return 1;
-  }
-#endif
-
   if (RtlGetCurrentThreadId() != ::GetCurrentThreadId()) {
     printf("TEST-FAILED | NativeNt | RtlGetCurrentThreadId() is broken\n");
     return 1;
   }
 
   const wchar_t kKernel32[] = L"kernel32.dll";
   DWORD verInfoSize = ::GetFileVersionInfoSizeW(kKernel32, nullptr);
   if (!verInfoSize) {
--- a/testing/cppunittest.ini
+++ b/testing/cppunittest.ini
@@ -39,18 +39,16 @@ skip-if = os != 'win'
 [TestNativeNt]
 skip-if = os != 'win'
 [TestUriValidation]
 skip-if = os != 'win'
 [TestSameBinary]
 skip-if = os != 'win'
 [TestTimeStampWin]
 skip-if = os != 'win'
-[TestSafeThreadLocal]
-skip-if = os != 'win'
 [TestBaseProfiler]
 [TestNonDereferenceable]
 [TestNotNull]
 [TestParseFTPList]
 [TestPLDHash]
 [TestPair]
 [TestPoisonArea]
 skip-if = os == 'android' # Bug 1147630