From 7a8980997d0e0dcf3f3a5d8ccf3c1d8c2840ea27 Mon Sep 17 00:00:00 2001 From: Tobias Sargeant Date: Wed, 30 Nov 2016 16:47:04 +0000 Subject: [PATCH] Do not generate a microdump if there are no webview pointers on the stack. The stack interest range is passed in MicrodumpExtraInfo from the client. If the extracted stack does not contain a pointer in this range, then we assume that this is not a WebView crash, and do not generate a microdump. If the stack extraction fails, we still generate a microdump (without a stack). BUG=664460 Change-Id: Ic762497f76f074a3621c7ec88a8c20ed768b9211 Reviewed-on: https://chromium-review.googlesource.com/412781 Reviewed-by: Primiano Tucci --- .../linux/handler/microdump_extra_info.h | 14 +- .../microdump_writer/microdump_writer.cc | 177 ++++++++++-------- .../microdump_writer_unittest.cc | 125 ++++++++++--- 3 files changed, 208 insertions(+), 108 deletions(-) diff --git a/src/client/linux/handler/microdump_extra_info.h b/src/client/linux/handler/microdump_extra_info.h index bf01f0c7..40cba1c4 100644 --- a/src/client/linux/handler/microdump_extra_info.h +++ b/src/client/linux/handler/microdump_extra_info.h @@ -40,11 +40,23 @@ struct MicrodumpExtraInfo { const char* gpu_fingerprint; const char* process_type; + // |interest_range_start| and |interest_range_end| specify a range + // in the target process address space. Microdumps are only + // generated if the PC or a word on the captured stack point into + // this range, or |suppress_microdump_based_on_interest_range| is + // false. + bool suppress_microdump_based_on_interest_range; + uintptr_t interest_range_start; + uintptr_t interest_range_end; + MicrodumpExtraInfo() : build_fingerprint(NULL), product_info(NULL), gpu_fingerprint(NULL), - process_type(NULL) {} + process_type(NULL), + suppress_microdump_based_on_interest_range(false), + interest_range_start(0), + interest_range_end(0) {} }; } diff --git a/src/client/linux/microdump_writer/microdump_writer.cc b/src/client/linux/microdump_writer/microdump_writer.cc index 6f5b4355..2bcef8ab 100644 --- a/src/client/linux/microdump_writer/microdump_writer.cc +++ b/src/client/linux/microdump_writer/microdump_writer.cc @@ -141,7 +141,11 @@ class MicrodumpWriter { dumper_(dumper), mapping_list_(mappings), microdump_extra_info_(microdump_extra_info), - log_line_(NULL) { + log_line_(NULL), + stack_copy_(NULL), + stack_len_(0), + stack_lower_bound_(0), + stack_pointer_(0) { log_line_ = reinterpret_cast(Alloc(kLineBufferSize)); if (log_line_) log_line_[0] = '\0'; // Clear out the log line buffer. @@ -159,8 +163,12 @@ class MicrodumpWriter { return dumper_->ThreadsSuspend() && dumper_->LateInit(); } - bool Dump() { - bool success; + void Dump() { + CaptureResult stack_capture_result = CaptureCrashingThreadStack(-1); + if (stack_capture_result == CAPTURE_UNINTERESTING) { + return; + } + LogLine("-----BEGIN BREAKPAD MICRODUMP-----"); DumpProductInformation(); DumpOSInformation(); @@ -169,15 +177,16 @@ class MicrodumpWriter { #if !defined(__LP64__) DumpFreeSpace(); #endif - success = DumpCrashingThread(); - if (success) - success = DumpMappings(); + if (stack_capture_result == CAPTURE_OK) + DumpThreadStack(); + DumpCPUState(); + DumpMappings(); LogLine("-----END BREAKPAD MICRODUMP-----"); - dumper_->ThreadsResume(); - return success; } private: + enum CaptureResult { CAPTURE_OK, CAPTURE_FAILED, CAPTURE_UNINTERESTING }; + // Writes one line to the system log. void LogLine(const char* msg) { #if defined(__ANDROID__) @@ -221,7 +230,59 @@ class MicrodumpWriter { // Writes out the current line buffer on the system log. void LogCommitLine() { LogLine(log_line_); - my_strlcpy(log_line_, "", kLineBufferSize); + log_line_[0] = 0; + } + + CaptureResult CaptureCrashingThreadStack(int max_stack_len) { + stack_pointer_ = UContextReader::GetStackPointer(ucontext_); + + if (!dumper_->GetStackInfo(reinterpret_cast(&stack_lower_bound_), + &stack_len_, stack_pointer_)) { + return CAPTURE_FAILED; + } + + if (max_stack_len >= 0 && + stack_len_ > static_cast(max_stack_len)) { + stack_len_ = max_stack_len; + } + + stack_copy_ = reinterpret_cast(Alloc(stack_len_)); + dumper_->CopyFromProcess(stack_copy_, dumper_->crash_thread(), + reinterpret_cast(stack_lower_bound_), + stack_len_); + + if (!microdump_extra_info_.suppress_microdump_based_on_interest_range) + return CAPTURE_OK; + + uintptr_t low_addr = microdump_extra_info_.interest_range_start; + uintptr_t high_addr = microdump_extra_info_.interest_range_end; + + uintptr_t pc = UContextReader::GetInstructionPointer(ucontext_); + if (low_addr <= pc && pc <= high_addr) return CAPTURE_OK; + + // Loop over all stack words that would have been on the stack in + // the target process. (i.e. ones that are >= |stack_pointer_| and + // < |stack_lower_bound_| + |stack_len_| in the target + // process). + // |stack_lower_bound_| is page aligned, and thus also pointer + // aligned. Because the stack pointer might be unaligned, we round + // the offset down to word alignment. |stack_pointer_| > + // |stack_lower_bound_|, so this never results in a negative + // offset. + // Regardless of the alignment of |stack_copy_|, the memory + // starting at |stack_copy_| + |offset| represents an aligned word + // in the target process. + uintptr_t offset = + ((stack_pointer_ - stack_lower_bound_) & ~(sizeof(uintptr_t) - 1)); + for (uint8_t* sp = stack_copy_ + offset; + sp <= stack_copy_ + stack_len_ - sizeof(uintptr_t); + sp += sizeof(uintptr_t)) { + uintptr_t addr; + my_memcpy(&addr, sp, sizeof(uintptr_t)); + if (low_addr <= addr && addr <= high_addr) return CAPTURE_OK; + } + + return CAPTURE_UNINTERESTING; } void DumpProductInformation() { @@ -315,87 +376,37 @@ class MicrodumpWriter { LogCommitLine(); } - bool DumpThreadStack(uint32_t thread_id, - uintptr_t stack_pointer, - int max_stack_len, - uint8_t** stack_copy) { - *stack_copy = NULL; - const void* stack; - size_t stack_len; - - if (!dumper_->GetStackInfo(&stack, &stack_len, stack_pointer)) { - // The stack pointer might not be available. In this case we don't hard - // fail, just produce a (almost useless) microdump w/o a stack section. - return true; - } - + void DumpThreadStack() { LogAppend("S 0 "); - LogAppend(stack_pointer); + LogAppend(stack_pointer_); LogAppend(" "); - LogAppend(reinterpret_cast(stack)); + LogAppend(stack_lower_bound_); LogAppend(" "); - LogAppend(stack_len); + LogAppend(stack_len_); LogCommitLine(); - if (max_stack_len >= 0 && - stack_len > static_cast(max_stack_len)) { - stack_len = max_stack_len; - } - - *stack_copy = reinterpret_cast(Alloc(stack_len)); - dumper_->CopyFromProcess(*stack_copy, thread_id, stack, stack_len); - - // Dump the content of the stack, splicing it into chunks which size is - // compatible with the max logcat line size (see LOGGER_ENTRY_MAX_PAYLOAD). const size_t STACK_DUMP_CHUNK_SIZE = 384; - for (size_t stack_off = 0; stack_off < stack_len; + for (size_t stack_off = 0; stack_off < stack_len_; stack_off += STACK_DUMP_CHUNK_SIZE) { LogAppend("S "); - LogAppend(reinterpret_cast(stack) + stack_off); + LogAppend(stack_lower_bound_ + stack_off); LogAppend(" "); - LogAppend(*stack_copy + stack_off, - std::min(STACK_DUMP_CHUNK_SIZE, stack_len - stack_off)); + LogAppend(stack_copy_ + stack_off, + std::min(STACK_DUMP_CHUNK_SIZE, stack_len_ - stack_off)); LogCommitLine(); } - return true; } - // Write information about the crashing thread. - bool DumpCrashingThread() { - const unsigned num_threads = dumper_->threads().size(); - - for (unsigned i = 0; i < num_threads; ++i) { - MDRawThread thread; - my_memset(&thread, 0, sizeof(thread)); - thread.thread_id = dumper_->threads()[i]; - - // Dump only the crashing thread. - if (static_cast(thread.thread_id) != dumper_->crash_thread()) - continue; - - assert(ucontext_); - assert(!dumper_->IsPostMortem()); - - uint8_t* stack_copy; - const uintptr_t stack_ptr = UContextReader::GetStackPointer(ucontext_); - if (!DumpThreadStack(thread.thread_id, stack_ptr, -1, &stack_copy)) - return false; - - RawContextCPU cpu; - my_memset(&cpu, 0, sizeof(RawContextCPU)); + void DumpCPUState() { + RawContextCPU cpu; + my_memset(&cpu, 0, sizeof(RawContextCPU)); #if !defined(__ARM_EABI__) && !defined(__mips__) - UContextReader::FillCPUContext(&cpu, ucontext_, float_state_); + UContextReader::FillCPUContext(&cpu, ucontext_, float_state_); #else - UContextReader::FillCPUContext(&cpu, ucontext_); + UContextReader::FillCPUContext(&cpu, ucontext_); #endif - DumpCPUState(&cpu); - } - return true; - } - - void DumpCPUState(RawContextCPU* cpu) { LogAppend("C "); - LogAppend(cpu, sizeof(*cpu)); + LogAppend(&cpu, sizeof(cpu)); LogCommitLine(); } @@ -547,7 +558,7 @@ class MicrodumpWriter { #endif // Write information about the mappings in effect. - bool DumpMappings() { + void DumpMappings() { // First write all the mappings from the dumper for (unsigned i = 0; i < dumper_->mappings().size(); ++i) { const MappingInfo& mapping = *dumper_->mappings()[i]; @@ -566,7 +577,6 @@ class MicrodumpWriter { ++iter) { DumpModule(iter->first, false, 0, iter->second); } - return true; } void* Alloc(unsigned bytes) { return dumper_->allocator()->Alloc(bytes); } @@ -579,6 +589,20 @@ class MicrodumpWriter { const MappingList& mapping_list_; const MicrodumpExtraInfo microdump_extra_info_; char* log_line_; + + // The local copy of crashed process stack memory, beginning at + // |stack_lower_bound_|. + uint8_t* stack_copy_; + + // The length of crashed process stack copy. + size_t stack_len_; + + // The address of the page containing the stack pointer in the + // crashed process. |stack_lower_bound_| <= |stack_pointer_| + uintptr_t stack_lower_bound_; + + // The stack pointer in the crashed process. + uintptr_t stack_pointer_; }; } // namespace @@ -603,7 +627,8 @@ bool WriteMicrodump(pid_t crashing_process, MicrodumpWriter writer(context, mappings, microdump_extra_info, &dumper); if (!writer.Init()) return false; - return writer.Dump(); + writer.Dump(); + return true; } } // namespace google_breakpad diff --git a/src/client/linux/microdump_writer/microdump_writer_unittest.cc b/src/client/linux/microdump_writer/microdump_writer_unittest.cc index 58a73118..b3008052 100644 --- a/src/client/linux/microdump_writer/microdump_writer_unittest.cc +++ b/src/client/linux/microdump_writer/microdump_writer_unittest.cc @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -47,6 +48,11 @@ using namespace google_breakpad; +extern "C" { +extern char __executable_start; +extern char __etext; +} + namespace { typedef testing::Test MicrodumpWriterTest; @@ -54,18 +60,29 @@ typedef testing::Test MicrodumpWriterTest; MicrodumpExtraInfo MakeMicrodumpExtraInfo( const char* build_fingerprint, const char* product_info, - const char* gpu_fingerprint) { + const char* gpu_fingerprint, + bool suppress_microdump_based_on_interest_range = false, + uintptr_t interest_range_start = 0, + uintptr_t interest_range_end = 0) { MicrodumpExtraInfo info; info.build_fingerprint = build_fingerprint; info.product_info = product_info; info.gpu_fingerprint = gpu_fingerprint; + info.suppress_microdump_based_on_interest_range = + suppress_microdump_based_on_interest_range; + info.interest_range_start = interest_range_start; + info.interest_range_end = interest_range_end; return info; } -void CrashAndGetMicrodump( - const MappingList& mappings, - const MicrodumpExtraInfo& microdump_extra_info, - scoped_array* buf) { +void AssertContainsMicrodump(const std::string& buf) { + ASSERT_NE(std::string::npos, buf.find("-----BEGIN BREAKPAD MICRODUMP-----")); + ASSERT_NE(std::string::npos, buf.find("-----END BREAKPAD MICRODUMP-----")); +} + +void CrashAndGetMicrodump(const MappingList& mappings, + const MicrodumpExtraInfo& microdump_extra_info, + std::string* microdump) { int fds[2]; ASSERT_NE(-1, pipe(fds)); @@ -86,7 +103,10 @@ void CrashAndGetMicrodump( ExceptionHandler::CrashContext context; memset(&context, 0, sizeof(context)); - + // Pretend the current context is the child context (which is + // approximately right) so that we have a valid stack pointer, and + // can fetch child stack data via ptrace. + getcontext(&context.context); // Set a non-zero tid to avoid tripping asserts. context.tid = child; @@ -105,17 +125,17 @@ void CrashAndGetMicrodump( // Read back the stderr file and check for the microdump marker. fsync(err_fd); lseek(err_fd, 0, SEEK_SET); - const size_t kBufSize = 64 * 1024; - buf->reset(new char[kBufSize]); - ASSERT_GT(read(err_fd, buf->get(), kBufSize), 0); + microdump->clear(); + char buf[1024]; + + while (true) { + int bytes_read = IGNORE_EINTR(read(err_fd, buf, 1024)); + if (bytes_read <= 0) break; + microdump->append(buf, buf + bytes_read); + } close(err_fd); close(fds[1]); - - ASSERT_NE(static_cast(0), strstr( - buf->get(), "-----BEGIN BREAKPAD MICRODUMP-----")); - ASSERT_NE(static_cast(0), strstr( - buf->get(), "-----END BREAKPAD MICRODUMP-----")); } void CheckMicrodumpContents(const string& microdump_content, @@ -191,23 +211,64 @@ TEST(MicrodumpWriterTest, BasicWithMappings) { memcpy(mapping.second, kModuleGUID, sizeof(MDGUID)); mappings.push_back(mapping); - scoped_array buf; + std::string buf; CrashAndGetMicrodump(mappings, MicrodumpExtraInfo(), &buf); + AssertContainsMicrodump(buf); #ifdef __LP64__ - ASSERT_NE(static_cast(0), strstr( - buf.get(), "M 0000000000001000 000000000000002A 0000000000001000 " - "33221100554477668899AABBCCDDEEFF0 libfoo.so")); + ASSERT_NE(std::string::npos, + buf.find("M 0000000000001000 000000000000002A 0000000000001000 " + "33221100554477668899AABBCCDDEEFF0 libfoo.so")); #else - ASSERT_NE(static_cast(0), strstr( - buf.get(), "M 00001000 0000002A 00001000 " - "33221100554477668899AABBCCDDEEFF0 libfoo.so")); + ASSERT_NE(std::string::npos, + buf.find("M 00001000 0000002A 00001000 " + "33221100554477668899AABBCCDDEEFF0 libfoo.so")); #endif // In absence of a product info in the minidump, the writer should just write // an unknown marker. - ASSERT_NE(static_cast(0), strstr( - buf.get(), "V UNKNOWN:0.0.0.0")); + ASSERT_NE(std::string::npos, buf.find("V UNKNOWN:0.0.0.0")); +} + +// Ensure that no output occurs if the interest region is set, but +// doesn't overlap anything on the stack. +TEST(MicrodumpWriterTest, NoOutputIfUninteresting) { + const char kProductInfo[] = "MockProduct:42.0.2311.99"; + const char kBuildFingerprint[] = + "aosp/occam/mako:5.1.1/LMY47W/12345678:userdegbug/dev-keys"; + const char kGPUFingerprint[] = + "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@ (GIT@Id3510ff6dc)"; + const MicrodumpExtraInfo kMicrodumpExtraInfo( + MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint, + true, 0xdeadbeef, 0xdeadbeef - 1)); + + std::string buf; + MappingList no_mappings; + + CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf); + ASSERT_EQ(0, buf.size()); +} + +// Ensure that output occurs if the interest region is set, and +// does overlap something on the stack. +TEST(MicrodumpWriterTest, OutputIfInteresting) { + const char kProductInfo[] = "MockProduct:42.0.2311.99"; + const char kBuildFingerprint[] = + "aosp/occam/mako:5.1.1/LMY47W/12345678:userdegbug/dev-keys"; + const char kGPUFingerprint[] = + "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@ (GIT@Id3510ff6dc)"; + + const MicrodumpExtraInfo kMicrodumpExtraInfo( + MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint, + true, + reinterpret_cast(&__executable_start), + reinterpret_cast(&__etext))); + + std::string buf; + MappingList no_mappings; + + CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf); + ASSERT_LT(0, buf.size()); } // Ensure that the product info and build fingerprint metadata show up in the @@ -220,38 +281,40 @@ TEST(MicrodumpWriterTest, BuildFingerprintAndProductInfo) { "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@ (GIT@Id3510ff6dc)"; const MicrodumpExtraInfo kMicrodumpExtraInfo( MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint)); - scoped_array buf; + std::string buf; MappingList no_mappings; CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf); - CheckMicrodumpContents(string(buf.get()), kMicrodumpExtraInfo); + AssertContainsMicrodump(buf); + CheckMicrodumpContents(buf, kMicrodumpExtraInfo); } TEST(MicrodumpWriterTest, NoProductInfo) { const char kBuildFingerprint[] = "foobar"; const char kGPUFingerprint[] = "bazqux"; - scoped_array buf; + std::string buf; MappingList no_mappings; const MicrodumpExtraInfo kMicrodumpExtraInfoNoProductInfo( MakeMicrodumpExtraInfo(kBuildFingerprint, NULL, kGPUFingerprint)); CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfoNoProductInfo, &buf); - CheckMicrodumpContents(string(buf.get()), kBuildFingerprint, - "UNKNOWN:0.0.0.0", kGPUFingerprint); + AssertContainsMicrodump(buf); + CheckMicrodumpContents(buf, kBuildFingerprint, "UNKNOWN:0.0.0.0", + kGPUFingerprint); } TEST(MicrodumpWriterTest, NoGPUInfo) { const char kProductInfo[] = "bazqux"; const char kBuildFingerprint[] = "foobar"; - scoped_array buf; + std::string buf; MappingList no_mappings; const MicrodumpExtraInfo kMicrodumpExtraInfoNoGPUInfo( MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, NULL)); CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfoNoGPUInfo, &buf); - CheckMicrodumpContents(string(buf.get()), kBuildFingerprint, - kProductInfo, "UNKNOWN"); + AssertContainsMicrodump(buf); + CheckMicrodumpContents(buf, kBuildFingerprint, kProductInfo, "UNKNOWN"); } } // namespace