diff --git a/client/ios_handler/in_process_intermediate_dump_handler.cc b/client/ios_handler/in_process_intermediate_dump_handler.cc index 82980583..06eaccee 100644 --- a/client/ios_handler/in_process_intermediate_dump_handler.cc +++ b/client/ios_handler/in_process_intermediate_dump_handler.cc @@ -22,12 +22,15 @@ #include #include +#include #include "build/build_config.h" #include "snapshot/snapshot_constants.h" #include "util/ios/ios_intermediate_dump_writer.h" #include "util/ios/raw_logging.h" +#include "util/ios/scoped_vm_map.h" #include "util/ios/scoped_vm_read.h" +#include "util/synchronization/scoped_spin_guard.h" namespace crashpad { namespace internal { @@ -1163,6 +1166,13 @@ void InProcessIntermediateDumpHandler::WriteCrashpadAnnotationsList( IOSIntermediateDumpWriter::ScopedArray annotations_array( writer, IntermediateDumpKey::kAnnotationObjects); ScopedVMRead current; + + // Use vm_read() to ensure that the linked-list AnnotationList head (which is + // a dummy node of type kInvalid) is valid and copy its memory into a + // newly-allocated buffer. + // + // In the case where the pointer has been clobbered or the memory range is not + // readable, skip reading all the Annotations. if (!current.Read(annotation_list->head())) { CRASHPAD_RAW_LOG("Unable to read annotation"); return; @@ -1173,6 +1183,12 @@ void InProcessIntermediateDumpHandler::WriteCrashpadAnnotationsList( index < kMaxNumberOfAnnotations; ++index) { ScopedVMRead node; + + // Like above, use vm_read() to ensure that the node in the linked list is + // valid and copy its memory into a newly-allocated buffer. + // + // In the case where the pointer has been clobbered or the memory range is + // not readable, skip reading this and all further Annotations. if (!node.Read(current->link_node())) { CRASHPAD_RAW_LOG("Unable to read annotation"); return; @@ -1187,6 +1203,36 @@ void InProcessIntermediateDumpHandler::WriteCrashpadAnnotationsList( continue; } + // For Annotations which support guarding reads from concurrent writes, map + // their memory read-write using vm_remap(), then declare a ScopedSpinGuard + // which lives for the duration of the read. + ScopedVMMap mapped_node; + std::optional annotation_guard; + if (node->concurrent_access_guard_mode() == + Annotation::ConcurrentAccessGuardMode::kScopedSpinGuard) { + constexpr vm_prot_t kDesiredProtection = VM_PROT_WRITE | VM_PROT_READ; + if (!mapped_node.Map(node.get()) || + (mapped_node.CurrentProtection() & kDesiredProtection) != + kDesiredProtection) { + CRASHPAD_RAW_LOG("Unable to map annotation"); + + // Skip this annotation rather than giving up entirely, since the linked + // node should still be valid. + continue; + } + + // TODO(https://crbug.com/crashpad/438): Pass down a `params` object into + // this method to optionally enable a timeout here. + constexpr uint64_t kTimeoutNanoseconds = 0; + annotation_guard = + mapped_node->TryCreateScopedSpinGuard(kTimeoutNanoseconds); + if (!annotation_guard) { + // This is expected if the process is writing to the Annotation, so + // don't log here and skip the annotation. + continue; + } + } + IOSIntermediateDumpWriter::ScopedArrayMap annotation_map(writer); WritePropertyCString(writer, IntermediateDumpKey::kAnnotationName, diff --git a/test/ios/BUILD.gn b/test/ios/BUILD.gn index 1987b782..c17f461c 100644 --- a/test/ios/BUILD.gn +++ b/test/ios/BUILD.gn @@ -70,6 +70,7 @@ source_set("xcuitests") { deps = [ "../../build:ios_enable_arc", "../../build:ios_xctest", + "../../client:common", "../../test/ios/host:app_shared_sources", "../../third_party/edo", "../../util", diff --git a/test/ios/crash_type_xctest.mm b/test/ios/crash_type_xctest.mm index e95760c7..8e32f11f 100644 --- a/test/ios/crash_type_xctest.mm +++ b/test/ios/crash_type_xctest.mm @@ -15,8 +15,11 @@ #import #include +#include + #import "Service/Sources/EDOClientService.h" #include "build/build_config.h" +#include "client/length_delimited_ring_buffer.h" #import "test/ios/host/cptest_shared_object.h" #include "util/mach/exception_types.h" #include "util/mach/mach_extensions.h" @@ -322,6 +325,31 @@ isEqualToString:@"same-name 3"]); XCTAssertTrue([[dict[@"objects"][2] valueForKeyPath:@"#TEST# one"] isEqualToString:@"moocow"]); + // Ensure `ring_buffer` is present but not `busy_ring_buffer`. + XCTAssertEqual(1u, [dict[@"ringbuffers"] count]); + NSData* ringBufferNSData = + [dict[@"ringbuffers"][0] valueForKeyPath:@"#TEST# ring_buffer"]; + crashpad::RingBufferData ringBufferData; + XCTAssertTrue(ringBufferData.DeserializeFromBuffer(ringBufferNSData.bytes, + ringBufferNSData.length)); + crashpad::LengthDelimitedRingBufferReader reader(ringBufferData); + + std::vector ringBufferEntry; + XCTAssertTrue(reader.Pop(ringBufferEntry)); + NSString* firstEntry = [[NSString alloc] initWithBytes:ringBufferEntry.data() + length:ringBufferEntry.size() + encoding:NSUTF8StringEncoding]; + XCTAssertEqualObjects(firstEntry, @"hello"); + ringBufferEntry.clear(); + + XCTAssertTrue(reader.Pop(ringBufferEntry)); + NSString* secondEntry = [[NSString alloc] initWithBytes:ringBufferEntry.data() + length:ringBufferEntry.size() + encoding:NSUTF8StringEncoding]; + XCTAssertEqualObjects(secondEntry, @"goodbye"); + ringBufferEntry.clear(); + + XCTAssertFalse(reader.Pop(ringBufferEntry)); } - (void)testDumpWithoutCrash { diff --git a/test/ios/host/cptest_application_delegate.mm b/test/ios/host/cptest_application_delegate.mm index 0af106e7..c2ee1949 100644 --- a/test/ios/host/cptest_application_delegate.mm +++ b/test/ios/host/cptest_application_delegate.mm @@ -38,6 +38,7 @@ #include "client/crash_report_database.h" #include "client/crashpad_client.h" #include "client/crashpad_info.h" +#include "client/ring_buffer_annotation.h" #include "client/simple_string_dictionary.h" #include "client/simulate_crash.h" #include "snapshot/minidump/process_snapshot_minidump.h" @@ -58,6 +59,9 @@ using Report = crashpad::CrashReportDatabase::Report; namespace { +constexpr crashpad::Annotation::Type kRingBufferType = + crashpad::Annotation::UserDefinedType(42); + base::FilePath GetDatabaseDir() { base::FilePath database_dir([NSFileManager.defaultManager URLsForDirectory:NSDocumentDirectory @@ -251,7 +255,8 @@ UIWindow* GetAnyWindow() { NSDictionary* dict = @{ @"simplemap" : [@{} mutableCopy], @"vector" : [@[] mutableCopy], - @"objects" : [@[] mutableCopy] + @"objects" : [@[] mutableCopy], + @"ringbuffers" : [@[] mutableCopy], }; for (const auto* module : process_snapshot->Modules()) { for (const auto& kv : module->AnnotationsSimpleMap()) { @@ -262,14 +267,18 @@ UIWindow* GetAnyWindow() { [dict[@"vector"] addObject:@(annotation.c_str())]; } for (const auto& annotation : module->AnnotationObjects()) { - if (annotation.type != + if (annotation.type == static_cast(crashpad::Annotation::Type::kString)) { - continue; + std::string value( + reinterpret_cast(annotation.value.data()), + annotation.value.size()); + [dict[@"objects"] + addObject:@{@(annotation.name.c_str()) : @(value.c_str())}]; + } else if (annotation.type == static_cast(kRingBufferType)) { + NSData* data = [NSData dataWithBytes:annotation.value.data() + length:annotation.value.size()]; + [dict[@"ringbuffers"] addObject:@{@(annotation.name.c_str()) : data}]; } - std::string value(reinterpret_cast(annotation.value.data()), - annotation.value.size()); - [dict[@"objects"] - addObject:@{@(annotation.name.c_str()) : @(value.c_str())}]; } } return [dict passByValue]; @@ -418,12 +427,23 @@ UIWindow* GetAnyWindow() { "#TEST# same-name"}; static crashpad::StringAnnotation<32> test_annotation_four{ "#TEST# same-name"}; + static crashpad::RingBufferAnnotation<32> test_ring_buffer_annotation( + kRingBufferType, "#TEST# ring_buffer"); + static crashpad::RingBufferAnnotation<32> test_busy_ring_buffer_annotation( + kRingBufferType, "#TEST# busy_ring_buffer"); test_annotation_one.Set("moocow"); test_annotation_two.Set("this will be cleared"); test_annotation_three.Set("same-name 3"); test_annotation_four.Set("same-name 4"); test_annotation_two.Clear(); + test_ring_buffer_annotation.Push("hello", 5); + test_ring_buffer_annotation.Push("goodbye", 7); + test_busy_ring_buffer_annotation.Push("busy", 4); + // Take the scoped spin guard on `test_busy_ring_buffer_annotation` to mimic + // an in-flight `Push()` so its contents are not included in the dump. + auto guard = test_busy_ring_buffer_annotation.TryCreateScopedSpinGuard( + /*timeout_nanos=*/0); abort(); }