win x86: Grab bag of restructuring to get tests working on x86-on-x86

snapshot/win/process_reader_win_test.cc

Index: snapshot/win/process_reader_win_test.cc
diff --git a/snapshot/win/process_reader_win_test.cc b/snapshot/win/process_reader_win_test.cc
index 1cb4bdda51ad37efe98fdc25f256adaf9cbcf352..219dae3bb171c8f7a525c8f4db947d77cc2ce723 100644
--- a/snapshot/win/process_reader_win_test.cc
+++ b/snapshot/win/process_reader_win_test.cc
@@ -19,6 +19,8 @@
 
 #include "gtest/gtest.h"
 #include "test/win/win_multiprocess.h"
+#include "util/synchronization/semaphore.h"
+#include "util/thread/thread.h"
 #include "util/win/scoped_process_suspend.h"
 
 namespace crashpad {
@@ -104,7 +106,7 @@ TEST(ProcessReaderWin, SelfOneThread) {
   // thread, not exactly one thread.
   ASSERT_GE(threads.size(), 1u);
 
-  EXPECT_EQ(GetThreadId(GetCurrentThread()), threads[0].id);
+  EXPECT_EQ(GetCurrentThreadId(), threads[0].id);

[scottmg, 2015/09/15 21:13:34]

No GetThreadId() on XP.

[Mark Mentovai, 2015/09/16 02:57:20]

scottmg wrote:
This is better anyway.

 #if defined(ARCH_CPU_64_BITS)
   EXPECT_NE(0, threads[0].context.Rip);
 #else
@@ -120,14 +122,36 @@ class ProcessReaderChildThreadSuspendCount final : public WinMultiprocess {
   ~ProcessReaderChildThreadSuspendCount() {}
 
  private:
+  enum { kCreatedThreads = 3 };
+
+  class SleepingThread : public Thread {
+   public:
+    SleepingThread() : done_(nullptr) {}
+
+    void SetHandle(Semaphore* done) {
+      done_= done;
+    }
+
+    void ThreadMain() override {
+      done_->Wait();
+    };
+
+   private:
+    Semaphore* done_;
+  };
+
   void WinMultiprocessParent() override {
+    char c;
+    CheckedReadFile(ReadPipeHandle(), &c, sizeof(c));
+    ASSERT_EQ(' ', c);
+
     {
       ProcessReaderWin process_reader;
       ASSERT_TRUE(process_reader.Initialize(ChildProcess(),
                                             ProcessSuspensionState::kRunning));
 
       const auto& threads = process_reader.Threads();
-      ASSERT_FALSE(threads.empty());
+      ASSERT_GE(threads.size(), static_cast<unsigned int>(kCreatedThreads + 1));

[Mark Mentovai, 2015/09/16 02:57:19]

enum : unsigned int should eliminate the need for this cast.

[scottmg, 2015/09/16 18:05:06]

Done.

       for (const auto& thread : threads)
         EXPECT_EQ(0u, thread.suspend_count);
     }
@@ -142,19 +166,33 @@ class ProcessReaderChildThreadSuspendCount final : public WinMultiprocess {
       // Confirm that thread counts are adjusted correctly for the process being
       // suspended.
       const auto& threads = process_reader.Threads();
-      ASSERT_FALSE(threads.empty());
+      ASSERT_GE(threads.size(), static_cast<unsigned int>(kCreatedThreads + 1));
       for (const auto& thread : threads)
         EXPECT_EQ(0u, thread.suspend_count);
     }
   }
 
   void WinMultiprocessChild() override {
-    WinVMAddress address = reinterpret_cast<WinVMAddress>(kTestMemory);
-    CheckedWriteFile(WritePipeHandle(), &address, sizeof(address));
+    // Create three dummy threads so we can confirm we read successfully read

[scottmg, 2015/09/15 21:13:34]

The size of the thread data structure was wrong on x86 and so only the first
thread was read successfully. So this is adding multiple threads to catch that
case.

+    // more than just the main thread.
+    SleepingThread threads[kCreatedThreads];
+    Semaphore done(0);
+    for (auto& thread : threads)
+      thread.SetHandle(&done);
+    for (auto& thread : threads)
+      thread.Start();
+
+    char c = ' ';
+    CheckedWriteFile(WritePipeHandle(), &c, sizeof(c));
 
     // Wait for the parent to signal that it's OK to exit by closing its end of
     // the pipe.
     CheckedReadFileAtEOF(ReadPipeHandle());
+
+    for (int i = 0; i < kCreatedThreads; ++i)

[Mark Mentovai, 2015/09/16 02:57:19]

This time, arraysize(threads).

[scottmg, 2015/09/16 18:05:06]

Done.

+      done.Signal();
+    for (auto& thread : threads)
+      thread.Join();
   }
 
   DISALLOW_COPY_AND_ASSIGN(ProcessReaderChildThreadSuspendCount);