Merge master 3983b80ca2fc into doc

build/run_tests.py

@@ -15,7 +15,6 @@
 # limitations under the License.
 
 import os
-import platform
 import subprocess
 import sys
 
@@ -25,25 +24,12 @@ import sys
 # location in the recipe.
 def main(args):
   if len(args) != 1:
-    print >> sys.stderr, \
+    print >> sys.stderr, 'usage: run_tests.py <binary_dir>'
-        'usage: run_tests.py {Debug|Release|Debug_x64|Release_x64}'
     return 1
 
   crashpad_dir = \
       os.path.join(os.path.dirname(os.path.abspath(__file__)), os.pardir)
-
+  binary_dir = args[0]
-  # In a standalone Crashpad build, the out directory is in the Crashpad root.
-  out_dir = os.path.join(crashpad_dir, 'out')
-  if not os.path.exists(out_dir):
-    # In an in-Chromium build, the out directory is in the Chromium root, and
-    # the Crashpad root is in third_party/crashpad/crashpad relative to the
-    # Chromium root.
-    chromium_dir = os.path.join(crashpad_dir, os.pardir, os.pardir, os.pardir)
-    out_dir = os.path.join(chromium_dir, 'out')
-  if not os.path.exists(out_dir):
-    raise Exception('could not determine out_dir', crashpad_dir)
-
-  binary_dir = os.path.join(out_dir, args[0])
 
   tests = [
       'crashpad_client_test',
@@ -59,12 +45,12 @@ def main(args):
     subprocess.check_call(os.path.join(binary_dir, test))
 
   if sys.platform == 'win32':
-    name = 'snapshot/win/end_to_end_test.py'
+    script = 'snapshot/win/end_to_end_test.py'
     print '-' * 80
-    print name
+    print script
     print '-' * 80
     subprocess.check_call(
-        [sys.executable, os.path.join(crashpad_dir, name), binary_dir])
+        [sys.executable, os.path.join(crashpad_dir, script), binary_dir])
 
   return 0
 

client/client_test.gyp

@@ -23,9 +23,9 @@
       'dependencies': [
         'client.gyp:crashpad_client',
         '../handler/handler.gyp:crashpad_handler',
+        '../test/test.gyp:crashpad_gmock_main',
         '../test/test.gyp:crashpad_test',
         '../third_party/gtest/gmock.gyp:gmock',
-        '../third_party/gtest/gmock.gyp:gmock_main',
         '../third_party/gtest/gtest.gyp:gtest',
         '../third_party/mini_chromium/mini_chromium.gyp:base',
         '../util/util.gyp:crashpad_util',

client/crash_report_database_win.cc

@@ -77,8 +77,8 @@ std::string ReadRestOfFileAsString(FileHandle file) {
   DCHECK_GT(end, read_from);
   size_t data_length = static_cast<size_t>(end - read_from);
   std::string buffer(data_length, '\0');
-  return LoggingReadFile(file, &buffer[0], data_length) ? buffer
+  return LoggingReadFileExactly(file, &buffer[0], data_length) ? buffer
-                                                        : std::string();
+                                                               : std::string();
 }
 
 // Helper structures, and conversions ------------------------------------------
@@ -417,7 +417,7 @@ void Metadata::Read() {
   }
 
   MetadataFileHeader header;
-  if (!LoggingReadFile(handle_.get(), &header, sizeof(header))) {
+  if (!LoggingReadFileExactly(handle_.get(), &header, sizeof(header))) {
     LOG(ERROR) << "failed to read header";
     return;
   }
@@ -438,7 +438,7 @@ void Metadata::Read() {
   std::vector<ReportDisk> reports;
   if (header.num_records > 0) {
     std::vector<MetadataFileReportRecord> records(header.num_records);
-    if (!LoggingReadFile(
+    if (!LoggingReadFileExactly(
             handle_.get(), &records[0], records_size.ValueOrDie())) {
       LOG(ERROR) << "failed to read records";
       return;

client/settings.cc

@@ -246,13 +246,10 @@ bool Settings::ReadSettings(FileHandle handle,
   if (LoggingSeekFile(handle, 0, SEEK_SET) != 0)
     return false;
 
-  bool read_result;
+  bool read_result =
-  if (log_read_error) {
+      log_read_error
-    read_result = LoggingReadFile(handle, out_data, sizeof(*out_data));
+          ? LoggingReadFileExactly(handle, out_data, sizeof(*out_data))
-  } else {
+          : ReadFileExactly(handle, out_data, sizeof(*out_data));
-    read_result =
-        ReadFile(handle, out_data, sizeof(*out_data)) == sizeof(*out_data);
-  }
 
   if (!read_result)
     return false;

compat/android/elf.h

@@ -0,0 +1,25 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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.
+
+#ifndef CRASHPAD_COMPAT_ANDROID_ELF_H_
+#define CRASHPAD_COMPAT_ANDROID_ELF_H_
+
+#include_next <elf.h>
+
+// Android 5.0.0 (API 21) NDK
+#if !defined(NT_PRSTATUS)
+#define NT_PRSTATUS 1
+#endif
+
+#endif  // CRASHPAD_COMPAT_ANDROID_ELF_H_

compat/android/linux/ptrace.h

@@ -0,0 +1,25 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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.
+
+#ifndef CRASHPAD_COMPAT_ANDROID_LINUX_PTRACE_H_
+#define CRASHPAD_COMPAT_ANDROID_LINUX_PTRACE_H_
+
+#include_next <linux/ptrace.h>
+
+// Android 5.0.0 (API 21) NDK
+#if !defined(PTRACE_GETREGSET)
+#define PTRACE_GETREGSET 0x4204
+#endif
+
+#endif  // CRASHPAD_COMPAT_ANDROID_LINUX_PTRACE_H_

compat/compat.gyp

@@ -83,6 +83,16 @@
             ],
           },
         }],
+        ['OS=="android"', {
+          'include_dirs': [
+            'android',
+          ],
+          'direct_dependent_settings': {
+            'include_dirs': [
+              'android',
+            ],
+          },
+        }],
       ],
     },
   ],

doc/developing.md

@@ -91,7 +91,9 @@ crashpad`, `gclient sync`, or `gclient runhooks`.
 The Ninja build files and build output are in the `out` directory. Both debug-
 and release-mode configurations are available. The examples below show the debug
 configuration. To build and test the release configuration, substitute `Release`
-for `Debug`.
+for `Debug`. On Windows, four configurations are available: `Debug` and
+`Release` produce 32-bit x86 executables, and `Debug_x64` and `Release_x64`
+produce x86_64 executables.
 
 ```
 $ cd ~/crashpad/crashpad
@@ -112,7 +114,7 @@ Kit)](https://developer.android.com/ndk/) runs on.
 If it’s not already present on your system, [download the NDK package for your
 system](https://developer.android.com/ndk/downloads/) and expand it to a
 suitable location. These instructions assume that it’s been expanded to
-`~/android-ndk-r13`.
+`~/android-ndk-r14`.
 
 To build Crashpad, portions of the NDK must be reassembled into a [standalone
 toolchain](https://developer.android.com/ndk/guides/standalone_toolchain.html).
@@ -126,8 +128,8 @@ desired. To build a standalone toolchain targeting 64-bit ARM and API level 21
 
 ```
 $ cd ~
-$ python android-ndk-r13/build/tools/make_standalone_toolchain.py \
+$ python android-ndk-r14/build/tools/make_standalone_toolchain.py \
-      --arch=arm64 --api=21 --install-dir=android-ndk-r13_arm64_api21
+      --arch=arm64 --api=21 --install-dir=android-ndk-r14_arm64_api21
 ```
 
 Note that Chrome uses Android API level 21 for 64-bit platforms and 16 for
@@ -144,14 +146,14 @@ not be permanent.
 
 ```
 $ cd ~/crashpad/crashpad
-$ CC_target=~/android-ndk-r13_arm64_api21/bin/clang \
+$ CC_target=~/android-ndk-r14_arm64_api21/bin/clang \
-  CXX_target=~/android-ndk-r13_arm64_api21/bin/clang++ \
+  CXX_target=~/android-ndk-r14_arm64_api21/bin/clang++ \
-  AR_target=~/android-ndk-r13_arm64_api21/bin/aarch64-linux-android-ar \
+  AR_target=~/android-ndk-r14_arm64_api21/bin/aarch64-linux-android-ar \
-  NM_target=~/android-ndk-r13_arm64_api21/bin/aarch64-linux-android-nm \
+  NM_target=~/android-ndk-r14_arm64_api21/bin/aarch64-linux-android-nm \
-  READELF_target=~/android-ndk-r13_arm64_api21/bin/aarch64-linux-android-readelf \
+  READELF_target=~/android-ndk-r14_arm64_api21/bin/aarch64-linux-android-readelf \
   python build/gyp_crashpad.py \
       -DOS=android -Dtarget_arch=arm64 -Dclang=1 \
-      --generator-output=out_android_arm64_api21 -f ninja-android
+      --generator-output=out/android_arm64_api21 -f ninja-android
 ```
 
 It is also possible to use GCC instead of Clang by making the appropriate
@@ -174,7 +176,7 @@ build, direct `ninja` to the specific `out` directory chosen by
 `--generator-output` above.
 
 ```
-$ ninja -C out_android_arm64_api21/out/Debug crashpad_test_test
+$ ninja -C out/android_arm64_api21/out/Debug crashpad_test_test
 ```
 
 ## Testing
@@ -193,11 +195,11 @@ $ out/Debug/crashpad_util_test
 ```
 
 A script is provided to run all of Crashpad’s tests. It accepts a single
-argument that tells it which configuration to test.
+argument, a path to the directory containing the test executables.
 
 ```
 $ cd ~/crashpad/crashpad
-$ python build/run_tests.py Debug
+$ python build/run_tests.py out/Debug
 ```
 
 ### Android
@@ -216,10 +218,10 @@ transferred to the device prior to running the test.
 
 ```
 $ cd ~/crashpad/crashpad
-$ adb push out_android_arm64_api21/out/Debug/crashpad_test_test /data/local/tmp/
+$ adb push out/android_arm64_api21/out/Debug/crashpad_test_test /data/local/tmp/
 [100%] /data/local/tmp/crashpad_test_test
 $ adb push \
-      out_android_arm64_api21/out/Debug/crashpad_test_test_multiprocess_exec_test_child \
+      out/android_arm64_api21/out/Debug/crashpad_test_test_multiprocess_exec_test_child \
       /data/local/tmp/
 [100%] /data/local/tmp/crashpad_test_test_multiprocess_exec_test_child
 $ adb shell mkdir -p /data/local/tmp/crashpad_test_data_root/test

doc/man.md

@@ -19,6 +19,7 @@ limitations under the License.
 ## Section 1: User Commands
 
  * [crashpad_database_util](../tools/crashpad_database_util.md)
+ * [crashpad_http_upload](../tools/crashpad_http_upload.md)
  * [generate_dump](../tools/generate_dump.md)
 
 ### macOS-Specific

handler/win/crash_other_program.cc

@@ -90,7 +90,8 @@ int CrashOtherProgram(int argc, wchar_t* argv[]) {
 
   // Wait until it's ready.
   char c;
-  if (!LoggingReadFile(child.stdout_read_handle(), &c, sizeof(c)) || c != ' ') {
+  if (!LoggingReadFileExactly(child.stdout_read_handle(), &c, sizeof(c)) ||
+      c != ' ') {
     LOG(ERROR) << "failed child communication";
     return EXIT_FAILURE;
   }

minidump/minidump_crashpad_info_writer_test.cc

@@ -70,7 +70,7 @@ TEST(MinidumpCrashpadInfoWriter, Empty) {
       base::WrapUnique(new MinidumpCrashpadInfoWriter());
   EXPECT_FALSE(crashpad_info_writer->IsUseful());
 
-  minidump_file_writer.AddStream(std::move(crashpad_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(crashpad_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -106,7 +106,7 @@ TEST(MinidumpCrashpadInfoWriter, ReportAndClientID) {
 
   EXPECT_TRUE(crashpad_info_writer->IsUseful());
 
-  minidump_file_writer.AddStream(std::move(crashpad_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(crashpad_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -148,7 +148,7 @@ TEST(MinidumpCrashpadInfoWriter, SimpleAnnotations) {
 
   EXPECT_TRUE(crashpad_info_writer->IsUseful());
 
-  minidump_file_writer.AddStream(std::move(crashpad_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(crashpad_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -189,7 +189,7 @@ TEST(MinidumpCrashpadInfoWriter, CrashpadModuleList) {
 
   EXPECT_TRUE(crashpad_info_writer->IsUseful());
 
-  minidump_file_writer.AddStream(std::move(crashpad_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(crashpad_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -265,7 +265,7 @@ TEST(MinidumpCrashpadInfoWriter, InitializeFromSnapshot) {
   EXPECT_TRUE(info_writer->IsUseful());
 
   MinidumpFileWriter minidump_file_writer;
-  minidump_file_writer.AddStream(std::move(info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));

minidump/minidump_exception_writer_test.cc

@@ -102,7 +102,7 @@ TEST(MinidumpExceptionWriter, Minimal) {
   InitializeMinidumpContextX86(context_x86_writer->context(), kSeed);
   exception_writer->SetContext(std::move(context_x86_writer));
 
-  minidump_file_writer.AddStream(std::move(exception_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(exception_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -161,7 +161,7 @@ TEST(MinidumpExceptionWriter, Standard) {
   exception_information.push_back(kExceptionInformation2);
   exception_writer->SetExceptionInformation(exception_information);
 
-  minidump_file_writer.AddStream(std::move(exception_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(exception_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -234,7 +234,7 @@ TEST(MinidumpExceptionWriter, InitializeFromSnapshot) {
   exception_writer->InitializeFromSnapshot(&exception_snapshot, thread_id_map);
 
   MinidumpFileWriter minidump_file_writer;
-  minidump_file_writer.AddStream(std::move(exception_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(exception_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -256,7 +256,7 @@ TEST(MinidumpExceptionWriterDeathTest, NoContext) {
   MinidumpFileWriter minidump_file_writer;
   auto exception_writer = base::WrapUnique(new MinidumpExceptionWriter());
 
-  minidump_file_writer.AddStream(std::move(exception_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(exception_writer)));
 
   StringFile string_file;
   ASSERT_DEATH_CHECK(minidump_file_writer.WriteEverything(&string_file),

minidump/minidump_file_writer.cc

@@ -74,11 +74,13 @@ void MinidumpFileWriter::InitializeFromSnapshot(
   const SystemSnapshot* system_snapshot = process_snapshot->System();
   auto system_info = base::WrapUnique(new MinidumpSystemInfoWriter());
   system_info->InitializeFromSnapshot(system_snapshot);
-  AddStream(std::move(system_info));
+  bool add_stream_result = AddStream(std::move(system_info));
+  DCHECK(add_stream_result);
 
   auto misc_info = base::WrapUnique(new MinidumpMiscInfoWriter());
   misc_info->InitializeFromSnapshot(process_snapshot);
-  AddStream(std::move(misc_info));
+  add_stream_result = AddStream(std::move(misc_info));
+  DCHECK(add_stream_result);
 
   auto memory_list = base::WrapUnique(new MinidumpMemoryListWriter());
   auto thread_list = base::WrapUnique(new MinidumpThreadListWriter());
@@ -86,33 +88,29 @@ void MinidumpFileWriter::InitializeFromSnapshot(
   MinidumpThreadIDMap thread_id_map;
   thread_list->InitializeFromSnapshot(process_snapshot->Threads(),
                                       &thread_id_map);
-  AddStream(std::move(thread_list));
+  add_stream_result = AddStream(std::move(thread_list));
+  DCHECK(add_stream_result);
 
   const ExceptionSnapshot* exception_snapshot = process_snapshot->Exception();
   if (exception_snapshot) {
     auto exception = base::WrapUnique(new MinidumpExceptionWriter());
     exception->InitializeFromSnapshot(exception_snapshot, thread_id_map);
-    AddStream(std::move(exception));
+    add_stream_result = AddStream(std::move(exception));
+    DCHECK(add_stream_result);
   }
 
   auto module_list = base::WrapUnique(new MinidumpModuleListWriter());
   module_list->InitializeFromSnapshot(process_snapshot->Modules());
-  AddStream(std::move(module_list));
+  add_stream_result = AddStream(std::move(module_list));
-
+  DCHECK(add_stream_result);
-  for (const auto& module : process_snapshot->Modules()) {
-    for (const UserMinidumpStream* stream : module->CustomMinidumpStreams()) {
-      auto user_stream = base::WrapUnique(new MinidumpUserStreamWriter());
-      user_stream->InitializeFromSnapshot(stream);
-      AddStream(std::move(user_stream));
-    }
-  }
 
   auto unloaded_modules = process_snapshot->UnloadedModules();
   if (!unloaded_modules.empty()) {
     auto unloaded_module_list =
         base::WrapUnique(new MinidumpUnloadedModuleListWriter());
     unloaded_module_list->InitializeFromSnapshot(unloaded_modules);
-    AddStream(std::move(unloaded_module_list));
+    add_stream_result = AddStream(std::move(unloaded_module_list));
+    DCHECK(add_stream_result);
   }
 
   auto crashpad_info = base::WrapUnique(new MinidumpCrashpadInfoWriter());
@@ -121,7 +119,8 @@ void MinidumpFileWriter::InitializeFromSnapshot(
   // Since the MinidumpCrashpadInfo stream is an extension, it’s safe to not add
   // it to the minidump file if it wouldn’t carry any useful information.
   if (crashpad_info->IsUseful()) {
-    AddStream(std::move(crashpad_info));
+    add_stream_result = AddStream(std::move(crashpad_info));
+    DCHECK(add_stream_result);
   }
 
   std::vector<const MemoryMapRegionSnapshot*> memory_map_snapshot =
@@ -130,21 +129,50 @@ void MinidumpFileWriter::InitializeFromSnapshot(
     auto memory_info_list =
         base::WrapUnique(new MinidumpMemoryInfoListWriter());
     memory_info_list->InitializeFromSnapshot(memory_map_snapshot);
-    AddStream(std::move(memory_info_list));
+    add_stream_result = AddStream(std::move(memory_info_list));
+    DCHECK(add_stream_result);
   }
 
   std::vector<HandleSnapshot> handles_snapshot = process_snapshot->Handles();
   if (!handles_snapshot.empty()) {
     auto handle_data_writer = base::WrapUnique(new MinidumpHandleDataWriter());
     handle_data_writer->InitializeFromSnapshot(handles_snapshot);
-    AddStream(std::move(handle_data_writer));
+    add_stream_result = AddStream(std::move(handle_data_writer));
+    DCHECK(add_stream_result);
   }
 
   memory_list->AddFromSnapshot(process_snapshot->ExtraMemory());
-  if (exception_snapshot)
+  if (exception_snapshot) {
     memory_list->AddFromSnapshot(exception_snapshot->ExtraMemory());
+  }
 
-  AddStream(std::move(memory_list));
+  // These user streams must be added last. Otherwise, a user stream with the
+  // same type as a well-known stream could preempt the well-known stream. As it
+  // stands now, earlier-discovered user streams can still preempt
+  // later-discovered ones. The well-known memory list stream is added after
+  // these user streams, but only with a check here to avoid adding a user
+  // stream that would preempt the memory list stream.
+  for (const auto& module : process_snapshot->Modules()) {
+    for (const UserMinidumpStream* stream : module->CustomMinidumpStreams()) {
+      if (stream->stream_type() == kMinidumpStreamTypeMemoryList) {
+        LOG(WARNING) << "discarding duplicate stream of type "
+                     << stream->stream_type();
+        continue;
+      }
+      auto user_stream = base::WrapUnique(new MinidumpUserStreamWriter());
+      user_stream->InitializeFromSnapshot(stream);
+      AddStream(std::move(user_stream));
+    }
+  }
+
+  // The memory list stream should be added last. This keeps the “extra memory”
+  // at the end so that if the minidump file is truncated, other, more critical
+  // data is more likely to be preserved. Note that non-“extra” memory regions
+  // will not have to ride at the end of the file. Thread stack memory, for
+  // example, exists as a children of threads, and appears alongside them in the
+  // file, despite also being mentioned by the memory list stream.
+  add_stream_result = AddStream(std::move(memory_list));
+  DCHECK(add_stream_result);
 }
 
 void MinidumpFileWriter::SetTimestamp(time_t timestamp) {
@@ -153,18 +181,22 @@ void MinidumpFileWriter::SetTimestamp(time_t timestamp) {
   internal::MinidumpWriterUtil::AssignTimeT(&header_.TimeDateStamp, timestamp);
 }
 
-void MinidumpFileWriter::AddStream(
+bool MinidumpFileWriter::AddStream(
     std::unique_ptr<internal::MinidumpStreamWriter> stream) {
   DCHECK_EQ(state(), kStateMutable);
 
   MinidumpStreamType stream_type = stream->StreamType();
 
   auto rv = stream_types_.insert(stream_type);
-  CHECK(rv.second) << "stream_type " << stream_type << " already present";
+  if (!rv.second) {
+    LOG(WARNING) << "discarding duplicate stream of type " << stream_type;
+    return false;
+  }
 
   streams_.push_back(stream.release());
 
   DCHECK_EQ(streams_.size(), stream_types_.size());
+  return true;
 }
 
 bool MinidumpFileWriter::WriteEverything(FileWriterInterface* file_writer) {

minidump/minidump_file_writer.h

@@ -84,11 +84,16 @@ class MinidumpFileWriter final : public internal::MinidumpWritable {
   //!
   //! At most one object of each stream type (as obtained from
   //! internal::MinidumpStreamWriter::StreamType()) may be added to a
-  //! MinidumpFileWriter object. It is an error to attempt to add multiple
+  //! MinidumpFileWriter object. If an attempt is made to add a stream whose
-  //! streams with the same stream type.
+  //! type matches an existing stream’s type, this method discards the new
+  //! stream.
   //!
   //! \note Valid in #kStateMutable.
-  void AddStream(std::unique_ptr<internal::MinidumpStreamWriter> stream);
+  //!
+  //! \return `true` on success. `false` on failure, as occurs when an attempt
+  //!     is made to add a stream whose type matches an existing stream’s type,
+  //!     with a message logged.
+  bool AddStream(std::unique_ptr<internal::MinidumpStreamWriter> stream);
 
   // MinidumpWritable:
 

minidump/minidump_file_writer_test.cc

@@ -98,7 +98,7 @@ TEST(MinidumpFileWriter, OneStream) {
   const uint8_t kStreamValue = 0x5a;
   auto stream =
       base::WrapUnique(new TestStream(kStreamType, kStreamSize, kStreamValue));
-  minidump_file.AddStream(std::move(stream));
+  ASSERT_TRUE(minidump_file.AddStream(std::move(stream)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file.WriteEverything(&string_file));
@@ -137,7 +137,7 @@ TEST(MinidumpFileWriter, ThreeStreams) {
   const uint8_t kStream0Value = 0x5a;
   auto stream0 = base::WrapUnique(
       new TestStream(kStream0Type, kStream0Size, kStream0Value));
-  minidump_file.AddStream(std::move(stream0));
+  ASSERT_TRUE(minidump_file.AddStream(std::move(stream0)));
 
   // Make the second stream’s type be a smaller quantity than the first stream’s
   // to test that the streams show up in the order that they were added, not in
@@ -147,14 +147,14 @@ TEST(MinidumpFileWriter, ThreeStreams) {
   const uint8_t kStream1Value = 0xa5;
   auto stream1 = base::WrapUnique(
       new TestStream(kStream1Type, kStream1Size, kStream1Value));
-  minidump_file.AddStream(std::move(stream1));
+  ASSERT_TRUE(minidump_file.AddStream(std::move(stream1)));
 
   const size_t kStream2Size = 1;
   const MinidumpStreamType kStream2Type = static_cast<MinidumpStreamType>(0x7e);
   const uint8_t kStream2Value = 0x36;
   auto stream2 = base::WrapUnique(
       new TestStream(kStream2Type, kStream2Size, kStream2Value));
-  minidump_file.AddStream(std::move(stream2));
+  ASSERT_TRUE(minidump_file.AddStream(std::move(stream2)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file.WriteEverything(&string_file));
@@ -221,7 +221,7 @@ TEST(MinidumpFileWriter, ZeroLengthStream) {
   const size_t kStreamSize = 0;
   const MinidumpStreamType kStreamType = static_cast<MinidumpStreamType>(0x4d);
   auto stream = base::WrapUnique(new TestStream(kStreamType, kStreamSize, 0));
-  minidump_file.AddStream(std::move(stream));
+  ASSERT_TRUE(minidump_file.AddStream(std::move(stream)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file.WriteEverything(&string_file));
@@ -439,24 +439,48 @@ TEST(MinidumpFileWriter, InitializeFromSnapshot_CrashpadInfo) {
                   string_file.string(), directory[6].Location));
 }
 
-TEST(MinidumpFileWriterDeathTest, SameStreamType) {
+TEST(MinidumpFileWriter, SameStreamType) {
   MinidumpFileWriter minidump_file;
 
-  const size_t kStream0Size = 5;
+  const size_t kStream0Size = 3;
-  const MinidumpStreamType kStream0Type = static_cast<MinidumpStreamType>(0x4d);
+  const MinidumpStreamType kStreamType = static_cast<MinidumpStreamType>(0x4d);
   const uint8_t kStream0Value = 0x5a;
   auto stream0 = base::WrapUnique(
-      new TestStream(kStream0Type, kStream0Size, kStream0Value));
+      new TestStream(kStreamType, kStream0Size, kStream0Value));
-  minidump_file.AddStream(std::move(stream0));
+  ASSERT_TRUE(minidump_file.AddStream(std::move(stream0)));
 
-  // It is an error to add a second stream of the same type.
+  // An attempt to add a second stream of the same type should fail.
-  const size_t kStream1Size = 3;
+  const size_t kStream1Size = 5;
-  const MinidumpStreamType kStream1Type = static_cast<MinidumpStreamType>(0x4d);
   const uint8_t kStream1Value = 0xa5;
   auto stream1 = base::WrapUnique(
-      new TestStream(kStream1Type, kStream1Size, kStream1Value));
+      new TestStream(kStreamType, kStream1Size, kStream1Value));
-  ASSERT_DEATH_CHECK(minidump_file.AddStream(std::move(stream1)),
+  ASSERT_FALSE(minidump_file.AddStream(std::move(stream1)));
-                     "already present");
+
+  StringFile string_file;
+  ASSERT_TRUE(minidump_file.WriteEverything(&string_file));
+
+  const size_t kDirectoryOffset = sizeof(MINIDUMP_HEADER);
+  const size_t kStream0Offset = kDirectoryOffset + sizeof(MINIDUMP_DIRECTORY);
+  const size_t kFileSize = kStream0Offset + kStream0Size;
+
+  ASSERT_EQ(kFileSize, string_file.string().size());
+
+  const MINIDUMP_DIRECTORY* directory;
+  const MINIDUMP_HEADER* header =
+      MinidumpHeaderAtStart(string_file.string(), &directory);
+  ASSERT_NO_FATAL_FAILURE(VerifyMinidumpHeader(header, 1, 0));
+  ASSERT_TRUE(directory);
+
+  EXPECT_EQ(kStreamType, directory[0].StreamType);
+  EXPECT_EQ(kStream0Size, directory[0].Location.DataSize);
+  EXPECT_EQ(kStream0Offset, directory[0].Location.Rva);
+
+  const uint8_t* stream_data = MinidumpWritableAtLocationDescriptor<uint8_t>(
+      string_file.string(), directory[0].Location);
+  ASSERT_TRUE(stream_data);
+
+  std::string expected_stream(kStream0Size, kStream0Value);
+  EXPECT_EQ(0, memcmp(stream_data, expected_stream.c_str(), kStream0Size));
 }
 
 }  // namespace

minidump/minidump_handle_writer_test.cc

@@ -59,7 +59,7 @@ void GetHandleDataStream(
 TEST(MinidumpHandleDataWriter, Empty) {
   MinidumpFileWriter minidump_file_writer;
   auto handle_data_writer = base::WrapUnique(new MinidumpHandleDataWriter());
-  minidump_file_writer.AddStream(std::move(handle_data_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(handle_data_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -92,7 +92,7 @@ TEST(MinidumpHandleDataWriter, OneHandle) {
 
   handle_data_writer->InitializeFromSnapshot(snapshot);
 
-  minidump_file_writer.AddStream(std::move(handle_data_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(handle_data_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -150,7 +150,7 @@ TEST(MinidumpHandleDataWriter, RepeatedTypeName) {
 
   handle_data_writer->InitializeFromSnapshot(snapshot);
 
-  minidump_file_writer.AddStream(std::move(handle_data_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(handle_data_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));

minidump/minidump_memory_info_writer_test.cc

@@ -60,7 +60,8 @@ TEST(MinidumpMemoryInfoWriter, Empty) {
   MinidumpFileWriter minidump_file_writer;
   auto memory_info_list_writer =
       base::WrapUnique(new MinidumpMemoryInfoListWriter());
-  minidump_file_writer.AddStream(std::move(memory_info_list_writer));
+  ASSERT_TRUE(
+      minidump_file_writer.AddStream(std::move(memory_info_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -97,7 +98,8 @@ TEST(MinidumpMemoryInfoWriter, OneRegion) {
   memory_map.push_back(memory_map_region.get());
   memory_info_list_writer->InitializeFromSnapshot(memory_map);
 
-  minidump_file_writer.AddStream(std::move(memory_info_list_writer));
+  ASSERT_TRUE(
+      minidump_file_writer.AddStream(std::move(memory_info_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));

minidump/minidump_memory_writer_test.cc

@@ -79,7 +79,7 @@ TEST(MinidumpMemoryWriter, EmptyMemoryList) {
   MinidumpFileWriter minidump_file_writer;
   auto memory_list_writer = base::WrapUnique(new MinidumpMemoryListWriter());
 
-  minidump_file_writer.AddStream(std::move(memory_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(memory_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -107,7 +107,7 @@ TEST(MinidumpMemoryWriter, OneMemoryRegion) {
       new TestMinidumpMemoryWriter(kBaseAddress, kSize, kValue));
   memory_list_writer->AddMemory(std::move(memory_writer));
 
-  minidump_file_writer.AddStream(std::move(memory_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(memory_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -148,7 +148,7 @@ TEST(MinidumpMemoryWriter, TwoMemoryRegions) {
       new TestMinidumpMemoryWriter(kBaseAddress1, kSize1, kValue1));
   memory_list_writer->AddMemory(std::move(memory_writer_1));
 
-  minidump_file_writer.AddStream(std::move(memory_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(memory_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -247,7 +247,7 @@ TEST(MinidumpMemoryWriter, ExtraMemory) {
   auto memory_list_writer = base::WrapUnique(new MinidumpMemoryListWriter());
   memory_list_writer->AddExtraMemory(test_memory_stream->memory());
 
-  minidump_file_writer.AddStream(std::move(test_memory_stream));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(test_memory_stream)));
 
   const uint64_t kBaseAddress1 = 0x2000;
   const size_t kSize1 = 0x0400;
@@ -257,7 +257,7 @@ TEST(MinidumpMemoryWriter, ExtraMemory) {
       new TestMinidumpMemoryWriter(kBaseAddress1, kSize1, kValue1));
   memory_list_writer->AddMemory(std::move(memory_writer));
 
-  minidump_file_writer.AddStream(std::move(memory_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(memory_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -335,7 +335,7 @@ TEST(MinidumpMemoryWriter, AddFromSnapshot) {
   memory_list_writer->AddFromSnapshot(memory_snapshots);
 
   MinidumpFileWriter minidump_file_writer;
-  minidump_file_writer.AddStream(std::move(memory_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(memory_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));

minidump/minidump_misc_info_writer_test.cc

@@ -193,7 +193,7 @@ TEST(MinidumpMiscInfoWriter, Empty) {
   MinidumpFileWriter minidump_file_writer;
   auto misc_info_writer = base::WrapUnique(new MinidumpMiscInfoWriter());
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -214,7 +214,7 @@ TEST(MinidumpMiscInfoWriter, ProcessId) {
 
   misc_info_writer->SetProcessID(kProcessId);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -240,7 +240,7 @@ TEST(MinidumpMiscInfoWriter, ProcessTimes) {
   misc_info_writer->SetProcessTimes(
       kProcessCreateTime, kProcessUserTime, kProcessKernelTime);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -273,7 +273,7 @@ TEST(MinidumpMiscInfoWriter, ProcessorPowerInfo) {
                                           kProcessorMaxIdleState,
                                           kProcessorCurrentIdleState);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -300,7 +300,7 @@ TEST(MinidumpMiscInfoWriter, ProcessIntegrityLevel) {
 
   misc_info_writer->SetProcessIntegrityLevel(kProcessIntegrityLevel);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -323,7 +323,7 @@ TEST(MinidumpMiscInfoWriter, ProcessExecuteFlags) {
 
   misc_info_writer->SetProcessExecuteFlags(kProcessExecuteFlags);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -346,7 +346,7 @@ TEST(MinidumpMiscInfoWriter, ProtectedProcess) {
 
   misc_info_writer->SetProtectedProcess(kProtectedProcess);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -383,7 +383,7 @@ TEST(MinidumpMiscInfoWriter, TimeZone) {
                                 kDaylightDate,
                                 kDaylightBias);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -444,7 +444,7 @@ TEST(MinidumpMiscInfoWriter, TimeZoneStringsOverflow) {
                                 kSystemTimeZero,
                                 kDaylightBias);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -485,7 +485,7 @@ TEST(MinidumpMiscInfoWriter, BuildStrings) {
 
   misc_info_writer->SetBuildString(kBuildString, kDebugBuildString);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -522,7 +522,7 @@ TEST(MinidumpMiscInfoWriter, BuildStringsOverflow) {
 
   misc_info_writer->SetBuildString(build_string, debug_build_string);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -565,7 +565,7 @@ TEST(MinidumpMiscInfoWriter, XStateData) {
 
   misc_info_writer->SetXStateData(kXStateData);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -587,7 +587,7 @@ TEST(MinidumpMiscInfoWriter, ProcessCookie) {
 
   misc_info_writer->SetProcessCookie(kProcessCookie);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -649,7 +649,7 @@ TEST(MinidumpMiscInfoWriter, Everything) {
                                 kDaylightBias);
   misc_info_writer->SetBuildString(kBuildString, kDebugBuildString);
 
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -789,7 +789,7 @@ TEST(MinidumpMiscInfoWriter, InitializeFromSnapshot) {
   misc_info_writer->InitializeFromSnapshot(&process_snapshot);
 
   MinidumpFileWriter minidump_file_writer;
-  minidump_file_writer.AddStream(std::move(misc_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(misc_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));

minidump/minidump_module_writer_test.cc

@@ -67,7 +67,7 @@ TEST(MinidumpModuleWriter, EmptyModuleList) {
   MinidumpFileWriter minidump_file_writer;
   auto module_list_writer = base::WrapUnique(new MinidumpModuleListWriter());
 
-  minidump_file_writer.AddStream(std::move(module_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(module_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -276,7 +276,7 @@ TEST(MinidumpModuleWriter, EmptyModule) {
   module_writer->SetName(kModuleName);
 
   module_list_writer->AddModule(std::move(module_writer));
-  minidump_file_writer.AddStream(std::move(module_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(module_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -367,7 +367,7 @@ TEST(MinidumpModuleWriter, OneModule) {
   module_writer->SetMiscDebugRecord(std::move(misc_debug_writer));
 
   module_list_writer->AddModule(std::move(module_writer));
-  minidump_file_writer.AddStream(std::move(module_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(module_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -442,7 +442,7 @@ TEST(MinidumpModuleWriter, OneModule_CodeViewUsesPDB20_MiscUsesUTF16) {
   module_writer->SetMiscDebugRecord(std::move(misc_debug_writer));
 
   module_list_writer->AddModule(std::move(module_writer));
-  minidump_file_writer.AddStream(std::move(module_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(module_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -534,7 +534,7 @@ TEST(MinidumpModuleWriter, ThreeModules) {
 
   module_list_writer->AddModule(std::move(module_writer_2));
 
-  minidump_file_writer.AddStream(std::move(module_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(module_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -727,7 +727,7 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) {
   module_list_writer->InitializeFromSnapshot(module_snapshots);
 
   MinidumpFileWriter minidump_file_writer;
-  minidump_file_writer.AddStream(std::move(module_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(module_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -759,7 +759,7 @@ TEST(MinidumpModuleWriterDeathTest, NoModuleName) {
   auto module_list_writer = base::WrapUnique(new MinidumpModuleListWriter());
   auto module_writer = base::WrapUnique(new MinidumpModuleWriter());
   module_list_writer->AddModule(std::move(module_writer));
-  minidump_file_writer.AddStream(std::move(module_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(module_list_writer)));
 
   StringFile string_file;
   ASSERT_DEATH_CHECK(minidump_file_writer.WriteEverything(&string_file),

minidump/minidump_system_info_writer_test.cc

@@ -82,7 +82,7 @@ TEST(MinidumpSystemInfoWriter, Empty) {
 
   system_info_writer->SetCSDVersion(std::string());
 
-  minidump_file_writer.AddStream(std::move(system_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(system_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -153,7 +153,7 @@ TEST(MinidumpSystemInfoWriter, X86_Win) {
   system_info_writer->SetCPUX86VersionAndFeatures(kCPUVersion, kCPUFeatures);
   system_info_writer->SetCPUX86AMDExtendedFeatures(kAMDFeatures);
 
-  minidump_file_writer.AddStream(std::move(system_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(system_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -213,7 +213,7 @@ TEST(MinidumpSystemInfoWriter, AMD64_Mac) {
   system_info_writer->SetCSDVersion(kCSDVersion);
   system_info_writer->SetCPUOtherFeatures(kCPUFeatures[0], kCPUFeatures[1]);
 
-  minidump_file_writer.AddStream(std::move(system_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(system_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -255,7 +255,7 @@ TEST(MinidumpSystemInfoWriter, X86_CPUVendorFromRegisters) {
       kCPUVendor[0], kCPUVendor[1], kCPUVendor[2]);
   system_info_writer->SetCSDVersion(std::string());
 
-  minidump_file_writer.AddStream(std::move(system_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(system_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -335,7 +335,7 @@ TEST(MinidumpSystemInfoWriter, InitializeFromSnapshot_X86) {
   system_info_writer->InitializeFromSnapshot(&system_snapshot);
 
   MinidumpFileWriter minidump_file_writer;
-  minidump_file_writer.AddStream(std::move(system_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(system_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -431,7 +431,7 @@ TEST(MinidumpSystemInfoWriter, InitializeFromSnapshot_AMD64) {
   system_info_writer->InitializeFromSnapshot(&system_snapshot);
 
   MinidumpFileWriter minidump_file_writer;
-  minidump_file_writer.AddStream(std::move(system_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(system_info_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -469,7 +469,7 @@ TEST(MinidumpSystemInfoWriter, InitializeFromSnapshot_AMD64) {
 TEST(MinidumpSystemInfoWriterDeathTest, NoCSDVersion) {
   MinidumpFileWriter minidump_file_writer;
   auto system_info_writer = base::WrapUnique(new MinidumpSystemInfoWriter());
-  minidump_file_writer.AddStream(std::move(system_info_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(system_info_writer)));
 
   StringFile string_file;
   ASSERT_DEATH_CHECK(minidump_file_writer.WriteEverything(&string_file),

minidump/minidump_test.gyp

@@ -23,9 +23,9 @@
       'dependencies': [
         'minidump.gyp:crashpad_minidump',
         '../snapshot/snapshot_test.gyp:crashpad_snapshot_test_lib',
+        '../test/test.gyp:crashpad_gtest_main',
         '../test/test.gyp:crashpad_test',
         '../third_party/gtest/gtest.gyp:gtest',
-        '../third_party/gtest/gtest.gyp:gtest_main',
         '../third_party/mini_chromium/mini_chromium.gyp:base',
         '../util/util.gyp:crashpad_util',
       ],

minidump/minidump_thread_writer_test.cc

@@ -81,7 +81,7 @@ TEST(MinidumpThreadWriter, EmptyThreadList) {
   MinidumpFileWriter minidump_file_writer;
   auto thread_list_writer = base::WrapUnique(new MinidumpThreadListWriter());
 
-  minidump_file_writer.AddStream(std::move(thread_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(thread_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -161,7 +161,7 @@ TEST(MinidumpThreadWriter, OneThread_x86_NoStack) {
   thread_writer->SetContext(std::move(context_x86_writer));
 
   thread_list_writer->AddThread(std::move(thread_writer));
-  minidump_file_writer.AddStream(std::move(thread_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(thread_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -231,7 +231,7 @@ TEST(MinidumpThreadWriter, OneThread_AMD64_Stack) {
   thread_writer->SetContext(std::move(context_amd64_writer));
 
   thread_list_writer->AddThread(std::move(thread_writer));
-  minidump_file_writer.AddStream(std::move(thread_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(thread_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -363,8 +363,8 @@ TEST(MinidumpThreadWriter, ThreeThreads_x86_MemoryList) {
 
   thread_list_writer->AddThread(std::move(thread_writer_2));
 
-  minidump_file_writer.AddStream(std::move(thread_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(thread_list_writer)));
-  minidump_file_writer.AddStream(std::move(memory_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(memory_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -623,8 +623,8 @@ void RunInitializeFromSnapshotTest(bool thread_id_collision) {
   thread_list_writer->InitializeFromSnapshot(thread_snapshots, &thread_id_map);
 
   MinidumpFileWriter minidump_file_writer;
-  minidump_file_writer.AddStream(std::move(thread_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(thread_list_writer)));
-  minidump_file_writer.AddStream(std::move(memory_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(memory_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -702,7 +702,7 @@ TEST(MinidumpThreadWriterDeathTest, NoContext) {
   auto thread_writer = base::WrapUnique(new MinidumpThreadWriter());
 
   thread_list_writer->AddThread(std::move(thread_writer));
-  minidump_file_writer.AddStream(std::move(thread_list_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(thread_list_writer)));
 
   StringFile string_file;
   ASSERT_DEATH_CHECK(minidump_file_writer.WriteEverything(&string_file),

minidump/minidump_unloaded_module_writer_test.cc

@@ -82,7 +82,8 @@ TEST(MinidumpUnloadedModuleWriter, EmptyModule) {
 
   unloaded_module_list_writer->AddUnloadedModule(
       std::move(unloaded_module_writer));
-  minidump_file_writer.AddStream(std::move(unloaded_module_list_writer));
+  ASSERT_TRUE(
+      minidump_file_writer.AddStream(std::move(unloaded_module_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -128,7 +129,8 @@ TEST(MinidumpUnloadedModuleWriter, OneModule) {
 
   unloaded_module_list_writer->AddUnloadedModule(
       std::move(unloaded_module_writer));
-  minidump_file_writer.AddStream(std::move(unloaded_module_list_writer));
+  ASSERT_TRUE(
+      minidump_file_writer.AddStream(std::move(unloaded_module_list_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));

minidump/minidump_user_stream_writer_test.cc

@@ -57,7 +57,7 @@ TEST(MinidumpUserStreamWriter, NoData) {
   auto stream =
       base::WrapUnique(new UserMinidumpStream(kTestStreamId, nullptr));
   user_stream_writer->InitializeFromSnapshot(stream.get());
-  minidump_file_writer.AddStream(std::move(user_stream_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(user_stream_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
@@ -84,7 +84,7 @@ TEST(MinidumpUserStreamWriter, OneStream) {
   auto stream =
       base::WrapUnique(new UserMinidumpStream(kTestStreamId, test_data));
   user_stream_writer->InitializeFromSnapshot(stream.get());
-  minidump_file_writer.AddStream(std::move(user_stream_writer));
+  ASSERT_TRUE(minidump_file_writer.AddStream(std::move(user_stream_writer)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));

snapshot/api/module_annotations_win_test.cc

@@ -28,7 +28,7 @@ class ModuleAnnotationsMultiprocessTest final : public WinMultiprocess {
   void WinMultiprocessParent() override {
     // Read the child executable module.
     HMODULE module = nullptr;
-    CheckedReadFile(ReadPipeHandle(), &module, sizeof(module));
+    CheckedReadFileExactly(ReadPipeHandle(), &module, sizeof(module));
 
     // Reopen the child process with necessary access.
     HANDLE process_handle =
@@ -70,7 +70,7 @@ class ModuleAnnotationsMultiprocessTest final : public WinMultiprocess {
 
     // Wait until a signal from the parent process to terminate.
     char c;
-    CheckedReadFile(ReadPipeHandle(), &c, sizeof(c));
+    CheckedReadFileExactly(ReadPipeHandle(), &c, sizeof(c));
   }
 };
 

snapshot/mac/mach_o_image_annotations_reader_test.cc

@@ -245,7 +245,7 @@ class TestMachOImageAnnotationsReader final
     // Wait for the child process to indicate that it’s done setting up its
     // annotations via the CrashpadInfo interface.
     char c;
-    CheckedReadFile(ReadPipeHandle(), &c, sizeof(c));
+    CheckedReadFileExactly(ReadPipeHandle(), &c, sizeof(c));
 
     // Verify the “simple map” annotations set via the CrashpadInfo interface.
     const std::vector<ProcessReader::Module>& modules =
@@ -333,7 +333,7 @@ class TestMachOImageAnnotationsReader final
     CheckedWriteFile(WritePipeHandle(), &c, sizeof(c));
 
     // Wait for the parent to indicate that it’s safe to crash.
-    CheckedReadFile(ReadPipeHandle(), &c, sizeof(c));
+    CheckedReadFileExactly(ReadPipeHandle(), &c, sizeof(c));
 
     // Direct an exception message to the exception server running in the
     // parent.

snapshot/mac/process_reader.cc

@@ -116,7 +116,9 @@ bool ProcessReader::Initialize(task_t task) {
     return false;
   }
 
-  is_64_bit_ = process_info_.Is64Bit();
+  if (!process_info_.Is64Bit(&is_64_bit_)) {
+    return false;
+  }
 
   task_memory_.reset(new TaskMemory(task));
   task_ = task;
@@ -125,6 +127,11 @@ bool ProcessReader::Initialize(task_t task) {
   return true;
 }
 
+void ProcessReader::StartTime(timeval* start_time) const {
+  bool rv = process_info_.StartTime(start_time);
+  DCHECK(rv);
+}
+
 bool ProcessReader::CPUTimes(timeval* user_time, timeval* system_time) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
 

snapshot/mac/process_reader.h

@@ -124,9 +124,7 @@ class ProcessReader {
   //! \brief Determines the target process’ start time.
   //!
   //! \param[out] start_time The time that the process started.
-  void StartTime(timeval* start_time) const {
+  void StartTime(timeval* start_time) const;
-    process_info_.StartTime(start_time);
-  }
 
   //! \brief Determines the target process’ execution time.
   //!

snapshot/mac/process_reader_test.cc

@@ -105,7 +105,7 @@ class ProcessReaderChild final : public MachMultiprocess {
     FileHandle read_handle = ReadPipeHandle();
 
     mach_vm_address_t address;
-    CheckedReadFile(read_handle, &address, sizeof(address));
+    CheckedReadFileExactly(read_handle, &address, sizeof(address));
 
     std::string read_string;
     ASSERT_TRUE(process_reader.Memory()->ReadCString(address, &read_string));
@@ -448,15 +448,15 @@ class ProcessReaderThreadedChild final : public MachMultiprocess {
          thread_index < thread_count_ + 1;
          ++thread_index) {
       uint64_t thread_id;
-      CheckedReadFile(read_handle, &thread_id, sizeof(thread_id));
+      CheckedReadFileExactly(read_handle, &thread_id, sizeof(thread_id));
 
       TestThreadPool::ThreadExpectation expectation;
-      CheckedReadFile(read_handle,
+      CheckedReadFileExactly(read_handle,
-                      &expectation.stack_address,
+                             &expectation.stack_address,
-                      sizeof(expectation.stack_address));
+                             sizeof(expectation.stack_address));
-      CheckedReadFile(read_handle,
+      CheckedReadFileExactly(read_handle,
-                      &expectation.suspend_count,
+                             &expectation.suspend_count,
-                      sizeof(expectation.suspend_count));
+                             sizeof(expectation.suspend_count));
 
       // There can’t be any duplicate thread IDs.
       EXPECT_EQ(0u, thread_map.count(thread_id));
@@ -730,7 +730,8 @@ class ProcessReaderModulesChild final : public MachMultiprocess {
     FileHandle read_handle = ReadPipeHandle();
 
     uint32_t expect_modules;
-    CheckedReadFile(read_handle, &expect_modules, sizeof(expect_modules));
+    CheckedReadFileExactly(
+        read_handle, &expect_modules, sizeof(expect_modules));
 
     ASSERT_EQ(expect_modules, modules.size());
 
@@ -740,16 +741,17 @@ class ProcessReaderModulesChild final : public MachMultiprocess {
           "index %zu, name %s", index, modules[index].name.c_str()));
 
       uint32_t expect_name_length;
-      CheckedReadFile(
+      CheckedReadFileExactly(
           read_handle, &expect_name_length, sizeof(expect_name_length));
 
       // The NUL terminator is not read.
       std::string expect_name(expect_name_length, '\0');
-      CheckedReadFile(read_handle, &expect_name[0], expect_name_length);
+      CheckedReadFileExactly(read_handle, &expect_name[0], expect_name_length);
       EXPECT_EQ(expect_name, modules[index].name);
 
       mach_vm_address_t expect_address;
-      CheckedReadFile(read_handle, &expect_address, sizeof(expect_address));
+      CheckedReadFileExactly(
+          read_handle, &expect_address, sizeof(expect_address));
       ASSERT_TRUE(modules[index].reader);
       EXPECT_EQ(expect_address, modules[index].reader->Address());
 

snapshot/snapshot_test.gyp

@@ -57,9 +57,9 @@
         'snapshot.gyp:crashpad_snapshot_api',
         '../client/client.gyp:crashpad_client',
         '../compat/compat.gyp:crashpad_compat',
+        '../test/test.gyp:crashpad_gtest_main',
         '../test/test.gyp:crashpad_test',
         '../third_party/gtest/gtest.gyp:gtest',
-        '../third_party/gtest/gtest.gyp:gtest_main',
         '../third_party/mini_chromium/mini_chromium.gyp:base',
         '../util/util.gyp:crashpad_util',
       ],

snapshot/win/crashpad_snapshot_test_extra_memory_ranges.cc

@@ -43,7 +43,7 @@ int wmain(int argc, wchar_t* argv[]) {
 
   HANDLE in = GetStdHandle(STD_INPUT_HANDLE);
   PCHECK(in != INVALID_HANDLE_VALUE) << "GetStdHandle";
-  CheckedReadFile(in, &c, sizeof(c));
+  CheckedReadFileExactly(in, &c, sizeof(c));
   CHECK(c == 'd' || c == ' ');
 
   // If 'd' we crash with a debug break, otherwise exit normally.

snapshot/win/crashpad_snapshot_test_simple_annotations.cc

@@ -42,7 +42,7 @@ int wmain(int argc, wchar_t* argv[]) {
 
   HANDLE in = GetStdHandle(STD_INPUT_HANDLE);
   PCHECK(in != INVALID_HANDLE_VALUE) << "GetStdHandle";
-  crashpad::CheckedReadFile(in, &c, sizeof(c));
+  crashpad::CheckedReadFileExactly(in, &c, sizeof(c));
   CHECK(c == 'd' || c == ' ');
 
   // If 'd' we crash with a debug break, otherwise exit normally.

snapshot/win/exception_snapshot_win_test.cc

@@ -148,9 +148,9 @@ void TestCrashingChild(const base::string16& directory_modification) {
 
   // The child tells us (approximately) where it will crash.
   WinVMAddress break_near_address;
-  LoggingReadFile(child.stdout_read_handle(),
+  LoggingReadFileExactly(child.stdout_read_handle(),
-                  &break_near_address,
+                         &break_near_address,
-                  sizeof(break_near_address));
+                         sizeof(break_near_address));
   delegate.set_break_near(break_near_address);
 
   // Wait for the child to crash and the exception information to be validated.
@@ -250,9 +250,9 @@ void TestDumpWithoutCrashingChild(
 
   // The child tells us (approximately) where it will capture a dump.
   WinVMAddress dump_near_address;
-  LoggingReadFile(child.stdout_read_handle(),
+  LoggingReadFileExactly(child.stdout_read_handle(),
-                  &dump_near_address,
+                         &dump_near_address,
-                  sizeof(dump_near_address));
+                         sizeof(dump_near_address));
   delegate.set_dump_near(dump_near_address);
 
   // Wait for the child to crash and the exception information to be validated.

snapshot/win/extra_memory_ranges_test.cc

@@ -58,7 +58,7 @@ void TestExtraMemoryRanges(TestType type,
   // Wait for the child process to indicate that it's done setting up its
   // annotations via the CrashpadInfo interface.
   char c;
-  CheckedReadFile(child.stdout_read_handle(), &c, sizeof(c));
+  CheckedReadFileExactly(child.stdout_read_handle(), &c, sizeof(c));
 
   ProcessSnapshotWin snapshot;
   ASSERT_TRUE(snapshot.Initialize(

snapshot/win/pe_image_annotations_reader_test.cc

@@ -62,7 +62,7 @@ void TestAnnotationsOnCrash(TestType type,
   // Wait for the child process to indicate that it's done setting up its
   // annotations via the CrashpadInfo interface.
   char c;
-  CheckedReadFile(child.stdout_read_handle(), &c, sizeof(c));
+  CheckedReadFileExactly(child.stdout_read_handle(), &c, sizeof(c));
 
   ProcessReaderWin process_reader;
   ASSERT_TRUE(process_reader.Initialize(child.process_handle(),

snapshot/win/process_reader_win_test.cc

@@ -69,7 +69,7 @@ class ProcessReaderChild final : public WinMultiprocess {
 #endif
 
     WinVMAddress address;
-    CheckedReadFile(ReadPipeHandle(), &address, sizeof(address));
+    CheckedReadFileExactly(ReadPipeHandle(), &address, sizeof(address));
 
     char buffer[sizeof(kTestMemory)];
     ASSERT_TRUE(
@@ -142,7 +142,7 @@ class ProcessReaderChildThreadSuspendCount final : public WinMultiprocess {
 
   void WinMultiprocessParent() override {
     char c;
-    CheckedReadFile(ReadPipeHandle(), &c, sizeof(c));
+    CheckedReadFileExactly(ReadPipeHandle(), &c, sizeof(c));
     ASSERT_EQ(' ', c);
 
     {

snapshot/win/process_snapshot_win_test.cc

@@ -48,7 +48,8 @@ void TestImageReaderChild(const base::string16& directory_modification) {
   child.Start();
 
   char c;
-  ASSERT_TRUE(LoggingReadFile(child.stdout_read_handle(), &c, sizeof(c)));
+  ASSERT_TRUE(
+      LoggingReadFileExactly(child.stdout_read_handle(), &c, sizeof(c)));
   ASSERT_EQ(' ', c);
 
   ScopedProcessSuspend suspend(child.process_handle());

test/gmock_main.cc

@@ -0,0 +1,23 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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 "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "test/main_arguments.h"
+
+int main(int argc, char* argv[]) {
+  crashpad::test::InitializeMainArguments(argc, argv);
+  testing::InitGoogleMock(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/gtest_main.cc

@@ -0,0 +1,22 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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 "gtest/gtest.h"
+#include "test/main_arguments.h"
+
+int main(int argc, char* argv[]) {
+  crashpad::test::InitializeMainArguments(argc, argv);
+  testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/main_arguments.cc

@@ -0,0 +1,38 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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 "test/main_arguments.h"
+
+#include "base/logging.h"
+
+namespace crashpad {
+namespace test {
+
+const std::vector<std::string>* g_arguments;
+
+void InitializeMainArguments(int argc, char* argv[]) {
+  CHECK(!g_arguments);
+  CHECK(argc);
+  CHECK(argv);
+
+  g_arguments = new const std::vector<std::string>(argv, argv + argc);
+}
+
+const std::vector<std::string>& GetMainArguments() {
+  CHECK(g_arguments);
+  return *g_arguments;
+}
+
+}  // namespace test
+}  // namespace crashpad

test/main_arguments.h

@@ -0,0 +1,49 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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.
+
+#ifndef CRASHPAD_TEST_MAIN_ARGUMENTS_H_
+#define CRASHPAD_TEST_MAIN_ARGUMENTS_H_
+
+#include <string>
+#include <vector>
+
+namespace crashpad {
+namespace test {
+
+//! \brief Saves the arguments to `main()` for later use.
+//!
+//! Call this function from a test program’s `main()` function so that tests
+//! that require access to these variables can retrieve them from
+//! GetMainArguments().
+//!
+//! The contents of \a argv, limited to \a argc elements, will be copied, so
+//! that subsequent modifications to these variables by `main()` will not affect
+//! the state returned by GetMainArguments().
+//!
+//! This function must be called exactly once during the lifetime of a test
+//! program.
+void InitializeMainArguments(int argc, char* argv[]);
+
+//! \brief Retrieves pointers to the arguments to `main()`.
+//!
+//! Tests that need to access the original values of a test program’s `main()`
+//! function’s parameters at process creation can use this function to retrieve
+//! them, provided that `main()` called InitializeMainArguments() before making
+//! any changes to its arguments.
+const std::vector<std::string>& GetMainArguments();
+
+}  // namespace test
+}  // namespace crashpad
+
+#endif  // CRASHPAD_TEST_MAIN_ARGUMENTS_H_

test/main_arguments_test.cc

@@ -0,0 +1,34 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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 "test/main_arguments.h"
+
+#include "gtest/gtest.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+TEST(MainArguments, GetMainArguments) {
+  // Make sure that InitializeMainArguments() has been called and that
+  // GetMainArguments() provides reasonable values.
+  const std::vector<std::string>& arguments = GetMainArguments();
+
+  ASSERT_FALSE(arguments.empty());
+  EXPECT_FALSE(arguments[0].empty());
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad

test/multiprocess_exec_test.cc

@@ -39,7 +39,7 @@ class TestMultiprocessExec final : public MultiprocessExec {
     char c = 'z';
     ASSERT_TRUE(LoggingWriteFile(WritePipeHandle(), &c, 1));
 
-    ASSERT_TRUE(LoggingReadFile(ReadPipeHandle(), &c, 1));
+    ASSERT_TRUE(LoggingReadFileExactly(ReadPipeHandle(), &c, 1));
     EXPECT_EQ('Z', c);
   }
 

test/multiprocess_posix_test.cc

@@ -39,11 +39,11 @@ class TestMultiprocess final : public Multiprocess {
   void MultiprocessParent() override {
     FileHandle read_handle = ReadPipeHandle();
     char c;
-    CheckedReadFile(read_handle, &c, 1);
+    CheckedReadFileExactly(read_handle, &c, 1);
     EXPECT_EQ('M', c);
 
     pid_t pid;
-    CheckedReadFile(read_handle, &pid, sizeof(pid));
+    CheckedReadFileExactly(read_handle, &pid, sizeof(pid));
     EXPECT_EQ(pid, ChildPID());
 
     c = 'm';
@@ -63,7 +63,7 @@ class TestMultiprocess final : public Multiprocess {
     pid_t pid = getpid();
     CheckedWriteFile(write_handle, &pid, sizeof(pid));
 
-    CheckedReadFile(ReadPipeHandle(), &c, 1);
+    CheckedReadFileExactly(ReadPipeHandle(), &c, 1);
     EXPECT_EQ('m', c);
   }
 

test/test.gyp

@@ -42,6 +42,8 @@
         'mac/mach_errors.h',
         'mac/mach_multiprocess.cc',
         'mac/mach_multiprocess.h',
+        'main_arguments.cc',
+        'main_arguments.h',
         'multiprocess.h',
         'multiprocess_exec.h',
         'multiprocess_exec_posix.cc',
@@ -63,6 +65,11 @@
         'win/win_multiprocess.cc',
         'win/win_multiprocess.h',
       ],
+      'direct_dependent_settings': {
+        'include_dirs': [
+          '..',
+        ],
+      },
       'conditions': [
         ['OS=="mac"', {
           'link_settings': {
@@ -87,5 +94,28 @@
         }],
       ],
     },
+    {
+      'target_name': 'crashpad_gtest_main',
+      'type': 'static_library',
+      'dependencies': [
+        'crashpad_test',
+        '../third_party/gtest/gtest.gyp:gtest',
+      ],
+      'sources': [
+        'gtest_main.cc',
+      ],
+    },
+    {
+      'target_name': 'crashpad_gmock_main',
+      'type': 'static_library',
+      'dependencies': [
+        'crashpad_test',
+        '../third_party/gtest/gmock.gyp:gmock',
+        '../third_party/gtest/gtest.gyp:gtest',
+      ],
+      'sources': [
+        'gmock_main.cc',
+      ],
+    },
   ],
 }

test/test_test.gyp

@@ -22,10 +22,10 @@
       'type': 'executable',
       'dependencies': [
         'crashpad_test_test_multiprocess_exec_test_child',
+        'test.gyp:crashpad_gmock_main',
         'test.gyp:crashpad_test',
         '../compat/compat.gyp:crashpad_compat',
         '../third_party/gtest/gmock.gyp:gmock',
-        '../third_party/gtest/gmock.gyp:gmock_main',
         '../third_party/gtest/gtest.gyp:gtest',
         '../third_party/mini_chromium/mini_chromium.gyp:base',
         '../util/util.gyp:crashpad_util',
@@ -36,6 +36,7 @@
       'sources': [
         'hex_string_test.cc',
         'mac/mach_multiprocess_test.cc',
+        'main_arguments_test.cc',
         'multiprocess_exec_test.cc',
         'multiprocess_posix_test.cc',
         'paths_test.cc',

test/win/win_child_process.cc

@@ -203,7 +203,7 @@ std::unique_ptr<WinChildProcess::Handles> WinChildProcess::Launch() {
   // immediately, and test code expects process initialization to have
   // completed so it can, for example, read the process memory.
   char c;
-  if (!LoggingReadFile(handles_for_parent->read.get(), &c, sizeof(c))) {
+  if (!LoggingReadFileExactly(handles_for_parent->read.get(), &c, sizeof(c))) {
     ADD_FAILURE() << "LoggedReadFile";
     return std::unique_ptr<Handles>();
   }

tools/crashpad_database_util.cc

@@ -560,13 +560,21 @@ int DatabaseUtilMain(int argc, char* argv[]) {
     return EXIT_FAILURE;
   }
 
+  bool used_stdin = false;
   for (const base::FilePath new_report_path : options.new_report_paths) {
     std::unique_ptr<FileReaderInterface> file_reader;
 
-    bool is_stdin = false;
     if (new_report_path.value() == FILE_PATH_LITERAL("-")) {
-      is_stdin = true;
+      if (used_stdin) {
-      file_reader.reset(new WeakStdioFileReader(stdin));
+        fprintf(stderr,
+                "%" PRFilePath
+                ": Only one --new-report may be read from standard input\n",
+                me.value().c_str());
+        return EXIT_FAILURE;
+      }
+      used_stdin = true;
+      file_reader.reset(new WeakFileHandleFileReader(
+          StdioFileHandle(StdioStream::kStandardInput)));
     } else {
       std::unique_ptr<FileReader> file_path_reader(new FileReader());
       if (!file_path_reader->Open(new_report_path)) {
@@ -597,7 +605,7 @@ int DatabaseUtilMain(int argc, char* argv[]) {
           !LoggingWriteFile(new_report->handle, buf, read_result)) {
         return EXIT_FAILURE;
       }
-    } while (read_result == sizeof(buf));
+    } while (read_result > 0);
 
     call_error_writing_crash_report.Disarm();
 
@@ -607,13 +615,6 @@ int DatabaseUtilMain(int argc, char* argv[]) {
       return EXIT_FAILURE;
     }
 
-    file_reader.reset();
-    if (is_stdin) {
-      if (fclose(stdin) == EOF) {
-        STDIO_PLOG(ERROR) << "fclose";
-      }
-    }
-
     const char* prefix = (show_operations > 1) ? "New report ID: " : "";
     printf("%s%s\n", prefix, uuid.ToString().c_str());
   }

tools/crashpad_http_upload.cc

@@ -163,7 +163,8 @@ int HTTPUploadMain(int argc, char* argv[]) {
       return EXIT_FAILURE;
     }
   } else {
-    file_writer.reset(new WeakStdioFileWriter(stdout));
+    file_writer.reset(new WeakFileHandleFileWriter(
+        StdioFileHandle(StdioStream::kStandardOutput)));
   }
 
   http_multipart_builder.SetGzipEnabled(options.upload_gzip);

util/file/delimited_file_reader.cc

@@ -0,0 +1,109 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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/file/delimited_file_reader.h"
+
+#include <sys/types.h>
+
+#include <algorithm>
+#include <limits>
+
+#include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
+
+namespace crashpad {
+
+DelimitedFileReader::DelimitedFileReader(FileReaderInterface* file_reader)
+    : file_reader_(file_reader), buf_pos_(0), buf_len_(0), eof_(false) {
+  static_assert(sizeof(buf_) <= std::numeric_limits<decltype(buf_pos_)>::max(),
+                "buf_pos_ must cover buf_");
+  static_assert(sizeof(buf_) <= std::numeric_limits<decltype(buf_len_)>::max(),
+                "buf_len_ must cover buf_");
+}
+
+DelimitedFileReader::~DelimitedFileReader() {}
+
+DelimitedFileReader::Result DelimitedFileReader::GetDelim(char delimiter,
+                                                          std::string* field) {
+  if (eof_) {
+    DCHECK_EQ(buf_pos_, buf_len_);
+
+    // Allow subsequent calls to attempt to read more data from the file. If the
+    // file is still at EOF in the future, the read will return 0 and cause
+    // kEndOfFile to be returned anyway.
+    eof_ = false;
+
+    return Result::kEndOfFile;
+  }
+
+  std::string local_field;
+  while (true) {
+    if (buf_pos_ == buf_len_) {
+      // buf_ is empty. Refill it.
+      FileOperationResult read_result = file_reader_->Read(buf_, sizeof(buf_));
+      if (read_result < 0) {
+        return Result::kError;
+      } else if (read_result == 0) {
+        if (!local_field.empty()) {
+          // The file ended with a field that wasn’t terminated by a delimiter
+          // character.
+          //
+          // This is EOF, but EOF can’t be returned because there’s a field that
+          // needs to be returned to the caller. Cache the detected EOF so it
+          // can be returned next time. This is done to support proper semantics
+          // for weird “files” like terminal input that can reach EOF and then
+          // “grow”, allowing subsequent reads past EOF to block while waiting
+          // for more data. Once EOF is detected by a read that returns 0, that
+          // EOF signal should propagate to the caller before attempting a new
+          // read. Here, it will be returned on the next call to this method
+          // without attempting to read more data.
+          eof_ = true;
+          field->swap(local_field);
+          return Result::kSuccess;
+        }
+        return Result::kEndOfFile;
+      }
+
+      DCHECK_LE(static_cast<size_t>(read_result), arraysize(buf_));
+      DCHECK(
+          base::IsValueInRangeForNumericType<decltype(buf_len_)>(read_result));
+      buf_len_ = static_cast<decltype(buf_len_)>(read_result);
+      buf_pos_ = 0;
+    }
+
+    const char* const start = buf_ + buf_pos_;
+    const char* const end = buf_ + buf_len_;
+    const char* const found = std::find(start, end, delimiter);
+
+    local_field.append(start, found);
+    buf_pos_ = static_cast<decltype(buf_pos_)>(found - buf_);
+    DCHECK_LE(buf_pos_, buf_len_);
+
+    if (found != end) {
+      // A real delimiter character was found. Append it to the field being
+      // built and return it.
+      local_field.push_back(delimiter);
+      ++buf_pos_;
+      DCHECK_LE(buf_pos_, buf_len_);
+      field->swap(local_field);
+      return Result::kSuccess;
+    }
+  }
+}
+
+DelimitedFileReader::Result DelimitedFileReader::GetLine(std::string* line) {
+  return GetDelim('\n', line);
+}
+
+}  // namespace crashpad

util/file/delimited_file_reader.h

@@ -0,0 +1,93 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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.
+
+#ifndef CRASHPAD_UTIL_FILE_DELIMITED_FILE_READER_H_
+#define CRASHPAD_UTIL_FILE_DELIMITED_FILE_READER_H_
+
+#include <stdint.h>
+
+#include <string>
+
+#include "base/macros.h"
+#include "util/file/file_reader.h"
+
+namespace crashpad {
+
+//! \brief Reads a file one field or line at a time.
+//!
+//! The file is interpreted as a series of fields separated by delimiter
+//! characters. When the delimiter character is the newline character
+//! (<code>'\\n'</code>), the file is interpreted as a series of lines.
+//!
+//! It is safe to mix GetDelim() and GetLine() calls, if appropriate for the
+//! format being interpreted.
+//!
+//! This is a replacement for the standard library’s `getdelim()` and
+//! `getline()` functions, adapted to work with FileReaderInterface objects
+//! instead of `FILE*` streams.
+class DelimitedFileReader {
+ public:
+  //! \brief The result of a GetDelim() or GetLine() call.
+  enum class Result {
+    //! \brief An error occurred, and a message was logged.
+    kError = -1,
+
+    //! \brief A field or line was read from the file.
+    kSuccess,
+
+    //! \brief The end of the file was encountered.
+    kEndOfFile,
+  };
+
+  explicit DelimitedFileReader(FileReaderInterface* file_reader);
+  ~DelimitedFileReader();
+
+  //! \brief Reads a single field from the file.
+  //!
+  //! \param[in] delimiter The delimiter character that terminates the field.
+  //!     It is safe to call this method multiple times while changing the value
+  //!     of this parameter, if appropriate for the format being interpreted.
+  //! \param[out] field The field read from the file. This parameter will
+  //!     include the field’s terminating delimiter character unless the field
+  //!     was at the end of the file and was read without such a character.
+  //!     This parameter will not be empty.
+  //!
+  //! \return a #Result value. \a field is only valid when Result::kSuccess is
+  //!     returned.
+  Result GetDelim(char delimiter, std::string* field);
+
+  //! \brief Reads a single line from the file.
+  //!
+  //! \param[out] line The line read from the file. This parameter will include
+  //!     the line terminating delimiter character unless the line was at the
+  //!     end of the file and was read without such a character. This parameter
+  //!     will not be empty.
+  //!
+  //! \return a #Result value. \a line is only valid when Result::kSuccess is
+  //!     returned.
+  Result GetLine(std::string* line);
+
+ private:
+  char buf_[4096];
+  FileReaderInterface* file_reader_;  // weak
+  uint16_t buf_pos_;  // Index into buf_ of the start of the next field.
+  uint16_t buf_len_;  // The size of buf_ that’s been filled.
+  bool eof_;  // Caches the EOF signal when detected following a partial field.
+
+  DISALLOW_COPY_AND_ASSIGN(DelimitedFileReader);
+};
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_UTIL_FILE_DELIMITED_FILE_READER_H_

util/file/delimited_file_reader_test.cc

@@ -0,0 +1,363 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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/file/delimited_file_reader.h"
+
+#include <vector>
+
+#include "base/format_macros.h"
+#include "base/strings/stringprintf.h"
+#include "gtest/gtest.h"
+#include "util/file/string_file.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+TEST(DelimitedFileReader, EmptyFile) {
+  StringFile string_file;
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string line;
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+}
+
+TEST(DelimitedFileReader, EmptyOneLineFile) {
+  StringFile string_file;
+  string_file.SetString("\n");
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string line;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ(string_file.string(), line);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+}
+
+TEST(DelimitedFileReader, SmallOneLineFile) {
+  StringFile string_file;
+  string_file.SetString("one line\n");
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string line;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ(string_file.string(), line);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+}
+
+TEST(DelimitedFileReader, SmallOneLineFileWithoutNewline) {
+  StringFile string_file;
+  string_file.SetString("no newline");
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string line;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ(string_file.string(), line);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+}
+
+TEST(DelimitedFileReader, SmallMultiLineFile) {
+  StringFile string_file;
+  string_file.SetString("first\nsecond line\n3rd\n");
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string line;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ("first\n", line);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ("second line\n", line);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ("3rd\n", line);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+}
+
+TEST(DelimitedFileReader, SmallMultiFieldFile) {
+  StringFile string_file;
+  string_file.SetString("first,second field\ntwo lines,3rd,");
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string field;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim(',', &field));
+  EXPECT_EQ("first,", field);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim(',', &field));
+  EXPECT_EQ("second field\ntwo lines,", field);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim(',', &field));
+  EXPECT_EQ("3rd,", field);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetDelim(',', &field));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetDelim(',', &field));
+}
+
+TEST(DelimitedFileReader, SmallMultiFieldFile_MixedDelimiters) {
+  StringFile string_file;
+  string_file.SetString("first,second, still 2nd\t3rd\nalso\tnewline\n55555$");
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string field;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim(',', &field));
+  EXPECT_EQ("first,", field);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim('\t', &field));
+  EXPECT_EQ("second, still 2nd\t", field);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&field));
+  EXPECT_EQ("3rd\n", field);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim('\n', &field));
+  EXPECT_EQ("also\tnewline\n", field);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim('$', &field));
+  EXPECT_EQ("55555$", field);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetDelim('?', &field));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&field));
+}
+
+TEST(DelimitedFileReader, EmptyLineMultiLineFile) {
+  StringFile string_file;
+  string_file.SetString("first\n\n\n4444\n");
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string line;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ("first\n", line);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ("\n", line);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ("\n", line);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ("4444\n", line);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+}
+
+TEST(DelimitedFileReader, LongOneLineFile) {
+  std::string contents(50000, '!');
+  contents[1] = '?';
+  contents.push_back('\n');
+
+  StringFile string_file;
+  string_file.SetString(contents);
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string line;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ(contents, line);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+}
+
+void TestLongMultiLineFile(int base_length) {
+  std::vector<std::string> lines;
+  std::string contents;
+  for (size_t line_index = 0; line_index <= 'z' - 'a'; ++line_index) {
+    char c = 'a' + static_cast<char>(line_index);
+
+    // Mix up the lengths a little.
+    std::string line(base_length + line_index * ((line_index % 3) - 1), c);
+
+    // Mix up the data a little too.
+    ASSERT_LT(line_index, line.size());
+    line[line_index] -= ('a' - 'A');
+
+    line.push_back('\n');
+    contents.append(line);
+    lines.push_back(line);
+  }
+
+  StringFile string_file;
+  string_file.SetString(contents);
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string line;
+  for (size_t line_index = 0; line_index < lines.size(); ++line_index) {
+    SCOPED_TRACE(base::StringPrintf("line_index %" PRIuS, line_index));
+    ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+              delimited_file_reader.GetLine(&line));
+    EXPECT_EQ(lines[line_index], line);
+  }
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+}
+
+TEST(DelimitedFileReader, LongMultiLineFile) {
+  TestLongMultiLineFile(500);
+}
+
+TEST(DelimitedFileReader, ReallyLongMultiLineFile) {
+  TestLongMultiLineFile(5000);
+}
+
+TEST(DelimitedFileReader, EmbeddedNUL) {
+  const char kString[] = "embedded\0NUL\n";
+  StringFile string_file;
+  string_file.SetString(std::string(kString, arraysize(kString) - 1));
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string line;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetLine(&line));
+  EXPECT_EQ(string_file.string(), line);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetLine(&line));
+}
+
+TEST(DelimitedFileReader, NULDelimiter) {
+  const char kString[] = "aa\0b\0ccc\0";
+  StringFile string_file;
+  string_file.SetString(std::string(kString, arraysize(kString) - 1));
+  DelimitedFileReader delimited_file_reader(&string_file);
+
+  std::string field;
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim('\0', &field));
+  EXPECT_EQ(std::string("aa\0", 3), field);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim('\0', &field));
+  EXPECT_EQ(std::string("b\0", 2), field);
+  ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+            delimited_file_reader.GetDelim('\0', &field));
+  EXPECT_EQ(std::string("ccc\0", 4), field);
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetDelim('\0', &field));
+
+  // The file is still at EOF.
+  EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+            delimited_file_reader.GetDelim('\0', &field));
+}
+
+TEST(DelimitedFileReader, EdgeCases) {
+  const size_t kSizes[] = {4094, 4095, 4096, 4097, 8190, 8191, 8192, 8193};
+  for (size_t index = 0; index < arraysize(kSizes); ++index) {
+    size_t size = kSizes[index];
+    SCOPED_TRACE(
+        base::StringPrintf("index %" PRIuS ", size %" PRIuS, index, size));
+
+    std::string line_0(size, '$');
+    line_0.push_back('\n');
+
+    StringFile string_file;
+    string_file.SetString(line_0);
+    DelimitedFileReader delimited_file_reader(&string_file);
+
+    std::string line;
+    ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+              delimited_file_reader.GetLine(&line));
+    EXPECT_EQ(line_0, line);
+    EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+              delimited_file_reader.GetLine(&line));
+
+    // The file is still at EOF.
+    EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+              delimited_file_reader.GetLine(&line));
+
+    std::string line_1(size, '@');
+    line_1.push_back('\n');
+
+    string_file.SetString(line_0 + line_1);
+    ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+              delimited_file_reader.GetLine(&line));
+    EXPECT_EQ(line_0, line);
+    ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+              delimited_file_reader.GetLine(&line));
+    EXPECT_EQ(line_1, line);
+    EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+              delimited_file_reader.GetLine(&line));
+
+    // The file is still at EOF.
+    EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+              delimited_file_reader.GetLine(&line));
+
+    line_1[size] = '?';
+
+    string_file.SetString(line_0 + line_1);
+    ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+              delimited_file_reader.GetLine(&line));
+    EXPECT_EQ(line_0, line);
+    ASSERT_EQ(DelimitedFileReader::Result::kSuccess,
+              delimited_file_reader.GetLine(&line));
+    EXPECT_EQ(line_1, line);
+    EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+              delimited_file_reader.GetLine(&line));
+
+    // The file is still at EOF.
+    EXPECT_EQ(DelimitedFileReader::Result::kEndOfFile,
+              delimited_file_reader.GetLine(&line));
+  }
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad

util/file/file_io.cc

@@ -15,42 +15,131 @@
 #include "util/file/file_io.h"
 
 #include "base/logging.h"
+#include "base/macros.h"
 #include "base/numerics/safe_conversions.h"
 
 namespace crashpad {
 
-bool LoggingReadFile(FileHandle file, void* buffer, size_t size) {
+namespace {
-  FileOperationResult expect = base::checked_cast<FileOperationResult>(size);
+
-  FileOperationResult rv = ReadFile(file, buffer, size);
+class FileIOReadExactly final : public internal::ReadExactlyInternal {
-  if (rv < 0) {
+ public:
-    PLOG(ERROR) << "read";
+  explicit FileIOReadExactly(FileHandle file)
-    return false;
+      : ReadExactlyInternal(), file_(file) {}
+  ~FileIOReadExactly() {}
+
+ private:
+  // ReadExactlyInternal:
+  FileOperationResult Read(void* buffer, size_t size, bool can_log) override {
+    FileOperationResult rv = ReadFile(file_, buffer, size);
+    if (rv < 0) {
+      PLOG_IF(ERROR, can_log) << internal::kNativeReadFunctionName;
+      return -1;
+    }
+    return rv;
   }
-  if (rv != expect) {
+
-    LOG(ERROR) << "read: expected " << expect << ", observed " << rv;
+  FileHandle file_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileIOReadExactly);
+};
+
+class FileIOWriteAll final : public internal::WriteAllInternal {
+ public:
+  explicit FileIOWriteAll(FileHandle file) : WriteAllInternal(), file_(file) {}
+  ~FileIOWriteAll() {}
+
+ private:
+  // WriteAllInternal:
+  FileOperationResult Write(const void* buffer, size_t size) override {
+    return internal::NativeWriteFile(file_, buffer, size);
+  }
+
+  FileHandle file_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileIOWriteAll);
+};
+
+}  // namespace
+
+namespace internal {
+
+bool ReadExactlyInternal::ReadExactly(void* buffer, size_t size, bool can_log) {
+  char* buffer_c = static_cast<char*>(buffer);
+  size_t total_bytes = 0;
+  size_t remaining = size;
+  while (remaining > 0) {
+    FileOperationResult bytes_read = Read(buffer_c, remaining, can_log);
+    if (bytes_read < 0) {
+      return false;
+    }
+
+    DCHECK_LE(static_cast<size_t>(bytes_read), remaining);
+
+    if (bytes_read == 0) {
+      break;
+    }
+
+    buffer_c += bytes_read;
+    remaining -= bytes_read;
+    total_bytes += bytes_read;
+  }
+
+  if (total_bytes != size) {
+    LOG_IF(ERROR, can_log) << "ReadExactly: expected " << size << ", observed "
+                           << total_bytes;
     return false;
   }
 
   return true;
 }
 
+bool WriteAllInternal::WriteAll(const void* buffer, size_t size) {
+  const char* buffer_c = static_cast<const char*>(buffer);
+
+  while (size > 0) {
+    FileOperationResult bytes_written = Write(buffer_c, size);
+    if (bytes_written < 0) {
+      return false;
+    }
+
+    DCHECK_NE(bytes_written, 0);
+
+    buffer_c += bytes_written;
+    size -= bytes_written;
+  }
+
+  return true;
+}
+
+}  // namespace internal
+
+bool ReadFileExactly(FileHandle file, void* buffer, size_t size) {
+  FileIOReadExactly read_exactly(file);
+  return read_exactly.ReadExactly(buffer, size, false);
+}
+
+bool LoggingReadFileExactly(FileHandle file, void* buffer, size_t size) {
+  FileIOReadExactly read_exactly(file);
+  return read_exactly.ReadExactly(buffer, size, true);
+}
+
+bool WriteFile(FileHandle file, const void* buffer, size_t size) {
+  FileIOWriteAll write_all(file);
+  return write_all.WriteAll(buffer, size);
+}
+
 bool LoggingWriteFile(FileHandle file, const void* buffer, size_t size) {
-  FileOperationResult expect = base::checked_cast<FileOperationResult>(size);
+  if (!WriteFile(file, buffer, size)) {
-  FileOperationResult rv = WriteFile(file, buffer, size);
+    PLOG(ERROR) << internal::kNativeWriteFunctionName;
-  if (rv < 0) {
-    PLOG(ERROR) << "write";
-    return false;
-  }
-  if (rv != expect) {
-    LOG(ERROR) << "write: expected " << expect << ", observed " << rv;
     return false;
   }
 
   return true;
 }
 
-void CheckedReadFile(FileHandle file, void* buffer, size_t size) {
+void CheckedReadFileExactly(FileHandle file, void* buffer, size_t size) {
-  CHECK(LoggingReadFile(file, buffer, size));
+  CHECK(LoggingReadFileExactly(file, buffer, size));
 }
 
 void CheckedWriteFile(FileHandle file, const void* buffer, size_t size) {
@@ -61,9 +150,9 @@ void CheckedReadFileAtEOF(FileHandle file) {
   char c;
   FileOperationResult rv = ReadFile(file, &c, 1);
   if (rv < 0) {
-    PCHECK(rv == 0) << "read";
+    PCHECK(rv == 0) << internal::kNativeReadFunctionName;
   } else {
-    CHECK_EQ(rv, 0) << "read";
+    CHECK_EQ(rv, 0) << internal::kNativeReadFunctionName;
   }
 }
 

util/file/file_io.h

@@ -26,27 +26,6 @@
 #include "util/win/scoped_handle.h"
 #endif
 
-//! \file
-
-#if defined(OS_POSIX) || DOXYGEN
-
-//! \brief A `PLOG()` macro usable for standard input/output error conditions.
-//!
-//! The `PLOG()` macro uses `errno` on POSIX and is appropriate to report
-//! errors from standard input/output functions. On Windows, `PLOG()` uses
-//! `GetLastError()`, and cannot be used to report errors from standard
-//! input/output functions. This macro uses `PLOG()` when appropriate for
-//! standard I/O functions, and `LOG()` otherwise.
-#define STDIO_PLOG(x) PLOG(x)
-
-#else
-
-#define STDIO_PLOG(x) LOG(x)
-#define fseeko(file, offset, whence) _fseeki64(file, offset, whence)
-#define ftello(file) _ftelli64(file)
-
-#endif
-
 namespace base {
 class FilePath;
 }  // namespace base
@@ -114,82 +93,205 @@ enum class FileLocking : bool {
   kExclusive,
 };
 
-//! \brief Reads from a file, retrying when interrupted on POSIX or following a
+//! \brief Determines the FileHandle that StdioFileHandle() returns.
-//!     short read.
+enum class StdioStream {
+  //! \brief Standard input, or `stdin`.
+  kStandardInput,
+
+  //! \brief Standard output, or `stdout`.
+  kStandardOutput,
+
+  //! \brief Standard error, or `stderr`.
+  kStandardError,
+};
+
+namespace internal {
+
+//! \brief The name of the native read function used by ReadFile().
 //!
-//! This function reads into \a buffer, stopping only when \a size bytes have
+//! This value may be useful for logging.
-//! been read or when end-of-file has been reached. On Windows, reading from
+//!
-//! sockets is not currently supported.
+//! \sa kNativeWriteFunctionName
+extern const char kNativeReadFunctionName[];
+
+//! \brief The name of the native write function used by WriteFile().
+//!
+//! This value may be useful for logging.
+//!
+//! \sa kNativeReadFunctionName
+extern const char kNativeWriteFunctionName[];
+
+//! \brief The internal implementation of ReadFileExactly() and its wrappers.
+//!
+//! The logic is exposed so that it may be reused by FileReaderInterface, and
+//! so that it may be tested without requiring large files to be read. It is not
+//! intended to be used more generally. Use ReadFileExactly(),
+//! LoggingReadFileExactly(), CheckedReadFileExactly(), or
+//! FileReaderInterface::ReadExactly() instead.
+class ReadExactlyInternal {
+ public:
+  //! \brief Calls Read(), retrying following a short read, ensuring that
+  //!     exactly \a size bytes are read.
+  //!
+  //! \return `true` on success. `false` if the underlying Read() fails or if
+  //!     fewer than \a size bytes were read. When returning `false`, if \a
+  //!     can_log is `true`, logs a message.
+  bool ReadExactly(void* buffer, size_t size, bool can_log);
+
+ protected:
+  ReadExactlyInternal() {}
+  ~ReadExactlyInternal() {}
+
+ private:
+  //! \brief Wraps a read operation, such as ReadFile().
+  //!
+  //! \return The number of bytes read and placed into \a buffer, or `-1` on
+  //!     error. When returning `-1`, if \a can_log is `true`, logs a message.
+  virtual FileOperationResult Read(void* buffer, size_t size, bool can_log) = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(ReadExactlyInternal);
+};
+
+//! \brief The internal implementation of WriteFile() and its wrappers.
+//!
+//! The logic is exposed so that it may be tested without requiring large files
+//! to be written. It is not intended to be used more generally. Use
+//! WriteFile(), LoggingWriteFile(), CheckedWriteFile(), or
+//! FileWriterInterface::Write() instead.
+class WriteAllInternal {
+ public:
+  //! \brief Calls Write(), retrying following a short write, ensuring that
+  //!     exactly \a size bytes are written.
+  //!
+  //! \return `true` on success. `false` if the underlying Write() fails or if
+  //!     fewer than \a size bytes were written.
+  bool WriteAll(const void* buffer, size_t size);
+
+ protected:
+  WriteAllInternal() {}
+  ~WriteAllInternal() {}
+
+ private:
+  //! \brief Wraps a write operation, such as NativeWriteFile().
+  //!
+  //! \return The number of bytes written from \a buffer, or `-1` on error.
+  virtual FileOperationResult Write(const void* buffer, size_t size) = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(WriteAllInternal);
+};
+
+//! \brief Writes to a file, retrying when interrupted on POSIX.
+//!
+//! Fewer than \a size bytes may be written to \a file. This can happen if the
+//! underlying write operation returns before writing the entire buffer, or if
+//! the buffer is too large to write in a single operation, possibly due to a
+//! limitation of a data type used to express the number of bytes written.
+//!
+//! This function adapts native write operations for uniform use by WriteFile().
+//! This function should only be called by WriteFile(). Other code should call
+//! WriteFile() or another function that wraps WriteFile().
+//!
+//! \param[in] file The file to write to.
+//! \param[in] buffer A buffer containing data to be written.
+//! \param[in] size The number of bytes from \a buffer to write.
+//!
+//! \return The number of bytes actually written from \a buffer to \a file on
+//!     success. `-1` on error, with `errno` or `GetLastError()` set
+//!     appropriately.
+FileOperationResult NativeWriteFile(FileHandle file,
+                                    const void* buffer,
+                                    size_t size);
+
+}  // namespace internal
+
+//! \brief Reads from a file, retrying when interrupted before reading any data
+//!     on POSIX.
+//!
+//! This function reads into \a buffer. Fewer than \a size bytes may be read.
+//! On Windows, reading from sockets is not currently supported.
 //!
 //! \return The number of bytes read and placed into \a buffer, or `-1` on
 //!     error, with `errno` or `GetLastError()` set appropriately. On error, a
 //!     portion of \a file may have been read into \a buffer.
 //!
 //! \sa WriteFile
-//! \sa LoggingReadFile
+//! \sa ReadFileExactly
-//! \sa CheckedReadFile
+//! \sa LoggingReadFileExactly
+//! \sa CheckedReadFileExactly
 //! \sa CheckedReadFileAtEOF
 FileOperationResult ReadFile(FileHandle file, void* buffer, size_t size);
 
-//! \brief Writes to a file, retrying when interrupted or following a short
+//! \brief Writes to a file, retrying when interrupted on POSIX or following a
-//!     write on POSIX.
+//!     short write.
 //!
 //! This function writes to \a file, stopping only when \a size bytes have been
 //! written.
 //!
-//! \return The number of bytes written from \a buffer, or `-1` on error, with
+//! \return `true` on success. `false` on error, with `errno` or
-//!     `errno` or `GetLastError()` set appropriately. On error, a portion of
+//!     `GetLastError()` set appropriately. On error, a portion of \a buffer may
-//!     \a buffer may have been written to \a file.
+//!     have been written to \a file.
 //!
 //! \sa ReadFile
 //! \sa LoggingWriteFile
 //! \sa CheckedWriteFile
-FileOperationResult WriteFile(FileHandle file, const void* buffer, size_t size);
+bool WriteFile(FileHandle file, const void* buffer, size_t size);
 
-//! \brief Wraps ReadFile(), ensuring that exactly \a size bytes are read.
+//! \brief Wraps ReadFile(), retrying following a short read, ensuring that
+//!     exactly \a size bytes are read.
 //!
-//! \return `true` on success. If \a size is out of the range of possible
+//! \return `true` on success. If the underlying ReadFile() fails, or if fewer
-//!     ReadFile() return values, if the underlying ReadFile() fails, or if
+//!     than \a size bytes were read, this function logs a message and
-//!     other than \a size bytes were read, this function logs a message and
 //!     returns `false`.
 //!
 //! \sa LoggingWriteFile
 //! \sa ReadFile
-//! \sa CheckedReadFile
+//! \sa LoggingReadFileExactly
+//! \sa CheckedReadFileExactly
 //! \sa CheckedReadFileAtEOF
-bool LoggingReadFile(FileHandle file, void* buffer, size_t size);
+bool ReadFileExactly(FileHandle file, void* buffer, size_t size);
+
+//! \brief Wraps ReadFile(), retrying following a short read, ensuring that
+//!     exactly \a size bytes are read.
+//!
+//! \return `true` on success. If the underlying ReadFile() fails, or if fewer
+//!     than \a size bytes were read, this function logs a message and
+//!     returns `false`.
+//!
+//! \sa LoggingWriteFile
+//! \sa ReadFile
+//! \sa ReadFileExactly
+//! \sa CheckedReadFileExactly
+//! \sa CheckedReadFileAtEOF
+bool LoggingReadFileExactly(FileHandle file, void* buffer, size_t size);
 
 //! \brief Wraps WriteFile(), ensuring that exactly \a size bytes are written.
 //!
-//! \return `true` on success. If \a size is out of the range of possible
+//! \return `true` on success. If the underlying WriteFile() fails, or if fewer
-//!     WriteFile() return values, if the underlying WriteFile() fails, or if
+//!     than \a size bytes were written, this function logs a message and
-//!     other than \a size bytes were written, this function logs a message and
 //!     returns `false`.
 //!
-//! \sa LoggingReadFile
+//! \sa LoggingReadFileExactly
 //! \sa WriteFile
 //! \sa CheckedWriteFile
 bool LoggingWriteFile(FileHandle file, const void* buffer, size_t size);
 
 //! \brief Wraps ReadFile(), ensuring that exactly \a size bytes are read.
 //!
-//! If \a size is out of the range of possible ReadFile() return values, if the
+//! If the underlying ReadFile() fails, or if fewer than \a size bytes were
-//! underlying ReadFile() fails, or if other than \a size bytes were read, this
+//! read, this function causes execution to terminate without returning.
-//! function causes execution to terminate without returning.
 //!
 //! \sa CheckedWriteFile
 //! \sa ReadFile
-//! \sa LoggingReadFile
+//! \sa LoggingReadFileExactly
 //! \sa CheckedReadFileAtEOF
-void CheckedReadFile(FileHandle file, void* buffer, size_t size);
+void CheckedReadFileExactly(FileHandle file, void* buffer, size_t size);
 
 //! \brief Wraps WriteFile(), ensuring that exactly \a size bytes are written.
 //!
-//! If \a size is out of the range of possible WriteFile() return values, if the
+//! if the underlying WriteFile() fails, or if fewer than \a size bytes were
-//! underlying WriteFile() fails, or if other than \a size bytes were written,
+//! written, this function causes execution to terminate without returning.
-//! this function causes execution to terminate without returning.
 //!
-//! \sa CheckedReadFile
+//! \sa CheckedReadFileExactly
 //! \sa WriteFile
 //! \sa LoggingWriteFile
 void CheckedWriteFile(FileHandle file, const void* buffer, size_t size);
@@ -200,7 +302,7 @@ void CheckedWriteFile(FileHandle file, const void* buffer, size_t size);
 //! If the underlying ReadFile() fails, or if a byte actually is read, this
 //! function causes execution to terminate without returning.
 //!
-//! \sa CheckedReadFile
+//! \sa CheckedReadFileExactly
 //! \sa ReadFile
 void CheckedReadFileAtEOF(FileHandle file);
 
@@ -345,12 +447,27 @@ void CheckedCloseFile(FileHandle file);
 //! \brief Determines the size of a file.
 //!
 //! \param[in] file The handle to the file for which the size should be
-//!     retrived.
+//!     retrieved.
 //!
 //! \return The size of the file. If an error occurs when attempting to
 //!     determine its size, returns `-1` with an error logged.
 FileOffset LoggingFileSizeByHandle(FileHandle file);
 
+//! \brief Returns a FileHandle corresponding to the requested standard I/O
+//!     stream.
+//!
+//! The returned FileHandle should not be closed on POSIX, where it is
+//! important to maintain valid file descriptors occupying the slots reserved
+//! for these streams. If a need to close such a stream arises on POSIX,
+//! `dup2()` should instead be used to replace the existing file descriptor with
+//! one opened to `/dev/null`. See CloseStdinAndStdout().
+//!
+//! \param[in] stdio_stream The requested standard I/O stream.
+//!
+//! \return A corresponding FileHandle on success. kInvalidFileHandle on error,
+//!     with a message logged.
+FileHandle StdioFileHandle(StdioStream stdio_stream);
+
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_FILE_FILE_IO_H_

util/file/file_io_posix.cc

@@ -19,63 +19,49 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include <algorithm>
+#include <limits>
+
 #include "base/files/file_path.h"
 #include "base/logging.h"
-#include "base/numerics/safe_conversions.h"
 #include "base/posix/eintr_wrapper.h"
 
+namespace crashpad {
+
 namespace {
 
 struct ReadTraits {
-  using VoidBufferType = void*;
+  using BufferType = void*;
-  using CharBufferType = char*;
+  static FileOperationResult Operate(int fd, BufferType buffer, size_t size) {
-  static crashpad::FileOperationResult Operate(int fd,
-                                               CharBufferType buffer,
-                                               size_t size) {
     return read(fd, buffer, size);
   }
 };
 
 struct WriteTraits {
-  using VoidBufferType = const void*;
+  using BufferType = const void*;
-  using CharBufferType = const char*;
+  static FileOperationResult Operate(int fd, BufferType buffer, size_t size) {
-  static crashpad::FileOperationResult Operate(int fd,
-                                               CharBufferType buffer,
-                                               size_t size) {
     return write(fd, buffer, size);
   }
 };
 
 template <typename Traits>
-crashpad::FileOperationResult
+FileOperationResult ReadOrWrite(int fd,
-ReadOrWrite(int fd, typename Traits::VoidBufferType buffer, size_t size) {
+                                typename Traits::BufferType buffer,
-  typename Traits::CharBufferType buffer_c =
+                                size_t size) {
-      reinterpret_cast<typename Traits::CharBufferType>(buffer);
+  constexpr size_t kMaxReadWriteSize =
+      static_cast<size_t>(std::numeric_limits<ssize_t>::max());
+  const size_t requested_bytes = std::min(size, kMaxReadWriteSize);
 
-  crashpad::FileOperationResult total_bytes = 0;
+  FileOperationResult transacted_bytes =
-  while (size > 0) {
+      HANDLE_EINTR(Traits::Operate(fd, buffer, requested_bytes));
-    crashpad::FileOperationResult bytes =
+  if (transacted_bytes < 0) {
-        HANDLE_EINTR(Traits::Operate(fd, buffer_c, size));
+    return -1;
-    if (bytes < 0) {
-      return bytes;
-    } else if (bytes == 0) {
-      break;
-    }
-
-    buffer_c += bytes;
-    size -= bytes;
-    total_bytes += bytes;
   }
 
-  return total_bytes;
+  DCHECK_LE(static_cast<size_t>(transacted_bytes), requested_bytes);
+  return transacted_bytes;
 }
 
-}  // namespace
-
-namespace crashpad {
-
-namespace {
-
 FileHandle OpenFileForOutput(int rdwr_or_wronly,
                              const base::FilePath& path,
                              FileWriteMode mode,
@@ -108,14 +94,21 @@ FileHandle OpenFileForOutput(int rdwr_or_wronly,
 
 }  // namespace
 
-FileOperationResult ReadFile(FileHandle file, void* buffer, size_t size) {
+namespace internal {
-  return ReadOrWrite<ReadTraits>(file, buffer, size);
+
+const char kNativeReadFunctionName[] = "read";
+const char kNativeWriteFunctionName[] = "write";
+
+FileOperationResult NativeWriteFile(FileHandle file,
+                                    const void* buffer,
+                                    size_t size) {
+  return ReadOrWrite<WriteTraits>(file, buffer, size);
 }
 
-FileOperationResult WriteFile(FileHandle file,
+}  // namespace internal
-                              const void* buffer,
+
-                              size_t size) {
+FileOperationResult ReadFile(FileHandle file, void* buffer, size_t size) {
-  return ReadOrWrite<WriteTraits>(file, buffer, size);
+  return ReadOrWrite<ReadTraits>(file, buffer, size);
 }
 
 FileHandle OpenFileForRead(const base::FilePath& path) {
@@ -199,4 +192,18 @@ FileOffset LoggingFileSizeByHandle(FileHandle file) {
   return st.st_size;
 }
 
+FileHandle StdioFileHandle(StdioStream stdio_stream) {
+  switch (stdio_stream) {
+    case StdioStream::kStandardInput:
+      return STDIN_FILENO;
+    case StdioStream::kStandardOutput:
+      return STDOUT_FILENO;
+    case StdioStream::kStandardError:
+      return STDERR_FILENO;
+  }
+
+  NOTREACHED();
+  return kInvalidFileHandle;
+}
+
 }  // namespace crashpad

util/file/file_io_test.cc

@@ -14,9 +14,15 @@
 
 #include "util/file/file_io.h"
 
+#include <stdio.h>
+
+#include <limits>
+#include <type_traits>
+
 #include "base/atomicops.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "test/errors.h"
 #include "test/file.h"
@@ -28,6 +34,342 @@ namespace crashpad {
 namespace test {
 namespace {
 
+using testing::_;
+using testing::InSequence;
+using testing::Return;
+
+class MockReadExactly : public internal::ReadExactlyInternal {
+ public:
+  MockReadExactly() : ReadExactlyInternal() {}
+  ~MockReadExactly() {}
+
+  // Since it’s more convenient for the test to use uintptr_t than void*,
+  // ReadExactlyInt() and ReadInt() adapt the types.
+
+  bool ReadExactlyInt(uintptr_t data, size_t size, bool can_log) {
+    return ReadExactly(reinterpret_cast<void*>(data), size, can_log);
+  }
+
+  MOCK_METHOD3(ReadInt, FileOperationResult(uintptr_t, size_t, bool));
+
+  // ReadExactlyInternal:
+  FileOperationResult Read(void* data, size_t size, bool can_log) {
+    return ReadInt(reinterpret_cast<uintptr_t>(data), size, can_log);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockReadExactly);
+};
+
+TEST(FileIO, ReadExactly_Zero) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(_, _, false)).Times(0);
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(100, 0, false));
+}
+
+TEST(FileIO, ReadExactly_SingleSmallSuccess) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(1000, 1, false)).WillOnce(Return(1));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(1000, 1, false));
+}
+
+TEST(FileIO, ReadExactly_SingleSmallSuccessCanLog) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(1000, 1, true)).WillOnce(Return(1));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(1000, 1, true));
+}
+
+TEST(FileIO, ReadExactly_SingleSmallFailure) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(1000, 1, false)).WillOnce(Return(-1));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(1000, 1, false));
+}
+
+TEST(FileIO, ReadExactly_SingleSmallFailureCanLog) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(1000, 1, true)).WillOnce(Return(-1));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(1000, 1, true));
+}
+
+TEST(FileIO, ReadExactly_DoubleSmallSuccess) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(0x1000, 2, false)).WillOnce(Return(1));
+  EXPECT_CALL(read_exactly, ReadInt(0x1001, 1, false)).WillOnce(Return(1));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(0x1000, 2, false));
+}
+
+TEST(FileIO, ReadExactly_DoubleSmallShort) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(0x20000, 2, false)).WillOnce(Return(1));
+  EXPECT_CALL(read_exactly, ReadInt(0x20001, 1, false)).WillOnce(Return(0));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(0x20000, 2, false));
+}
+
+TEST(FileIO, ReadExactly_DoubleSmallShortCanLog) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(0x20000, 2, true)).WillOnce(Return(1));
+  EXPECT_CALL(read_exactly, ReadInt(0x20001, 1, true)).WillOnce(Return(0));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(0x20000, 2, true));
+}
+
+TEST(FileIO, ReadExactly_Medium) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(0x80000000, 0x20000000, false))
+      .WillOnce(Return(0x10000000));
+  EXPECT_CALL(read_exactly, ReadInt(0x90000000, 0x10000000, false))
+      .WillOnce(Return(0x8000000));
+  EXPECT_CALL(read_exactly, ReadInt(0x98000000, 0x8000000, false))
+      .WillOnce(Return(0x4000000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9c000000, 0x4000000, false))
+      .WillOnce(Return(0x2000000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9e000000, 0x2000000, false))
+      .WillOnce(Return(0x1000000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9f000000, 0x1000000, false))
+      .WillOnce(Return(0x800000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9f800000, 0x800000, false))
+      .WillOnce(Return(0x400000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fc00000, 0x400000, false))
+      .WillOnce(Return(0x200000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fe00000, 0x200000, false))
+      .WillOnce(Return(0x100000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ff00000, 0x100000, false))
+      .WillOnce(Return(0x80000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ff80000, 0x80000, false))
+      .WillOnce(Return(0x40000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffc0000, 0x40000, false))
+      .WillOnce(Return(0x20000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffe0000, 0x20000, false))
+      .WillOnce(Return(0x10000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fff0000, 0x10000, false))
+      .WillOnce(Return(0x8000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fff8000, 0x8000, false))
+      .WillOnce(Return(0x4000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fffc000, 0x4000, false))
+      .WillOnce(Return(0x2000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fffe000, 0x2000, false))
+      .WillOnce(Return(0x1000));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffff000, 0x1000, false))
+      .WillOnce(Return(0x800));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffff800, 0x800, false))
+      .WillOnce(Return(0x400));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffffc00, 0x400, false))
+      .WillOnce(Return(0x200));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffffe00, 0x200, false))
+      .WillOnce(Return(0x100));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fffff00, 0x100, false))
+      .WillOnce(Return(0x80));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fffff80, 0x80, false))
+      .WillOnce(Return(0x40));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fffffc0, 0x40, false))
+      .WillOnce(Return(0x20));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fffffe0, 0x20, false))
+      .WillOnce(Return(0x10));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffffff0, 0x10, false))
+      .WillOnce(Return(0x8));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffffff8, 0x8, false))
+      .WillOnce(Return(0x4));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffffffc, 0x4, false))
+      .WillOnce(Return(0x2));
+  EXPECT_CALL(read_exactly, ReadInt(0x9ffffffe, 0x2, false))
+      .WillOnce(Return(0x1));
+  EXPECT_CALL(read_exactly, ReadInt(0x9fffffff, 0x1, false))
+      .WillOnce(Return(0x1));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(0x80000000, 0x20000000, false));
+}
+
+TEST(FileIO, ReadExactly_LargeSuccess) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  constexpr size_t max = std::numeric_limits<uint32_t>::max();
+  constexpr size_t increment = std::numeric_limits<int32_t>::max();
+  EXPECT_CALL(read_exactly, ReadInt(0, max, false)).WillOnce(Return(increment));
+  EXPECT_CALL(read_exactly, ReadInt(increment, max - increment, false))
+      .WillOnce(Return(increment));
+  EXPECT_CALL(read_exactly, ReadInt(2 * increment, 1, false))
+      .WillOnce(Return(1));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(0, max, false));
+}
+
+TEST(FileIO, ReadExactly_LargeShort) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(0, 0xffffffff, false))
+      .WillOnce(Return(0x7fffffff));
+  EXPECT_CALL(read_exactly, ReadInt(0x7fffffff, 0x80000000, false))
+      .WillOnce(Return(0x10000000));
+  EXPECT_CALL(read_exactly, ReadInt(0x8fffffff, 0x70000000, false))
+      .WillOnce(Return(0));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(0, 0xffffffff, false));
+}
+
+TEST(FileIO, ReadExactly_LargeFailure) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  EXPECT_CALL(read_exactly, ReadInt(0, 0xffffffff, false))
+      .WillOnce(Return(0x7fffffff));
+  EXPECT_CALL(read_exactly, ReadInt(0x7fffffff, 0x80000000, false))
+      .WillOnce(Return(-1));
+  EXPECT_FALSE(read_exactly.ReadExactlyInt(0, 0xffffffff, false));
+}
+
+TEST(FileIO, ReadExactly_TripleMax) {
+  MockReadExactly read_exactly;
+  InSequence in_sequence;
+  constexpr size_t max = std::numeric_limits<size_t>::max();
+  constexpr size_t increment =
+      std::numeric_limits<std::make_signed<size_t>::type>::max();
+  EXPECT_CALL(read_exactly, ReadInt(0, max, false)).WillOnce(Return(increment));
+  EXPECT_CALL(read_exactly, ReadInt(increment, max - increment, false))
+      .WillOnce(Return(increment));
+  EXPECT_CALL(read_exactly, ReadInt(2 * increment, 1, false))
+      .WillOnce(Return(1));
+  EXPECT_TRUE(read_exactly.ReadExactlyInt(0, max, false));
+}
+
+class MockWriteAll : public internal::WriteAllInternal {
+ public:
+  MockWriteAll() : WriteAllInternal() {}
+  ~MockWriteAll() {}
+
+  // Since it’s more convenient for the test to use uintptr_t than const void*,
+  // WriteAllInt() and WriteInt() adapt the types.
+
+  bool WriteAllInt(uintptr_t data, size_t size) {
+    return WriteAll(reinterpret_cast<const void*>(data), size);
+  }
+
+  MOCK_METHOD2(WriteInt, FileOperationResult(uintptr_t, size_t));
+
+  // WriteAllInternal:
+  FileOperationResult Write(const void* data, size_t size) {
+    return WriteInt(reinterpret_cast<uintptr_t>(data), size);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockWriteAll);
+};
+
+TEST(FileIO, WriteAll_Zero) {
+  MockWriteAll write_all;
+  InSequence in_sequence;
+  EXPECT_CALL(write_all, WriteInt(_, _)).Times(0);
+  EXPECT_TRUE(write_all.WriteAllInt(100, 0));
+}
+
+TEST(FileIO, WriteAll_SingleSmallSuccess) {
+  MockWriteAll write_all;
+  InSequence in_sequence;
+  EXPECT_CALL(write_all, WriteInt(1000, 1)).WillOnce(Return(1));
+  EXPECT_TRUE(write_all.WriteAllInt(1000, 1));
+}
+
+TEST(FileIO, WriteAll_SingleSmallFailure) {
+  MockWriteAll write_all;
+  InSequence in_sequence;
+  EXPECT_CALL(write_all, WriteInt(1000, 1)).WillOnce(Return(-1));
+  EXPECT_FALSE(write_all.WriteAllInt(1000, 1));
+}
+
+TEST(FileIO, WriteAll_DoubleSmall) {
+  MockWriteAll write_all;
+  InSequence in_sequence;
+  EXPECT_CALL(write_all, WriteInt(0x1000, 2)).WillOnce(Return(1));
+  EXPECT_CALL(write_all, WriteInt(0x1001, 1)).WillOnce(Return(1));
+  EXPECT_TRUE(write_all.WriteAllInt(0x1000, 2));
+}
+
+TEST(FileIO, WriteAll_Medium) {
+  MockWriteAll write_all;
+  InSequence in_sequence;
+  EXPECT_CALL(write_all, WriteInt(0x80000000, 0x20000000))
+      .WillOnce(Return(0x10000000));
+  EXPECT_CALL(write_all, WriteInt(0x90000000, 0x10000000))
+      .WillOnce(Return(0x8000000));
+  EXPECT_CALL(write_all, WriteInt(0x98000000, 0x8000000))
+      .WillOnce(Return(0x4000000));
+  EXPECT_CALL(write_all, WriteInt(0x9c000000, 0x4000000))
+      .WillOnce(Return(0x2000000));
+  EXPECT_CALL(write_all, WriteInt(0x9e000000, 0x2000000))
+      .WillOnce(Return(0x1000000));
+  EXPECT_CALL(write_all, WriteInt(0x9f000000, 0x1000000))
+      .WillOnce(Return(0x800000));
+  EXPECT_CALL(write_all, WriteInt(0x9f800000, 0x800000))
+      .WillOnce(Return(0x400000));
+  EXPECT_CALL(write_all, WriteInt(0x9fc00000, 0x400000))
+      .WillOnce(Return(0x200000));
+  EXPECT_CALL(write_all, WriteInt(0x9fe00000, 0x200000))
+      .WillOnce(Return(0x100000));
+  EXPECT_CALL(write_all, WriteInt(0x9ff00000, 0x100000))
+      .WillOnce(Return(0x80000));
+  EXPECT_CALL(write_all, WriteInt(0x9ff80000, 0x80000))
+      .WillOnce(Return(0x40000));
+  EXPECT_CALL(write_all, WriteInt(0x9ffc0000, 0x40000))
+      .WillOnce(Return(0x20000));
+  EXPECT_CALL(write_all, WriteInt(0x9ffe0000, 0x20000))
+      .WillOnce(Return(0x10000));
+  EXPECT_CALL(write_all, WriteInt(0x9fff0000, 0x10000))
+      .WillOnce(Return(0x8000));
+  EXPECT_CALL(write_all, WriteInt(0x9fff8000, 0x8000)).WillOnce(Return(0x4000));
+  EXPECT_CALL(write_all, WriteInt(0x9fffc000, 0x4000)).WillOnce(Return(0x2000));
+  EXPECT_CALL(write_all, WriteInt(0x9fffe000, 0x2000)).WillOnce(Return(0x1000));
+  EXPECT_CALL(write_all, WriteInt(0x9ffff000, 0x1000)).WillOnce(Return(0x800));
+  EXPECT_CALL(write_all, WriteInt(0x9ffff800, 0x800)).WillOnce(Return(0x400));
+  EXPECT_CALL(write_all, WriteInt(0x9ffffc00, 0x400)).WillOnce(Return(0x200));
+  EXPECT_CALL(write_all, WriteInt(0x9ffffe00, 0x200)).WillOnce(Return(0x100));
+  EXPECT_CALL(write_all, WriteInt(0x9fffff00, 0x100)).WillOnce(Return(0x80));
+  EXPECT_CALL(write_all, WriteInt(0x9fffff80, 0x80)).WillOnce(Return(0x40));
+  EXPECT_CALL(write_all, WriteInt(0x9fffffc0, 0x40)).WillOnce(Return(0x20));
+  EXPECT_CALL(write_all, WriteInt(0x9fffffe0, 0x20)).WillOnce(Return(0x10));
+  EXPECT_CALL(write_all, WriteInt(0x9ffffff0, 0x10)).WillOnce(Return(0x8));
+  EXPECT_CALL(write_all, WriteInt(0x9ffffff8, 0x8)).WillOnce(Return(0x4));
+  EXPECT_CALL(write_all, WriteInt(0x9ffffffc, 0x4)).WillOnce(Return(0x2));
+  EXPECT_CALL(write_all, WriteInt(0x9ffffffe, 0x2)).WillOnce(Return(0x1));
+  EXPECT_CALL(write_all, WriteInt(0x9fffffff, 0x1)).WillOnce(Return(0x1));
+  EXPECT_TRUE(write_all.WriteAllInt(0x80000000, 0x20000000));
+}
+
+TEST(FileIO, WriteAll_LargeSuccess) {
+  MockWriteAll write_all;
+  InSequence in_sequence;
+  constexpr size_t max = std::numeric_limits<uint32_t>::max();
+  constexpr size_t increment = std::numeric_limits<int32_t>::max();
+  EXPECT_CALL(write_all, WriteInt(0, max)).WillOnce(Return(increment));
+  EXPECT_CALL(write_all, WriteInt(increment, max - increment))
+      .WillOnce(Return(increment));
+  EXPECT_CALL(write_all, WriteInt(2 * increment, 1)).WillOnce(Return(1));
+  EXPECT_TRUE(write_all.WriteAllInt(0, max));
+}
+
+TEST(FileIO, WriteAll_LargeFailure) {
+  MockWriteAll write_all;
+  InSequence in_sequence;
+  EXPECT_CALL(write_all, WriteInt(0, 0xffffffff)).WillOnce(Return(0x7fffffff));
+  EXPECT_CALL(write_all, WriteInt(0x7fffffff, 0x80000000)).WillOnce(Return(-1));
+  EXPECT_FALSE(write_all.WriteAllInt(0, 0xffffffff));
+}
+
+TEST(FileIO, WriteAll_TripleMax) {
+  MockWriteAll write_all;
+  InSequence in_sequence;
+  constexpr size_t max = std::numeric_limits<size_t>::max();
+  constexpr size_t increment =
+      std::numeric_limits<std::make_signed<size_t>::type>::max();
+  EXPECT_CALL(write_all, WriteInt(0, max)).WillOnce(Return(increment));
+  EXPECT_CALL(write_all, WriteInt(increment, max - increment))
+      .WillOnce(Return(increment));
+  EXPECT_CALL(write_all, WriteInt(2 * increment, 1)).WillOnce(Return(1));
+  EXPECT_TRUE(write_all.WriteAllInt(0, max));
+}
+
 void TestOpenFileForWrite(FileHandle (*opener)(const base::FilePath&,
                                                FileWriteMode,
                                                FilePermissions)) {
@@ -324,6 +666,26 @@ TEST(FileIO, FileSizeByHandle) {
   EXPECT_EQ(9, LoggingFileSizeByHandle(file_handle.get()));
 }
 
+FileHandle FileHandleForFILE(FILE* file) {
+  int fd = fileno(file);
+#if defined(OS_POSIX)
+  return fd;
+#elif defined(OS_WIN)
+  return reinterpret_cast<HANDLE>(_get_osfhandle(fd));
+#else
+#error Port
+#endif
+}
+
+TEST(FileIO, StdioFileHandle) {
+  EXPECT_EQ(FileHandleForFILE(stdin),
+            StdioFileHandle(StdioStream::kStandardInput));
+  EXPECT_EQ(FileHandleForFILE(stdout),
+            StdioFileHandle(StdioStream::kStandardOutput));
+  EXPECT_EQ(FileHandleForFILE(stderr),
+            StdioFileHandle(StdioStream::kStandardError));
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad

util/file/file_io_win.cc

@@ -14,9 +14,11 @@
 
 #include "util/file/file_io.h"
 
+#include <algorithm>
+#include <limits>
+
 #include "base/files/file_path.h"
 #include "base/logging.h"
-#include "base/numerics/safe_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 
 namespace {
@@ -37,6 +39,16 @@ namespace crashpad {
 
 namespace {
 
+// kMaxReadWriteSize needs to be limited to the range of DWORD for the calls to
+// ::ReadFile() and ::WriteFile(), and also limited to the range of
+// FileOperationResult to be able to adequately express the number of bytes read
+// and written in the return values from ReadFile() and NativeWriteFile(). In a
+// 64-bit build, the former will control, and the limit will be (2^32)-1. In a
+// 32-bit build, the latter will control, and the limit will be (2^31)-1.
+constexpr size_t kMaxReadWriteSize = std::min(
+    static_cast<size_t>(std::numeric_limits<DWORD>::max()),
+    static_cast<size_t>(std::numeric_limits<FileOperationResult>::max()));
+
 FileHandle OpenFileForOutput(DWORD access,
                              const base::FilePath& path,
                              FileWriteMode mode,
@@ -70,51 +82,58 @@ FileHandle OpenFileForOutput(DWORD access,
 
 }  // namespace
 
-// TODO(scottmg): Handle > DWORD sized writes if necessary.
+namespace internal {
+
+const char kNativeReadFunctionName[] = "ReadFile";
+const char kNativeWriteFunctionName[] = "WriteFile";
+
+FileOperationResult NativeWriteFile(FileHandle file,
+                                    const void* buffer,
+                                    size_t size) {
+  // TODO(scottmg): This might need to handle the limit for pipes across a
+  // network in the future.
+
+  const DWORD write_size =
+      static_cast<DWORD>(std::min(size, kMaxReadWriteSize));
+
+  DWORD bytes_written;
+  if (!::WriteFile(file, buffer, write_size, &bytes_written, nullptr))
+    return -1;
+
+  CHECK_NE(bytes_written, static_cast<DWORD>(-1));
+  DCHECK_LE(static_cast<size_t>(bytes_written), write_size);
+  return bytes_written;
+}
+
+}  // namespace internal
 
 FileOperationResult ReadFile(FileHandle file, void* buffer, size_t size) {
   DCHECK(!IsSocketHandle(file));
-  DWORD size_dword = base::checked_cast<DWORD>(size);
+
-  DWORD total_read = 0;
+  const DWORD read_size = static_cast<DWORD>(std::min(size, kMaxReadWriteSize));
-  char* buffer_c = reinterpret_cast<char*>(buffer);
+
-  while (size_dword > 0) {
+  while (true) {
     DWORD bytes_read;
-    BOOL success = ::ReadFile(file, buffer_c, size_dword, &bytes_read, nullptr);
+    BOOL success = ::ReadFile(file, buffer, read_size, &bytes_read, nullptr);
     if (!success) {
       if (GetLastError() == ERROR_BROKEN_PIPE) {
         // When reading a pipe and the write handle has been closed, ReadFile
         // fails with ERROR_BROKEN_PIPE, but only once all pending data has been
-        // read.
+        // read. Treat this as EOF.
-        break;
+        return 0;
-      } else if (GetLastError() != ERROR_MORE_DATA) {
-        return -1;
       }
-    } else if (bytes_read == 0 && GetFileType(file) != FILE_TYPE_PIPE) {
+      return -1;
+    }
+
+    CHECK_NE(bytes_read, static_cast<DWORD>(-1));
+    DCHECK_LE(bytes_read, read_size);
+    if (bytes_read != 0 || GetFileType(file) != FILE_TYPE_PIPE) {
       // Zero bytes read for a file indicates reaching EOF. Zero bytes read from
       // a pipe indicates only that there was a zero byte WriteFile issued on
       // the other end, so continue reading.
-      break;
+      return bytes_read;
     }
-
-    buffer_c += bytes_read;
-    size_dword -= bytes_read;
-    total_read += bytes_read;
   }
-  return total_read;
-}
-
-FileOperationResult WriteFile(FileHandle file,
-                              const void* buffer,
-                              size_t size) {
-  // TODO(scottmg): This might need to handle the limit for pipes across a
-  // network in the future.
-  DWORD size_dword = base::checked_cast<DWORD>(size);
-  DWORD bytes_written;
-  BOOL rv = ::WriteFile(file, buffer, size_dword, &bytes_written, nullptr);
-  if (!rv)
-    return -1;
-  CHECK_EQ(bytes_written, size_dword);
-  return bytes_written;
 }
 
 FileHandle OpenFileForRead(const base::FilePath& path) {
@@ -245,4 +264,26 @@ FileOffset LoggingFileSizeByHandle(FileHandle file) {
   return file_size.QuadPart;
 }
 
+FileHandle StdioFileHandle(StdioStream stdio_stream) {
+  DWORD standard_handle;
+  switch (stdio_stream) {
+    case StdioStream::kStandardInput:
+      standard_handle = STD_INPUT_HANDLE;
+      break;
+    case StdioStream::kStandardOutput:
+      standard_handle = STD_OUTPUT_HANDLE;
+      break;
+    case StdioStream::kStandardError:
+      standard_handle = STD_ERROR_HANDLE;
+      break;
+    default:
+      NOTREACHED();
+      return INVALID_HANDLE_VALUE;
+  }
+
+  HANDLE handle = GetStdHandle(standard_handle);
+  PLOG_IF(ERROR, handle == INVALID_HANDLE_VALUE) << "GetStdHandle";
+  return handle;
+}
+
 }  // namespace crashpad

util/file/file_reader.cc

@@ -20,18 +20,31 @@
 
 namespace crashpad {
 
-bool FileReaderInterface::ReadExactly(void* data, size_t size) {
+namespace {
-  FileOperationResult expect = base::checked_cast<FileOperationResult>(size);
+
-  FileOperationResult rv = Read(data, size);
+class FileReaderReadExactly final : public internal::ReadExactlyInternal {
-  if (rv < 0) {
+ public:
-    // Read() will have logged its own error.
+  explicit FileReaderReadExactly(FileReaderInterface* file_reader)
-    return false;
+      : ReadExactlyInternal(), file_reader_(file_reader) {}
-  } else if (rv != expect) {
+  ~FileReaderReadExactly() {}
-    LOG(ERROR) << "ReadExactly(): expected " << expect << ", observed " << rv;
+
-    return false;
+ private:
+  // ReadExactlyInternal:
+  FileOperationResult Read(void* buffer, size_t size, bool can_log) override {
+    DCHECK(can_log);
+    return file_reader_->Read(buffer, size);
   }
 
-  return true;
+  FileReaderInterface* file_reader_;  // weak
+
+  DISALLOW_COPY_AND_ASSIGN(FileReaderReadExactly);
+};
+
+}  // namespace
+
+bool FileReaderInterface::ReadExactly(void* data, size_t size) {
+  FileReaderReadExactly read_exactly(this);
+  return read_exactly.ReadExactly(data, size, true);
 }
 
 WeakFileHandleFileReader::WeakFileHandleFileReader(FileHandle file_handle)
@@ -44,13 +57,10 @@ WeakFileHandleFileReader::~WeakFileHandleFileReader() {
 FileOperationResult WeakFileHandleFileReader::Read(void* data, size_t size) {
   DCHECK_NE(file_handle_, kInvalidFileHandle);
 
-  // Don’t use LoggingReadFile(), which insists on a full read and only returns
-  // a bool. This method permits short reads and returns the number of bytes
-  // read.
   base::checked_cast<FileOperationResult>(size);
   FileOperationResult rv = ReadFile(file_handle_, data, size);
   if (rv < 0) {
-    PLOG(ERROR) << "read";
+    PLOG(ERROR) << internal::kNativeReadFunctionName;
     return -1;
   }
 
@@ -98,43 +108,4 @@ FileOffset FileReader::Seek(FileOffset offset, int whence) {
   return weak_file_handle_file_reader_.Seek(offset, whence);
 }
 
-WeakStdioFileReader::WeakStdioFileReader(FILE* file)
-    : file_(file) {
-}
-
-WeakStdioFileReader::~WeakStdioFileReader() {
-}
-
-FileOperationResult WeakStdioFileReader::Read(void* data, size_t size) {
-  DCHECK(file_);
-
-  size_t rv = fread(data, 1, size, file_);
-  if (rv != size && ferror(file_)) {
-    STDIO_PLOG(ERROR) << "fread";
-    return -1;
-  }
-  if (rv > size) {
-    LOG(ERROR) << "fread: expected " << size << ", observed " << rv;
-    return -1;
-  }
-
-  return rv;
-}
-
-FileOffset WeakStdioFileReader::Seek(FileOffset offset, int whence) {
-  DCHECK(file_);
-  if (fseeko(file_, offset, whence) == -1) {
-    STDIO_PLOG(ERROR) << "fseeko";
-    return -1;
-  }
-
-  FileOffset new_offset = ftello(file_);
-  if (new_offset == -1) {
-    STDIO_PLOG(ERROR) << "ftello";
-    return -1;
-  }
-
-  return new_offset;
-}
-
 }  // namespace crashpad

util/file/file_reader.h

@@ -15,7 +15,6 @@
 #ifndef CRASHPAD_UTIL_FILE_FILE_READER_H_
 #define CRASHPAD_UTIL_FILE_FILE_READER_H_
 
-#include <stdio.h>
 #include <sys/types.h>
 
 #include "base/files/file_path.h"
@@ -42,7 +41,7 @@ class FileReaderInterface : public virtual FileSeekerInterface {
   //! \brief Wraps Read(), ensuring that the read succeeded and exactly \a size
   //!     bytes were read.
   //!
-  //! Semantically, this behaves as LoggingReadFile().
+  //! Semantically, this behaves as LoggingReadFileExactly().
   //!
   //! \return `true` if the operation succeeded, `false` if it failed, with an
   //!     error message logged. Short reads are treated as failures.
@@ -142,40 +141,6 @@ class FileReader : public FileReaderInterface {
   DISALLOW_COPY_AND_ASSIGN(FileReader);
 };
 
-//! \brief A file reader backed by a standard input/output `FILE*`.
-//!
-//! This class accepts an already-open `FILE*`. It is not responsible for
-//! opening or closing this `FILE*`. Users of this class must ensure that the
-//! `FILE*` is closed appropriately elsewhere. Objects of this class may be used
-//! to read from `FILE*` objects not associated with filesystem-based files,
-//! although special attention should be paid to the Seek() method, which may
-//! not function on `FILE*` objects that do not refer to disk-based files.
-//!
-//! This class is expected to be used when other code is responsible for
-//! opening `FILE*` objects and already provides `FILE*` objects. A good use
-//! would be a WeakStdioFileReader for `stdin`.
-class WeakStdioFileReader : public FileReaderInterface {
- public:
-  explicit WeakStdioFileReader(FILE* file);
-  ~WeakStdioFileReader() override;
-
-  // FileReaderInterface:
-  FileOperationResult Read(void* data, size_t size) override;
-
-  // FileSeekerInterface:
-
-  //! \copydoc FileReaderInterface::Seek()
-  //!
-  //! \note This method is only guaranteed to function on `FILE*` objects
-  //!     referring to disk-based files.
-  FileOffset Seek(FileOffset offset, int whence) override;
-
- private:
-  FILE* file_;  // weak
-
-  DISALLOW_COPY_AND_ASSIGN(WeakStdioFileReader);
-};
-
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_FILE_FILE_READER_H_

util/file/file_reader_test.cc

@@ -0,0 +1,205 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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/file/file_reader.h"
+
+#include <stdint.h>
+
+#include <limits>
+#include <type_traits>
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+using testing::_;
+using testing::InSequence;
+using testing::Return;
+
+class MockFileReader : public FileReaderInterface {
+ public:
+  MockFileReader() : FileReaderInterface() {}
+  ~MockFileReader() override {}
+
+  // Since it’s more convenient for the test to use uintptr_t than void*,
+  // ReadExactlyInt() and ReadInt() adapt the types.
+
+  bool ReadExactlyInt(uintptr_t data, size_t size) {
+    return ReadExactly(reinterpret_cast<void*>(data), size);
+  }
+
+  MOCK_METHOD2(ReadInt, FileOperationResult(uintptr_t, size_t));
+
+  // FileReaderInterface:
+  FileOperationResult Read(void* data, size_t size) {
+    return ReadInt(reinterpret_cast<uintptr_t>(data), size);
+  }
+
+  // FileSeekerInterface:
+  MOCK_METHOD2(Seek, FileOffset(FileOffset, int));
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockFileReader);
+};
+
+TEST(FileReader, ReadExactly_Zero) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  EXPECT_CALL(file_reader, ReadInt(_, _)).Times(0);
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_TRUE(file_reader.ReadExactlyInt(100, 0));
+}
+
+TEST(FileReader, ReadExactly_SingleSmallSuccess) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  EXPECT_CALL(file_reader, ReadInt(1000, 1)).WillOnce(Return(1));
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_TRUE(file_reader.ReadExactlyInt(1000, 1));
+}
+
+TEST(FileReader, ReadExactly_SingleSmallFailure) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  EXPECT_CALL(file_reader, ReadInt(1000, 1)).WillOnce(Return(-1));
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_FALSE(file_reader.ReadExactlyInt(1000, 1));
+}
+
+TEST(FileReader, ReadExactly_DoubleSmallSuccess) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  EXPECT_CALL(file_reader, ReadInt(0x1000, 2)).WillOnce(Return(1));
+  EXPECT_CALL(file_reader, ReadInt(0x1001, 1)).WillOnce(Return(1));
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_TRUE(file_reader.ReadExactlyInt(0x1000, 2));
+}
+
+TEST(FileReader, ReadExactly_DoubleSmallShort) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  EXPECT_CALL(file_reader, ReadInt(0x20000, 2)).WillOnce(Return(1));
+  EXPECT_CALL(file_reader, ReadInt(0x20001, 1)).WillOnce(Return(0));
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_FALSE(file_reader.ReadExactlyInt(0x20000, 2));
+}
+
+TEST(FileReader, ReadExactly_Medium) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  EXPECT_CALL(file_reader, ReadInt(0x80000000, 0x20000000))
+      .WillOnce(Return(0x10000000));
+  EXPECT_CALL(file_reader, ReadInt(0x90000000, 0x10000000))
+      .WillOnce(Return(0x8000000));
+  EXPECT_CALL(file_reader, ReadInt(0x98000000, 0x8000000))
+      .WillOnce(Return(0x4000000));
+  EXPECT_CALL(file_reader, ReadInt(0x9c000000, 0x4000000))
+      .WillOnce(Return(0x2000000));
+  EXPECT_CALL(file_reader, ReadInt(0x9e000000, 0x2000000))
+      .WillOnce(Return(0x1000000));
+  EXPECT_CALL(file_reader, ReadInt(0x9f000000, 0x1000000))
+      .WillOnce(Return(0x800000));
+  EXPECT_CALL(file_reader, ReadInt(0x9f800000, 0x800000))
+      .WillOnce(Return(0x400000));
+  EXPECT_CALL(file_reader, ReadInt(0x9fc00000, 0x400000))
+      .WillOnce(Return(0x200000));
+  EXPECT_CALL(file_reader, ReadInt(0x9fe00000, 0x200000))
+      .WillOnce(Return(0x100000));
+  EXPECT_CALL(file_reader, ReadInt(0x9ff00000, 0x100000))
+      .WillOnce(Return(0x80000));
+  EXPECT_CALL(file_reader, ReadInt(0x9ff80000, 0x80000))
+      .WillOnce(Return(0x40000));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffc0000, 0x40000))
+      .WillOnce(Return(0x20000));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffe0000, 0x20000))
+      .WillOnce(Return(0x10000));
+  EXPECT_CALL(file_reader, ReadInt(0x9fff0000, 0x10000))
+      .WillOnce(Return(0x8000));
+  EXPECT_CALL(file_reader, ReadInt(0x9fff8000, 0x8000))
+      .WillOnce(Return(0x4000));
+  EXPECT_CALL(file_reader, ReadInt(0x9fffc000, 0x4000))
+      .WillOnce(Return(0x2000));
+  EXPECT_CALL(file_reader, ReadInt(0x9fffe000, 0x2000))
+      .WillOnce(Return(0x1000));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffff000, 0x1000)).WillOnce(Return(0x800));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffff800, 0x800)).WillOnce(Return(0x400));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffffc00, 0x400)).WillOnce(Return(0x200));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffffe00, 0x200)).WillOnce(Return(0x100));
+  EXPECT_CALL(file_reader, ReadInt(0x9fffff00, 0x100)).WillOnce(Return(0x80));
+  EXPECT_CALL(file_reader, ReadInt(0x9fffff80, 0x80)).WillOnce(Return(0x40));
+  EXPECT_CALL(file_reader, ReadInt(0x9fffffc0, 0x40)).WillOnce(Return(0x20));
+  EXPECT_CALL(file_reader, ReadInt(0x9fffffe0, 0x20)).WillOnce(Return(0x10));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffffff0, 0x10)).WillOnce(Return(0x8));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffffff8, 0x8)).WillOnce(Return(0x4));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffffffc, 0x4)).WillOnce(Return(0x2));
+  EXPECT_CALL(file_reader, ReadInt(0x9ffffffe, 0x2)).WillOnce(Return(0x1));
+  EXPECT_CALL(file_reader, ReadInt(0x9fffffff, 0x1)).WillOnce(Return(0x1));
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_TRUE(file_reader.ReadExactlyInt(0x80000000, 0x20000000));
+}
+
+TEST(FileReader, ReadExactly_LargeSuccess) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  constexpr size_t max = std::numeric_limits<uint32_t>::max();
+  constexpr size_t increment = std::numeric_limits<int32_t>::max();
+  EXPECT_CALL(file_reader, ReadInt(0, max)).WillOnce(Return(increment));
+  EXPECT_CALL(file_reader, ReadInt(increment, max - increment))
+      .WillOnce(Return(increment));
+  EXPECT_CALL(file_reader, ReadInt(2 * increment, 1)).WillOnce(Return(1));
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_TRUE(file_reader.ReadExactlyInt(0, max));
+}
+
+TEST(FileReader, ReadExactly_LargeShort) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  EXPECT_CALL(file_reader, ReadInt(0, 0xffffffff)).WillOnce(Return(0x7fffffff));
+  EXPECT_CALL(file_reader, ReadInt(0x7fffffff, 0x80000000))
+      .WillOnce(Return(0x10000000));
+  EXPECT_CALL(file_reader, ReadInt(0x8fffffff, 0x70000000)).WillOnce(Return(0));
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_FALSE(file_reader.ReadExactlyInt(0, 0xffffffff));
+}
+
+TEST(FileReader, ReadExactly_LargeFailure) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  EXPECT_CALL(file_reader, ReadInt(0, 0xffffffff)).WillOnce(Return(0x7fffffff));
+  EXPECT_CALL(file_reader, ReadInt(0x7fffffff, 0x80000000))
+      .WillOnce(Return(-1));
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_FALSE(file_reader.ReadExactlyInt(0, 0xffffffff));
+}
+
+TEST(FileReader, ReadExactly_TripleMax) {
+  MockFileReader file_reader;
+  InSequence in_sequence;
+  constexpr size_t max = std::numeric_limits<size_t>::max();
+  constexpr size_t increment =
+      std::numeric_limits<std::make_signed<size_t>::type>::max();
+  EXPECT_CALL(file_reader, ReadInt(0, max)).WillOnce(Return(increment));
+  EXPECT_CALL(file_reader, ReadInt(increment, max - increment))
+      .WillOnce(Return(increment));
+  EXPECT_CALL(file_reader, ReadInt(2 * increment, 1)).WillOnce(Return(1));
+  EXPECT_CALL(file_reader, Seek(_, _)).Times(0);
+  EXPECT_TRUE(file_reader.ReadExactlyInt(0, max));
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad

util/file/file_writer.cc

@@ -192,66 +192,4 @@ FileOffset FileWriter::Seek(FileOffset offset, int whence) {
   return weak_file_handle_file_writer_.Seek(offset, whence);
 }
 
-WeakStdioFileWriter::WeakStdioFileWriter(FILE* file)
-    : file_(file) {
-}
-
-WeakStdioFileWriter::~WeakStdioFileWriter() {
-}
-
-bool WeakStdioFileWriter::Write(const void* data, size_t size) {
-  DCHECK(file_);
-
-  size_t rv = fwrite(data, 1, size, file_);
-  if (rv != size) {
-    if (ferror(file_)) {
-      STDIO_PLOG(ERROR) << "fwrite";
-    } else {
-      LOG(ERROR) << "fwrite: expected " << size << ", observed " << rv;
-    }
-    return false;
-  }
-
-  return true;
-}
-
-bool WeakStdioFileWriter::WriteIoVec(std::vector<WritableIoVec>* iovecs) {
-  DCHECK(file_);
-
-  if (iovecs->empty()) {
-    LOG(ERROR) << "WriteIoVec(): no iovecs";
-    return false;
-  }
-
-  for (const WritableIoVec& iov : *iovecs) {
-    if (!Write(iov.iov_base, iov.iov_len)) {
-      return false;
-    }
-  }
-
-#ifndef NDEBUG
-  // The interface says that |iovecs| is not sacred, so scramble it to make sure
-  // that nobody depends on it.
-  memset(&(*iovecs)[0], 0xa5, sizeof((*iovecs)[0]) * iovecs->size());
-#endif
-
-  return true;
-}
-
-FileOffset WeakStdioFileWriter::Seek(FileOffset offset, int whence) {
-  DCHECK(file_);
-  if (fseeko(file_, offset, whence) == -1) {
-    STDIO_PLOG(ERROR) << "fseeko";
-    return -1;
-  }
-
-  FileOffset new_offset = ftello(file_);
-  if (new_offset == -1) {
-    STDIO_PLOG(ERROR) << "ftello";
-    return -1;
-  }
-
-  return new_offset;
-}
-
 }  // namespace crashpad

util/file/file_writer.h

@@ -167,41 +167,6 @@ class FileWriter : public FileWriterInterface {
   DISALLOW_COPY_AND_ASSIGN(FileWriter);
 };
 
-//! \brief A file writer backed by a standard input/output `FILE*`.
-//!
-//! This class accepts an already-open `FILE*`. It is not responsible for
-//! opening or closing this `FILE*`. Users of this class must ensure that the
-//! `FILE*` is closed appropriately elsewhere. Objects of this class may be used
-//! to write to `FILE*` objects not associated with filesystem-based files,
-//! although special attention should be paid to the Seek() method, which may
-//! not function on `FILE*` objects that do not refer to disk-based files.
-//!
-//! This class is expected to be used when other code is responsible for
-//! opening `FILE*` objects and already provides `FILE*` objects. A good use
-//! would be a WeakStdioFileWriter for `stdout`.
-class WeakStdioFileWriter : public FileWriterInterface {
- public:
-  explicit WeakStdioFileWriter(FILE* file);
-  ~WeakStdioFileWriter() override;
-
-  // FileWriterInterface:
-  bool Write(const void* data, size_t size) override;
-  bool WriteIoVec(std::vector<WritableIoVec>* iovecs) override;
-
-  // FileSeekerInterface:
-
-  //! \copydoc FileWriterInterface::Seek()
-  //!
-  //! \note This method is only guaranteed to function on `FILE*` objects
-  //!     referring to disk-based files.
-  FileOffset Seek(FileOffset offset, int whence) override;
-
- private:
-  FILE* file_;  // weak
-
-  DISALLOW_COPY_AND_ASSIGN(WeakStdioFileWriter);
-};
-
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_FILE_FILE_WRITER_H_

util/mach/child_port_handshake.cc

@@ -403,20 +403,20 @@ bool ChildPortHandshake::RunClientInternal_ReadPipe(int client_read_fd,
                                                     child_port_token_t* token,
                                                     std::string* service_name) {
   // Read the token from the pipe.
-  if (!LoggingReadFile(client_read_fd, token, sizeof(*token))) {
+  if (!LoggingReadFileExactly(client_read_fd, token, sizeof(*token))) {
     return false;
   }
 
   // Read the service name from the pipe.
   uint32_t service_name_length;
-  if (!LoggingReadFile(
+  if (!LoggingReadFileExactly(
-      client_read_fd, &service_name_length, sizeof(service_name_length))) {
+          client_read_fd, &service_name_length, sizeof(service_name_length))) {
     return false;
   }
 
   service_name->resize(service_name_length);
   if (!service_name->empty() &&
-      !LoggingReadFile(
+      !LoggingReadFileExactly(
           client_read_fd, &(*service_name)[0], service_name_length)) {
     return false;
   }

util/mach/exception_ports_test.cc

@@ -244,7 +244,7 @@ class TestExceptionPorts : public MachMultiprocess,
       CheckedWriteFile(test_exception_ports_->WritePipeHandle(), &c, 1);
 
       // Wait for the parent process to say that its end is set up.
-      CheckedReadFile(test_exception_ports_->ReadPipeHandle(), &c, 1);
+      CheckedReadFileExactly(test_exception_ports_->ReadPipeHandle(), &c, 1);
       EXPECT_EQ('\0', c);
 
       // Regardless of where ExceptionPorts::SetExceptionPort() ran,
@@ -352,7 +352,7 @@ class TestExceptionPorts : public MachMultiprocess,
     // Wait for the child process to be ready. It needs to have all of its
     // threads set up before proceeding if in kSetOutOfProcess mode.
     char c;
-    CheckedReadFile(ReadPipeHandle(), &c, 1);
+    CheckedReadFileExactly(ReadPipeHandle(), &c, 1);
     EXPECT_EQ('\0', c);
 
     mach_port_t local_port = LocalPort();

util/mach/mach_message_server_test.cc

@@ -344,7 +344,7 @@ class TestMachMessageServer : public MachMessageServer::Interface,
     if (options_.parent_wait_for_child_pipe) {
       // Wait until the child is done sending what it’s going to send.
       char c;
-      CheckedReadFile(ReadPipeHandle(), &c, 1);
+      CheckedReadFileExactly(ReadPipeHandle(), &c, 1);
       EXPECT_EQ('\0', c);
     }
 
@@ -397,7 +397,7 @@ class TestMachMessageServer : public MachMessageServer::Interface,
     if (options_.child_wait_for_parent_pipe_early) {
       // Wait until the parent is done setting things up on its end.
       char c;
-      CheckedReadFile(ReadPipeHandle(), &c, 1);
+      CheckedReadFileExactly(ReadPipeHandle(), &c, 1);
       EXPECT_EQ('\0', c);
     }
 
@@ -431,7 +431,7 @@ class TestMachMessageServer : public MachMessageServer::Interface,
 
     if (options_.child_wait_for_parent_pipe_late) {
       char c;
-      CheckedReadFile(ReadPipeHandle(), &c, 1);
+      CheckedReadFileExactly(ReadPipeHandle(), &c, 1);
       ASSERT_EQ('\0', c);
     }
   }

util/net/http_transport_test.cc

@@ -81,7 +81,7 @@ class HTTPTransportTestFixture : public MultiprocessExec {
     // The child will write the HTTP server port number as a packed unsigned
     // short to stdout.
     uint16_t port;
-    ASSERT_TRUE(LoggingReadFile(ReadPipeHandle(), &port, sizeof(port)));
+    ASSERT_TRUE(LoggingReadFileExactly(ReadPipeHandle(), &port, sizeof(port)));
 
     // Then the parent will tell the web server what response code to send
     // for the HTTP request.

util/posix/process_info.h

@@ -25,6 +25,7 @@
 
 #include "base/macros.h"
 #include "build/build_config.h"
+#include "util/misc/initialization_state.h"
 #include "util/misc/initialization_state_dcheck.h"
 
 #if defined(OS_MACOSX)
@@ -113,13 +114,21 @@ class ProcessInfo {
   //! `execve()` as a result of executing a setuid or setgid executable.
   bool DidChangePrivileges() const;
 
-  //! \return `true` if the target task is a 64-bit process.
+  //! \brief Determines the target process’ bitness.
-  bool Is64Bit() const;
+  //!
+  //! \param[out] is_64_bit `true` if the target task is a 64-bit process.
+  //!
+  //! \return `true` on success, with \a is_64_bit set. Otherwise, `false` with
+  //!     a message logged.
+  bool Is64Bit(bool* is_64_bit) const;
 
   //! \brief Determines the target process’ start time.
   //!
   //! \param[out] start_time The time that the process started.
-  void StartTime(timeval* start_time) const;
+  //!
+  //! \return `true` on success, with \a start_time set. Otherwise, `false` with
+  //!     a message logged.
+  bool StartTime(timeval* start_time) const;
 
   //! \brief Obtains the arguments used to launch a process.
   //!
@@ -141,6 +150,25 @@ class ProcessInfo {
  private:
 #if defined(OS_MACOSX)
   kinfo_proc kern_proc_info_;
+#elif defined(OS_LINUX) || defined(OS_ANDROID)
+  // Some members are marked mutable so that they can be lazily initialized by
+  // const methods. These are always InitializationState-protected so that
+  // multiple successive calls will always produce the same return value and out
+  // parameters. This is necessary for intergration with the Snapshot interface.
+  // See https://crashpad.chromium.org/bug/9.
+  std::set<gid_t> supplementary_groups_;
+  mutable timeval start_time_;
+  pid_t pid_;
+  pid_t ppid_;
+  uid_t uid_;
+  uid_t euid_;
+  uid_t suid_;
+  gid_t gid_;
+  gid_t egid_;
+  gid_t sgid_;
+  mutable InitializationState start_time_initialized_;
+  mutable InitializationState is_64_bit_initialized_;
+  mutable bool is_64_bit_;
 #endif
   InitializationStateDcheck initialized_;
 

util/posix/process_info_linux.cc

@@ -0,0 +1,520 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// 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/posix/process_info.h"
+
+#include <ctype.h>
+#include <elf.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/ptrace.h>
+#include <sys/uio.h>
+#include <sys/user.h>
+#include <sys/wait.h>
+#include <time.h>
+#include <unistd.h>
+
+#include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
+#include "base/logging.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_piece.h"
+#include "util/file/delimited_file_reader.h"
+#include "util/file/file_reader.h"
+
+namespace crashpad {
+
+namespace {
+
+// If the string |pattern| is matched exactly at the start of |input|, advance
+// |input| past |pattern| and return true.
+bool AdvancePastPrefix(const char** input, const char* pattern) {
+  size_t length = strlen(pattern);
+  if (strncmp(*input, pattern, length) == 0) {
+    *input += length;
+    return true;
+  }
+  return false;
+}
+
+#define MAKE_ADAPTER(type, function)                                        \
+  bool ConvertStringToNumber(const base::StringPiece& input, type* value) { \
+    return function(input, value);                                          \
+  }
+MAKE_ADAPTER(int, base::StringToInt)
+MAKE_ADAPTER(unsigned int, base::StringToUint)
+MAKE_ADAPTER(uint64_t, base::StringToUint64)
+#undef MAKE_ADAPTER
+
+// Attempt to convert a prefix of |input| to numeric type T. On success, set
+// |value| to the number, advance |input| past the number, and return true.
+template <typename T>
+bool AdvancePastNumber(const char** input, T* value) {
+  size_t length = 0;
+  if (std::numeric_limits<T>::is_signed && **input == '-') {
+    ++length;
+  }
+  while (isdigit((*input)[length])) {
+    ++length;
+  }
+  bool success = ConvertStringToNumber(base::StringPiece(*input, length),
+                                       value);
+  if (success) {
+    *input += length;
+    return true;
+  }
+  return false;
+}
+
+bool ReadEntireFile(const char* path, std::string* contents) {
+  FileReader file;
+  if (!file.Open(base::FilePath(path))) {
+    return false;
+  }
+
+  char buffer[4096];
+  FileOperationResult length;
+  while ((length = file.Read(buffer, sizeof(buffer))) > 0) {
+    contents->append(buffer, length);
+  }
+  return length >= 0;
+}
+
+void SubtractTimespec(const timespec& t1, const timespec& t2,
+                      timespec* result) {
+  result->tv_sec = t1.tv_sec - t2.tv_sec;
+  result->tv_nsec = t1.tv_nsec - t2.tv_nsec;
+  if (result->tv_nsec < 0) {
+    result->tv_sec -= 1;
+    result->tv_nsec += static_cast<long>(1E9);
+  }
+}
+
+void TimespecToTimeval(const timespec& ts, timeval* tv) {
+  tv->tv_sec = ts.tv_sec;
+  tv->tv_usec = ts.tv_nsec / 1000;
+}
+
+class ScopedPtraceDetach {
+ public:
+  explicit ScopedPtraceDetach(pid_t pid) : pid_(pid) {}
+  ~ScopedPtraceDetach() {
+    if (ptrace(PTRACE_DETACH, pid_, nullptr, nullptr) != 0) {
+      PLOG(ERROR) << "ptrace";
+    }
+  }
+
+ private:
+  pid_t pid_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedPtraceDetach);
+};
+
+}  // namespace
+
+ProcessInfo::ProcessInfo()
+    : supplementary_groups_(),
+      start_time_(),
+      pid_(-1),
+      ppid_(-1),
+      uid_(-1),
+      euid_(-1),
+      suid_(-1),
+      gid_(-1),
+      egid_(-1),
+      sgid_(-1),
+      start_time_initialized_(),
+      is_64_bit_initialized_(),
+      is_64_bit_(false),
+      initialized_() {}
+
+ProcessInfo::~ProcessInfo() {}
+
+bool ProcessInfo::Initialize(pid_t pid) {
+  INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
+
+  pid_ = pid;
+
+  {
+    char path[32];
+    snprintf(path, sizeof(path), "/proc/%d/status", pid_);
+    FileReader status_file;
+    if (!status_file.Open(base::FilePath(path))) {
+      return false;
+    }
+
+    DelimitedFileReader status_file_line_reader(&status_file);
+
+    bool have_ppid = false;
+    bool have_uids = false;
+    bool have_gids = false;
+    bool have_groups = false;
+    std::string line;
+    DelimitedFileReader::Result result;
+    while ((result = status_file_line_reader.GetLine(&line)) ==
+           DelimitedFileReader::Result::kSuccess) {
+      if (line.back() != '\n') {
+        LOG(ERROR) << "format error: unterminated line at EOF";
+        return false;
+      }
+
+      bool understood_line = false;
+      const char* line_c = line.c_str();
+      if (AdvancePastPrefix(&line_c, "PPid:\t")) {
+        if (have_ppid) {
+          LOG(ERROR) << "format error: multiple PPid lines";
+          return false;
+        }
+        have_ppid = AdvancePastNumber(&line_c, &ppid_);
+        if (!have_ppid) {
+          LOG(ERROR) << "format error: unrecognized PPid format";
+          return false;
+        }
+        understood_line = true;
+      } else if (AdvancePastPrefix(&line_c, "Uid:\t")) {
+        if (have_uids) {
+          LOG(ERROR) << "format error: multiple Uid lines";
+          return false;
+        }
+        uid_t fsuid;
+        have_uids = AdvancePastNumber(&line_c, &uid_) &&
+                    AdvancePastPrefix(&line_c, "\t") &&
+                    AdvancePastNumber(&line_c, &euid_) &&
+                    AdvancePastPrefix(&line_c, "\t") &&
+                    AdvancePastNumber(&line_c, &suid_) &&
+                    AdvancePastPrefix(&line_c, "\t") &&
+                    AdvancePastNumber(&line_c, &fsuid);
+        if (!have_uids) {
+          LOG(ERROR) << "format error: unrecognized Uid format";
+          return false;
+        }
+        understood_line = true;
+      } else if (AdvancePastPrefix(&line_c, "Gid:\t")) {
+        if (have_gids) {
+          LOG(ERROR) << "format error: multiple Gid lines";
+          return false;
+        }
+        gid_t fsgid;
+        have_gids = AdvancePastNumber(&line_c, &gid_) &&
+                    AdvancePastPrefix(&line_c, "\t") &&
+                    AdvancePastNumber(&line_c, &egid_) &&
+                    AdvancePastPrefix(&line_c, "\t") &&
+                    AdvancePastNumber(&line_c, &sgid_) &&
+                    AdvancePastPrefix(&line_c, "\t") &&
+                    AdvancePastNumber(&line_c, &fsgid);
+        if (!have_gids) {
+          LOG(ERROR) << "format error: unrecognized Gid format";
+          return false;
+        }
+        understood_line = true;
+      } else if (AdvancePastPrefix(&line_c, "Groups:\t")) {
+        if (have_groups) {
+          LOG(ERROR) << "format error: multiple Groups lines";
+          return false;
+        }
+        gid_t group;
+        while (AdvancePastNumber(&line_c, &group)) {
+          supplementary_groups_.insert(group);
+          if (!AdvancePastPrefix(&line_c, " ")) {
+            LOG(ERROR) << "format error: unrecognized Groups format";
+            return false;
+          }
+        }
+        have_groups = true;
+        understood_line = true;
+      }
+
+      if (understood_line && line_c != &line.back()) {
+        LOG(ERROR) << "format error: unconsumed trailing data";
+        return false;
+      }
+    }
+    if (result != DelimitedFileReader::Result::kEndOfFile) {
+      return false;
+    }
+    if (!have_ppid || !have_uids || !have_gids || !have_groups) {
+      LOG(ERROR) << "format error: missing fields";
+      return false;
+    }
+  }
+
+  INITIALIZATION_STATE_SET_VALID(initialized_);
+  return true;
+}
+
+pid_t ProcessInfo::ProcessID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return pid_;
+}
+
+pid_t ProcessInfo::ParentProcessID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return ppid_;
+}
+
+uid_t ProcessInfo::RealUserID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return uid_;
+}
+
+uid_t ProcessInfo::EffectiveUserID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return euid_;
+}
+
+uid_t ProcessInfo::SavedUserID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return suid_;
+}
+
+gid_t ProcessInfo::RealGroupID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return gid_;
+}
+
+gid_t ProcessInfo::EffectiveGroupID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return egid_;
+}
+
+gid_t ProcessInfo::SavedGroupID() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return sgid_;
+}
+
+std::set<gid_t> ProcessInfo::SupplementaryGroups() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return supplementary_groups_;
+}
+
+std::set<gid_t> ProcessInfo::AllGroups() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+
+  std::set<gid_t> all_groups = SupplementaryGroups();
+  all_groups.insert(RealGroupID());
+  all_groups.insert(EffectiveGroupID());
+  all_groups.insert(SavedGroupID());
+  return all_groups;
+}
+
+bool ProcessInfo::DidChangePrivileges() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  // TODO(jperaza): Is this possible to determine?
+  return false;
+}
+
+bool ProcessInfo::Is64Bit(bool* is_64_bit) const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+
+  if (is_64_bit_initialized_.is_uninitialized()) {
+    is_64_bit_initialized_.set_invalid();
+
+#if defined(ARCH_CPU_64_BITS)
+    const bool am_64_bit = true;
+#else
+    const bool am_64_bit = false;
+#endif
+
+    if (pid_ == getpid()) {
+      is_64_bit_ = am_64_bit;
+    } else {
+      if (ptrace(PTRACE_ATTACH, pid_, nullptr, nullptr) != 0) {
+        PLOG(ERROR) << "ptrace";
+        return false;
+      }
+
+      ScopedPtraceDetach ptrace_detach(pid_);
+
+      if (HANDLE_EINTR(waitpid(pid_, nullptr, __WALL)) < 0) {
+        PLOG(ERROR) << "waitpid";
+        return false;
+      }
+
+      // Allocate more buffer space than is required to hold registers for this
+      // process. If the kernel fills the extra space, the target process uses
+      // more/larger registers than this process. If the kernel fills less space
+      // than sizeof(regs) then the target process uses smaller/fewer registers.
+      struct {
+#if defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARM64)
+        using PrStatusType = user_regs_struct;
+#elif defined(ARCH_CPU_ARMEL)
+        using PrStatusType = user_regs;
+#endif
+        PrStatusType regs;
+        char extra;
+      } regbuf;
+
+      iovec iov;
+      iov.iov_base = &regbuf;
+      iov.iov_len = sizeof(regbuf);
+      if (ptrace(PTRACE_GETREGSET,
+                 pid_,
+                 reinterpret_cast<void*>(NT_PRSTATUS),
+                 &iov) != 0) {
+        switch (errno) {
+#if defined(ARCH_CPU_ARMEL)
+          case EIO:
+            // PTRACE_GETREGSET, introduced in Linux 2.6.34 (2225a122ae26),
+            // requires kernel support enabled by HAVE_ARCH_TRACEHOOK. This has
+            // been set for x86 (including x86_64) since Linux 2.6.28
+            // (99bbc4b1e677a), but for ARM only since Linux 3.5.0
+            // (0693bf68148c4). Fortunately, 64-bit ARM support only appeared in
+            // Linux 3.7.0, so if PTRACE_GETREGSET fails on ARM with EIO,
+            // indicating that the request is not supported, the kernel must be
+            // old enough that 64-bit ARM isn’t supported either.
+            //
+            // TODO(mark): Once helpers to interpret the kernel version are
+            // available, add a DCHECK to ensure that the kernel is older than
+            // 3.5.
+            is_64_bit_ = false;
+            break;
+#endif
+          default:
+            PLOG(ERROR) << "ptrace";
+            return false;
+        }
+      } else {
+        is_64_bit_ = am_64_bit == (iov.iov_len == sizeof(regbuf.regs));
+      }
+    }
+
+    is_64_bit_initialized_.set_valid();
+  }
+
+  if (!is_64_bit_initialized_.is_valid()) {
+    return false;
+  }
+
+  *is_64_bit = is_64_bit_;
+  return true;
+}
+
+bool ProcessInfo::StartTime(timeval* start_time) const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+
+  if (start_time_initialized_.is_uninitialized()) {
+    start_time_initialized_.set_invalid();
+
+    char path[32];
+    snprintf(path, sizeof(path), "/proc/%d/stat", pid_);
+    std::string stat_contents;
+    if (!ReadEntireFile(path, &stat_contents)) {
+      return false;
+    }
+
+    // The process start time is the 22nd column.
+    // The second column is the executable name in parentheses.
+    // The executable name may have parentheses itself, so find the end of the
+    // second column by working backwards to find the last closing parens and
+    // then count forward to the 22nd column.
+    size_t stat_pos = stat_contents.rfind(')');
+    if (stat_pos == std::string::npos) {
+      LOG(ERROR) << "format error";
+      return false;
+    }
+
+    for (int index = 1; index < 21; ++index) {
+      stat_pos = stat_contents.find(' ', stat_pos);
+      if (stat_pos == std::string::npos) {
+        break;
+      }
+      ++stat_pos;
+    }
+    if (stat_pos >= stat_contents.size()) {
+      LOG(ERROR) << "format error";
+      return false;
+    }
+
+    const char* ticks_ptr = &stat_contents[stat_pos];
+
+    // start time is in jiffies instead of clock ticks pre 2.6.
+    uint64_t ticks_after_boot;
+    if (!AdvancePastNumber<uint64_t>(&ticks_ptr, &ticks_after_boot)) {
+      LOG(ERROR) << "format error";
+      return false;
+    }
+    long clock_ticks_per_s = sysconf(_SC_CLK_TCK);
+    if (clock_ticks_per_s <= 0) {
+      PLOG(ERROR) << "sysconf";
+      return false;
+    }
+    timeval time_after_boot;
+    time_after_boot.tv_sec = ticks_after_boot / clock_ticks_per_s;
+    time_after_boot.tv_usec =
+        (ticks_after_boot % clock_ticks_per_s) *
+        (static_cast<long>(1E6) / clock_ticks_per_s);
+
+    timespec uptime;
+    if (clock_gettime(CLOCK_BOOTTIME, &uptime) != 0) {
+      PLOG(ERROR) << "clock_gettime";
+      return false;
+    }
+
+    timespec current_time;
+    if (clock_gettime(CLOCK_REALTIME, &current_time) != 0) {
+      PLOG(ERROR) << "clock_gettime";
+      return false;
+    }
+
+    timespec boot_time_ts;
+    SubtractTimespec(current_time, uptime, &boot_time_ts);
+    timeval boot_time_tv;
+    TimespecToTimeval(boot_time_ts, &boot_time_tv);
+    timeradd(&boot_time_tv, &time_after_boot, &start_time_);
+
+    start_time_initialized_.set_valid();
+  }
+
+  if (!start_time_initialized_.is_valid()) {
+    return false;
+  }
+
+  *start_time = start_time_;
+  return true;
+}
+
+bool ProcessInfo::Arguments(std::vector<std::string>* argv) const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+
+  char path[32];
+  snprintf(path, sizeof(path), "/proc/%d/cmdline", pid_);
+  FileReader cmdline_file;
+  if (!cmdline_file.Open(base::FilePath(path))) {
+    return false;
+  }
+
+  DelimitedFileReader cmdline_file_field_reader(&cmdline_file);
+
+  std::vector<std::string> local_argv;
+  std::string argument;
+  DelimitedFileReader::Result result;
+  while ((result = cmdline_file_field_reader.GetDelim('\0', &argument)) ==
+         DelimitedFileReader::Result::kSuccess) {
+    if (argument.back() != '\0') {
+      LOG(ERROR) << "format error";
+      return false;
+    }
+    argument.pop_back();
+    local_argv.push_back(argument);
+  }
+  if (result != DelimitedFileReader::Result::kEndOfFile) {
+    return false;
+  }
+
+  argv->swap(local_argv);
+  return true;
+}
+
+}  // namespace crashpad

util/posix/process_info_mac.cc

@@ -132,14 +132,16 @@ bool ProcessInfo::DidChangePrivileges() const {
   return kern_proc_info_.kp_proc.p_flag & P_SUGID;
 }
 
-bool ProcessInfo::Is64Bit() const {
+bool ProcessInfo::Is64Bit(bool* is_64_bit) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  return kern_proc_info_.kp_proc.p_flag & P_LP64;
+  *is_64_bit = kern_proc_info_.kp_proc.p_flag & P_LP64;
+  return true;
 }
 
-void ProcessInfo::StartTime(timeval* start_time) const {
+bool ProcessInfo::StartTime(timeval* start_time) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   *start_time = kern_proc_info_.kp_proc.p_starttime;
+  return true;
 }
 
 bool ProcessInfo::Arguments(std::vector<std::string>* argv) const {

util/posix/process_info_test.cc

@@ -14,32 +14,27 @@
 
 #include "util/posix/process_info.h"
 
+#include <signal.h>
 #include <time.h>
-#include <stdio.h>
 #include <unistd.h>
 
+#include <algorithm>
 #include <set>
 #include <string>
 #include <vector>
 
-#include "base/files/scoped_file.h"
+#include "base/strings/stringprintf.h"
 #include "build/build_config.h"
 #include "gtest/gtest.h"
 #include "test/errors.h"
+#include "test/main_arguments.h"
 #include "util/misc/implicit_cast.h"
 
-#if defined(OS_MACOSX)
-#include <crt_externs.h>
-#endif
-
 namespace crashpad {
 namespace test {
 namespace {
 
-void TestSelfProcess(const ProcessInfo& process_info) {
+void TestProcessSelfOrClone(const ProcessInfo& process_info) {
-  EXPECT_EQ(getpid(), process_info.ProcessID());
-  EXPECT_EQ(getppid(), process_info.ParentProcessID());
-
   // There’s no system call to obtain the saved set-user ID or saved set-group
   // ID in an easy way. Normally, they are the same as the effective user ID and
   // effective group ID, so just check against those.
@@ -47,6 +42,7 @@ void TestSelfProcess(const ProcessInfo& process_info) {
   const uid_t euid = geteuid();
   EXPECT_EQ(euid, process_info.EffectiveUserID());
   EXPECT_EQ(euid, process_info.SavedUserID());
+
   const gid_t gid = getgid();
   EXPECT_EQ(gid, process_info.RealGroupID());
   const gid_t egid = getegid();
@@ -78,15 +74,18 @@ void TestSelfProcess(const ProcessInfo& process_info) {
   // The test executable isn’t expected to change privileges.
   EXPECT_FALSE(process_info.DidChangePrivileges());
 
+  bool is_64_bit;
+  ASSERT_TRUE(process_info.Is64Bit(&is_64_bit));
 #if defined(ARCH_CPU_64_BITS)
-  EXPECT_TRUE(process_info.Is64Bit());
+  EXPECT_TRUE(is_64_bit);
 #else
-  EXPECT_FALSE(process_info.Is64Bit());
+  EXPECT_FALSE(is_64_bit);
 #endif
 
   // Test StartTime(). This program must have started at some time in the past.
   timeval start_time;
-  process_info.StartTime(&start_time);
+  ASSERT_TRUE(process_info.StartTime(&start_time));
+  EXPECT_FALSE(start_time.tv_sec == 0 && start_time.tv_usec == 0);
   time_t now;
   time(&now);
   EXPECT_LE(start_time.tv_sec, now);
@@ -94,53 +93,37 @@ void TestSelfProcess(const ProcessInfo& process_info) {
   std::vector<std::string> argv;
   ASSERT_TRUE(process_info.Arguments(&argv));
 
-  // gtest argv processing scrambles argv, but it leaves argc and argv[0]
+  const std::vector<std::string>& expect_argv = GetMainArguments();
-  // intact, so test those.
 
-#if defined(OS_MACOSX)
+  // expect_argv always contains the initial view of the arguments at the time
-  int expect_argc = *_NSGetArgc();
+  // the program was invoked. argv may contain this view, or it may contain the
-  char** expect_argv = *_NSGetArgv();
+  // current view of arguments after gtest argv processing. argv may be a subset
-#elif defined(OS_LINUX) || defined(OS_ANDROID)
+  // of expect_argv.
-  std::vector<std::string> expect_arg_vector;
+  //
-  {
+  // gtest argv processing always leaves argv[0] intact, so this can be checked
-    base::ScopedFILE cmdline(fopen("/proc/self/cmdline", "re"));
+  // directly.
-    ASSERT_NE(nullptr, cmdline.get()) << ErrnoMessage("fopen");
+  ASSERT_FALSE(expect_argv.empty());
+  ASSERT_FALSE(argv.empty());
+  EXPECT_EQ(expect_argv[0], argv[0]);
 
-    int expect_arg_char;
+  EXPECT_LE(argv.size(), expect_argv.size());
-    std::string expect_arg_string;
+
-    while ((expect_arg_char = fgetc(cmdline.get())) != EOF) {
+  // Everything else in argv should have a match in expect_argv too, but things
-      if (expect_arg_char != '\0') {
+  // may have moved around.
-        expect_arg_string.append(1, expect_arg_char);
+  for (size_t arg_index = 1; arg_index < argv.size(); ++arg_index) {
-      } else {
+    const std::string& arg = argv[arg_index];
-        expect_arg_vector.push_back(expect_arg_string);
+    SCOPED_TRACE(
-        expect_arg_string.clear();
+        base::StringPrintf("arg_index %zu, arg %s", arg_index, arg.c_str()));
-      }
+    EXPECT_NE(expect_argv.end(), std::find(argv.begin(), argv.end(), arg));
-    }
-    ASSERT_EQ(0, ferror(cmdline.get())) << ErrnoMessage("fgetc");
-    ASSERT_TRUE(expect_arg_string.empty());
   }
-
-  std::vector<const char*> expect_argv_storage;
-  for (const std::string& expect_arg_string : expect_arg_vector) {
-    expect_argv_storage.push_back(expect_arg_string.c_str());
-  }
-
-  int expect_argc = expect_argv_storage.size();
-  const char* const* expect_argv =
-      !expect_argv_storage.empty() ? &expect_argv_storage[0] : nullptr;
-#else
-#error Obtain expect_argc and expect_argv correctly on your system.
-#endif
-
-  int argc = implicit_cast<int>(argv.size());
-  EXPECT_EQ(expect_argc, argc);
-
-  ASSERT_GE(expect_argc, 1);
-  ASSERT_GE(argc, 1);
-
-  EXPECT_EQ(std::string(expect_argv[0]), argv[0]);
 }
 
+void TestSelfProcess(const ProcessInfo& process_info) {
+  EXPECT_EQ(getpid(), process_info.ProcessID());
+  EXPECT_EQ(getppid(), process_info.ParentProcessID());
+
+  TestProcessSelfOrClone(process_info);
+}
 
 TEST(ProcessInfo, Self) {
   ProcessInfo process_info;
@@ -173,6 +156,24 @@ TEST(ProcessInfo, Pid1) {
   EXPECT_FALSE(process_info.AllGroups().empty());
 }
 
+TEST(ProcessInfo, Forked) {
+  pid_t pid = fork();
+  if (pid == 0) {
+    raise(SIGSTOP);
+    _exit(0);
+  }
+  ASSERT_GE(pid, 0) << ErrnoMessage("fork");
+
+  ProcessInfo process_info;
+  ASSERT_TRUE(process_info.Initialize(pid));
+
+  EXPECT_EQ(pid, process_info.ProcessID());
+  EXPECT_EQ(getpid(), process_info.ParentProcessID());
+
+  TestProcessSelfOrClone(process_info);
+  kill(pid, SIGKILL);
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad

util/thread/worker_thread_test.cc

@@ -30,8 +30,11 @@ class WorkDelegate : public WorkerThread::Delegate {
   ~WorkDelegate() {}
 
   void DoWork(const WorkerThread* thread) override {
-    if (++work_count_ == waiting_for_count_)
+    if (work_count_ < waiting_for_count_) {
-      semaphore_.Signal();
+      if (++work_count_ == waiting_for_count_) {
+        semaphore_.Signal();
+      }
+    }
   }
 
   void SetDesiredWorkCount(int times) {
@@ -59,6 +62,7 @@ TEST(WorkerThread, DoWork) {
   WorkerThread thread(0.05, &delegate);
 
   uint64_t start = ClockMonotonicNanoseconds();
+
   delegate.SetDesiredWorkCount(2);
   thread.Start(0);
   EXPECT_TRUE(thread.is_running());
@@ -103,12 +107,12 @@ TEST(WorkerThread, DoWorkNow) {
   WorkDelegate delegate;
   WorkerThread thread(100, &delegate);
 
+  uint64_t start = ClockMonotonicNanoseconds();
+
   delegate.SetDesiredWorkCount(1);
   thread.Start(0);
   EXPECT_TRUE(thread.is_running());
 
-  uint64_t start = ClockMonotonicNanoseconds();
-
   delegate.WaitForWorkCount();
   EXPECT_EQ(1, delegate.work_count());
 

util/util.gyp

@@ -30,6 +30,8 @@
         '<(INTERMEDIATE_DIR)',
       ],
       'sources': [
+        'file/delimited_file_reader.cc',
+        'file/delimited_file_reader.h',
         'file/file_io.cc',
         'file/file_io.h',
         'file/file_io_posix.cc',
@@ -135,6 +137,7 @@
         'posix/drop_privileges.cc',
         'posix/drop_privileges.h',
         'posix/process_info.h',
+        'posix/process_info_linux.cc',
         'posix/process_info_mac.cc',
         'posix/signals.cc',
         'posix/signals.h',
@@ -313,6 +316,13 @@
           ],
         }],
       ],
+      'target_conditions': [
+        ['OS=="android"', {
+          'sources/': [
+            ['include', '^posix/process_info_linux\\.cc$'],
+          ],
+        }],
+      ],
     },
   ],
 }

util/util_test.gyp

@@ -24,9 +24,9 @@
         'util.gyp:crashpad_util',
         '../client/client.gyp:crashpad_client',
         '../compat/compat.gyp:crashpad_compat',
+        '../test/test.gyp:crashpad_gmock_main',
         '../test/test.gyp:crashpad_test',
         '../third_party/gtest/gmock.gyp:gmock',
-        '../third_party/gtest/gmock.gyp:gmock_main',
         '../third_party/gtest/gtest.gyp:gtest',
         '../third_party/mini_chromium/mini_chromium.gyp:base',
         '../third_party/zlib/zlib.gyp:zlib',
@@ -35,7 +35,9 @@
         '..',
       ],
       'sources': [
+        'file/delimited_file_reader_test.cc',
         'file/file_io_test.cc',
+        'file/file_reader_test.cc',
         'file/string_file_test.cc',
         'mac/launchd_test.mm',
         'mac/mac_util_test.mm',
@@ -120,6 +122,18 @@
             ],
           },
         }],
+        ['OS=="android"', {
+          # Things not yet ported to Android
+          'sources/' : [
+            ['exclude', '^net/http_transport_test\\.cc$'],
+          ]
+        }],
+        ['OS=="android" or OS=="linux"' , {
+          # Things not yet ported to Android or Linux
+          'sources/' : [
+            ['exclude', '^numeric/checked_address_range_test\\.cc$'],
+          ]
+        }],
       ],
     },
   ],

util/win/exception_handler_server.cc

@@ -396,7 +396,8 @@ bool ExceptionHandlerServer::ServiceClientConnection(
     const internal::PipeServiceContext& service_context) {
   ClientToServerMessage message;
 
-  if (!LoggingReadFile(service_context.pipe(), &message, sizeof(message)))
+  if (!LoggingReadFileExactly(
+          service_context.pipe(), &message, sizeof(message)))
     return false;
 
   switch (message.type) {

util/win/exception_handler_server_test.cc

@@ -143,10 +143,11 @@ TEST_F(ExceptionHandlerServerTest, StopWhileConnected) {
 
 std::wstring ReadWString(FileHandle handle) {
   size_t length = 0;
-  EXPECT_TRUE(LoggingReadFile(handle, &length, sizeof(length)));
+  EXPECT_TRUE(LoggingReadFileExactly(handle, &length, sizeof(length)));
   std::wstring str(length, L'\0');
   if (length > 0) {
-    EXPECT_TRUE(LoggingReadFile(handle, &str[0], length * sizeof(str[0])));
+    EXPECT_TRUE(
+        LoggingReadFileExactly(handle, &str[0], length * sizeof(str[0])));
   }
   return str;
 }

util/win/process_info_test.cc

@@ -156,7 +156,7 @@ void TestOtherProcess(const base::string16& directory_modification) {
 
   // The child sends us a code address we can look up in the memory map.
   WinVMAddress code_address;
-  CheckedReadFile(
+  CheckedReadFileExactly(
       child.stdout_read_handle(), &code_address, sizeof(code_address));
 
   ASSERT_TRUE(process_info.Initialize(child.process_handle()));

util/win/scoped_process_suspend_test.cc

@@ -80,7 +80,7 @@ class ScopedProcessSuspendTest final : public WinChildProcess {
   int Run() override {
     char c;
     // Wait for notification from parent.
-    EXPECT_TRUE(LoggingReadFile(ReadPipeHandle(), &c, sizeof(c)));
+    EXPECT_TRUE(LoggingReadFileExactly(ReadPipeHandle(), &c, sizeof(c)));
     EXPECT_EQ(' ', c);
     return EXIT_SUCCESS;
   }