meta: update 2023-05-03

.github/workflows/build.yml

@@ -26,7 +26,7 @@ jobs:
 
       - name: Build crashpad
         run: |
-          cmake -B cmake-build
+          cmake -B cmake-build -D CRASHPAD_BUILD_TOOLS=On
           cmake --build cmake-build --parallel
 
       - name: Build crashpad with client-side stack traces

AUTHORS

@@ -13,3 +13,4 @@ Vewd Software AS
 LG Electronics, Inc.
 MIPS Technologies, Inc.
 Darshan Sen <raisinten@gmail.com>
+Ho Cheung <uioptt24@gmail.com>

client/annotation_test.cc

@@ -161,8 +161,9 @@ TEST_F(Annotation, BaseAnnotationShouldNotSupportSpinGuard) {
       crashpad::Annotation::Type::kString, "no-spin-guard", buffer);
   EXPECT_EQ(annotation.concurrent_access_guard_mode(),
             crashpad::Annotation::ConcurrentAccessGuardMode::kUnguarded);
-#ifdef NDEBUG
-  // This fails a DCHECK() in debug builds, so only test it for NDEBUG builds.
+#if !DCHECK_IS_ON()
+  // This fails a DCHECK() in debug builds, so only test it when DCHECK()
+  // is not on.
   EXPECT_EQ(std::nullopt, annotation.TryCreateScopedSpinGuard(0));
 #endif
 }

client/crashpad_client_linux_test.cc

@@ -225,6 +225,9 @@ void ValidateDump(const StartHandlerForSelfTestOptions& options,
 #if defined(COMPILER_GCC)
 __attribute__((noinline))
 #endif
+#if defined(__clang__)
+__attribute__((optnone))
+#endif
 int RecurseInfinitely(int* ptr) {
   int buf[1 << 20];
   return *ptr + RecurseInfinitely(buf);

client/ios_handler/exception_processor.mm

@@ -38,6 +38,7 @@
 #include <sys/types.h>
 #include <unwind.h>
 
+#include <atomic>
 #include <exception>
 #include <type_traits>
 #include <typeinfo>
@@ -218,7 +219,7 @@ id HandleUncaughtException(NativeCPUContext* cpu_context, id exception) {
   // preprocessor didn't catch anything, so pass the frames or just the context
   // to the exception_delegate.
   void FinalizeUncaughtNSException(id exception) {
-    if ([last_exception_ isEqual:exception] &&
+    if (last_exception() == exception &&
         !last_handled_intermediate_dump_.empty() &&
         exception_delegate_->MoveIntermediateDumpAtPathToPending(
             last_handled_intermediate_dump_)) {
@@ -227,8 +228,11 @@ void FinalizeUncaughtNSException(id exception) {
     }
 
     std::string name, reason;
-    SetNSExceptionAnnotations(exception, name, reason);
-    NSArray<NSNumber*>* address_array = [exception callStackReturnAddresses];
+    NSArray<NSNumber*>* address_array = nil;
+    if ([exception isKindOfClass:[NSException class]]) {
+      SetNSExceptionAnnotations(exception, name, reason);
+      address_array = [exception callStackReturnAddresses];
+    }
 
     if ([address_array count] > 0) {
       static StringAnnotation<256> name_key("UncaughtNSException");
@@ -258,11 +262,8 @@ id MaybeCallNextPreprocessor(id exception) {
   // Restore the objc_setExceptionPreprocessor and NSUncaughtExceptionHandler.
   void Uninstall();
 
-  NSException* last_exception() { return last_exception_; }
-  void set_last_exception(NSException* exception) {
-    [last_exception_ release];
-    last_exception_ = [exception retain];
-  }
+  void* last_exception() { return last_exception_; }
+  void set_last_exception(void* exception) { last_exception_ = exception; }
 
  private:
   ExceptionPreprocessorState() = default;
@@ -272,9 +273,11 @@ void set_last_exception(NSException* exception) {
   // HANDLE_UNCAUGHT_NSEXCEPTION.
   base::FilePath last_handled_intermediate_dump_;
 
-  // Recorded last NSException in case the exception is caught and thrown again
-  // (without using objc_exception_rethrow.)
-  NSException* last_exception_ = nil;
+  // Recorded last NSException pointer in case the exception is caught and
+  // thrown again (without using objc_exception_rethrow) as an
+  // unsafe_unretained reference. Stored as a void* as the only safe
+  // operation is pointer comparison.
+  std::atomic<void*> last_exception_ = nil;
 
   ObjcExceptionDelegate* exception_delegate_ = nullptr;
   objc_exception_preprocessor next_preprocessor_ = nullptr;
@@ -291,7 +294,9 @@ static __attribute__((noinline)) id HANDLE_UNCAUGHT_NSEXCEPTION(
     id exception,
     const char* sinkhole) {
   std::string name, reason;
-  SetNSExceptionAnnotations(exception, name, reason);
+  if ([exception isKindOfClass:[NSException class]]) {
+    SetNSExceptionAnnotations(exception, name, reason);
+  }
   LOG(WARNING) << "Handling Objective-C exception name: " << name
                << " reason: " << reason << " with sinkhole: " << sinkhole;
   NativeCPUContext cpu_context{};
@@ -326,7 +331,7 @@ id ObjcExceptionPreprocessor(id exception) {
   // ignore it.
   ExceptionPreprocessorState* preprocessor_state =
       ExceptionPreprocessorState::Get();
-  if ([preprocessor_state->last_exception() isEqual:exception]) {
+  if (preprocessor_state->last_exception() == exception) {
     return preprocessor_state->MaybeCallNextPreprocessor(exception);
   }
   preprocessor_state->set_last_exception(exception);
@@ -339,9 +344,14 @@ id ObjcExceptionPreprocessor(id exception) {
   static StringAnnotation<1024> lastexception_bt("lastexception_bt");
   auto* key = seen_first_exception ? &lastexception : &firstexception;
   auto* bt_key = seen_first_exception ? &lastexception_bt : &firstexception_bt;
-  NSString* value = [NSString
-      stringWithFormat:@"%@ reason %@", [exception name], [exception reason]];
-  key->Set(base::SysNSStringToUTF8(value));
+
+  if ([exception isKindOfClass:[NSException class]]) {
+    NSString* value = [NSString
+        stringWithFormat:@"%@ reason %@", [exception name], [exception reason]];
+    key->Set(base::SysNSStringToUTF8(value));
+  } else {
+    key->Set(base::SysNSStringToUTF8([exception description]));
+  }
 
   // This exception preprocessor runs prior to the one in libobjc, which sets
   // the -[NSException callStackReturnAddresses].

client/ios_handler/in_process_handler.cc

@@ -19,7 +19,6 @@
 
 #include <algorithm>
 
-#include "base/cxx17_backports.h"
 #include "base/logging.h"
 #include "client/ios_handler/in_process_intermediate_dump_handler.h"
 #include "client/prune_crash_reports.h"

client/ring_buffer_annotation_load_test_main.cc

@@ -315,9 +315,11 @@ class RingBufferAnnotationSnapshot final {
       int next_value;
       base::StringPiece str(reinterpret_cast<const char*>(&bytes[0]),
                             bytes.size());
-      if (!HexStringToInt(str, &next_value)) {
-        fprintf(
-            stderr, "Couldn't parse value: [%s]\n", str.as_string().c_str());
+      if (!base::HexStringToInt(str, &next_value)) {
+        fprintf(stderr,
+                "Couldn't parse value: [%.*s]\n",
+                base::checked_cast<int>(bytes.size()),
+                bytes.data());
         abort();
       }
       if (value == std::numeric_limits<int>::max()) {

handler/BUILD.gn

@@ -49,6 +49,10 @@ static_library("handler") {
       "linux/cros_crash_report_exception_handler.cc",
       "linux/cros_crash_report_exception_handler.h",
     ]
+    # TODO(https://crbug.com/1420445): Remove this config when M115 branches.
+    configs += [
+      "../build:crashpad_is_in_chromium",
+    ]
   }
 
   if (crashpad_is_win) {

handler/linux/cros_crash_report_exception_handler.cc

@@ -17,6 +17,10 @@
 #include <vector>
 
 #include "base/logging.h"
+#include "base/strings/stringprintf.h"
+#if defined(CRASHPAD_IS_IN_CHROMIUM)
+#include "base/system/sys_info.h"
+#endif // CRASHPAD_IS_IN_CHROMIUM
 #include "client/settings.h"
 #include "handler/linux/capture_snapshot.h"
 #include "handler/minidump_to_upload_parameters.h"
@@ -246,6 +250,29 @@ bool CrosCrashReportExceptionHandler::HandleExceptionWithConnection(
   // crash_reporter needs to know the pid and uid of the crashing process.
   std::vector<std::string> argv({"/sbin/crash_reporter"});
 
+#if defined(CRASHPAD_IS_IN_CHROMIUM)
+  int32_t major_version = 0, minor_version = 0, bugfix_version = 0;
+  base::SysInfo::OperatingSystemVersionNumbers(
+      &major_version, &minor_version, &bugfix_version);
+  // The version on which https://crrev.com/c/4265753 landed.
+  constexpr int32_t kFixedVersion = 15363;
+  // TODO(https://crbug.com/1420445): Remove this check (and the
+  // CRASHPAD_IS_IN_CHROMIUM defines) when M115 branches.
+  // (Lacros is guaranteed not to be more than 2 milestones ahead of ash, and
+  // M113 on ash has the relevant crash_reporter change.)
+  if (major_version >= kFixedVersion) {
+    // Used to distinguish between non-fatal and fatal crashes.
+    const ExceptionSnapshot* const exception_snapshot = snapshot->Exception();
+    if (exception_snapshot) {
+      // convert to int32, since crashpad uses -1 as a signal for non-fatal
+      // crashes.
+      argv.push_back(base::StringPrintf(
+          "--chrome_signal=%d",
+          static_cast<int32_t>(exception_snapshot->Exception())));
+    }
+  }
+#endif // CRASHPAD_IS_IN_CHROMIUM
+
   argv.push_back("--chrome_memfd=" + std::to_string(file_writer.fd()));
 
   const pid_t pid = process_snapshot->ProcessID();

snapshot/BUILD.gn

@@ -493,6 +493,7 @@ bundle_data("snapshot_test_ios_data") {
     "ios/testdata/crash-1fa088dda0adb41459d063078a0f384a0bb8eefa",
     "ios/testdata/crash-5726011582644224",
     "ios/testdata/crash-6605504629637120",
+    "ios/testdata/crash-c44acfcbccd8c7a8",
   ]
 
   outputs = [ "{{bundle_resources_dir}}/crashpad_test_data/" +

snapshot/ios/exception_snapshot_ios_intermediate_dump.cc

@@ -175,18 +175,21 @@ bool ExceptionSnapshotIOSIntermediateDump::InitializeFromMachException(
     const std::vector<uint8_t>& bytes = code_dump->bytes();
     const mach_exception_data_type_t* code =
         reinterpret_cast<const mach_exception_data_type_t*>(bytes.data());
-    if (bytes.size() == 0 || !code) {
+    if (bytes.size() == 0 ||
+        bytes.size() % sizeof(mach_exception_data_type_t) != 0 || !code) {
       LOG(ERROR) << "Invalid mach exception code.";
     } else {
-      // TODO: rationalize with the macOS implementation.
       mach_msg_type_number_t code_count =
           bytes.size() / sizeof(mach_exception_data_type_t);
       for (mach_msg_type_number_t code_index = 0; code_index < code_count;
            ++code_index) {
         codes_.push_back(code[code_index]);
       }
+      DCHECK_GE(code_count, 1u);
       exception_info_ = code[0];
-      exception_address_ = code[1];
+      if (code_count >= 2) {
+        exception_address_ = code[1];
+      }
     }
   }
 

snapshot/ios/module_snapshot_ios_intermediate_dump.cc

@@ -51,10 +51,13 @@ bool ModuleSnapshotIOSIntermediateDump::Initialize(
   GetDataStringFromMap(image_data, Key::kName, &name_);
   GetDataValueFromMap(image_data, Key::kAddress, &address_);
   GetDataValueFromMap(image_data, Key::kSize, &size_);
-  GetDataValueFromMap(image_data, Key::kSourceVersion, &source_version_);
   GetDataValueFromMap(image_data, Key::kFileType, &filetype_);
 
   // These keys are often missing.
+  GetDataValueFromMap(image_data,
+                      Key::kSourceVersion,
+                      &source_version_,
+                      LogMissingDataValueFromMap::kDontLogIfMissing);
   GetDataValueFromMap(image_data,
                       Key::kTimestamp,
                       &timestamp_,

snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc

@@ -16,7 +16,8 @@
 
 #include <mach-o/loader.h>
 
-#include "base/cxx17_backports.h"
+#include <algorithm>
+
 #include "base/files/scoped_file.h"
 #include "base/posix/eintr_wrapper.h"
 #include "build/build_config.h"
@@ -760,6 +761,11 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, FuzzTestCases) {
       FILE_PATH_LITERAL("snapshot/ios/testdata/crash-6605504629637120"));
   crashpad::internal::ProcessSnapshotIOSIntermediateDump process_snapshot3;
   EXPECT_FALSE(process_snapshot3.InitializeWithFilePath(fuzz_path, {}));
+
+  fuzz_path = TestPaths::TestDataRoot().Append(
+      FILE_PATH_LITERAL("snapshot/ios/testdata/crash-c44acfcbccd8c7a8"));
+  crashpad::internal::ProcessSnapshotIOSIntermediateDump process_snapshot4;
+  EXPECT_TRUE(process_snapshot4.InitializeWithFilePath(fuzz_path, {}));
 }
 
 }  // namespace

snapshot/linux/system_snapshot_linux.cc

@@ -62,8 +62,8 @@ bool ReadCPUsOnline(uint32_t* first_cpu, uint8_t* cpu_count) {
     std::string left, right;
     if (SplitStringFirst(range, '-', &left, &right)) {
       unsigned int start, end;
-      if (!StringToUint(base::StringPiece(left), &start) ||
-          !StringToUint(base::StringPiece(right), &end) || end <= start) {
+      if (!base::StringToUint(base::StringPiece(left), &start) ||
+          !base::StringToUint(base::StringPiece(right), &end) || end <= start) {
         LOG(ERROR) << "format error: " << range;
         return false;
       }
@@ -78,7 +78,7 @@ bool ReadCPUsOnline(uint32_t* first_cpu, uint8_t* cpu_count) {
       }
     } else {
       unsigned int cpuno;
-      if (!StringToUint(base::StringPiece(range), &cpuno)) {
+      if (!base::StringToUint(base::StringPiece(range), &cpuno)) {
         LOG(ERROR) << "format error";
         return false;
       }
@@ -399,13 +399,13 @@ void SystemSnapshotLinux::ReadKernelVersion(const std::string& version_string) {
     return;
   }
 
-  if (!StringToInt(base::StringPiece(versions[0]), &os_version_major_)) {
+  if (!base::StringToInt(base::StringPiece(versions[0]), &os_version_major_)) {
     LOG(WARNING) << "no kernel version";
     return;
   }
   DCHECK_GE(os_version_major_, 3);
 
-  if (!StringToInt(base::StringPiece(versions[1]), &os_version_minor_)) {
+  if (!base::StringToInt(base::StringPiece(versions[1]), &os_version_minor_)) {
     LOG(WARNING) << "no major revision";
     return;
   }
@@ -415,8 +415,8 @@ void SystemSnapshotLinux::ReadKernelVersion(const std::string& version_string) {
   if (minor_rev_end == std::string::npos) {
     minor_rev_end = versions[2].size();
   }
-  if (!StringToInt(base::StringPiece(versions[2].c_str(), minor_rev_end),
-                   &os_version_bugfix_)) {
+  if (!base::StringToInt(base::StringPiece(versions[2].c_str(), minor_rev_end),
+                         &os_version_bugfix_)) {
     LOG(WARNING) << "no minor revision";
     return;
   }

snapshot/minidump/thread_snapshot_minidump.cc

@@ -17,7 +17,8 @@
 #include <stddef.h>
 #include <string.h>
 
-#include "base/cxx17_backports.h"
+#include <algorithm>
+
 #include "minidump/minidump_context.h"
 
 namespace crashpad {

snapshot/sanitized/memory_snapshot_sanitized.cc

@@ -16,6 +16,8 @@
 
 #include <string.h>
 
+#include "util/linux/pac_helper.h"
+
 namespace crashpad {
 namespace internal {
 
@@ -62,8 +64,9 @@ class MemorySanitizer : public MemorySnapshot::Delegate {
     auto words =
         reinterpret_cast<Pointer*>(static_cast<char*>(data) + aligned_offset);
     for (size_t index = 0; index < word_count; ++index) {
-      if (words[index] > MemorySnapshotSanitized::kSmallWordMax &&
-          !ranges_->Contains(words[index])) {
+      auto word = StripPACBits(words[index]);
+      if (word > MemorySnapshotSanitized::kSmallWordMax &&
+          !ranges_->Contains(word)) {
         words[index] = defaced;
       }
     }

snapshot/sanitized/process_snapshot_sanitized.cc

@@ -17,6 +17,7 @@
 #include <stdint.h>
 
 #include "snapshot/cpu_context.h"
+#include "util/linux/pac_helper.h"
 #include "util/numeric/safe_assignment.h"
 
 namespace crashpad {
@@ -61,7 +62,8 @@ class StackReferencesAddressRange : public MemorySnapshot::Delegate {
                                             aligned_sp_offset);
     size_t word_count = (size - aligned_sp_offset) / sizeof(Pointer);
     for (size_t index = 0; index < word_count; ++index) {
-      if (words[index] >= low_ && words[index] < high_) {
+      auto word = StripPACBits(words[index]);
+      if (word >= low_ && word < high_) {
         return true;
       }
     }

snapshot/win/exception_snapshot_win.cc

@@ -14,6 +14,8 @@
 
 #include "snapshot/win/exception_snapshot_win.h"
 
+#include <algorithm>
+
 #include "base/logging.h"
 #include "snapshot/capture_memory.h"
 #include "snapshot/memory_snapshot.h"
@@ -261,8 +263,12 @@ bool ExceptionSnapshotWin::InitializeFromExceptionPointers(
     exception_code_ = first_record.ExceptionCode;
     exception_flags_ = first_record.ExceptionFlags;
     exception_address_ = first_record.ExceptionAddress;
-    for (DWORD i = 0; i < first_record.NumberParameters; ++i)
+
+    const DWORD number_parameters = std::min<DWORD>(
+        first_record.NumberParameters, EXCEPTION_MAXIMUM_PARAMETERS);
+    for (DWORD i = 0; i < number_parameters; ++i) {
       codes_.push_back(first_record.ExceptionInformation[i]);
+    }
     if (first_record.ExceptionRecord) {
       // https://crashpad.chromium.org/bug/43
       LOG(WARNING) << "dropping chained ExceptionRecord";