win: Add implementation of ProcessInfo

util/win/process_info_test.cc

Index: util/win/process_info_test.cc
diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5dd86f6cc342f0328f6697e82bcafaa20470fb58
--- /dev/null
+++ b/util/win/process_info_test.cc
@@ -0,0 +1,112 @@
+// Copyright 2014 The Crashpad Authors. All rights reserved.

[Mark Mentovai, 2015/03/05 17:32:23]

2015 (I’ve been goofing this up a lot too, a PRESUBMIT checking new files could
help).

[scottmg, 2015/03/05 20:31:08]

Done.

+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "util/win/process_info.h"
+
+#include <rpc.h>
+
+#include "base/files/file_path.h"
+#include "gtest/gtest.h"
+#include "util/misc/uuid.h"
+#include "util/test/executable_path.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+void TestSelfProcess(const ProcessInfo& process_info) {

[Mark Mentovai, 2015/03/05 17:32:23]

Why don’t you just put this in-line into the TEST() below? It’s not used
anywhere else, and the TEST() is really short.

[scottmg, 2015/03/05 20:31:07]

Done.

+  EXPECT_EQ(GetCurrentProcessId(), process_info.ProcessID());
+  EXPECT_GT(process_info.ParentProcessID(), 0u);
+
+#if defined(ARCH_CPU_64_BITS)

[Mark Mentovai, 2015/03/05 17:32:23]

#include "build/build_config.h"

[scottmg, 2015/03/05 20:31:07]

Done.

+  EXPECT_TRUE(process_info.Is64Bit());

[Mark Mentovai, 2015/03/05 17:32:23]

You can check that WoW64 is false here too.

[scottmg, 2015/03/05 20:31:07]

Done.

+#else
+  EXPECT_FALSE(process_info.Is64Bit());
+  // Assume we won't be running these tests on a 32 bit host OS.
+  EXPECT_TRUE(process_info.IsWow64());
+#endif
+
+  std::wstring command_line;
+  EXPECT_TRUE(process_info.CommandLine(&command_line));
+  EXPECT_EQ(std::wstring(GetCommandLineW()), command_line);

[Mark Mentovai, 2015/03/05 17:32:23]

Can’t you just use GetCommandLine() and let the SDK macros give you the right
variant? Or is UNICODE not defined here?

[scottmg, 2015/03/05 20:31:07]

Done.

+
+  std::vector<std::wstring> modules;
+  EXPECT_TRUE(process_info.Modules(&modules));
+  ASSERT_GT(modules.size(), 0u);
+  EXPECT_EQ(L"ntdll.dll", modules[0].substr(modules[0].size() - 9));

[Mark Mentovai, 2015/03/05 17:32:23]

Will the module name that shows up here always be a pathname? Will it always
have \ separators? If so, you can look for L"\\ntdll.dll".

[Mark Mentovai, 2015/03/05 17:32:23]

Not kosher unless you’ve checked that the size >= 9. Also, wcslen(kNtdllName)
would be better than 9.

[scottmg, 2015/03/05 20:31:07]

Done.

[scottmg, 2015/03/05 20:31:07]

Done.

+}
+
+TEST(ProcessInfo, Self) {
+  ProcessInfo process_info;
+  ASSERT_TRUE(process_info.Initialize(GetCurrentProcess()));
+  TestSelfProcess(process_info);
+}
+
+TEST(ProcessInfo, SomeOtherProcess) {
+  ProcessInfo process_info;
+
+  ::UUID system_uuid;
+  ASSERT_EQ(RPC_S_OK, UuidCreate(&system_uuid));
+  UUID started_uuid(reinterpret_cast<const uint8_t*>(&system_uuid.Data1));
+  ASSERT_EQ(RPC_S_OK, UuidCreate(&system_uuid));
+  UUID done_uuid(reinterpret_cast<const uint8_t*>(&system_uuid.Data1));
+
+  HANDLE started =
+      CreateEvent(nullptr, true, false, started_uuid.ToString16().c_str());
+  ASSERT_TRUE(started);
+  HANDLE done =
+      CreateEvent(nullptr, true, false, done_uuid.ToString16().c_str());
+  ASSERT_TRUE(done);
+
+  base::FilePath test_executable = ExecutablePath();
+  std::wstring child_test_executable =
+      test_executable.RemoveFinalExtension().value() +
+      L"_process_info_test_child.exe";
+  std::wstring command_line = child_test_executable + L" " +

[Mark Mentovai, 2015/03/05 17:32:23]

Ideally we’d have a utility to properly escape command lines given arguments,
but for now, a TODO will suffice, and this will be fine as long as there are no
spaces or other bogons in the path.

[scottmg, 2015/03/05 20:31:07]

TODO for now. At the moment it's just the two guids, so it should be fine in
this code.

+                              started_uuid.ToString16() + L" " +
+                              done_uuid.ToString16();
+  STARTUPINFO si = {0};
+  si.cb = sizeof(si);
+  PROCESS_INFORMATION pi;
+  ASSERT_TRUE(CreateProcess(child_test_executable.c_str(),
+                            &command_line[0],
+                            nullptr,
+                            nullptr,
+                            false,
+                            0,
+                            nullptr,
+                            nullptr,
+                            &si,
+                            &pi));
+  ASSERT_EQ(WaitForSingleObject(started, INFINITE), WAIT_OBJECT_0);
+
+  ASSERT_TRUE(process_info.Initialize(pi.hProcess));
+
+  std::vector<std::wstring> modules;
+  EXPECT_TRUE(process_info.Modules(&modules));
+  ASSERT_GE(modules.size(), 2u);
+  EXPECT_EQ(L"ntdll.dll", modules[0].substr(modules[0].size() - 9));

[Mark Mentovai, 2015/03/05 17:32:23]

Do you want to look for the main executable as well?

[Mark Mentovai, 2015/03/05 17:32:23]

The same comments about wcslen() and checking the size apply here.

[scottmg, 2015/03/05 20:31:07]

The main executable doesn't appear in the list at the moment. I guess inserting
it at 0, and checking for both that, and ntdll at 1 might makes sense.
Alternatively we could make the main module a separate field, but maybe that
will just be an annoying difference when compared with Mac.

[scottmg, 2015/03/05 20:31:07]

Done.

[Mark Mentovai, 2015/03/05 21:53:36]

scottmg wrote:
See previous comments.

If you do decide that this interface should expose the main executable at index
0, it’s easy to test. Otherwise, you should still test that it shows up
somewhere in the list. You could do this with a loop over the modules, or I’m
sure you could cook up a crafty lambda to do it more concisely and slash or
obtusely.
Well, I imagine that given the TODOs, the modules that this class returns will
be more than just strings in the future. Having Module MainModule() and
vector<Module> AllModulesExceptMain() seems like kind of a weird interface. But
Modules() and MainModuleIndex() might be OK.

Up to you, though.

+  EXPECT_EQ(L"lz32.dll", modules.back().substr(modules.back().size() - 8));

[Mark Mentovai, 2015/03/05 17:32:23]

This was a surprise until I read the test file, but once I did, I realized what
a good idea this is. Can you just add a comment here saying that it’s a weird
module that the test executable loads?

[scottmg, 2015/03/05 20:31:07]

Done.

+
+  SetEvent(done);
+
+  CloseHandle(pi.hThread);

[Mark Mentovai, 2015/03/05 17:32:23]

Scopers? There are ASSERTs in here that translate to early returns and thus
leaks.

[scottmg, 2015/03/05 20:31:07]

Done.

+  CloseHandle(pi.hProcess);
+  CloseHandle(done);
+  CloseHandle(started);
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad