From ae7d8a9ba461134ae8eb7f7a86dfc02bb41a85d6 Mon Sep 17 00:00:00 2001 From: Justin Cohen Date: Fri, 15 Jul 2022 14:36:38 -0400 Subject: [PATCH] ios: Use fewer vm_reads when iterating modules. Rather than vm_reading each individual module load_command, load all of the commands at once. This saves nearly 200ms on an iPhone 12 Pro. Change-Id: I06f56c3ecbdf74f78759648ea62bcccd027f304c Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3764242 Reviewed-by: Mark Mentovai Commit-Queue: Justin Cohen --- .../in_process_intermediate_dump_handler.cc | 121 ++++++++---------- .../in_process_intermediate_dump_handler.h | 10 +- 2 files changed, 60 insertions(+), 71 deletions(-) diff --git a/client/ios_handler/in_process_intermediate_dump_handler.cc b/client/ios_handler/in_process_intermediate_dump_handler.cc index 6b431581..93128931 100644 --- a/client/ios_handler/in_process_intermediate_dump_handler.cc +++ b/client/ios_handler/in_process_intermediate_dump_handler.cc @@ -1053,68 +1053,63 @@ void InProcessIntermediateDumpHandler::WriteModuleInfoAtAddress( return; } - const load_command* command_ptr = reinterpret_cast( - reinterpret_cast(address) + 1); + const load_command* unsafe_command_ptr = + reinterpret_cast( + reinterpret_cast(address) + 1); - ScopedVMRead command; - if (!command.Read(command_ptr)) { - CRASHPAD_RAW_LOG("Invalid module command"); + // Rather than using an individual ScopedVMRead for each load_command, load + // the entire block of commands at once. + ScopedVMRead all_commands; + if (!all_commands.Read(unsafe_command_ptr, header->sizeofcmds)) { + CRASHPAD_RAW_LOG("Unable to read module load_commands."); return; } + // All the *_vm_read_ptr variables in the load_command loop below have been + // vm_read in `all_commands` above, and may be dereferenced without additional + // ScopedVMReads. + const load_command* command_vm_read_ptr = + reinterpret_cast(all_commands.get()); + // Make sure that the basic load command structure doesn’t overflow the // space allotted for load commands, as well as iterating through ncmds. vm_size_t slide = 0; for (uint32_t cmd_index = 0, cumulative_cmd_size = 0; - cmd_index <= header->ncmds && cumulative_cmd_size < header->sizeofcmds; - ++cmd_index, cumulative_cmd_size += command->cmdsize) { - if (command->cmd == LC_SEGMENT_64) { - ScopedVMRead segment; - if (!segment.Read(command_ptr)) { - CRASHPAD_RAW_LOG("Invalid LC_SEGMENT_64 segment"); - return; - } - const segment_command_64* segment_ptr = - reinterpret_cast(command_ptr); - if (strcmp(segment->segname, SEG_TEXT) == 0) { - WriteProperty(writer, IntermediateDumpKey::kSize, &segment->vmsize); - slide = address - segment->vmaddr; - } else if (strcmp(segment->segname, SEG_DATA) == 0) { - WriteDataSegmentAnnotations(writer, segment_ptr, slide); - } - } else if (command->cmd == LC_ID_DYLIB) { - ScopedVMRead dylib; - if (!dylib.Read(command_ptr)) { - CRASHPAD_RAW_LOG("Invalid LC_ID_DYLIB segment"); - return; + cmd_index < header->ncmds && cumulative_cmd_size < header->sizeofcmds; + ++cmd_index) { + if (command_vm_read_ptr->cmd == LC_SEGMENT_64) { + const segment_command_64* segment_vm_read_ptr = + reinterpret_cast(command_vm_read_ptr); + if (strcmp(segment_vm_read_ptr->segname, SEG_TEXT) == 0) { + WriteProperty( + writer, IntermediateDumpKey::kSize, &segment_vm_read_ptr->vmsize); + slide = address - segment_vm_read_ptr->vmaddr; + } else if (strcmp(segment_vm_read_ptr->segname, SEG_DATA) == 0) { + WriteDataSegmentAnnotations(writer, segment_vm_read_ptr, slide); } + } else if (command_vm_read_ptr->cmd == LC_ID_DYLIB) { + const dylib_command* dylib_vm_read_ptr = + reinterpret_cast(command_vm_read_ptr); WriteProperty(writer, IntermediateDumpKey::kDylibCurrentVersion, - &dylib->dylib.current_version); - } else if (command->cmd == LC_SOURCE_VERSION) { - ScopedVMRead source_version; - if (!source_version.Read(command_ptr)) { - CRASHPAD_RAW_LOG("Invalid LC_SOURCE_VERSION segment"); - return; - } + &dylib_vm_read_ptr->dylib.current_version); + } else if (command_vm_read_ptr->cmd == LC_SOURCE_VERSION) { + const source_version_command* source_version_vm_read_ptr = + reinterpret_cast(command_vm_read_ptr); WriteProperty(writer, IntermediateDumpKey::kSourceVersion, - &source_version->version); - } else if (command->cmd == LC_UUID) { - ScopedVMRead uuid; - if (!uuid.Read(command_ptr)) { - CRASHPAD_RAW_LOG("Invalid LC_UUID segment"); - return; - } - WriteProperty(writer, IntermediateDumpKey::kUUID, &uuid->uuid); + &source_version_vm_read_ptr->version); + } else if (command_vm_read_ptr->cmd == LC_UUID) { + const uuid_command* uuid_vm_read_ptr = + reinterpret_cast(command_vm_read_ptr); + WriteProperty( + writer, IntermediateDumpKey::kUUID, &uuid_vm_read_ptr->uuid); } - command_ptr = reinterpret_cast( - reinterpret_cast(command_ptr) + command->cmdsize); - if (!command.Read(command_ptr)) { - CRASHPAD_RAW_LOG("Invalid module command"); - return; - } + cumulative_cmd_size += command_vm_read_ptr->cmdsize; + command_vm_read_ptr = reinterpret_cast( + reinterpret_cast(command_vm_read_ptr) + + command_vm_read_ptr->cmdsize); } WriteProperty(writer, IntermediateDumpKey::kFileType, &header->filetype); @@ -1122,40 +1117,32 @@ void InProcessIntermediateDumpHandler::WriteModuleInfoAtAddress( void InProcessIntermediateDumpHandler::WriteDataSegmentAnnotations( IOSIntermediateDumpWriter* writer, - const segment_command_64* segment_ptr, + const segment_command_64* segment_vm_read_ptr, vm_size_t slide) { - ScopedVMRead segment; - if (!segment.Read(segment_ptr)) { - CRASHPAD_RAW_LOG("Unable to read SEG_DATA."); - return; - } - const section_64* section_ptr = reinterpret_cast( - reinterpret_cast(segment_ptr) + sizeof(segment_command_64)); - for (uint32_t sect_index = 0; sect_index <= segment->nsects; ++sect_index) { - ScopedVMRead section; - if (!section.Read(section_ptr)) { - CRASHPAD_RAW_LOG("Unable to read SEG_DATA section."); - return; - } - if (strcmp(section->sectname, "crashpad_info") == 0) { + const section_64* section_vm_read_ptr = reinterpret_cast( + reinterpret_cast(segment_vm_read_ptr) + + sizeof(segment_command_64)); + for (uint32_t sect_index = 0; sect_index <= segment_vm_read_ptr->nsects; + ++sect_index) { + if (strcmp(section_vm_read_ptr->sectname, "crashpad_info") == 0) { ScopedVMRead crashpad_info; - if (crashpad_info.Read(section->addr + slide) && + if (crashpad_info.Read(section_vm_read_ptr->addr + slide) && crashpad_info->size() == sizeof(CrashpadInfo) && crashpad_info->signature() == CrashpadInfo::kSignature && crashpad_info->version() == 1) { WriteCrashpadAnnotationsList(writer, crashpad_info.get()); WriteCrashpadSimpleAnnotationsDictionary(writer, crashpad_info.get()); } - } else if (strcmp(section->sectname, "__crash_info") == 0) { + } else if (strcmp(section_vm_read_ptr->sectname, "__crash_info") == 0) { ScopedVMRead crash_info; - if (!crash_info.Read(section->addr + slide) || + if (!crash_info.Read(section_vm_read_ptr->addr + slide) || (crash_info->version != 4 && crash_info->version != 5)) { continue; } WriteAppleCrashReporterAnnotations(writer, crash_info.get()); } - section_ptr = reinterpret_cast( - reinterpret_cast(section_ptr) + sizeof(section_64)); + section_vm_read_ptr = reinterpret_cast( + reinterpret_cast(section_vm_read_ptr) + sizeof(section_64)); } } diff --git a/client/ios_handler/in_process_intermediate_dump_handler.h b/client/ios_handler/in_process_intermediate_dump_handler.h index f6db28d5..49a520dc 100644 --- a/client/ios_handler/in_process_intermediate_dump_handler.h +++ b/client/ios_handler/in_process_intermediate_dump_handler.h @@ -139,10 +139,12 @@ class InProcessIntermediateDumpHandler final { bool is_dyld); //! \brief Extract and write Apple crashreporter_annotations_t data and - //! Crashpad annotations. - static void WriteDataSegmentAnnotations(IOSIntermediateDumpWriter* writer, - const segment_command_64* segment_ptr, - vm_size_t slide); + //! Crashpad annotations. Note that \a segment_vm_read_ptr has already + //! been read via vm_read and may be dereferenced without a ScopedVMRead. + static void WriteDataSegmentAnnotations( + IOSIntermediateDumpWriter* writer, + const segment_command_64* segment_vm_read_ptr, + vm_size_t slide); //! \brief Write Crashpad annotations list. static void WriteCrashpadAnnotationsList(IOSIntermediateDumpWriter* writer,