diff --git a/build/run_tests.py b/build/run_tests.py index f6306852..00b40f6b 100755 --- a/build/run_tests.py +++ b/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, \ - 'usage: run_tests.py {Debug|Release|Debug_x64|Release_x64}' + print >> sys.stderr, 'usage: run_tests.py ' return 1 crashpad_dir = \ os.path.join(os.path.dirname(os.path.abspath(__file__)), os.pardir) - - # 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]) + binary_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 diff --git a/client/client_test.gyp b/client/client_test.gyp index 66ec59ea..dec8f603 100644 --- a/client/client_test.gyp +++ b/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', diff --git a/client/crash_report_database_win.cc b/client/crash_report_database_win.cc index 4fccd3fd..6629d25d 100644 --- a/client/crash_report_database_win.cc +++ b/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(end - read_from); std::string buffer(data_length, '\0'); - return LoggingReadFile(file, &buffer[0], data_length) ? buffer - : std::string(); + return LoggingReadFileExactly(file, &buffer[0], data_length) ? buffer + : 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 reports; if (header.num_records > 0) { std::vector records(header.num_records); - if (!LoggingReadFile( + if (!LoggingReadFileExactly( handle_.get(), &records[0], records_size.ValueOrDie())) { LOG(ERROR) << "failed to read records"; return; diff --git a/client/settings.cc b/client/settings.cc index 7757ecb0..15d16f2e 100644 --- a/client/settings.cc +++ b/client/settings.cc @@ -246,13 +246,10 @@ bool Settings::ReadSettings(FileHandle handle, if (LoggingSeekFile(handle, 0, SEEK_SET) != 0) return false; - bool read_result; - if (log_read_error) { - read_result = LoggingReadFile(handle, out_data, sizeof(*out_data)); - } else { - read_result = - ReadFile(handle, out_data, sizeof(*out_data)) == sizeof(*out_data); - } + bool read_result = + log_read_error + ? LoggingReadFileExactly(handle, out_data, sizeof(*out_data)) + : ReadFileExactly(handle, out_data, sizeof(*out_data)); if (!read_result) return false; diff --git a/compat/android/elf.h b/compat/android/elf.h new file mode 100644 index 00000000..a3fc2a9a --- /dev/null +++ b/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 + +// Android 5.0.0 (API 21) NDK +#if !defined(NT_PRSTATUS) +#define NT_PRSTATUS 1 +#endif + +#endif // CRASHPAD_COMPAT_ANDROID_ELF_H_ diff --git a/compat/android/linux/ptrace.h b/compat/android/linux/ptrace.h new file mode 100644 index 00000000..7db46aa3 --- /dev/null +++ b/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 + +// Android 5.0.0 (API 21) NDK +#if !defined(PTRACE_GETREGSET) +#define PTRACE_GETREGSET 0x4204 +#endif + +#endif // CRASHPAD_COMPAT_ANDROID_LINUX_PTRACE_H_ diff --git a/compat/compat.gyp b/compat/compat.gyp index f1cd4933..56daa3f8 100644 --- a/compat/compat.gyp +++ b/compat/compat.gyp @@ -83,6 +83,16 @@ ], }, }], + ['OS=="android"', { + 'include_dirs': [ + 'android', + ], + 'direct_dependent_settings': { + 'include_dirs': [ + 'android', + ], + }, + }], ], }, ], diff --git a/doc/developing.md b/doc/developing.md index 4505d610..9beec52c 100644 --- a/doc/developing.md +++ b/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 \ - --arch=arm64 --api=21 --install-dir=android-ndk-r13_arm64_api21 +$ python android-ndk-r14/build/tools/make_standalone_toolchain.py \ + --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 \ - CXX_target=~/android-ndk-r13_arm64_api21/bin/clang++ \ - AR_target=~/android-ndk-r13_arm64_api21/bin/aarch64-linux-android-ar \ - NM_target=~/android-ndk-r13_arm64_api21/bin/aarch64-linux-android-nm \ - READELF_target=~/android-ndk-r13_arm64_api21/bin/aarch64-linux-android-readelf \ +$ CC_target=~/android-ndk-r14_arm64_api21/bin/clang \ + CXX_target=~/android-ndk-r14_arm64_api21/bin/clang++ \ + AR_target=~/android-ndk-r14_arm64_api21/bin/aarch64-linux-android-ar \ + NM_target=~/android-ndk-r14_arm64_api21/bin/aarch64-linux-android-nm \ + 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 diff --git a/doc/man.md b/doc/man.md index ff5fd413..0344fe7e 100644 --- a/doc/man.md +++ b/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 diff --git a/handler/win/crash_other_program.cc b/handler/win/crash_other_program.cc index d191aac0..012b4ba3 100644 --- a/handler/win/crash_other_program.cc +++ b/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; } diff --git a/minidump/minidump_crashpad_info_writer_test.cc b/minidump/minidump_crashpad_info_writer_test.cc index 85eec274..0b620663 100644 --- a/minidump/minidump_crashpad_info_writer_test.cc +++ b/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)); diff --git a/minidump/minidump_exception_writer_test.cc b/minidump/minidump_exception_writer_test.cc index 8635190b..613c5dd1 100644 --- a/minidump/minidump_exception_writer_test.cc +++ b/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), diff --git a/minidump/minidump_file_writer.cc b/minidump/minidump_file_writer.cc index 6a91ca6b..ebff2bab 100644 --- a/minidump/minidump_file_writer.cc +++ b/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)); - - 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)); - } - } + add_stream_result = AddStream(std::move(module_list)); + DCHECK(add_stream_result); 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 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 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 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) { diff --git a/minidump/minidump_file_writer.h b/minidump/minidump_file_writer.h index b093dade..05cecb14 100644 --- a/minidump/minidump_file_writer.h +++ b/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 - //! streams with the same stream type. + //! MinidumpFileWriter object. If an attempt is made to add a stream whose + //! type matches an existing stream’s type, this method discards the new + //! stream. //! //! \note Valid in #kStateMutable. - void AddStream(std::unique_ptr 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 stream); // MinidumpWritable: diff --git a/minidump/minidump_file_writer_test.cc b/minidump/minidump_file_writer_test.cc index f3432fb8..d8d0f394 100644 --- a/minidump/minidump_file_writer_test.cc +++ b/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(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(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 MinidumpStreamType kStream0Type = static_cast(0x4d); + const size_t kStream0Size = 3; + const MinidumpStreamType kStreamType = static_cast(0x4d); const uint8_t kStream0Value = 0x5a; auto stream0 = base::WrapUnique( - new TestStream(kStream0Type, kStream0Size, kStream0Value)); - minidump_file.AddStream(std::move(stream0)); + new TestStream(kStreamType, kStream0Size, kStream0Value)); + ASSERT_TRUE(minidump_file.AddStream(std::move(stream0))); - // It is an error to add a second stream of the same type. - const size_t kStream1Size = 3; - const MinidumpStreamType kStream1Type = static_cast(0x4d); + // An attempt to add a second stream of the same type should fail. + const size_t kStream1Size = 5; const uint8_t kStream1Value = 0xa5; auto stream1 = base::WrapUnique( - new TestStream(kStream1Type, kStream1Size, kStream1Value)); - ASSERT_DEATH_CHECK(minidump_file.AddStream(std::move(stream1)), - "already present"); + new TestStream(kStreamType, kStream1Size, kStream1Value)); + ASSERT_FALSE(minidump_file.AddStream(std::move(stream1))); + + 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( + 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 diff --git a/minidump/minidump_handle_writer_test.cc b/minidump/minidump_handle_writer_test.cc index 982da713..db4cae5b 100644 --- a/minidump/minidump_handle_writer_test.cc +++ b/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)); diff --git a/minidump/minidump_memory_info_writer_test.cc b/minidump/minidump_memory_info_writer_test.cc index ef6535b7..8debff56 100644 --- a/minidump/minidump_memory_info_writer_test.cc +++ b/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)); diff --git a/minidump/minidump_memory_writer_test.cc b/minidump/minidump_memory_writer_test.cc index d9e87aee..65fcca28 100644 --- a/minidump/minidump_memory_writer_test.cc +++ b/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)); diff --git a/minidump/minidump_misc_info_writer_test.cc b/minidump/minidump_misc_info_writer_test.cc index 9a0c7654..8f2b5517 100644 --- a/minidump/minidump_misc_info_writer_test.cc +++ b/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)); diff --git a/minidump/minidump_module_writer_test.cc b/minidump/minidump_module_writer_test.cc index 7c9dcdaa..f5484744 100644 --- a/minidump/minidump_module_writer_test.cc +++ b/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), diff --git a/minidump/minidump_system_info_writer_test.cc b/minidump/minidump_system_info_writer_test.cc index 2a869c6b..4f35d3d1 100644 --- a/minidump/minidump_system_info_writer_test.cc +++ b/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), diff --git a/minidump/minidump_test.gyp b/minidump/minidump_test.gyp index cdae4caa..acf4d635 100644 --- a/minidump/minidump_test.gyp +++ b/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', ], diff --git a/minidump/minidump_thread_writer_test.cc b/minidump/minidump_thread_writer_test.cc index 1384efe3..61332363 100644 --- a/minidump/minidump_thread_writer_test.cc +++ b/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)); - minidump_file_writer.AddStream(std::move(memory_list_writer)); + ASSERT_TRUE(minidump_file_writer.AddStream(std::move(thread_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)); - minidump_file_writer.AddStream(std::move(memory_list_writer)); + ASSERT_TRUE(minidump_file_writer.AddStream(std::move(thread_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), diff --git a/minidump/minidump_unloaded_module_writer_test.cc b/minidump/minidump_unloaded_module_writer_test.cc index dba46918..002c82bb 100644 --- a/minidump/minidump_unloaded_module_writer_test.cc +++ b/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)); diff --git a/minidump/minidump_user_stream_writer_test.cc b/minidump/minidump_user_stream_writer_test.cc index 97d41eec..e4e8cccb 100644 --- a/minidump/minidump_user_stream_writer_test.cc +++ b/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)); diff --git a/snapshot/api/module_annotations_win_test.cc b/snapshot/api/module_annotations_win_test.cc index 30e880f3..ee028683 100644 --- a/snapshot/api/module_annotations_win_test.cc +++ b/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)); } }; diff --git a/snapshot/mac/mach_o_image_annotations_reader_test.cc b/snapshot/mac/mach_o_image_annotations_reader_test.cc index 7a94eb2c..50e646ae 100644 --- a/snapshot/mac/mach_o_image_annotations_reader_test.cc +++ b/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& 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. diff --git a/snapshot/mac/process_reader.cc b/snapshot/mac/process_reader.cc index 46b83c13..46083ab3 100644 --- a/snapshot/mac/process_reader.cc +++ b/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_); diff --git a/snapshot/mac/process_reader.h b/snapshot/mac/process_reader.h index c3696110..72bd1dc6 100644 --- a/snapshot/mac/process_reader.h +++ b/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 { - process_info_.StartTime(start_time); - } + void StartTime(timeval* start_time) const; //! \brief Determines the target process’ execution time. //! diff --git a/snapshot/mac/process_reader_test.cc b/snapshot/mac/process_reader_test.cc index e679d2ef..87fb6394 100644 --- a/snapshot/mac/process_reader_test.cc +++ b/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, - &expectation.stack_address, - sizeof(expectation.stack_address)); - CheckedReadFile(read_handle, - &expectation.suspend_count, - sizeof(expectation.suspend_count)); + CheckedReadFileExactly(read_handle, + &expectation.stack_address, + sizeof(expectation.stack_address)); + CheckedReadFileExactly(read_handle, + &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()); diff --git a/snapshot/snapshot_test.gyp b/snapshot/snapshot_test.gyp index 4ca9f4d5..b31b0601 100644 --- a/snapshot/snapshot_test.gyp +++ b/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', ], diff --git a/snapshot/win/crashpad_snapshot_test_extra_memory_ranges.cc b/snapshot/win/crashpad_snapshot_test_extra_memory_ranges.cc index 58b145e5..e6875fdd 100644 --- a/snapshot/win/crashpad_snapshot_test_extra_memory_ranges.cc +++ b/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. diff --git a/snapshot/win/crashpad_snapshot_test_simple_annotations.cc b/snapshot/win/crashpad_snapshot_test_simple_annotations.cc index b66a19e1..11e7b4e7 100644 --- a/snapshot/win/crashpad_snapshot_test_simple_annotations.cc +++ b/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. diff --git a/snapshot/win/exception_snapshot_win_test.cc b/snapshot/win/exception_snapshot_win_test.cc index af0cc3a6..03aa0499 100644 --- a/snapshot/win/exception_snapshot_win_test.cc +++ b/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(), - &break_near_address, - sizeof(break_near_address)); + LoggingReadFileExactly(child.stdout_read_handle(), + &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(), - &dump_near_address, - sizeof(dump_near_address)); + LoggingReadFileExactly(child.stdout_read_handle(), + &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. diff --git a/snapshot/win/extra_memory_ranges_test.cc b/snapshot/win/extra_memory_ranges_test.cc index d6d56596..57d41bb5 100644 --- a/snapshot/win/extra_memory_ranges_test.cc +++ b/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( diff --git a/snapshot/win/pe_image_annotations_reader_test.cc b/snapshot/win/pe_image_annotations_reader_test.cc index 523a4494..f791f80e 100644 --- a/snapshot/win/pe_image_annotations_reader_test.cc +++ b/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(), diff --git a/snapshot/win/process_reader_win_test.cc b/snapshot/win/process_reader_win_test.cc index b9befc9a..af57c622 100644 --- a/snapshot/win/process_reader_win_test.cc +++ b/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); { diff --git a/snapshot/win/process_snapshot_win_test.cc b/snapshot/win/process_snapshot_win_test.cc index a3e31a4e..82b29035 100644 --- a/snapshot/win/process_snapshot_win_test.cc +++ b/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()); diff --git a/test/gmock_main.cc b/test/gmock_main.cc new file mode 100644 index 00000000..05492817 --- /dev/null +++ b/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(); +} diff --git a/test/gtest_main.cc b/test/gtest_main.cc new file mode 100644 index 00000000..5608d2c2 --- /dev/null +++ b/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(); +} diff --git a/test/main_arguments.cc b/test/main_arguments.cc new file mode 100644 index 00000000..c28d2fa3 --- /dev/null +++ b/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* g_arguments; + +void InitializeMainArguments(int argc, char* argv[]) { + CHECK(!g_arguments); + CHECK(argc); + CHECK(argv); + + g_arguments = new const std::vector(argv, argv + argc); +} + +const std::vector& GetMainArguments() { + CHECK(g_arguments); + return *g_arguments; +} + +} // namespace test +} // namespace crashpad diff --git a/test/main_arguments.h b/test/main_arguments.h new file mode 100644 index 00000000..d6c4d5c8 --- /dev/null +++ b/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 +#include + +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& GetMainArguments(); + +} // namespace test +} // namespace crashpad + +#endif // CRASHPAD_TEST_MAIN_ARGUMENTS_H_ diff --git a/test/main_arguments_test.cc b/test/main_arguments_test.cc new file mode 100644 index 00000000..85d15984 --- /dev/null +++ b/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& arguments = GetMainArguments(); + + ASSERT_FALSE(arguments.empty()); + EXPECT_FALSE(arguments[0].empty()); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/test/multiprocess_exec_test.cc b/test/multiprocess_exec_test.cc index f6231e9c..3e958aa8 100644 --- a/test/multiprocess_exec_test.cc +++ b/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); } diff --git a/test/multiprocess_posix_test.cc b/test/multiprocess_posix_test.cc index be927111..9f557fab 100644 --- a/test/multiprocess_posix_test.cc +++ b/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); } diff --git a/test/test.gyp b/test/test.gyp index d84ff640..eb0a28b9 100644 --- a/test/test.gyp +++ b/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', + ], + }, ], } diff --git a/test/test_test.gyp b/test/test_test.gyp index 2c58c170..053d9c82 100644 --- a/test/test_test.gyp +++ b/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', diff --git a/test/win/win_child_process.cc b/test/win/win_child_process.cc index 156aa6dc..06ab21b2 100644 --- a/test/win/win_child_process.cc +++ b/test/win/win_child_process.cc @@ -203,7 +203,7 @@ std::unique_ptr 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(); } diff --git a/tools/crashpad_database_util.cc b/tools/crashpad_database_util.cc index e7d89bc4..b2c4cb49 100644 --- a/tools/crashpad_database_util.cc +++ b/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 file_reader; - bool is_stdin = false; if (new_report_path.value() == FILE_PATH_LITERAL("-")) { - is_stdin = true; - file_reader.reset(new WeakStdioFileReader(stdin)); + if (used_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 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()); } diff --git a/tools/crashpad_http_upload.cc b/tools/crashpad_http_upload.cc index 55ab9550..d4ec3f9c 100644 --- a/tools/crashpad_http_upload.cc +++ b/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); diff --git a/util/file/delimited_file_reader.cc b/util/file/delimited_file_reader.cc new file mode 100644 index 00000000..a55c2a7c --- /dev/null +++ b/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 + +#include +#include + +#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::max(), + "buf_pos_ must cover buf_"); + static_assert(sizeof(buf_) <= std::numeric_limits::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(read_result), arraysize(buf_)); + DCHECK( + base::IsValueInRangeForNumericType(read_result)); + buf_len_ = static_cast(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(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 diff --git a/util/file/delimited_file_reader.h b/util/file/delimited_file_reader.h new file mode 100644 index 00000000..93fd7380 --- /dev/null +++ b/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 + +#include + +#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 +//! ('\\n'), 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_ diff --git a/util/file/delimited_file_reader_test.cc b/util/file/delimited_file_reader_test.cc new file mode 100644 index 00000000..63b50836 --- /dev/null +++ b/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 + +#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 lines; + std::string contents; + for (size_t line_index = 0; line_index <= 'z' - 'a'; ++line_index) { + char c = 'a' + static_cast(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 diff --git a/util/file/file_io.cc b/util/file/file_io.cc index 98eb3404..6cae837a 100644 --- a/util/file/file_io.cc +++ b/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) { - FileOperationResult expect = base::checked_cast(size); - FileOperationResult rv = ReadFile(file, buffer, size); - if (rv < 0) { - PLOG(ERROR) << "read"; - return false; +namespace { + +class FileIOReadExactly final : public internal::ReadExactlyInternal { + public: + explicit FileIOReadExactly(FileHandle file) + : 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(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(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(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(size); - FileOperationResult rv = WriteFile(file, buffer, size); - if (rv < 0) { - PLOG(ERROR) << "write"; - return false; - } - if (rv != expect) { - LOG(ERROR) << "write: expected " << expect << ", observed " << rv; + if (!WriteFile(file, buffer, size)) { + PLOG(ERROR) << internal::kNativeWriteFunctionName; return false; } return true; } -void CheckedReadFile(FileHandle file, void* buffer, size_t size) { - CHECK(LoggingReadFile(file, buffer, size)); +void CheckedReadFileExactly(FileHandle file, void* buffer, size_t 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; } } diff --git a/util/file/file_io.h b/util/file/file_io.h index ff20488d..87c40702 100644 --- a/util/file/file_io.h +++ b/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 -//! short read. +//! \brief Determines the FileHandle that StdioFileHandle() returns. +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 -//! been read or when end-of-file has been reached. On Windows, reading from -//! sockets is not currently supported. +//! This value may be useful for logging. +//! +//! \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 CheckedReadFile +//! \sa ReadFileExactly +//! \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 -//! write on POSIX. +//! \brief Writes to a file, retrying when interrupted on POSIX or following a +//! 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 -//! `errno` or `GetLastError()` set appropriately. On error, a portion of -//! \a buffer may have been written to \a file. +//! \return `true` on success. `false` on error, with `errno` or +//! `GetLastError()` set appropriately. On error, a portion of \a buffer may +//! 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 -//! ReadFile() return values, if the underlying ReadFile() fails, or if -//! other than \a size bytes were read, this function logs a message and +//! \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 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 -//! WriteFile() return values, if the underlying WriteFile() fails, or if -//! other than \a size bytes were written, this function logs a message and +//! \return `true` on success. If the underlying WriteFile() fails, or if fewer +//! 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 -//! underlying ReadFile() fails, or if other than \a size bytes were read, this -//! function causes execution to terminate without returning. +//! If the underlying ReadFile() fails, or if fewer than \a size bytes were +//! read, this 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 -//! underlying WriteFile() fails, or if other than \a size bytes were written, -//! this function causes execution to terminate without returning. +//! if the underlying WriteFile() fails, or if fewer than \a size bytes were +//! written, 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_ diff --git a/util/file/file_io_posix.cc b/util/file/file_io_posix.cc index 1534db15..49441244 100644 --- a/util/file/file_io_posix.cc +++ b/util/file/file_io_posix.cc @@ -19,63 +19,49 @@ #include #include +#include +#include + #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 CharBufferType = char*; - static crashpad::FileOperationResult Operate(int fd, - CharBufferType buffer, - size_t size) { + using BufferType = void*; + static FileOperationResult Operate(int fd, BufferType buffer, size_t size) { return read(fd, buffer, size); } }; struct WriteTraits { - using VoidBufferType = const void*; - using CharBufferType = const char*; - static crashpad::FileOperationResult Operate(int fd, - CharBufferType buffer, - size_t size) { + using BufferType = const void*; + static FileOperationResult Operate(int fd, BufferType buffer, size_t size) { return write(fd, buffer, size); } }; template -crashpad::FileOperationResult -ReadOrWrite(int fd, typename Traits::VoidBufferType buffer, size_t size) { - typename Traits::CharBufferType buffer_c = - reinterpret_cast(buffer); +FileOperationResult ReadOrWrite(int fd, + typename Traits::BufferType buffer, + size_t size) { + constexpr size_t kMaxReadWriteSize = + static_cast(std::numeric_limits::max()); + const size_t requested_bytes = std::min(size, kMaxReadWriteSize); - crashpad::FileOperationResult total_bytes = 0; - while (size > 0) { - crashpad::FileOperationResult bytes = - HANDLE_EINTR(Traits::Operate(fd, buffer_c, size)); - if (bytes < 0) { - return bytes; - } else if (bytes == 0) { - break; - } - - buffer_c += bytes; - size -= bytes; - total_bytes += bytes; + FileOperationResult transacted_bytes = + HANDLE_EINTR(Traits::Operate(fd, buffer, requested_bytes)); + if (transacted_bytes < 0) { + return -1; } - return total_bytes; + DCHECK_LE(static_cast(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) { - return ReadOrWrite(file, buffer, size); +namespace internal { + +const char kNativeReadFunctionName[] = "read"; +const char kNativeWriteFunctionName[] = "write"; + +FileOperationResult NativeWriteFile(FileHandle file, + const void* buffer, + size_t size) { + return ReadOrWrite(file, buffer, size); } -FileOperationResult WriteFile(FileHandle file, - const void* buffer, - size_t size) { - return ReadOrWrite(file, buffer, size); +} // namespace internal + +FileOperationResult ReadFile(FileHandle file, void* buffer, size_t size) { + return ReadOrWrite(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 diff --git a/util/file/file_io_test.cc b/util/file/file_io_test.cc index b8444d41..c4e60585 100644 --- a/util/file/file_io_test.cc +++ b/util/file/file_io_test.cc @@ -14,9 +14,15 @@ #include "util/file/file_io.h" +#include + +#include +#include + #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(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(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::max(); + constexpr size_t increment = std::numeric_limits::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::max(); + constexpr size_t increment = + std::numeric_limits::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(data), size); + } + + MOCK_METHOD2(WriteInt, FileOperationResult(uintptr_t, size_t)); + + // WriteAllInternal: + FileOperationResult Write(const void* data, size_t size) { + return WriteInt(reinterpret_cast(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::max(); + constexpr size_t increment = std::numeric_limits::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::max(); + constexpr size_t increment = + std::numeric_limits::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(_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 diff --git a/util/file/file_io_win.cc b/util/file/file_io_win.cc index 5144869e..467fc3c8 100644 --- a/util/file/file_io_win.cc +++ b/util/file/file_io_win.cc @@ -14,9 +14,11 @@ #include "util/file/file_io.h" +#include +#include + #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(std::numeric_limits::max()), + static_cast(std::numeric_limits::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(std::min(size, kMaxReadWriteSize)); + + DWORD bytes_written; + if (!::WriteFile(file, buffer, write_size, &bytes_written, nullptr)) + return -1; + + CHECK_NE(bytes_written, static_cast(-1)); + DCHECK_LE(static_cast(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(size); - DWORD total_read = 0; - char* buffer_c = reinterpret_cast(buffer); - while (size_dword > 0) { + + const DWORD read_size = static_cast(std::min(size, kMaxReadWriteSize)); + + 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. - break; - } else if (GetLastError() != ERROR_MORE_DATA) { - return -1; + // read. Treat this as EOF. + return 0; } - } else if (bytes_read == 0 && GetFileType(file) != FILE_TYPE_PIPE) { + return -1; + } + + CHECK_NE(bytes_read, static_cast(-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(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 diff --git a/util/file/file_reader.cc b/util/file/file_reader.cc index 4f5ae773..6f272f01 100644 --- a/util/file/file_reader.cc +++ b/util/file/file_reader.cc @@ -20,18 +20,31 @@ namespace crashpad { -bool FileReaderInterface::ReadExactly(void* data, size_t size) { - FileOperationResult expect = base::checked_cast(size); - FileOperationResult rv = Read(data, size); - if (rv < 0) { - // Read() will have logged its own error. - return false; - } else if (rv != expect) { - LOG(ERROR) << "ReadExactly(): expected " << expect << ", observed " << rv; - return false; +namespace { + +class FileReaderReadExactly final : public internal::ReadExactlyInternal { + public: + explicit FileReaderReadExactly(FileReaderInterface* file_reader) + : ReadExactlyInternal(), file_reader_(file_reader) {} + ~FileReaderReadExactly() {} + + 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(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 diff --git a/util/file/file_reader.h b/util/file/file_reader.h index 39225e54..d44be1c1 100644 --- a/util/file/file_reader.h +++ b/util/file/file_reader.h @@ -15,7 +15,6 @@ #ifndef CRASHPAD_UTIL_FILE_FILE_READER_H_ #define CRASHPAD_UTIL_FILE_FILE_READER_H_ -#include #include #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_ diff --git a/util/file/file_reader_test.cc b/util/file/file_reader_test.cc new file mode 100644 index 00000000..7c168bb0 --- /dev/null +++ b/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 + +#include +#include + +#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(data), size); + } + + MOCK_METHOD2(ReadInt, FileOperationResult(uintptr_t, size_t)); + + // FileReaderInterface: + FileOperationResult Read(void* data, size_t size) { + return ReadInt(reinterpret_cast(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::max(); + constexpr size_t increment = std::numeric_limits::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::max(); + constexpr size_t increment = + std::numeric_limits::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 diff --git a/util/file/file_writer.cc b/util/file/file_writer.cc index d22a1a3b..a8d92566 100644 --- a/util/file/file_writer.cc +++ b/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* 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 diff --git a/util/file/file_writer.h b/util/file/file_writer.h index 007e5f3a..ed261ec5 100644 --- a/util/file/file_writer.h +++ b/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* 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_ diff --git a/util/mach/child_port_handshake.cc b/util/mach/child_port_handshake.cc index 51aad905..e3624d70 100644 --- a/util/mach/child_port_handshake.cc +++ b/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( - client_read_fd, &service_name_length, sizeof(service_name_length))) { + if (!LoggingReadFileExactly( + 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; } diff --git a/util/mach/exception_ports_test.cc b/util/mach/exception_ports_test.cc index 37c7d118..f3d40029 100644 --- a/util/mach/exception_ports_test.cc +++ b/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(); diff --git a/util/mach/mach_message_server_test.cc b/util/mach/mach_message_server_test.cc index a2d2eb1f..2c4c8e1d 100644 --- a/util/mach/mach_message_server_test.cc +++ b/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); } } diff --git a/util/net/http_transport_test.cc b/util/net/http_transport_test.cc index 8dfa68a4..cb5c4f1f 100644 --- a/util/net/http_transport_test.cc +++ b/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. diff --git a/util/posix/process_info.h b/util/posix/process_info.h index 1aa23d8c..5e1a6108 100644 --- a/util/posix/process_info.h +++ b/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. - bool Is64Bit() const; + //! \brief Determines the target process’ bitness. + //! + //! \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 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_; diff --git a/util/posix/process_info_linux.cc b/util/posix/process_info_linux.cc new file mode 100644 index 00000000..bd4202a8 --- /dev/null +++ b/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 +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#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 +bool AdvancePastNumber(const char** input, T* value) { + size_t length = 0; + if (std::numeric_limits::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(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 ProcessInfo::SupplementaryGroups() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return supplementary_groups_; +} + +std::set ProcessInfo::AllGroups() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + std::set 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 = ®buf; + iov.iov_len = sizeof(regbuf); + if (ptrace(PTRACE_GETREGSET, + pid_, + reinterpret_cast(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(&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(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, ¤t_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* 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 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 diff --git a/util/posix/process_info_mac.cc b/util/posix/process_info_mac.cc index 17d205a6..4a1fc58a 100644 --- a/util/posix/process_info_mac.cc +++ b/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* argv) const { diff --git a/util/posix/process_info_test.cc b/util/posix/process_info_test.cc index c07b1ef7..6a1a132b 100644 --- a/util/posix/process_info_test.cc +++ b/util/posix/process_info_test.cc @@ -14,32 +14,27 @@ #include "util/posix/process_info.h" +#include #include -#include #include +#include #include #include #include -#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 -#endif - namespace crashpad { namespace test { namespace { -void TestSelfProcess(const ProcessInfo& process_info) { - EXPECT_EQ(getpid(), process_info.ProcessID()); - EXPECT_EQ(getppid(), process_info.ParentProcessID()); - +void TestProcessSelfOrClone(const ProcessInfo& process_info) { // 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 argv; ASSERT_TRUE(process_info.Arguments(&argv)); - // gtest argv processing scrambles argv, but it leaves argc and argv[0] - // intact, so test those. + const std::vector& expect_argv = GetMainArguments(); -#if defined(OS_MACOSX) - int expect_argc = *_NSGetArgc(); - char** expect_argv = *_NSGetArgv(); -#elif defined(OS_LINUX) || defined(OS_ANDROID) - std::vector expect_arg_vector; - { - base::ScopedFILE cmdline(fopen("/proc/self/cmdline", "re")); - ASSERT_NE(nullptr, cmdline.get()) << ErrnoMessage("fopen"); + // expect_argv always contains the initial view of the arguments at the time + // the program was invoked. argv may contain this view, or it may contain the + // current view of arguments after gtest argv processing. argv may be a subset + // of expect_argv. + // + // gtest argv processing always leaves argv[0] intact, so this can be checked + // directly. + ASSERT_FALSE(expect_argv.empty()); + ASSERT_FALSE(argv.empty()); + EXPECT_EQ(expect_argv[0], argv[0]); - int expect_arg_char; - std::string expect_arg_string; - while ((expect_arg_char = fgetc(cmdline.get())) != EOF) { - if (expect_arg_char != '\0') { - expect_arg_string.append(1, expect_arg_char); - } else { - expect_arg_vector.push_back(expect_arg_string); - expect_arg_string.clear(); - } - } - ASSERT_EQ(0, ferror(cmdline.get())) << ErrnoMessage("fgetc"); - ASSERT_TRUE(expect_arg_string.empty()); + EXPECT_LE(argv.size(), expect_argv.size()); + + // Everything else in argv should have a match in expect_argv too, but things + // may have moved around. + for (size_t arg_index = 1; arg_index < argv.size(); ++arg_index) { + const std::string& arg = argv[arg_index]; + SCOPED_TRACE( + base::StringPrintf("arg_index %zu, arg %s", arg_index, arg.c_str())); + EXPECT_NE(expect_argv.end(), std::find(argv.begin(), argv.end(), arg)); } - - std::vector 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(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 diff --git a/util/thread/worker_thread_test.cc b/util/thread/worker_thread_test.cc index 0cbee187..044734c8 100644 --- a/util/thread/worker_thread_test.cc +++ b/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_) - semaphore_.Signal(); + if (work_count_ < waiting_for_count_) { + 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()); diff --git a/util/util.gyp b/util/util.gyp index f1f004c7..4890012a 100644 --- a/util/util.gyp +++ b/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$'], + ], + }], + ], }, ], } diff --git a/util/util_test.gyp b/util/util_test.gyp index a33e474e..495c5e61 100644 --- a/util/util_test.gyp +++ b/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$'], + ] + }], ], }, ], diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 70955c82..fdc159c0 100644 --- a/util/win/exception_handler_server.cc +++ b/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) { diff --git a/util/win/exception_handler_server_test.cc b/util/win/exception_handler_server_test.cc index e94b91d5..e9f865be 100644 --- a/util/win/exception_handler_server_test.cc +++ b/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; } diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index e377a6ee..dfa6564d 100644 --- a/util/win/process_info_test.cc +++ b/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())); diff --git a/util/win/scoped_process_suspend_test.cc b/util/win/scoped_process_suspend_test.cc index c968f710..adddfe27 100644 --- a/util/win/scoped_process_suspend_test.cc +++ b/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; }