GWP-ASan: Add crash handler PartitionAlloc support

Teach the crash handler to look for crash keys for either malloc or
partitionalloc's allocator information and to include allocator
information with a crash report.

Bug: 956824
Change-Id: Iad99346aeea263c1effbedf8483fa6c26cd5e578
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1598478
Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657402}

components/gwp_asan/crash_handler/crash.proto

@@ -23,6 +23,11 @@ message Crash {
     FREE_INVALID_ADDRESS = 5;
   }
 
+  enum Allocator {
+    MALLOC = 0;
+    PARTITIONALLOC = 1;
+  }
+
   message AllocationInfo {
     repeated uint64 stack_trace = 1 [packed = true];
     optional uint64 thread_id = 2;
@@ -52,4 +57,6 @@ message Crash {
   // Report an internal GWP-ASan error encountered in the crash handler while
   // analyzing this crash.
   optional string internal_error = 10;
+
+  optional Allocator allocator = 11;
 }

components/gwp_asan/crash_handler/crash_analyzer.cc

@@ -33,9 +33,12 @@ using GetMetadataReturnType = AllocatorState::GetMetadataReturnType;
 bool CrashAnalyzer::GetExceptionInfo(
     const crashpad::ProcessSnapshot& process_snapshot,
     gwp_asan::Crash* proto) {
-  crashpad::VMAddress gpa_ptr = GetAllocatorAddress(process_snapshot);
-  // If the annotation wasn't present, GWP-ASan wasn't enabled.
-  if (!gpa_ptr)
+  crashpad::VMAddress malloc_key =
+      GetAllocatorAddress(process_snapshot, kMallocCrashKey);
+  crashpad::VMAddress partitionalloc_key =
+      GetAllocatorAddress(process_snapshot, kPartitionAllocCrashKey);
+  // If the annotations weren't present, GWP-ASan wasn't enabled.
+  if (!malloc_key && !partitionalloc_key)
     return false;
 
   const crashpad::ExceptionSnapshot* exception = process_snapshot.Exception();
@@ -65,15 +68,31 @@ bool CrashAnalyzer::GetExceptionInfo(
     return false;
   }
 
-  return AnalyzeCrashedAllocator(*process_snapshot.Memory(), *exception,
-                                 gpa_ptr, proto);
+  if (malloc_key) {
+    if (AnalyzeCrashedAllocator(*process_snapshot.Memory(), *exception,
+                                malloc_key, proto)) {
+      proto->set_allocator(Crash_Allocator_MALLOC);
+      return true;
+    }
+  }
+
+  if (partitionalloc_key) {
+    if (AnalyzeCrashedAllocator(*process_snapshot.Memory(), *exception,
+                                partitionalloc_key, proto)) {
+      proto->set_allocator(Crash_Allocator_PARTITIONALLOC);
+      return true;
+    }
+  }
+
+  return false;
 }
 
 crashpad::VMAddress CrashAnalyzer::GetAllocatorAddress(
-    const crashpad::ProcessSnapshot& process_snapshot) {
+    const crashpad::ProcessSnapshot& process_snapshot,
+    const char* annotation_name) {
   for (auto* module : process_snapshot.Modules()) {
     for (auto annotation : module->AnnotationObjects()) {
-      if (annotation.name != kMallocCrashKey)
+      if (annotation.name != annotation_name)
         continue;
 
       if (annotation.type !=

components/gwp_asan/crash_handler/crash_analyzer.h

@@ -79,9 +79,10 @@ class CrashAnalyzer {
       const crashpad::ExceptionSnapshot& exception);
 
   // If the allocator annotation is present in the given snapshot, then return
-  // the address for the GuardedPageAllocator in the crashing process.
+  // the address for the AllocatorState in the crashing process.
   static crashpad::VMAddress GetAllocatorAddress(
-      const crashpad::ProcessSnapshot& process_snapshot);
+      const crashpad::ProcessSnapshot& process_snapshot,
+      const char* annotation_name);
 
   // This method implements the underlying logic for GetExceptionInfo(). It
   // analyzes the AllocatorState of the crashing process, if the exception is

components/gwp_asan/crash_handler/crash_handler.cc

@@ -73,6 +73,17 @@ const char* ErrorToString(Crash_ErrorType type) {
   }
 }
 
+const char* AllocatorToString(Crash_Allocator allocator) {
+  switch (allocator) {
+    case Crash::MALLOC:
+      return "malloc";
+    case Crash::PARTITIONALLOC:
+      return "partitionalloc";
+    default:
+      return "unexpected allocator type";
+  }
+}
+
 std::unique_ptr<crashpad::MinidumpUserExtensionStreamDataSource>
 HandleException(const crashpad::ProcessSnapshot& snapshot) {
   gwp_asan::Crash proto;
@@ -85,7 +96,8 @@ HandleException(const crashpad::ProcessSnapshot& snapshot) {
     LOG(ERROR) << "Detected GWP-ASan crash with missing metadata.";
   } else {
     LOG(ERROR) << "Detected GWP-ASan crash for allocation at 0x" << std::hex
-               << proto.allocation_address() << std::dec << " of type "
+               << proto.allocation_address() << std::dec << " ("
+               << AllocatorToString(proto.allocator()) << ") of type "
                << ErrorToString(proto.error_type());
   }
 

components/gwp_asan/crash_handler/crash_handler_unittest.cc

@@ -84,10 +84,21 @@ MULTIPROCESS_TEST_MAIN(CrashingProcess) {
   CHECK(!directory.empty());
   std::string test_name = cmd_line->GetSwitchValueASCII("test-name");
   CHECK(!test_name.empty());
+  std::string allocator = cmd_line->GetSwitchValueASCII("allocator");
+
+  const char* annotation_name;
+  if (allocator == "malloc") {
+    annotation_name = kMallocCrashKey;
+  } else if (allocator == "partitionalloc") {
+    annotation_name = kPartitionAllocCrashKey;
+  } else {
+    LOG(ERROR) << "Unknown allocator";
+    return kSuccess;
+  }
 
   std::string gpa_addr = gpa->GetCrashKey();
   static crashpad::Annotation gpa_annotation(
-      crashpad::Annotation::Type::kString, kMallocCrashKey,
+      crashpad::Annotation::Type::kString, annotation_name,
       const_cast<char*>(gpa_addr.c_str()));
   gpa_annotation.SetSize(gpa_addr.size());
 
@@ -159,16 +170,25 @@ MULTIPROCESS_TEST_MAIN(CrashingProcess) {
   return kSuccess;
 }
 
-class CrashHandlerTest : public base::MultiProcessTest {
+class CrashHandlerTest : public base::MultiProcessTest,
+                         public testing::WithParamInterface<const char*> {
  protected:
+  CrashHandlerTest() : allocator_(GetParam()) {}
+
   // Launch a child process and wait for it to crash. Set |gwp_asan_found_| if a
   // GWP-ASan data was found and if so, read it into |proto_|.
   void SetUp() final {
     base::ScopedTempDir database_dir;
     ASSERT_TRUE(database_dir.CreateUniqueTempDir());
-    ASSERT_TRUE(runTestProcess(
-        database_dir.GetPath(),
-        ::testing::UnitTest::GetInstance()->current_test_info()->name()));
+
+    // Remove the parameterized test suffix from the test name.
+    std::string test_name(
+        testing::UnitTest::GetInstance()->current_test_info()->name());
+    size_t separator = test_name.find("/");
+    ASSERT_NE(separator, std::string::npos);
+    test_name.erase(separator);
+
+    ASSERT_TRUE(runTestProcess(database_dir.GetPath(), test_name.c_str()));
 
     bool minidump_found;
     readGwpAsanStreamFromCrash(database_dir.GetPath(), &minidump_found,
@@ -185,6 +205,7 @@ class CrashHandlerTest : public base::MultiProcessTest {
         base::GetMultiProcessTestChildBaseCommandLine();
     cmd_line.AppendSwitchPath("directory", database_dir);
     cmd_line.AppendSwitchASCII("test-name", test_name);
+    cmd_line.AppendSwitchASCII("allocator", allocator_);
 
     base::LaunchOptions options;
 #if defined(OS_WIN)
@@ -269,9 +290,18 @@ class CrashHandlerTest : public base::MultiProcessTest {
 
     EXPECT_TRUE(proto_.has_missing_metadata());
     EXPECT_FALSE(proto_.missing_metadata());
+
+    EXPECT_TRUE(proto_.has_allocator());
+    if (allocator_ == "malloc")
+      EXPECT_EQ(proto_.allocator(), Crash_Allocator_MALLOC);
+    else if (allocator_ == "partitionalloc")
+      EXPECT_EQ(proto_.allocator(), Crash_Allocator_PARTITIONALLOC);
+    else
+      ASSERT_TRUE(false) << "Unknown allocator name";
   }
 
   gwp_asan::Crash proto_;
+  std::string allocator_;
   bool gwp_asan_found_;
 };
 
@@ -282,33 +312,33 @@ class CrashHandlerTest : public base::MultiProcessTest {
 #define MAYBE_DISABLED(name) name
 #endif  // defined(ADDRESS_SANITIZER) && defined(OS_WIN)
 
-TEST_F(CrashHandlerTest, MAYBE_DISABLED(UseAfterFree)) {
+TEST_P(CrashHandlerTest, MAYBE_DISABLED(UseAfterFree)) {
   ASSERT_TRUE(gwp_asan_found_);
   checkProto(Crash_ErrorType_USE_AFTER_FREE, true);
 }
 
-TEST_F(CrashHandlerTest, MAYBE_DISABLED(DoubleFree)) {
+TEST_P(CrashHandlerTest, MAYBE_DISABLED(DoubleFree)) {
   ASSERT_TRUE(gwp_asan_found_);
   checkProto(Crash_ErrorType_DOUBLE_FREE, true);
 }
 
-TEST_F(CrashHandlerTest, MAYBE_DISABLED(Underflow)) {
+TEST_P(CrashHandlerTest, MAYBE_DISABLED(Underflow)) {
   ASSERT_TRUE(gwp_asan_found_);
   checkProto(Crash_ErrorType_BUFFER_UNDERFLOW, false);
 }
 
-TEST_F(CrashHandlerTest, MAYBE_DISABLED(Overflow)) {
+TEST_P(CrashHandlerTest, MAYBE_DISABLED(Overflow)) {
   ASSERT_TRUE(gwp_asan_found_);
   checkProto(Crash_ErrorType_BUFFER_OVERFLOW, false);
 }
 
-TEST_F(CrashHandlerTest, MAYBE_DISABLED(FreeInvalidAddress)) {
+TEST_P(CrashHandlerTest, MAYBE_DISABLED(FreeInvalidAddress)) {
   ASSERT_TRUE(gwp_asan_found_);
   checkProto(Crash_ErrorType_FREE_INVALID_ADDRESS, false);
   EXPECT_TRUE(proto_.has_free_invalid_address());
 }
 
-TEST_F(CrashHandlerTest, MAYBE_DISABLED(MissingMetadata)) {
+TEST_P(CrashHandlerTest, MAYBE_DISABLED(MissingMetadata)) {
   ASSERT_TRUE(gwp_asan_found_);
 
   ASSERT_TRUE(proto_.has_missing_metadata());
@@ -324,10 +354,14 @@ TEST_F(CrashHandlerTest, MAYBE_DISABLED(MissingMetadata)) {
   EXPECT_TRUE(proto_.has_region_size());
 }
 
-TEST_F(CrashHandlerTest, MAYBE_DISABLED(UnrelatedException)) {
+TEST_P(CrashHandlerTest, MAYBE_DISABLED(UnrelatedException)) {
   ASSERT_FALSE(gwp_asan_found_);
 }
 
+INSTANTIATE_TEST_SUITE_P(VaryAllocator,
+                         CrashHandlerTest,
+                         testing::Values("malloc", "partitionalloc"));
+
 }  // namespace
 
 }  // namespace internal