Bug 1404147 Convert tid_t to integer on all platforms to resolve signed/unsigned comparison warnings draft
authorTom Ritter <tom@mozilla.com>
Tue, 03 Oct 2017 23:04:24 -0500
changeset 674602 2bb9a06bb7658c4aa30558e52571e74cce3b6071
parent 674381 135c068cf7b7f1076bf2ca5350ab32e3024fed2b
child 734390 bf41e25cd1a09eec59388bdb108d05b3b528a45f
push id82899
push userbmo:tom@mozilla.com
push dateWed, 04 Oct 2017 04:05:08 +0000
bugs1404147
milestone58.0a1
Bug 1404147 Convert tid_t to integer on all platforms to resolve signed/unsigned comparison warnings MozReview-Commit-ID: G5E3GpELkPs
tools/profiler/core/ProfileBufferEntry.cpp
tools/profiler/core/platform-linux-android.cpp
tools/profiler/core/platform-macos.cpp
tools/profiler/core/platform-win32.cpp
tools/profiler/core/platform.cpp
tools/profiler/core/platform.h
tools/profiler/tasktracer/GeckoTaskTracer.cpp
tools/profiler/tests/gtest/ThreadProfileTest.cpp
old mode 100644
new mode 100755
old mode 100644
new mode 100755
--- a/tools/profiler/core/platform-linux-android.cpp
+++ b/tools/profiler/core/platform-linux-android.cpp
@@ -64,17 +64,17 @@
 #include "mozilla/PodOperations.h"
 #include "mozilla/DebugOnly.h"
 
 #include <string.h>
 #include <list>
 
 using namespace mozilla;
 
-/* static */ Thread::tid_t
+/* static */ int
 Thread::GetCurrentId()
 {
   return gettid();
 }
 
 static void
 PopulateRegsFromContext(Registers& aRegs, ucontext_t* aContext)
 {
old mode 100644
new mode 100755
--- a/tools/profiler/core/platform-macos.cpp
+++ b/tools/profiler/core/platform-macos.cpp
@@ -26,17 +26,17 @@
 #include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
 #include <math.h>
 
 // this port is based off of v8 svn revision 9837
 
-/* static */ Thread::tid_t
+/* static */ int
 Thread::GetCurrentId()
 {
   return gettid();
 }
 
 class PlatformData
 {
 public:
old mode 100644
new mode 100755
--- a/tools/profiler/core/platform-win32.cpp
+++ b/tools/profiler/core/platform-win32.cpp
@@ -35,20 +35,22 @@
 #ifdef __MINGW32__
 #include <immintrin.h> // for _mm_pause
 #endif
 
 #include "nsWindowsDllInterceptor.h"
 #include "mozilla/StackWalk_windows.h"
 #include "mozilla/WindowsVersion.h"
 
-/* static */ Thread::tid_t
+/* static */ int
 Thread::GetCurrentId()
 {
-  return GetCurrentThreadId();
+  DWORD threadId = GetCurrentThreadId();
+  MOZ_ASSERT(threadId <= INT32_MAX, "native thread ID is > INT32_MAX");
+  return int(threadId);
 }
 
 static void
 PopulateRegsFromContext(Registers& aRegs, CONTEXT* aContext)
 {
 #if defined(GP_ARCH_amd64)
   aRegs.mPC = reinterpret_cast<Address>(aContext->Rip);
   aRegs.mSP = reinterpret_cast<Address>(aContext->Rsp);
old mode 100644
new mode 100755
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -2110,17 +2110,17 @@ ParseFeaturesFromStringArray(const char*
 
 // Find the ThreadInfo for the current thread. This should only be called in
 // places where TLSInfo can't be used. On success, *aIndexOut is set to the
 // index if it is non-null.
 static ThreadInfo*
 FindLiveThreadInfo(PSLockRef aLock, int* aIndexOut = nullptr)
 {
   ThreadInfo* ret = nullptr;
-  Thread::tid_t id = Thread::GetCurrentId();
+  int id = Thread::GetCurrentId();
   const CorePS::ThreadVector& liveThreads = CorePS::LiveThreads(aLock);
   for (uint32_t i = 0; i < liveThreads.size(); i++) {
     ThreadInfo* info = liveThreads.at(i);
     if (info->ThreadId() == id) {
       if (aIndexOut) {
         *aIndexOut = i;
       }
       ret = info;
@@ -2724,17 +2724,17 @@ locked_profiler_start(PSLockRef aLock, i
 
   // Fall back to the default values if the passed-in values are unreasonable.
   int entries = aEntries > 0 ? aEntries : PROFILER_DEFAULT_ENTRIES;
   double interval = aInterval > 0 ? aInterval : PROFILER_DEFAULT_INTERVAL;
 
   ActivePS::Create(aLock, entries, interval, aFeatures, aFilters, aFilterCount);
 
   // Set up profiling for each registered thread, if appropriate.
-  Thread::tid_t tid = Thread::GetCurrentId();
+  int tid = Thread::GetCurrentId();
   const CorePS::ThreadVector& liveThreads = CorePS::LiveThreads(aLock);
   for (uint32_t i = 0; i < liveThreads.size(); i++) {
     ThreadInfo* info = liveThreads.at(i);
 
     if (ActivePS::ShouldProfileThread(aLock, info)) {
       info->StartProfiling();
       if (ActivePS::FeatureJS(aLock)) {
         info->StartJSSampling();
@@ -2873,17 +2873,17 @@ locked_profiler_stop(PSLockRef aLock)
 
 #ifdef MOZ_TASK_TRACER
   if (ActivePS::FeatureTaskTracer(aLock)) {
     tasktracer::StopLogging();
   }
 #endif
 
   // Stop sampling live threads.
-  Thread::tid_t tid = Thread::GetCurrentId();
+  int tid = Thread::GetCurrentId();
   CorePS::ThreadVector& liveThreads = CorePS::LiveThreads(aLock);
   for (uint32_t i = 0; i < liveThreads.size(); i++) {
     ThreadInfo* info = liveThreads.at(i);
     if (info->IsBeingProfiled()) {
       if (ActivePS::FeatureJS(aLock)) {
         info->StopJSSampling();
         if (info->ThreadId() == tid) {
           // We can manually poll the current thread so it stops profiling
@@ -3158,17 +3158,17 @@ profiler_get_backtrace()
   }
 
   ThreadInfo* info = TLSInfo::Info(lock);
   if (!info) {
     MOZ_ASSERT(info);
     return nullptr;
   }
 
-  Thread::tid_t tid = Thread::GetCurrentId();
+  int tid = Thread::GetCurrentId();
 
   TimeStamp now = TimeStamp::Now();
 
   Registers regs;
 #if defined(HAVE_NATIVE_UNWIND)
   regs.SyncPopulate();
 #else
   regs.Clear();
old mode 100644
new mode 100755
--- a/tools/profiler/core/platform.h
+++ b/tools/profiler/core/platform.h
@@ -93,23 +93,17 @@ typedef uint8_t* Address;
 // Thread
 //
 // This class has static methods for the different platform specific
 // functions. Add methods here to cope with differences between the
 // supported platforms.
 
 class Thread {
 public:
-#if defined(GP_OS_windows)
-  typedef DWORD tid_t;
-#else
-  typedef ::pid_t tid_t;
-#endif
-
-  static tid_t GetCurrentId();
+  static int GetCurrentId();
 };
 
 // ----------------------------------------------------------------------------
 // Miscellaneous
 
 class PlatformData;
 
 // We can't new/delete the type safely without defining it
old mode 100644
new mode 100755
--- a/tools/profiler/tasktracer/GeckoTaskTracer.cpp
+++ b/tools/profiler/tasktracer/GeckoTaskTracer.cpp
@@ -342,16 +342,19 @@ LogBegin(uint64_t aTaskId, uint64_t aSou
   // [1 taskId beginTime processId threadId]
   TraceInfoLogType* log = info->AppendLog();
   if (log) {
     log->mBegin.mType = ACTION_BEGIN;
     log->mBegin.mTaskId = aTaskId;
     log->mBegin.mTime = GetTimestamp();
     log->mBegin.mPid = getpid();
     log->mBegin.mTid = Thread::GetCurrentId();
+
+    MOZ_ASSERT(log->mBegin.mPid >= 0, "native process ID is < 0 (signed integer overflow)");
+    MOZ_ASSERT(log->mBegin.mTid >= 0, "native thread ID is < 0  (signed integer overflow)");
   }
 }
 
 void
 LogEnd(uint64_t aTaskId, uint64_t aSourceEventId)
 {
   TraceInfoHolder info = GetOrCreateTraceInfo();
   ENSURE_TRUE_VOID(info);
old mode 100644
new mode 100755
--- a/tools/profiler/tests/gtest/ThreadProfileTest.cpp
+++ b/tools/profiler/tests/gtest/ThreadProfileTest.cpp
@@ -6,35 +6,35 @@
 
 #include "gtest/gtest.h"
 
 #include "ProfileBufferEntry.h"
 #include "ThreadInfo.h"
 
 // Make sure we can initialize our thread profile
 TEST(ThreadProfile, Initialization) {
-  Thread::tid_t tid = 1000;
+  int tid = 1000;
   ThreadInfo info("testThread", tid, true, nullptr);
   info.StartProfiling();
 }
 
 // Make sure we can record one entry and read it
 TEST(ThreadProfile, InsertOneEntry) {
-  Thread::tid_t tid = 1000;
+  int tid = 1000;
   ThreadInfo info("testThread", tid, true, nullptr);
   auto pb = MakeUnique<ProfileBuffer>(10);
   pb->AddEntry(ProfileBufferEntry::Time(123.1));
   ASSERT_TRUE(pb->mEntries != nullptr);
   ASSERT_TRUE(pb->mEntries[pb->mReadPos].IsTime());
   ASSERT_TRUE(pb->mEntries[pb->mReadPos].u.mDouble == 123.1);
 }
 
 // See if we can insert some entries
 TEST(ThreadProfile, InsertEntriesNoWrap) {
-  Thread::tid_t tid = 1000;
+  int tid = 1000;
   ThreadInfo info("testThread", tid, true, nullptr);
   auto pb = MakeUnique<ProfileBuffer>(100);
   int test_size = 50;
   for (int i = 0; i < test_size; i++) {
     pb->AddEntry(ProfileBufferEntry::Time(i));
   }
   ASSERT_TRUE(pb->mEntries != nullptr);
   int readPos = pb->mReadPos;
@@ -42,17 +42,17 @@ TEST(ThreadProfile, InsertEntriesNoWrap)
     ASSERT_TRUE(pb->mEntries[readPos].IsTime());
     ASSERT_TRUE(pb->mEntries[readPos].u.mDouble == readPos);
     readPos = (readPos + 1) % pb->mEntrySize;
   }
 }
 
 // See if wrapping works as it should in the basic case
 TEST(ThreadProfile, InsertEntriesWrap) {
-  Thread::tid_t tid = 1000;
+  int tid = 1000;
   // we can fit only 24 entries in this buffer because of the empty slot
   int entries = 24;
   int buffer_size = entries + 1;
   ThreadInfo info("testThread", tid, true, nullptr);
   auto pb = MakeUnique<ProfileBuffer>(buffer_size);
   int test_size = 43;
   for (int i = 0; i < test_size; i++) {
     pb->AddEntry(ProfileBufferEntry::Time(i));