From 4b86b277735e43f178bc04bdfedcffe2fdbdf0f8 Mon Sep 17 00:00:00 2001 From: Justin Cohen Date: Sat, 20 Nov 2021 23:03:45 -0500 Subject: [PATCH] ios: Add IOSIntermediateDumpInterface. Change IOSIntermediateDumpReader to take a new interface that can be backed by a FilePath (as it is now) or a StringFile byte array, which can be useful for tests, especially with fuzzing. Change-Id: I02a25cfb7cd204975d1bcce80201bd10944f3f2e Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3270755 Commit-Queue: Justin Cohen Reviewed-by: Mark Mentovai --- client/ios_handler/in_process_handler.cc | 2 +- client/ios_handler/in_process_handler_test.cc | 1 - ..._process_intermediate_dump_handler_test.cc | 15 +++-- .../process_snapshot_ios_intermediate_dump.cc | 24 ++++++- .../process_snapshot_ios_intermediate_dump.h | 24 +++++-- ...ess_snapshot_ios_intermediate_dump_test.cc | 23 +++---- util/BUILD.gn | 2 + util/ios/ios_intermediate_dump_interface.cc | 60 +++++++++++++++++ util/ios/ios_intermediate_dump_interface.h | 67 +++++++++++++++++++ util/ios/ios_intermediate_dump_reader.cc | 28 ++++---- util/ios/ios_intermediate_dump_reader.h | 16 +++-- util/ios/ios_intermediate_dump_reader_test.cc | 29 ++++---- 12 files changed, 229 insertions(+), 62 deletions(-) create mode 100644 util/ios/ios_intermediate_dump_interface.cc create mode 100644 util/ios/ios_intermediate_dump_interface.h diff --git a/client/ios_handler/in_process_handler.cc b/client/ios_handler/in_process_handler.cc index d57b5f29..285218fd 100644 --- a/client/ios_handler/in_process_handler.cc +++ b/client/ios_handler/in_process_handler.cc @@ -178,7 +178,7 @@ void InProcessHandler::ProcessIntermediateDump( INITIALIZATION_STATE_DCHECK_VALID(initialized_); ProcessSnapshotIOSIntermediateDump process_snapshot; - if (process_snapshot.Initialize(file, annotations)) { + if (process_snapshot.InitializeWithFilePath(file, annotations)) { SaveSnapshot(process_snapshot); } } diff --git a/client/ios_handler/in_process_handler_test.cc b/client/ios_handler/in_process_handler_test.cc index 13dd366d..d7dfc053 100644 --- a/client/ios_handler/in_process_handler_test.cc +++ b/client/ios_handler/in_process_handler_test.cc @@ -142,7 +142,6 @@ TEST_F(InProcessHandlerTest, TestPendingFileLimit) { handler().ProcessIntermediateDumps({}); VerifyRemainingFileCount(0, 0); ClearFiles(); - } } // namespace diff --git a/client/ios_handler/in_process_intermediate_dump_handler_test.cc b/client/ios_handler/in_process_intermediate_dump_handler_test.cc index 26c40c0b..b79f93f6 100644 --- a/client/ios_handler/in_process_intermediate_dump_handler_test.cc +++ b/client/ios_handler/in_process_intermediate_dump_handler_test.cc @@ -93,7 +93,7 @@ class InProcessIntermediateDumpHandlerTest : public testing::Test { TEST_F(InProcessIntermediateDumpHandlerTest, TestSystem) { WriteReport(); internal::ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), {})); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), {})); // Snpahot const SystemSnapshot* system = process_snapshot.System(); @@ -145,7 +145,8 @@ TEST_F(InProcessIntermediateDumpHandlerTest, TestAnnotations) { WriteReport(); internal::ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), {{"after_dump", "post"}})); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath( + path(), {{"after_dump", "post"}})); auto process_map = process_snapshot.AnnotationsSimpleMap(); EXPECT_EQ(process_map.size(), 2u); @@ -199,7 +200,7 @@ TEST_F(InProcessIntermediateDumpHandlerTest, TestAnnotations) { TEST_F(InProcessIntermediateDumpHandlerTest, TestThreads) { WriteReport(); internal::ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), {})); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), {})); const auto& threads = process_snapshot.Threads(); ASSERT_GT(threads.size(), 0u); @@ -217,26 +218,26 @@ TEST_F(InProcessIntermediateDumpHandlerTest, TestThreads) { TEST_F(InProcessIntermediateDumpHandlerTest, TestProcess) { WriteReport(); internal::ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), {})); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), {})); EXPECT_EQ(process_snapshot.ProcessID(), getpid()); } TEST_F(InProcessIntermediateDumpHandlerTest, TestMachException) { WriteReport(); internal::ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), {})); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), {})); } TEST_F(InProcessIntermediateDumpHandlerTest, TestSignalException) { WriteReport(); internal::ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), {})); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), {})); } TEST_F(InProcessIntermediateDumpHandlerTest, TestNSException) { WriteReport(); internal::ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), {})); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), {})); } } // namespace diff --git a/snapshot/ios/process_snapshot_ios_intermediate_dump.cc b/snapshot/ios/process_snapshot_ios_intermediate_dump.cc index f4769ff6..3f9414d9 100644 --- a/snapshot/ios/process_snapshot_ios_intermediate_dump.cc +++ b/snapshot/ios/process_snapshot_ios_intermediate_dump.cc @@ -36,14 +36,24 @@ namespace internal { using Key = internal::IntermediateDumpKey; -bool ProcessSnapshotIOSIntermediateDump::Initialize( +bool ProcessSnapshotIOSIntermediateDump::InitializeWithFilePath( const base::FilePath& dump_path, const std::map& annotations) { + IOSIntermediateDumpFilePath dump_interface; + if (!dump_interface.Initialize(dump_path)) + return false; + + return InitializeWithFileInterface(dump_interface, annotations); +} + +bool ProcessSnapshotIOSIntermediateDump::InitializeWithFileInterface( + const IOSIntermediateDumpInterface& dump_interface, + const std::map& annotations) { INITIALIZATION_STATE_SET_INITIALIZING(initialized_); annotations_simple_map_ = annotations; - if (!reader_.Initialize(dump_path)) { + if (!reader_.Initialize(dump_interface)) { return false; } @@ -165,6 +175,16 @@ bool ProcessSnapshotIOSIntermediateDump::Initialize( return true; } +void ProcessSnapshotIOSIntermediateDump::SetClientID(const UUID& client_id) { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + client_id_ = client_id; +} + +void ProcessSnapshotIOSIntermediateDump::SetReportID(const UUID& report_id) { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + report_id_ = report_id; +} + pid_t ProcessSnapshotIOSIntermediateDump::ProcessID() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return p_pid_; diff --git a/snapshot/ios/process_snapshot_ios_intermediate_dump.h b/snapshot/ios/process_snapshot_ios_intermediate_dump.h index 61312f07..fb05cdaa 100644 --- a/snapshot/ios/process_snapshot_ios_intermediate_dump.h +++ b/snapshot/ios/process_snapshot_ios_intermediate_dump.h @@ -46,24 +46,38 @@ class ProcessSnapshotIOSIntermediateDump final : public ProcessSnapshot { //! \brief Initializes the object. //! - //! \param[in] dump_path A class containing various system data points. + //! \param[in] dump_path The intermediate dump to read. + //! \param[in] annotations Process annotations to set in each crash report. //! //! \return `true` if the snapshot could be created, `false` otherwise with //! an appropriate message logged. - bool Initialize(const base::FilePath& dump_path, - const std::map& annotations); + bool InitializeWithFilePath( + const base::FilePath& dump_path, + const std::map& annotations); + + //! \brief Initializes the object. + //! + //! \param[in] dump_interface An interface corresponding to an intermediate + //! dump file. + //! \param[in] annotations Process annotations to set in each crash report. + //! + //! \return `true` if the snapshot could be created, `false` otherwise with + //! an appropriate message logged. + bool InitializeWithFileInterface( + const IOSIntermediateDumpInterface& dump_interface, + const std::map& annotations); //! On iOS, the client ID is under the control of the snapshot producer, //! which may call this method to set the client ID. If this is not done, //! ClientID() will return an identifier consisting entirely of zeroes. - void SetClientID(const UUID& client_id) { client_id_ = client_id; } + void SetClientID(const UUID& client_id); //! \brief Sets the value to be returned by ReportID(). //! //! On iOS, the crash report ID is under the control of the snapshot //! producer, which may call this method to set the report ID. If this is not //! done, ReportID() will return an identifier consisting entirely of zeroes. - void SetReportID(const UUID& report_id) { report_id_ = report_id; } + void SetReportID(const UUID& report_id); // ProcessSnapshot: pid_t ProcessID() const override; diff --git a/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc b/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc index dc3ad272..910c124d 100644 --- a/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc +++ b/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc @@ -456,14 +456,14 @@ class ProcessSnapshotIOSIntermediateDumpTest : public testing::Test { TEST_F(ProcessSnapshotIOSIntermediateDumpTest, InitializeNoFile) { const base::FilePath file; ProcessSnapshotIOSIntermediateDump process_snapshot; - EXPECT_FALSE(process_snapshot.Initialize(file, annotations())); + EXPECT_FALSE(process_snapshot.InitializeWithFilePath(file, annotations())); EXPECT_TRUE(LoggingRemoveFile(path())); EXPECT_FALSE(IsRegularFile(path())); } TEST_F(ProcessSnapshotIOSIntermediateDumpTest, InitializeEmpty) { ProcessSnapshotIOSIntermediateDump process_snapshot; - EXPECT_FALSE(process_snapshot.Initialize(path(), annotations())); + EXPECT_FALSE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); } @@ -476,7 +476,7 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, InitializeMinimumDump) { { IOSIntermediateDumpWriter::ScopedMap map(writer(), Key::kProcessInfo); } } ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), annotations())); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); EXPECT_TRUE(DumpSnapshot(process_snapshot)); } @@ -489,7 +489,7 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, MissingSystemDump) { { IOSIntermediateDumpWriter::ScopedMap map(writer(), Key::kProcessInfo); } } ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_FALSE(process_snapshot.Initialize(path(), annotations())); + ASSERT_FALSE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); } @@ -501,7 +501,7 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, MissingProcessDump) { { IOSIntermediateDumpWriter::ScopedMap map(writer(), Key::kSystemInfo); } } ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_FALSE(process_snapshot.Initialize(path(), annotations())); + ASSERT_FALSE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); } @@ -526,7 +526,7 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, EmptySignalDump) { } } ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), annotations())); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); EXPECT_TRUE(DumpSnapshot(process_snapshot)); } @@ -552,7 +552,7 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, EmptyMachDump) { } } ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), annotations())); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); EXPECT_TRUE(DumpSnapshot(process_snapshot)); } @@ -578,7 +578,7 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, EmptyExceptionDump) { } } ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), annotations())); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); EXPECT_TRUE(DumpSnapshot(process_snapshot)); } @@ -608,7 +608,7 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, EmptyUncaughtNSExceptionDump) { } } ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), annotations())); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); EXPECT_TRUE(DumpSnapshot(process_snapshot)); } @@ -625,7 +625,7 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, FullReport) { WriteMachException(writer()); } ProcessSnapshotIOSIntermediateDump process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(path(), annotations())); + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); EXPECT_TRUE(DumpSnapshot(process_snapshot)); ExpectSnapshot(process_snapshot); @@ -634,9 +634,8 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, FullReport) { TEST_F(ProcessSnapshotIOSIntermediateDumpTest, FuzzTestCases) { base::FilePath fuzz_path = TestPaths::TestDataRoot().Append(FILE_PATH_LITERAL( "snapshot/ios/testdata/crash-1fa088dda0adb41459d063078a0f384a0bb8eefa")); - crashpad::internal::ProcessSnapshotIOSIntermediateDump process_snapshot; - EXPECT_TRUE(process_snapshot.Initialize(fuzz_path, {})); + EXPECT_TRUE(process_snapshot.InitializeWithFilePath(fuzz_path, {})); EXPECT_TRUE(LoggingRemoveFile(path())); } diff --git a/util/BUILD.gn b/util/BUILD.gn index a376c0c9..7b704408 100644 --- a/util/BUILD.gn +++ b/util/BUILD.gn @@ -369,6 +369,8 @@ crashpad_static_library("util") { "ios/ios_intermediate_dump_data.cc", "ios/ios_intermediate_dump_data.h", "ios/ios_intermediate_dump_format.h", + "ios/ios_intermediate_dump_interface.cc", + "ios/ios_intermediate_dump_interface.h", "ios/ios_intermediate_dump_list.cc", "ios/ios_intermediate_dump_list.h", "ios/ios_intermediate_dump_map.cc", diff --git a/util/ios/ios_intermediate_dump_interface.cc b/util/ios/ios_intermediate_dump_interface.cc new file mode 100644 index 00000000..bc583c4b --- /dev/null +++ b/util/ios/ios_intermediate_dump_interface.cc @@ -0,0 +1,60 @@ +// Copyright 2021 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/ios/ios_intermediate_dump_interface.h" + +#include "util/file/scoped_remove_file.h" + +namespace crashpad { +namespace internal { + +bool IOSIntermediateDumpFilePath::Initialize(const base::FilePath& path) { + INITIALIZATION_STATE_SET_INITIALIZING(initialized_); + ScopedRemoveFile file_remover(path); + handle_.reset(LoggingOpenFileForRead(path)); + if (!handle_.is_valid()) + return false; + + reader_ = std::make_unique(handle_.get()); + INITIALIZATION_STATE_SET_VALID(initialized_); + return true; +} + +FileReaderInterface* IOSIntermediateDumpFilePath::FileReader() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return reader_.get(); +} + +FileOffset IOSIntermediateDumpFilePath::Size() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return LoggingFileSizeByHandle(handle_.get()); +} + +IOSIntermediateDumpByteArray::IOSIntermediateDumpByteArray(const void* data, + size_t size) { + string_file_ = std::make_unique(); + string_file_->SetString( + std::string(reinterpret_cast(data), size)); +} + +FileReaderInterface* IOSIntermediateDumpByteArray::FileReader() const { + return string_file_.get(); +} + +FileOffset IOSIntermediateDumpByteArray::Size() const { + return string_file_->string().size(); +} + +} // namespace internal +} // namespace crashpad diff --git a/util/ios/ios_intermediate_dump_interface.h b/util/ios/ios_intermediate_dump_interface.h new file mode 100644 index 00000000..6d14d2f9 --- /dev/null +++ b/util/ios/ios_intermediate_dump_interface.h @@ -0,0 +1,67 @@ +// Copyright 2021 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_IOS_IOS_INTERMEDIATE_DUMP_INTERFACE_H_ +#define CRASHPAD_UTIL_IOS_IOS_INTERMEDIATE_DUMP_INTERFACE_H_ + +#include "base/files/file_path.h" +#include "util/file/file_reader.h" +#include "util/file/filesystem.h" +#include "util/file/string_file.h" +#include "util/misc/initialization_state_dcheck.h" + +namespace crashpad { +namespace internal { + +//! \brief The base class for reading data into an IOSIntermediateDumpReader. +class IOSIntermediateDumpInterface { + public: + virtual FileReaderInterface* FileReader() const = 0; + virtual FileOffset Size() const = 0; +}; + +//! \brief An intermediate dump backed by a FilePath. FilePath is unlinked +//! immediately upon initialization to ensure files are only processed once +//! in the event a crash is introduced by this intermediate dump. +class IOSIntermediateDumpFilePath : public IOSIntermediateDumpInterface { + public: + bool Initialize(const base::FilePath& path); + + // IOSIntermediateDumpInterface: + FileReaderInterface* FileReader() const override; + FileOffset Size() const override; + + private: + ScopedFileHandle handle_; + std::unique_ptr reader_; + InitializationStateDcheck initialized_; +}; + +//! \brief An intermediate dump backed by a byte array. +class IOSIntermediateDumpByteArray : public IOSIntermediateDumpInterface { + public: + IOSIntermediateDumpByteArray(const void* data, size_t size); + + // IOSIntermediateDumpInterface + FileReaderInterface* FileReader() const override; + FileOffset Size() const override; + + private: + std::unique_ptr string_file_; +}; + +} // namespace internal +} // namespace crashpad + +#endif // CRASHPAD_UTIL_IOS_IOS_INTERMEDIATE_DUMP_INTERFACE_H_ diff --git a/util/ios/ios_intermediate_dump_reader.cc b/util/ios/ios_intermediate_dump_reader.cc index 853d92cd..0b41eab5 100644 --- a/util/ios/ios_intermediate_dump_reader.cc +++ b/util/ios/ios_intermediate_dump_reader.cc @@ -29,31 +29,35 @@ namespace crashpad { namespace internal { -bool IOSIntermediateDumpReader::Initialize(const base::FilePath& path) { - ScopedFileHandle handle(LoggingOpenFileForRead(path)); - auto reader = std::make_unique(handle.get()); +bool IOSIntermediateDumpReader::Initialize( + const IOSIntermediateDumpInterface& dump_interface) { + INITIALIZATION_STATE_SET_INITIALIZING(initialized_); - // In the event a crash is introduced by this intermediate dump, don't ever - // read a file twice. To ensure this doesn't happen, immediately unlink. - LoggingRemoveFile(path); - - // Don't initialize invalid or empty files. - FileOffset size = LoggingFileSizeByHandle(handle.get()); - if (!handle.is_valid() || size == 0) { + // Don't initialize empty files. + FileOffset size = dump_interface.Size(); + if (size == 0) { return false; } - if (!Parse(reader.get(), size)) { + if (!Parse(dump_interface.FileReader(), size)) { LOG(ERROR) << "Intermediate dump parsing failed"; + // Intentially do not return false here, as it may be possible to extract a + // useful minidump out of a partial intermediate dump. } + INITIALIZATION_STATE_SET_VALID(initialized_); return true; } +const IOSIntermediateDumpMap* IOSIntermediateDumpReader::RootMap() { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return &intermediate_dump_; +} + bool IOSIntermediateDumpReader::Parse(FileReaderInterface* reader, FileOffset file_size) { std::stack stack; - stack.push(&minidump_); + stack.push(&intermediate_dump_); using Command = IOSIntermediateDumpWriter::CommandType; using Type = IOSIntermediateDumpObject::Type; diff --git a/util/ios/ios_intermediate_dump_reader.h b/util/ios/ios_intermediate_dump_reader.h index e1c42172..513d5227 100644 --- a/util/ios/ios_intermediate_dump_reader.h +++ b/util/ios/ios_intermediate_dump_reader.h @@ -15,9 +15,9 @@ #ifndef CRASHPAD_UTIL_IOS_IOS_INTERMEDIATE_DUMP_READER_H_ #define CRASHPAD_UTIL_IOS_IOS_INTERMEDIATE_DUMP_READER_H_ -#include "base/files/file_path.h" -#include "util/file/file_reader.h" +#include "util/ios/ios_intermediate_dump_interface.h" #include "util/ios/ios_intermediate_dump_map.h" +#include "util/misc/initialization_state_dcheck.h" namespace crashpad { namespace internal { @@ -31,26 +31,28 @@ class IOSIntermediateDumpReader { IOSIntermediateDumpReader& operator=(const IOSIntermediateDumpReader&) = delete; - //! \brief Open and parses \a path, ignoring empty files. + //! \brief Open and parses \a dump_interface. //! //! Will attempt to parse the binary file, similar to a JSON file, using the //! same format used by IOSIntermediateDumpWriter, resulting in an //! IOSIntermediateDumpMap //! - //! \param[in] path The intermediate dump to read. + //! \param[in] dump_interface An interface corresponding to an intermediate + //! dump file. //! //! \return On success, returns `true`, otherwise returns `false`. Clients may //! still attempt to parse RootMap, as partial minidumps may still be //! usable. - bool Initialize(const base::FilePath& path); + bool Initialize(const IOSIntermediateDumpInterface& dump_interface); //! \brief Returns an IOSIntermediateDumpMap corresponding to the root of the //! intermediate dump. - const IOSIntermediateDumpMap* RootMap() { return &minidump_; } + const IOSIntermediateDumpMap* RootMap(); private: bool Parse(FileReaderInterface* reader, FileOffset file_size); - IOSIntermediateDumpMap minidump_; + IOSIntermediateDumpMap intermediate_dump_; + InitializationStateDcheck initialized_; }; } // namespace internal diff --git a/util/ios/ios_intermediate_dump_reader_test.cc b/util/ios/ios_intermediate_dump_reader_test.cc index 68921208..a223b441 100644 --- a/util/ios/ios_intermediate_dump_reader_test.cc +++ b/util/ios/ios_intermediate_dump_reader_test.cc @@ -48,6 +48,7 @@ class IOSIntermediateDumpReaderTest : public testing::Test { writer_ = std::make_unique(); ASSERT_TRUE(writer_->Open(path_)); ASSERT_TRUE(IsRegularFile(path_)); + dump_interface_.Initialize(path_); } void TearDown() override { @@ -59,6 +60,7 @@ class IOSIntermediateDumpReaderTest : public testing::Test { int fd() { return fd_.get(); } const base::FilePath& path() const { return path_; } + const auto& dump_interface() const { return dump_interface_; } std::unique_ptr writer_; @@ -66,22 +68,20 @@ class IOSIntermediateDumpReaderTest : public testing::Test { base::ScopedFD fd_; ScopedTempDir temp_dir_; base::FilePath path_; + internal::IOSIntermediateDumpFilePath dump_interface_; }; TEST_F(IOSIntermediateDumpReaderTest, ReadNoFile) { internal::IOSIntermediateDumpReader reader; - EXPECT_FALSE(reader.Initialize(base::FilePath())); - EXPECT_TRUE(LoggingRemoveFile(path())); + internal::IOSIntermediateDumpFilePath dump_interface; + EXPECT_FALSE(dump_interface.Initialize(base::FilePath())); EXPECT_FALSE(IsRegularFile(path())); } TEST_F(IOSIntermediateDumpReaderTest, ReadEmptyFile) { internal::IOSIntermediateDumpReader reader; - EXPECT_FALSE(reader.Initialize(path())); + EXPECT_FALSE(reader.Initialize(dump_interface())); EXPECT_FALSE(IsRegularFile(path())); - - const auto root_map = reader.RootMap(); - EXPECT_TRUE(root_map->empty()); } TEST_F(IOSIntermediateDumpReaderTest, ReadHelloWorld) { @@ -89,7 +89,7 @@ TEST_F(IOSIntermediateDumpReaderTest, ReadHelloWorld) { EXPECT_TRUE( LoggingWriteFile(fd(), hello_world.c_str(), hello_world.length())); internal::IOSIntermediateDumpReader reader; - EXPECT_TRUE(reader.Initialize(path())); + EXPECT_TRUE(reader.Initialize(dump_interface())); EXPECT_FALSE(IsRegularFile(path())); const auto root_map = reader.RootMap(); @@ -113,11 +113,10 @@ TEST_F(IOSIntermediateDumpReaderTest, FuzzTestCases) { 0x1, 0x7, 0x16}; - EXPECT_TRUE(LoggingWriteFile(fd(), &fuzz1, sizeof(fuzz1))); - internal::IOSIntermediateDumpReader reader; - EXPECT_TRUE(reader.Initialize(path())); - EXPECT_FALSE(IsRegularFile(path())); + internal::IOSIntermediateDumpByteArray dump_interface(fuzz1, sizeof(fuzz1)); + internal::IOSIntermediateDumpReader reader; + EXPECT_TRUE(reader.Initialize(dump_interface)); const auto root_map = reader.RootMap(); EXPECT_TRUE(root_map->empty()); } @@ -136,7 +135,7 @@ TEST_F(IOSIntermediateDumpReaderTest, WriteBadPropertyDataLength) { size_t value_length = 999999; EXPECT_TRUE(LoggingWriteFile(fd(), &value_length, sizeof(size_t))); EXPECT_TRUE(LoggingWriteFile(fd(), &value, sizeof(value))); - EXPECT_TRUE(reader.Initialize(path())); + EXPECT_TRUE(reader.Initialize(dump_interface())); EXPECT_FALSE(IsRegularFile(path())); const auto root_map = reader.RootMap(); @@ -159,7 +158,7 @@ TEST_F(IOSIntermediateDumpReaderTest, InvalidArrayInArray) { writer_->AddProperty(Key::kVersion, &version); } EXPECT_TRUE(writer_->Close()); - EXPECT_TRUE(reader.Initialize(path())); + EXPECT_TRUE(reader.Initialize(dump_interface())); EXPECT_FALSE(IsRegularFile(path())); const auto root_map = reader.RootMap(); @@ -181,7 +180,7 @@ TEST_F(IOSIntermediateDumpReaderTest, InvalidPropertyInArray) { writer_->AddProperty(Key::kVersion, &version); } EXPECT_TRUE(writer_->Close()); - EXPECT_TRUE(reader.Initialize(path())); + EXPECT_TRUE(reader.Initialize(dump_interface())); EXPECT_FALSE(IsRegularFile(path())); const auto root_map = reader.RootMap(); @@ -218,7 +217,7 @@ TEST_F(IOSIntermediateDumpReaderTest, ReadValidData) { } EXPECT_TRUE(writer_->Close()); - EXPECT_TRUE(reader.Initialize(path())); + EXPECT_TRUE(reader.Initialize(dump_interface())); EXPECT_FALSE(IsRegularFile(path())); auto root_map = reader.RootMap();