Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 773acaa1 authored by Peter Collingbourne's avatar Peter Collingbourne
Browse files

Improvements to tombstone output. 

- Use "likelihood" instead of "probability" since that has connotations
  of being less precise, and our probability ordering isn't very precise
  anyway.

- Hide the fault address with SEGV_MTEAERR because it is not available.

- Pad the fault address with leading zeroes to make it clearer which
  bits of the top byte (and any following bytes such as PAC signature
  bits) are set.

Bug: 206015287
Change-Id: I5e1e99b7f3e967c44781d8550bbd7158eb421b64
parent d0a4e710

debuggerd/debuggerd_test.cpp

+53 −13
Original line number Diff line number Diff line
@@ -340,7 +340,12 @@ TEST_F(CrasherTest, smoke) { 


  std::string result;

  ConsumeFd(std::move(output_fd), &result);

  ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0xdead)");

#ifdef __LP64__

  ASSERT_MATCH(result,

               R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x000000000000dead)");

#else

  ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x0000dead)");

#endif



  if (mte_supported()) {

    // Test that the default TAGGED_ADDR_CTRL value is set.
@@ -370,8 +375,7 @@ TEST_F(CrasherTest, tagged_fault_addr) { 


  // The address can either be tagged (new kernels) or untagged (old kernels).

  ASSERT_MATCH(

      result,

      R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr (0x100000000000dead|0xdead))");

      result, R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x[01]00000000000dead)");

}



// Marked as weak to prevent the compiler from removing the malloc in the caller. In theory, the
@@ -422,6 +426,12 @@ static void SetTagCheckingLevelSync() { 
    abort();

  }

}



static void SetTagCheckingLevelAsync() {

  if (mallopt(M_BIONIC_SET_HEAP_TAGGING_LEVEL, M_HEAP_TAGGING_LEVEL_ASYNC) == 0) {

    abort();

  }

}

#endif



// Number of iterations required to reliably guarantee a GWP-ASan crash.
@@ -653,6 +663,36 @@ TEST_P(SizeParamCrasherTest, mte_underflow) { 
#endif

}



TEST_F(CrasherTest, mte_async) {

#if defined(__aarch64__)

  if (!mte_supported()) {

    GTEST_SKIP() << "Requires MTE";

  }



  int intercept_result;

  unique_fd output_fd;

  StartProcess([&]() {

    SetTagCheckingLevelAsync();

    volatile int* p = (volatile int*)malloc(16);

    p[-1] = 42;

  });



  StartIntercept(&output_fd);

  FinishCrasher();

  AssertDeath(SIGSEGV);

  FinishIntercept(&intercept_result);



  ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";



  std::string result;

  ConsumeFd(std::move(output_fd), &result);



  ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 8 \(SEGV_MTEAERR\), fault addr --------)");

#else

  GTEST_SKIP() << "Requires aarch64";

#endif

}



TEST_F(CrasherTest, mte_multiple_causes) {

#if defined(__aarch64__)

  if (!mte_supported()) {
@@ -703,7 +743,7 @@ TEST_F(CrasherTest, mte_multiple_causes) { 
  for (const auto& result : log_sources) {

    ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");

    ASSERT_THAT(result, HasSubstr("Note: multiple potential causes for this crash were detected, "

                                  "listing them in decreasing order of probability."));

                                  "listing them in decreasing order of likelihood."));

    // Adjacent untracked allocations may cause us to see the wrong underflow here (or only

    // overflows), so we can't match explicitly for an underflow message.

    ASSERT_MATCH(result,
@@ -890,7 +930,7 @@ TEST_F(CrasherTest, LD_PRELOAD) { 


  std::string result;

  ConsumeFd(std::move(output_fd), &result);

  ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0xdead)");

  ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x0+dead)");

}



TEST_F(CrasherTest, abort) {
@@ -1940,7 +1980,7 @@ TEST_F(CrasherTest, fault_address_before_first_map) { 


  std::string result;

  ConsumeFd(std::move(output_fd), &result);

  ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x1024)");

  ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x0+1024)");



  ASSERT_MATCH(result, R"(\nmemory map \(.*\):\n)");
@@ -1970,8 +2010,8 @@ TEST_F(CrasherTest, fault_address_after_last_map) { 
  std::string result;

  ConsumeFd(std::move(output_fd), &result);



  std::string match_str = R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr )";

  match_str += android::base::StringPrintf("0x%" PRIxPTR, crash_uptr);

  std::string match_str = R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x)";

  match_str += format_full_pointer(crash_uptr);

  ASSERT_MATCH(result, match_str);



  ASSERT_MATCH(result, R"(\nmemory map \(.*\): \(fault address prefixed with --->)\n)");
@@ -2018,8 +2058,8 @@ TEST_F(CrasherTest, fault_address_between_maps) { 
  std::string result;

  ConsumeFd(std::move(output_fd), &result);



  std::string match_str = R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr )";

  match_str += android::base::StringPrintf("%p", middle_ptr);

  std::string match_str = R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x)";

  match_str += format_full_pointer(reinterpret_cast<uintptr_t>(middle_ptr));

  ASSERT_MATCH(result, match_str);



  ASSERT_MATCH(result, R"(\nmemory map \(.*\): \(fault address prefixed with --->)\n)");
@@ -2056,8 +2096,8 @@ TEST_F(CrasherTest, fault_address_in_map) { 
  std::string result;

  ConsumeFd(std::move(output_fd), &result);



  std::string match_str = R"(signal 11 \(SIGSEGV\), code 2 \(SEGV_ACCERR\), fault addr )";

  match_str += android::base::StringPrintf("%p", ptr);

  std::string match_str = R"(signal 11 \(SIGSEGV\), code 2 \(SEGV_ACCERR\), fault addr 0x)";

  match_str += format_full_pointer(reinterpret_cast<uintptr_t>(ptr));

  ASSERT_MATCH(result, match_str);



  ASSERT_MATCH(result, R"(\nmemory map \(.*\): \(fault address prefixed with --->)\n)");
@@ -2181,7 +2221,7 @@ TEST_F(CrasherTest, verify_dex_pc_with_function_name) { 
  ConsumeFd(std::move(output_fd), &result);



  // Verify the process crashed properly.

  ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x0)");

  ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 1 \(SEGV_MAPERR\), fault addr 0x0*)");



  // Now verify that the dex_pc frame includes a proper function name.

  ASSERT_MATCH(result, R"( \[anon:dex\] \(Main\.\<init\>\+2)");

debuggerd/libdebuggerd/scudo.cpp

+1 −1
Original line number Diff line number Diff line
@@ -135,7 +135,7 @@ void ScudoCrashData::DumpCause(log_t* log, unwindstack::Unwinder* unwinder) cons 
  if (error_info_.reports[1].error_type != UNKNOWN) {

    _LOG(log, logtype::HEADER,

         "\nNote: multiple potential causes for this crash were detected, listing them in "

         "decreasing order of probability.\n");

         "decreasing order of likelihood.\n");

  }



  size_t report_num = 0;

debuggerd/libdebuggerd/tombstone_proto_to_text.cpp

+3 −3
Original line number Diff line number Diff line
@@ -292,6 +292,7 @@ static void print_tag_dump(CallbackType callback, const Tombstone& tombstone) { 


static void print_main_thread(CallbackType callback, const Tombstone& tombstone,

                              const Thread& thread) {

  int word_size = pointer_width(tombstone);

  print_thread_header(callback, tombstone, thread, true);



  const Signal& signal_info = tombstone.signal_info();
@@ -307,7 +308,7 @@ static void print_main_thread(CallbackType callback, const Tombstone& tombstone, 
  } else {

    std::string fault_addr_desc;

    if (signal_info.has_fault_address()) {

      fault_addr_desc = StringPrintf("0x%" PRIx64, signal_info.fault_address());

      fault_addr_desc = StringPrintf("0x%0*" PRIx64, 2 * word_size, signal_info.fault_address());

    } else {

      fault_addr_desc = "--------";

    }
@@ -331,7 +332,7 @@ static void print_main_thread(CallbackType callback, const Tombstone& tombstone, 
  if (tombstone.causes_size() > 1) {

    CBS("");

    CBL("Note: multiple potential causes for this crash were detected, listing them in decreasing "

        "order of probability.");

        "order of likelihood.");

  }



  for (const Cause& cause : tombstone.causes()) {
@@ -369,7 +370,6 @@ static void print_main_thread(CallbackType callback, const Tombstone& tombstone, 
    return;

  }



  int word_size = pointer_width(tombstone);

  const auto format_pointer = [word_size](uint64_t ptr) -> std::string {

    if (word_size == 8) {

      uint64_t top = ptr >> 32;

debuggerd/libdebuggerd/utility.cpp

+2 −1
Original line number Diff line number Diff line
@@ -275,9 +275,10 @@ bool signal_has_si_addr(const siginfo_t* si) { 
    case SIGBUS:

    case SIGFPE:

    case SIGILL:

    case SIGSEGV:

    case SIGTRAP:

      return true;

    case SIGSEGV:

      return si->si_code != SEGV_MTEAERR;

    default:

      return false;

  }