Skip to content

Commit b9637b0

Browse files
committed
[MTE] Print cause and alloc/dealloc traces to logcat.
This information clearly meets the bar for being dumped to logcat. If we omit the info, we may confuse the user into thinking that it's not available at all, especially if it's their first time seeing an MTE report. This also adds some functionality to the integration testing library to pull logcat messages and scan them to make sure the contents are in both places. Fixes: 187881237 Test: atest debuggerd_test # on QEMU w/ MTE. Change-Id: Icc17ea45bda7628331cc4812eaad3bc5c949b7a7 Merged-In: Icc17ea45bda7628331cc4812eaad3bc5c949b7a7
1 parent d04966a commit b9637b0

File tree

2 files changed

+74
-43
lines changed

2 files changed

+74
-43
lines changed

debuggerd/debuggerd_test.cpp

Lines changed: 68 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
#include <android-base/test_utils.h>
5252
#include <android-base/unique_fd.h>
5353
#include <cutils/sockets.h>
54+
#include <gmock/gmock.h>
5455
#include <gtest/gtest.h>
5556

5657
#include <libminijail.h>
@@ -65,6 +66,7 @@ using namespace std::chrono_literals;
6566

6667
using android::base::SendFileDescriptors;
6768
using android::base::unique_fd;
69+
using ::testing::HasSubstr;
6870

6971
#if defined(__LP64__)
7072
#define ARCH_SUFFIX "64"
@@ -307,6 +309,19 @@ static void ConsumeFd(unique_fd fd, std::string* output) {
307309
*output = std::move(result);
308310
}
309311

312+
class LogcatCollector {
313+
public:
314+
LogcatCollector() { system("logcat -c"); }
315+
316+
void Collect(std::string* output) {
317+
FILE* cmd_stdout = popen("logcat -d '*:S DEBUG'", "r");
318+
ASSERT_NE(cmd_stdout, nullptr);
319+
unique_fd tmp_fd(TEMP_FAILURE_RETRY(dup(fileno(cmd_stdout))));
320+
ConsumeFd(std::move(tmp_fd), output);
321+
pclose(cmd_stdout);
322+
}
323+
};
324+
310325
TEST_F(CrasherTest, smoke) {
311326
int intercept_result;
312327
unique_fd output_fd;
@@ -441,6 +456,7 @@ TEST_P(GwpAsanCrasherTest, gwp_asan_uaf) {
441456
}
442457

443458
GwpAsanTestParameters params = GetParam();
459+
LogcatCollector logcat_collector;
444460

445461
int intercept_result;
446462
unique_fd output_fd;
@@ -460,17 +476,18 @@ TEST_P(GwpAsanCrasherTest, gwp_asan_uaf) {
460476

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

463-
std::string result;
464-
ConsumeFd(std::move(output_fd), &result);
479+
std::vector<std::string> log_sources(2);
480+
ConsumeFd(std::move(output_fd), &log_sources[0]);
481+
logcat_collector.Collect(&log_sources[1]);
465482

466-
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 2 \(SEGV_ACCERR\))");
467-
ASSERT_MATCH(result, R"(Cause: \[GWP-ASan\]: )" + params.cause_needle);
468-
if (params.free_before_access) {
469-
ASSERT_MATCH(result, R"(deallocated by thread .*
470-
#00 pc)");
483+
for (const auto& result : log_sources) {
484+
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 2 \(SEGV_ACCERR\))");
485+
ASSERT_MATCH(result, R"(Cause: \[GWP-ASan\]: )" + params.cause_needle);
486+
if (params.free_before_access) {
487+
ASSERT_MATCH(result, R"(deallocated by thread .*\n.*#00 pc)");
488+
}
489+
ASSERT_MATCH(result, R"((^|\s)allocated by thread .*\n.*#00 pc)");
471490
}
472-
ASSERT_MATCH(result, R"(allocated by thread .*
473-
#00 pc)");
474491
}
475492

476493
struct SizeParamCrasherTest : CrasherTest, testing::WithParamInterface<size_t> {};
@@ -488,6 +505,8 @@ TEST_P(SizeParamCrasherTest, mte_uaf) {
488505
return;
489506
}
490507

508+
LogcatCollector logcat_collector;
509+
491510
int intercept_result;
492511
unique_fd output_fd;
493512
StartProcess([&]() {
@@ -504,16 +523,17 @@ TEST_P(SizeParamCrasherTest, mte_uaf) {
504523

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

507-
std::string result;
508-
ConsumeFd(std::move(output_fd), &result);
526+
std::vector<std::string> log_sources(2);
527+
ConsumeFd(std::move(output_fd), &log_sources[0]);
528+
logcat_collector.Collect(&log_sources[1]);
509529

510-
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
511-
ASSERT_MATCH(result, R"(Cause: \[MTE\]: Use After Free, 0 bytes into a )" +
512-
std::to_string(GetParam()) + R"(-byte allocation)");
513-
ASSERT_MATCH(result, R"(deallocated by thread .*
514-
#00 pc)");
515-
ASSERT_MATCH(result, R"(allocated by thread .*
516-
#00 pc)");
530+
for (const auto& result : log_sources) {
531+
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
532+
ASSERT_MATCH(result, R"(Cause: \[MTE\]: Use After Free, 0 bytes into a )" +
533+
std::to_string(GetParam()) + R"(-byte allocation)");
534+
ASSERT_MATCH(result, R"(deallocated by thread .*?\n.*#00 pc)");
535+
ASSERT_MATCH(result, R"((^|\s)allocated by thread .*?\n.*#00 pc)");
536+
}
517537
#else
518538
GTEST_SKIP() << "Requires aarch64";
519539
#endif
@@ -557,6 +577,7 @@ TEST_P(SizeParamCrasherTest, mte_overflow) {
557577
GTEST_SKIP() << "Requires MTE";
558578
}
559579

580+
LogcatCollector logcat_collector;
560581
int intercept_result;
561582
unique_fd output_fd;
562583
StartProcess([&]() {
@@ -572,14 +593,16 @@ TEST_P(SizeParamCrasherTest, mte_overflow) {
572593

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

575-
std::string result;
576-
ConsumeFd(std::move(output_fd), &result);
596+
std::vector<std::string> log_sources(2);
597+
ConsumeFd(std::move(output_fd), &log_sources[0]);
598+
logcat_collector.Collect(&log_sources[1]);
577599

578-
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
579-
ASSERT_MATCH(result, R"(Cause: \[MTE\]: Buffer Overflow, 0 bytes right of a )" +
580-
std::to_string(GetParam()) + R"(-byte allocation)");
581-
ASSERT_MATCH(result, R"(allocated by thread .*
582-
#00 pc)");
600+
for (const auto& result : log_sources) {
601+
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
602+
ASSERT_MATCH(result, R"(Cause: \[MTE\]: Buffer Overflow, 0 bytes right of a )" +
603+
std::to_string(GetParam()) + R"(-byte allocation)");
604+
ASSERT_MATCH(result, R"((^|\s)allocated by thread .*?\n.*#00 pc)");
605+
}
583606
#else
584607
GTEST_SKIP() << "Requires aarch64";
585608
#endif
@@ -612,7 +635,7 @@ TEST_P(SizeParamCrasherTest, mte_underflow) {
612635
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 9 \(SEGV_MTESERR\))");
613636
ASSERT_MATCH(result, R"(Cause: \[MTE\]: Buffer Underflow, 4 bytes left of a )" +
614637
std::to_string(GetParam()) + R"(-byte allocation)");
615-
ASSERT_MATCH(result, R"(allocated by thread .*
638+
ASSERT_MATCH(result, R"((^|\s)allocated by thread .*
616639
#00 pc)");
617640
#else
618641
GTEST_SKIP() << "Requires aarch64";
@@ -625,6 +648,8 @@ TEST_F(CrasherTest, mte_multiple_causes) {
625648
GTEST_SKIP() << "Requires MTE";
626649
}
627650

651+
LogcatCollector logcat_collector;
652+
628653
int intercept_result;
629654
unique_fd output_fd;
630655
StartProcess([]() {
@@ -657,17 +682,23 @@ TEST_F(CrasherTest, mte_multiple_causes) {
657682

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

660-
std::string result;
661-
ConsumeFd(std::move(output_fd), &result);
662-
663-
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
664-
ASSERT_MATCH(
665-
result,
666-
R"(Note: multiple potential causes for this crash were detected, listing them in decreasing order of probability.)");
667-
668-
// Adjacent untracked allocations may cause us to see the wrong underflow here (or only
669-
// overflows), so we can't match explicitly for an underflow message.
670-
ASSERT_MATCH(result, R"(Cause: \[MTE\]: Buffer Overflow, 0 bytes right of a 16-byte allocation)");
685+
std::vector<std::string> log_sources(2);
686+
ConsumeFd(std::move(output_fd), &log_sources[0]);
687+
logcat_collector.Collect(&log_sources[1]);
688+
689+
for (const auto& result : log_sources) {
690+
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
691+
ASSERT_THAT(result, HasSubstr("Note: multiple potential causes for this crash were detected, "
692+
"listing them in decreasing order of probability."));
693+
// Adjacent untracked allocations may cause us to see the wrong underflow here (or only
694+
// overflows), so we can't match explicitly for an underflow message.
695+
ASSERT_MATCH(result,
696+
R"(Cause: \[MTE\]: Buffer Overflow, 0 bytes right of a 16-byte allocation)");
697+
// Ensure there's at least two allocation traces (one for each cause).
698+
ASSERT_MATCH(
699+
result,
700+
R"((^|\s)allocated by thread .*?\n.*#00 pc(.|\n)*?(^|\s)allocated by thread .*?\n.*#00 pc)");
701+
}
671702
#else
672703
GTEST_SKIP() << "Requires aarch64";
673704
#endif

debuggerd/libdebuggerd/tombstone_proto_to_text.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -272,29 +272,29 @@ static void print_main_thread(CallbackType callback, const Tombstone& tombstone,
272272

273273
if (tombstone.causes_size() > 1) {
274274
CBS("");
275-
CBS("Note: multiple potential causes for this crash were detected, listing them in decreasing "
275+
CBL("Note: multiple potential causes for this crash were detected, listing them in decreasing "
276276
"order of probability.");
277277
}
278278

279279
for (const Cause& cause : tombstone.causes()) {
280280
if (tombstone.causes_size() > 1) {
281281
CBS("");
282-
CBS("Cause: %s", cause.human_readable().c_str());
282+
CBL("Cause: %s", cause.human_readable().c_str());
283283
}
284284

285285
if (cause.has_memory_error() && cause.memory_error().has_heap()) {
286286
const HeapObject& heap_object = cause.memory_error().heap();
287287

288288
if (heap_object.deallocation_backtrace_size() != 0) {
289289
CBS("");
290-
CBS("deallocated by thread %" PRIu64 ":", heap_object.deallocation_tid());
291-
print_backtrace(callback, tombstone, heap_object.deallocation_backtrace(), false);
290+
CBL("deallocated by thread %" PRIu64 ":", heap_object.deallocation_tid());
291+
print_backtrace(callback, tombstone, heap_object.deallocation_backtrace(), true);
292292
}
293293

294294
if (heap_object.allocation_backtrace_size() != 0) {
295295
CBS("");
296-
CBS("allocated by thread %" PRIu64 ":", heap_object.allocation_tid());
297-
print_backtrace(callback, tombstone, heap_object.allocation_backtrace(), false);
296+
CBL("allocated by thread %" PRIu64 ":", heap_object.allocation_tid());
297+
print_backtrace(callback, tombstone, heap_object.allocation_backtrace(), true);
298298
}
299299
}
300300
}

0 commit comments

Comments
 (0)