From c405b3e9a0600787de857742813a05f76b3b20a2 Mon Sep 17 00:00:00 2001 From: Justin Cohen Date: Thu, 13 Mar 2025 22:18:43 -0400 Subject: [PATCH] ios: Support intermediate dump extra_memory_ranges. The extra_memory_ranges stored in the intermediate dump and used by gwp-asan via the ProcessSnapshot are not needed in the minidump. If this memory were stored in the minidump with gwp-asan, this would double the size of every iOS minidump. Instead, add a second bag that is stored in the intermediate dump but not written to the minidump. Expose an iOS only ProcessSnapshot API that can be used by the gwp-asan analyzer to use the extra_memory_ranges without increasing the minidump file size. Change-Id: Ibf7b7fb89cda0a829727c02e5118dc2f6423769f Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/6351477 Reviewed-by: Joshua Peraza Commit-Queue: Justin Cohen --- client/BUILD.gn | 4 + client/crashpad_info.cc | 8 +- client/crashpad_info.h | 32 ++++++++ .../in_process_intermediate_dump_handler.cc | 39 +++++++++ .../in_process_intermediate_dump_handler.h | 5 ++ ..._process_intermediate_dump_handler_test.cc | 79 ++++++++++++++++++- snapshot/crashpad_info_size_test_module.cc | 7 ++ .../module_snapshot_ios_intermediate_dump.cc | 39 +++++++++ .../module_snapshot_ios_intermediate_dump.h | 3 + .../process_snapshot_ios_intermediate_dump.cc | 12 +++ .../process_snapshot_ios_intermediate_dump.h | 5 ++ util/ios/ios_intermediate_dump_format.h | 3 + 12 files changed, 231 insertions(+), 5 deletions(-) diff --git a/client/BUILD.gn b/client/BUILD.gn index 16838df8..0cfaaa19 100644 --- a/client/BUILD.gn +++ b/client/BUILD.gn @@ -226,6 +226,10 @@ source_set("client_test") { "../handler/win/wer:crashpad_wer_handler", ] } + + if (crashpad_is_ios) { + deps += [ "//minidump" ] + } } if (crashpad_is_linux || crashpad_is_android) { diff --git a/client/crashpad_info.cc b/client/crashpad_info.cc index 781c0e38..78a40d89 100644 --- a/client/crashpad_info.cc +++ b/client/crashpad_info.cc @@ -137,7 +137,13 @@ CrashpadInfo::CrashpadInfo() extra_memory_ranges_(nullptr), simple_annotations_(nullptr), user_data_minidump_stream_head_(nullptr), - annotations_list_(nullptr) {} + annotations_list_(nullptr) +#if BUILDFLAG(IS_IOS) + , + intermediate_dump_extra_memory_ranges_(nullptr) +#endif +{ +} UserDataMinidumpStreamHandle* CrashpadInfo::AddUserDataMinidumpStream( uint32_t stream_type, diff --git a/client/crashpad_info.h b/client/crashpad_info.h index f75fd179..b777a81f 100644 --- a/client/crashpad_info.h +++ b/client/crashpad_info.h @@ -100,6 +100,35 @@ struct CrashpadInfo { return extra_memory_ranges_; } +#if BUILDFLAG(IS_IOS) + //! \brief Sets the bag of extra memory ranges to be included in the iOS + //! intermediate dump. This memory is not included in the minidump. + //! + //! Extra memory ranges may exist in \a address_range_bag at the time that + //! this method is called, or they may be added, removed, or modified in \a + //! address_range_bag after this method is called. + //! + //! This is only supported on iOS. + //! + //! \param[in] address_range_bag A bag of address ranges. The CrashpadInfo + //! object does not take ownership of the SimpleAddressRangeBag object. + //! It is the caller’s responsibility to ensure that this pointer remains + //! valid while it is in effect for a CrashpadInfo object. + //! + //! \sa extra_memory_ranges() + void set_intermediate_dump_extra_memory_ranges( + SimpleAddressRangeBag* address_range_bag) { + intermediate_dump_extra_memory_ranges_ = address_range_bag; + } + + //! \return The simple extra memory ranges SimpleAddressRangeBag object. + //! + //! \sa set_extra_memory_ranges() + SimpleAddressRangeBag* intermediate_dump_extra_memory_ranges() const { + return intermediate_dump_extra_memory_ranges_; + } +#endif + //! \brief Sets the simple annotations dictionary. //! //! Simple annotations set on a CrashpadInfo structure are interpreted by @@ -305,6 +334,9 @@ struct CrashpadInfo { SimpleStringDictionary* simple_annotations_; // weak internal::UserDataMinidumpStreamListEntry* user_data_minidump_stream_head_; AnnotationList* annotations_list_; // weak +#if BUILDFLAG(IS_IOS) + SimpleAddressRangeBag* intermediate_dump_extra_memory_ranges_; // weak +#endif // It’s generally safe to add new fields without changing // kCrashpadInfoVersion, because readers should check size_ and ignore fields diff --git a/client/ios_handler/in_process_intermediate_dump_handler.cc b/client/ios_handler/in_process_intermediate_dump_handler.cc index d9a131c5..52c42a0f 100644 --- a/client/ios_handler/in_process_intermediate_dump_handler.cc +++ b/client/ios_handler/in_process_intermediate_dump_handler.cc @@ -1145,6 +1145,8 @@ void InProcessIntermediateDumpHandler::WriteDataSegmentAnnotations( WriteCrashpadAnnotationsList(writer, crashpad_info.get()); WriteCrashpadSimpleAnnotationsDictionary(writer, crashpad_info.get()); WriteCrashpadExtraMemoryRanges(writer, crashpad_info.get()); + WriteCrashpadIntermediateDumpExtraMemoryRanges(writer, + crashpad_info.get()); } } else if (strcmp(section_vm_read_ptr->sectname, "__crash_info") == 0) { ScopedVMRead crash_info; @@ -1288,5 +1290,42 @@ void InProcessIntermediateDumpHandler::WriteCrashpadExtraMemoryRanges( } } +void InProcessIntermediateDumpHandler:: + WriteCrashpadIntermediateDumpExtraMemoryRanges( + IOSIntermediateDumpWriter* writer, + CrashpadInfo* crashpad_info) { + if (!crashpad_info->intermediate_dump_extra_memory_ranges()) { + return; + } + + ScopedVMRead intermediate_dump_extra_memory; + if (!intermediate_dump_extra_memory.Read( + crashpad_info->intermediate_dump_extra_memory_ranges())) { + CRASHPAD_RAW_LOG( + "Unable to read intermediate dump extra memory ranges object"); + return; + } + + IOSIntermediateDumpWriter::ScopedArray module_extra_memory_regions_array( + writer, IntermediateDumpKey::kModuleIntermediateDumpExtraMemoryRegions); + + SimpleAddressRangeBag::Iterator iterator( + *(intermediate_dump_extra_memory.get())); + while (const SimpleAddressRangeBag::Entry* entry = iterator.Next()) { + const uint64_t& address = entry->base; + const uint64_t& size = entry->size; + IOSIntermediateDumpWriter::ScopedArrayMap memory_region_map(writer); + WriteProperty( + writer, + IntermediateDumpKey::kModuleIntermediateDumpExtraMemoryRegionAddress, + &address); + WritePropertyBytes( + writer, + IntermediateDumpKey::kModuleIntermediateDumpExtraMemoryRegionData, + reinterpret_cast(address), + size); + } +} + } // namespace internal } // namespace crashpad diff --git a/client/ios_handler/in_process_intermediate_dump_handler.h b/client/ios_handler/in_process_intermediate_dump_handler.h index e7bb3b63..40189a02 100644 --- a/client/ios_handler/in_process_intermediate_dump_handler.h +++ b/client/ios_handler/in_process_intermediate_dump_handler.h @@ -156,6 +156,11 @@ class InProcessIntermediateDumpHandler final { //! \brief Write Crashpad extra memory data. static void WriteCrashpadExtraMemoryRanges(IOSIntermediateDumpWriter* writer, CrashpadInfo* crashpad_info); + + //! \brief Write Crashpad intermediate dump extra memory data. + static void WriteCrashpadIntermediateDumpExtraMemoryRanges( + IOSIntermediateDumpWriter* writer, + CrashpadInfo* crashpad_info); }; } // namespace internal 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 2619d720..a97c76a0 100644 --- a/client/ios_handler/in_process_intermediate_dump_handler_test.cc +++ b/client/ios_handler/in_process_intermediate_dump_handler_test.cc @@ -25,7 +25,9 @@ #include "client/crashpad_info.h" #include "client/simple_string_dictionary.h" #include "gtest/gtest.h" +#include "minidump/minidump_file_writer.h" #include "snapshot/ios/process_snapshot_ios_intermediate_dump.h" +#include "snapshot/minidump/process_snapshot_minidump.h" #include "test/scoped_set_thread_name.h" #include "test/scoped_temp_dir.h" #include "test/test_paths.h" @@ -251,12 +253,14 @@ TEST_F(InProcessIntermediateDumpHandlerTest, TestExtraMemoryRanges) { } #endif - constexpr char kSomeExtraMemoryString[] = "extra memory range"; + // Put the string on the heap so the memory doesn't coalesce with the stack. + std::unique_ptr someExtraMemoryString( + new std::string("extra memory range")); crashpad::SimpleAddressRangeBag* ios_extra_ranges = new crashpad::SimpleAddressRangeBag(); crashpad::CrashpadInfo::GetCrashpadInfo()->set_extra_memory_ranges( ios_extra_ranges); - ios_extra_ranges->Insert((void*)kSomeExtraMemoryString, 18); + ios_extra_ranges->Insert((void*)someExtraMemoryString->c_str(), 18); WriteReportAndCloseWriter(); crashpad::CrashpadInfo::GetCrashpadInfo()->set_extra_memory_ranges(nullptr); internal::ProcessSnapshotIOSIntermediateDump process_snapshot; @@ -264,11 +268,78 @@ TEST_F(InProcessIntermediateDumpHandlerTest, TestExtraMemoryRanges) { ASSERT_EQ(process_snapshot.ExtraMemory().size(), 1LU); auto memory = process_snapshot.ExtraMemory()[0]; EXPECT_EQ(memory->Address(), - reinterpret_cast(kSomeExtraMemoryString)); + reinterpret_cast(someExtraMemoryString->c_str())); EXPECT_EQ(memory->Size(), 18LU); ReadToString delegate; ASSERT_TRUE(memory->Read(&delegate)); - EXPECT_EQ(delegate.result, kSomeExtraMemoryString); + EXPECT_EQ(delegate.result, someExtraMemoryString->c_str()); + + StringFile string_file; + MinidumpFileWriter minidump_file_writer; + minidump_file_writer.InitializeFromSnapshot(&process_snapshot); + ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); + + ProcessSnapshotMinidump process_snapshot_minidump; + EXPECT_TRUE(process_snapshot_minidump.Initialize(&string_file)); + bool found; + for (auto minidump_memory : process_snapshot_minidump.ExtraMemory()) { + if (minidump_memory->Address() == + reinterpret_cast(someExtraMemoryString->c_str()) && + minidump_memory->Size() == 18LU) { + found = true; + break; + } + } + EXPECT_TRUE(found); +} + +TEST_F(InProcessIntermediateDumpHandlerTest, + TestIntermediateDumpExtraMemoryRanges) { +#if TARGET_OS_SIMULATOR + // This test will fail on older ( someExtraMemoryString( + new std::string("extra memory range")); + crashpad::SimpleAddressRangeBag* ios_extra_ranges = + new crashpad::SimpleAddressRangeBag(); + crashpad::CrashpadInfo::GetCrashpadInfo() + ->set_intermediate_dump_extra_memory_ranges(ios_extra_ranges); + ios_extra_ranges->Insert((void*)someExtraMemoryString->c_str(), 18); + WriteReportAndCloseWriter(); + crashpad::CrashpadInfo::GetCrashpadInfo() + ->set_intermediate_dump_extra_memory_ranges(nullptr); + internal::ProcessSnapshotIOSIntermediateDump process_snapshot; + ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), {})); + ASSERT_EQ(process_snapshot.IntermediateDumpExtraMemory().size(), 1LU); + auto memory = process_snapshot.IntermediateDumpExtraMemory()[0]; + EXPECT_EQ(memory->Address(), + reinterpret_cast(someExtraMemoryString->c_str())); + EXPECT_EQ(memory->Size(), 18LU); + ReadToString delegate; + ASSERT_TRUE(memory->Read(&delegate)); + EXPECT_EQ(delegate.result, someExtraMemoryString->c_str()); + + StringFile string_file; + MinidumpFileWriter minidump_file_writer; + minidump_file_writer.InitializeFromSnapshot(&process_snapshot); + ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); + + ProcessSnapshotMinidump process_snapshot_minidump; + EXPECT_TRUE(process_snapshot_minidump.Initialize(&string_file)); + for (auto minidump_memory : process_snapshot_minidump.ExtraMemory()) { + EXPECT_FALSE( + minidump_memory->Address() == + reinterpret_cast(someExtraMemoryString->c_str()) && + minidump_memory->Size() == 18LU); + } } TEST_F(InProcessIntermediateDumpHandlerTest, TestThreads) { diff --git a/snapshot/crashpad_info_size_test_module.cc b/snapshot/crashpad_info_size_test_module.cc index 3318702f..9ec736ef 100644 --- a/snapshot/crashpad_info_size_test_module.cc +++ b/snapshot/crashpad_info_size_test_module.cc @@ -53,6 +53,9 @@ struct TestCrashpadInfo { #if !defined(CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL) void* user_data_minidump_stream_head_; void* annotations_list_; +#if BUILDFLAG(IS_IOS) + void* intermediate_dump_extra_memory_ranges_; +#endif // BUILDFLAG(IS_IOS) #endif // CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL #if defined(CRASHPAD_INFO_SIZE_TEST_MODULE_LARGE) uint8_t trailer_[64 * 1024]; @@ -95,6 +98,10 @@ TestCrashpadInfo g_test_crashpad_info = {'CPad', #if !defined(CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL) nullptr, nullptr, +#if BUILDFLAG(IS_IOS) + nullptr, +#endif // BUILDFLAG(IS_IOS) + #endif // CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL #if defined(CRASHPAD_INFO_SIZE_TEST_MODULE_LARGE) {} diff --git a/snapshot/ios/module_snapshot_ios_intermediate_dump.cc b/snapshot/ios/module_snapshot_ios_intermediate_dump.cc index 5c3b4c2a..16d2282c 100644 --- a/snapshot/ios/module_snapshot_ios_intermediate_dump.cc +++ b/snapshot/ios/module_snapshot_ios_intermediate_dump.cc @@ -154,6 +154,36 @@ bool ModuleSnapshotIOSIntermediateDump::Initialize( } } + const IOSIntermediateDumpList* intermediate_dump_extra_memory_regions_array = + image_data->GetAsList( + IntermediateDumpKey::kModuleIntermediateDumpExtraMemoryRegions); + if (intermediate_dump_extra_memory_regions_array) { + for (auto& region : *intermediate_dump_extra_memory_regions_array) { + vm_address_t address; + const IOSIntermediateDumpData* region_data = + region->GetAsData(Key::kModuleIntermediateDumpExtraMemoryRegionData); + if (!region_data) + continue; + if (GetDataValueFromMap( + region.get(), + Key::kModuleIntermediateDumpExtraMemoryRegionAddress, + &address)) { + const std::vector& bytes = region_data->bytes(); + vm_size_t data_size = bytes.size(); + if (data_size == 0) + continue; + + const vm_address_t data = + reinterpret_cast(bytes.data()); + + auto memory = + std::make_unique(); + memory->Initialize(address, data, data_size); + intermediate_dump_extra_memory_.push_back(std::move(memory)); + } + } + } + const IOSIntermediateDumpMap* crash_info_dump = image_data->GetAsMap(IntermediateDumpKey::kAnnotationsCrashInfo); if (crash_info_dump) { @@ -302,6 +332,15 @@ ModuleSnapshotIOSIntermediateDump::ExtraMemory() const { return extra_memory; } +std::vector +ModuleSnapshotIOSIntermediateDump::IntermediateDumpExtraMemory() const { + std::vector intermediate_dump_extra_memory; + for (const auto& memory : intermediate_dump_extra_memory_) { + intermediate_dump_extra_memory.push_back(memory.get()); + } + return intermediate_dump_extra_memory; +} + std::vector ModuleSnapshotIOSIntermediateDump::CustomMinidumpStreams() const { return std::vector(); diff --git a/snapshot/ios/module_snapshot_ios_intermediate_dump.h b/snapshot/ios/module_snapshot_ios_intermediate_dump.h index e53c772c..2b34d4cf 100644 --- a/snapshot/ios/module_snapshot_ios_intermediate_dump.h +++ b/snapshot/ios/module_snapshot_ios_intermediate_dump.h @@ -78,6 +78,7 @@ class ModuleSnapshotIOSIntermediateDump final : public ModuleSnapshot { // Used by ProcessSnapshot std::vector ExtraMemory() const; + std::vector IntermediateDumpExtraMemory() const; private: std::string name_; @@ -93,6 +94,8 @@ class ModuleSnapshotIOSIntermediateDump final : public ModuleSnapshot { std::vector annotation_objects_; std::vector> extra_memory_; + std::vector> + intermediate_dump_extra_memory_; InitializationStateDcheck initialized_; }; diff --git a/snapshot/ios/process_snapshot_ios_intermediate_dump.cc b/snapshot/ios/process_snapshot_ios_intermediate_dump.cc index ee98b4a2..de9caebe 100644 --- a/snapshot/ios/process_snapshot_ios_intermediate_dump.cc +++ b/snapshot/ios/process_snapshot_ios_intermediate_dump.cc @@ -313,6 +313,18 @@ ProcessSnapshotIOSIntermediateDump::ExtraMemory() const { return extra_memory; } +std::vector +ProcessSnapshotIOSIntermediateDump::IntermediateDumpExtraMemory() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + std::vector intermediate_dump_extra_memory; + for (const auto& module : modules_) { + for (const auto& memory : module->IntermediateDumpExtraMemory()) { + intermediate_dump_extra_memory.push_back(memory); + } + } + return intermediate_dump_extra_memory; +} + const ProcessMemory* ProcessSnapshotIOSIntermediateDump::Memory() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return nullptr; diff --git a/snapshot/ios/process_snapshot_ios_intermediate_dump.h b/snapshot/ios/process_snapshot_ios_intermediate_dump.h index 693b5b35..1bdedcf1 100644 --- a/snapshot/ios/process_snapshot_ios_intermediate_dump.h +++ b/snapshot/ios/process_snapshot_ios_intermediate_dump.h @@ -99,6 +99,11 @@ class ProcessSnapshotIOSIntermediateDump final : public ProcessSnapshot { std::vector ExtraMemory() const override; const ProcessMemory* Memory() const override; + // Returns ModuleSnapshotIOSIntermediateDump::IntermediateDumpExtraMemory + // stored in the intermediate dump. This is used by UserExtensionStreams for + // processing the snapshot. This memory will not be written to a minidump + std::vector IntermediateDumpExtraMemory() const; + private: // Retain the reader for the lifetime of the ProcessSnapshot so large chunks // of data do not need to be copied around (such as MemorySnapshot diff --git a/util/ios/ios_intermediate_dump_format.h b/util/ios/ios_intermediate_dump_format.h index 437bf943..32752f61 100644 --- a/util/ios/ios_intermediate_dump_format.h +++ b/util/ios/ios_intermediate_dump_format.h @@ -59,6 +59,9 @@ namespace internal { TD(kModuleExtraMemoryRegions, 3019) \ TD(kModuleExtraMemoryRegionAddress, 3020) \ TD(kModuleExtraMemoryRegionData, 3021) \ + TD(kModuleIntermediateDumpExtraMemoryRegions, 3022) \ + TD(kModuleIntermediateDumpExtraMemoryRegionAddress, 3023) \ + TD(kModuleIntermediateDumpExtraMemoryRegionData, 3024) \ TD(kProcessInfo, 4000) \ TD(kParentPID, 4001) \ TD(kPID, 4002) \