ios: Allow missing exception thread id from thread list.

It's expected that iOS intermediate dumps can be written with
missing information, but it's better to try and report as much as
possible rather than drop the incomplete minidump.

Bug: b/284959148
Change-Id: I04110b576a4ee552814234d559c9ba85db0382f0
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4582167
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

minidump/minidump_exception_writer.cc

@@ -34,13 +34,19 @@ MinidumpExceptionWriter::~MinidumpExceptionWriter() {
 
 void MinidumpExceptionWriter::InitializeFromSnapshot(
     const ExceptionSnapshot* exception_snapshot,
-    const MinidumpThreadIDMap& thread_id_map) {
+    const MinidumpThreadIDMap& thread_id_map,
+    bool allow_missing_thread_id_from_map) {
   DCHECK_EQ(state(), kStateMutable);
   DCHECK(!context_);
 
   auto thread_id_it = thread_id_map.find(exception_snapshot->ThreadID());
-  DCHECK(thread_id_it != thread_id_map.end());
-  SetThreadID(thread_id_it->second);
+  bool thread_id_missing = thread_id_it == thread_id_map.end();
+  if (allow_missing_thread_id_from_map && thread_id_missing) {
+    SetThreadID(static_cast<uint32_t>(exception_snapshot->ThreadID()));
+  } else {
+    DCHECK(!thread_id_missing);
+    SetThreadID(thread_id_it->second);
+  }
 
   SetExceptionCode(exception_snapshot->Exception());
   SetExceptionFlags(exception_snapshot->ExceptionInfo());

minidump/minidump_exception_writer.h

@@ -50,12 +50,17 @@ class MinidumpExceptionWriter final : public internal::MinidumpStreamWriter {
   //! \param[in] thread_id_map A MinidumpThreadIDMap to be consulted to
   //!     determine the 32-bit minidump thread ID to use for the thread
   //!     identified by \a exception_snapshot.
+  //! \param[in] allow_missing_thread_id_from_map Whether it is valid
+  //!     for \a exception_snapshot->ThreadID() to be absent from the
+  //!     \a thread_id_map, such as in an incomplete iOS intermediate dump. When
+  //!     false a missing thread id is considered invalid and will DCHECK.
   //!
   //! \note Valid in #kStateMutable. No mutator methods may be called before
   //!     this method, and it is not normally necessary to call any mutator
   //!     methods after this method.
   void InitializeFromSnapshot(const ExceptionSnapshot* exception_snapshot,
-                              const MinidumpThreadIDMap& thread_id_map);
+                              const MinidumpThreadIDMap& thread_id_map,
+                              bool allow_missing_thread_id_from_map);
 
   //! \brief Arranges for MINIDUMP_EXCEPTION_STREAM::ThreadContext to point to
   //!     the CPU context to be written by \a context.

minidump/minidump_exception_writer_test.cc

@@ -235,7 +235,10 @@ TEST(MinidumpExceptionWriter, InitializeFromSnapshot) {
   thread_id_map[kThreadID] = expect_exception.ThreadId;
 
   auto exception_writer = std::make_unique<MinidumpExceptionWriter>();
-  exception_writer->InitializeFromSnapshot(&exception_snapshot, thread_id_map);
+  exception_writer->InitializeFromSnapshot(
+      &exception_snapshot,
+      thread_id_map,
+      /*allow_missing_thread_id_from_map=*/false);
 
   MinidumpFileWriter minidump_file_writer;
   ASSERT_TRUE(minidump_file_writer.AddStream(std::move(exception_writer)));

minidump/minidump_file_writer.cc

@@ -18,6 +18,7 @@
 
 #include "base/check_op.h"
 #include "base/logging.h"
+#include "build/build_config.h"
 #include "minidump/minidump_crashpad_info_writer.h"
 #include "minidump/minidump_exception_writer.h"
 #include "minidump/minidump_handle_writer.h"
@@ -115,7 +116,16 @@ void MinidumpFileWriter::InitializeFromSnapshot(
   const ExceptionSnapshot* exception_snapshot = process_snapshot->Exception();
   if (exception_snapshot) {
     auto exception = std::make_unique<MinidumpExceptionWriter>();
-    exception->InitializeFromSnapshot(exception_snapshot, thread_id_map);
+#if BUILDFLAG(IS_IOS)
+    // It's expected that iOS intermediate dumps can be written with missing
+    // information, but it's better to try and report as much as possible
+    // rather than drop the incomplete minidump.
+    constexpr bool allow_missing_thread_id_from_map = true;
+#else
+    constexpr bool allow_missing_thread_id_from_map = false;
+#endif
+    exception->InitializeFromSnapshot(
+        exception_snapshot, thread_id_map, allow_missing_thread_id_from_map);
     add_stream_result = AddStream(std::move(exception));
     DCHECK(add_stream_result);
   }

snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc

@@ -775,6 +775,22 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, FuzzTestCases) {
   EXPECT_TRUE(process_snapshot4.InitializeWithFilePath(fuzz_path, {}));
 }
 
+TEST_F(ProcessSnapshotIOSIntermediateDumpTest, WriteNoThreads) {
+  {
+    IOSIntermediateDumpWriter::ScopedRootMap rootMap(writer());
+    uint8_t version = 1;
+    EXPECT_TRUE(writer()->AddProperty(Key::kVersion, &version));
+    WriteSystemInfo(writer());
+    WriteProcessInfo(writer());
+    WriteMachException(writer());
+  }
+  CloseWriter();
+  ProcessSnapshotIOSIntermediateDump process_snapshot;
+  ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), annotations()));
+  EXPECT_FALSE(IsRegularFile(path()));
+  EXPECT_TRUE(DumpSnapshot(process_snapshot));
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad