From fc6f700bb56d25919e40b13bd141832dcb8c6a7f Mon Sep 17 00:00:00 2001 From: "ted.mielczarek@gmail.com" Date: Tue, 6 Nov 2012 16:50:01 +0000 Subject: [PATCH] Allow processing dumps with missing stack memory for some threads r=mkrebs at https://breakpad.appspot.com/413002/ git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1077 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/google_breakpad/processor/minidump.h | 94 ++++---- .../processor/minidump_processor.h | 9 - src/processor/minidump.cc | 8 +- src/processor/minidump_processor.cc | 18 +- src/processor/minidump_processor_unittest.cc | 209 ++++++++++++++++-- src/processor/minidump_stackwalk.cc | 3 + src/processor/minidump_unittest.cc | 87 ++++++++ src/processor/stackwalker_amd64.cc | 4 +- src/processor/stackwalker_amd64_unittest.cc | 18 ++ src/processor/stackwalker_arm.cc | 4 +- src/processor/stackwalker_arm_unittest.cc | 15 ++ src/processor/stackwalker_ppc.cc | 6 +- src/processor/stackwalker_sparc.cc | 4 +- src/processor/stackwalker_x86.cc | 6 +- src/processor/stackwalker_x86_unittest.cc | 17 ++ src/processor/synth_minidump.h | 2 + 16 files changed, 412 insertions(+), 92 deletions(-) diff --git a/src/google_breakpad/processor/minidump.h b/src/google_breakpad/processor/minidump.h index 03337796..5e60a77c 100644 --- a/src/google_breakpad/processor/minidump.h +++ b/src/google_breakpad/processor/minidump.h @@ -190,31 +190,13 @@ class MinidumpContext : public MinidumpStream { const MDRawContextPPC* GetContextPPC() const; const MDRawContextSPARC* GetContextSPARC() const; const MDRawContextX86* GetContextX86() const; - + // Print a human-readable representation of the object to stdout. void Print(); - private: - friend class MinidumpThread; - friend class MinidumpException; - + protected: explicit MinidumpContext(Minidump* minidump); - bool Read(u_int32_t expected_size); - - // Free the CPU-specific context structure. - void FreeContext(); - - // If the minidump contains a SYSTEM_INFO_STREAM, makes sure that the - // system info stream gives an appropriate CPU type matching the context - // CPU type in context_cpu_type. Returns false if the CPU type does not - // match. Returns true if the CPU type matches or if the minidump does - // not contain a system info stream. - bool CheckAgainstSystemInfo(u_int32_t context_cpu_type); - - // Store this separately because of the weirdo AMD64 context - u_int32_t context_flags_; - // The CPU-specific context structure. union { MDRawContextBase* base; @@ -226,6 +208,25 @@ class MinidumpContext : public MinidumpStream { MDRawContextSPARC* ctx_sparc; MDRawContextARM* arm; } context_; + + // Store this separately because of the weirdo AMD64 context + u_int32_t context_flags_; + + private: + friend class MinidumpThread; + friend class MinidumpException; + + bool Read(u_int32_t expected_size); + + // Free the CPU-specific context structure. + void FreeContext(); + + // If the minidump contains a SYSTEM_INFO_STREAM, makes sure that the + // system info stream gives an appropriate CPU type matching the context + // CPU type in context_cpu_type. Returns false if the CPU type does not + // match. Returns true if the CPU type matches or if the minidump does + // not contain a system info stream. + bool CheckAgainstSystemInfo(u_int32_t context_cpu_type); }; @@ -268,12 +269,13 @@ class MinidumpMemoryRegion : public MinidumpObject, // Print a human-readable representation of the object to stdout. void Print(); + protected: + explicit MinidumpMemoryRegion(Minidump* minidump); + private: friend class MinidumpThread; friend class MinidumpMemoryList; - explicit MinidumpMemoryRegion(Minidump* minidump); - // Identify the base address and size of the memory region, and the // location it may be found in the minidump file. void SetDescriptor(MDMemoryDescriptor* descriptor); @@ -300,29 +302,35 @@ class MinidumpMemoryRegion : public MinidumpObject, // the thread that caused an exception, the context carried by // MinidumpException is probably desired instead of the CPU context // provided here. +// Note that a MinidumpThread may be valid() even if it does not +// contain a memory region or context. class MinidumpThread : public MinidumpObject { public: virtual ~MinidumpThread(); const MDRawThread* thread() const { return valid_ ? &thread_ : NULL; } - MinidumpMemoryRegion* GetMemory(); - MinidumpContext* GetContext(); + // GetMemory may return NULL even if the MinidumpThread is valid, + // if the thread memory cannot be read. + virtual MinidumpMemoryRegion* GetMemory(); + // GetContext may return NULL even if the MinidumpThread is valid. + virtual MinidumpContext* GetContext(); // The thread ID is used to determine if a thread is the exception thread, // so a special getter is provided to retrieve this data from the // MDRawThread structure. Returns false if the thread ID cannot be // determined. - bool GetThreadID(u_int32_t *thread_id) const; + virtual bool GetThreadID(u_int32_t *thread_id) const; // Print a human-readable representation of the object to stdout. void Print(); + protected: + explicit MinidumpThread(Minidump* minidump); + private: // These objects are managed by MinidumpThreadList. friend class MinidumpThreadList; - explicit MinidumpThread(Minidump* minidump); - // This works like MinidumpStream::Read, but is driven by // MinidumpThreadList. No size checking is done, because // MinidumpThreadList handles that directly. @@ -345,12 +353,12 @@ class MinidumpThreadList : public MinidumpStream { } static u_int32_t max_threads() { return max_threads_; } - unsigned int thread_count() const { + virtual unsigned int thread_count() const { return valid_ ? thread_count_ : 0; } // Sequential access to threads. - MinidumpThread* GetThreadAtIndex(unsigned int index) const; + virtual MinidumpThread* GetThreadAtIndex(unsigned int index) const; // Random access to threads. MinidumpThread* GetThreadByID(u_int32_t thread_id); @@ -358,6 +366,9 @@ class MinidumpThreadList : public MinidumpStream { // Print a human-readable representation of the object to stdout. void Print(); + protected: + explicit MinidumpThreadList(Minidump* aMinidump); + private: friend class Minidump; @@ -366,8 +377,6 @@ class MinidumpThreadList : public MinidumpStream { static const u_int32_t kStreamType = MD_THREAD_LIST_STREAM; - explicit MinidumpThreadList(Minidump* aMinidump); - bool Read(u_int32_t aExpectedSize); // The largest number of threads that will be read from a minidump. The @@ -526,6 +535,9 @@ class MinidumpModuleList : public MinidumpStream, // Print a human-readable representation of the object to stdout. void Print(); + protected: + explicit MinidumpModuleList(Minidump* minidump); + private: friend class Minidump; @@ -533,8 +545,6 @@ class MinidumpModuleList : public MinidumpStream, static const u_int32_t kStreamType = MD_MODULE_LIST_STREAM; - explicit MinidumpModuleList(Minidump* minidump); - bool Read(u_int32_t expected_size); // The largest number of modules that will be read from a minidump. The @@ -722,21 +732,21 @@ class MinidumpSystemInfo : public MinidumpStream { // Print a human-readable representation of the object to stdout. void Print(); - private: - friend class Minidump; - - static const u_int32_t kStreamType = MD_SYSTEM_INFO_STREAM; - + protected: explicit MinidumpSystemInfo(Minidump* minidump); - - bool Read(u_int32_t expected_size); - MDRawSystemInfo system_info_; // Textual representation of the OS service pack, for minidumps produced // by MiniDumpWriteDump on Windows. const string* csd_version_; + private: + friend class Minidump; + + static const u_int32_t kStreamType = MD_SYSTEM_INFO_STREAM; + + bool Read(u_int32_t expected_size); + // A string identifying the CPU vendor, if known. const string* cpu_vendor_; }; @@ -913,7 +923,7 @@ class Minidump { MinidumpMemoryList* GetMemoryList(); MinidumpException* GetException(); MinidumpAssertion* GetAssertion(); - MinidumpSystemInfo* GetSystemInfo(); + virtual MinidumpSystemInfo* GetSystemInfo(); MinidumpMiscInfo* GetMiscInfo(); MinidumpBreakpadInfo* GetBreakpadInfo(); MinidumpMemoryInfoList* GetMemoryInfoList(); diff --git a/src/google_breakpad/processor/minidump_processor.h b/src/google_breakpad/processor/minidump_processor.h index 5da11cd3..0e10f40b 100644 --- a/src/google_breakpad/processor/minidump_processor.h +++ b/src/google_breakpad/processor/minidump_processor.h @@ -73,15 +73,6 @@ enum ProcessResult { // one requesting // thread. - PROCESS_ERROR_NO_MEMORY_FOR_THREAD, // A thread had no - // memory region. - - PROCESS_ERROR_NO_STACKWALKER_FOR_THREAD, // We couldn't - // determine the - // StackWalker to walk - // the minidump's - // threads. - PROCESS_SYMBOL_SUPPLIER_INTERRUPTED // The minidump // processing was // interrupted by the diff --git a/src/processor/minidump.cc b/src/processor/minidump.cc index 19ba0354..0c13e00e 100644 --- a/src/processor/minidump.cc +++ b/src/processor/minidump.cc @@ -1354,15 +1354,15 @@ bool MinidumpThread::Read() { if (thread_.stack.memory.data_size == 0 || thread_.stack.memory.data_size > numeric_limits::max() - thread_.stack.start_of_memory_range) { + // This is ok, but log an error anyway. BPLOG(ERROR) << "MinidumpThread has a memory region problem, " << HexString(thread_.stack.start_of_memory_range) << "+" << HexString(thread_.stack.memory.data_size); - return false; + } else { + memory_ = new MinidumpMemoryRegion(minidump_); + memory_->SetDescriptor(&thread_.stack); } - memory_ = new MinidumpMemoryRegion(minidump_); - memory_->SetDescriptor(&thread_.stack); - valid_ = true; return true; } diff --git a/src/processor/minidump_processor.cc b/src/processor/minidump_processor.cc index a197cd10..db5b271a 100644 --- a/src/processor/minidump_processor.cc +++ b/src/processor/minidump_processor.cc @@ -208,7 +208,6 @@ ProcessResult MinidumpProcessor::Process( MinidumpMemoryRegion *thread_memory = thread->GetMemory(); if (!thread_memory) { BPLOG(ERROR) << "No memory region for " << thread_string; - return PROCESS_ERROR_NO_MEMORY_FOR_THREAD; } // Use process_state->modules_ instead of module_list, because the @@ -225,16 +224,19 @@ ProcessResult MinidumpProcessor::Process( thread_memory, process_state->modules_, frame_symbolizer_)); - if (!stackwalker.get()) { - BPLOG(ERROR) << "No stackwalker for " << thread_string; - return PROCESS_ERROR_NO_STACKWALKER_FOR_THREAD; - } scoped_ptr stack(new CallStack()); - if (!stackwalker->Walk(stack.get())) { - BPLOG(INFO) << "Stackwalker interrupt (missing symbols?) at " << + if (stackwalker.get()) { + if (!stackwalker->Walk(stack.get())) { + BPLOG(INFO) << "Stackwalker interrupt (missing symbols?) at " << thread_string; - interrupted = true; + interrupted = true; + } + } else { + // Threads with missing CPU contexts will hit this, but + // don't abort processing the rest of the dump just for + // one bad thread. + BPLOG(ERROR) << "No stackwalker for " << thread_string; } process_state->threads_.push_back(stack.release()); process_state->thread_memory_regions_.push_back(thread_memory); diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc index a8e1208e..db96efb7 100644 --- a/src/processor/minidump_processor_unittest.cc +++ b/src/processor/minidump_processor_unittest.cc @@ -51,6 +51,7 @@ #include "google_breakpad/processor/symbol_supplier.h" #include "processor/logging.h" #include "processor/scoped_ptr.h" +#include "processor/stackwalker_unittest_utils.h" using std::map; @@ -64,27 +65,86 @@ class MockMinidump : public Minidump { MOCK_CONST_METHOD0(path, string()); MOCK_CONST_METHOD0(header, const MDRawHeader*()); MOCK_METHOD0(GetThreadList, MinidumpThreadList*()); + MOCK_METHOD0(GetSystemInfo, MinidumpSystemInfo*()); + MOCK_METHOD0(GetBreakpadInfo, MinidumpBreakpadInfo*()); + MOCK_METHOD0(GetException, MinidumpException*()); + MOCK_METHOD0(GetAssertion, MinidumpAssertion*()); + MOCK_METHOD0(GetModuleList, MinidumpModuleList*()); }; -} + +class MockMinidumpThreadList : public MinidumpThreadList { + public: + MockMinidumpThreadList() : MinidumpThreadList(NULL) {} + + MOCK_CONST_METHOD0(thread_count, unsigned int()); + MOCK_CONST_METHOD1(GetThreadAtIndex, MinidumpThread*(unsigned int)); +}; + +class MockMinidumpThread : public MinidumpThread { + public: + MockMinidumpThread() : MinidumpThread(NULL) {} + + MOCK_CONST_METHOD1(GetThreadID, bool(u_int32_t*)); + MOCK_METHOD0(GetContext, MinidumpContext*()); + MOCK_METHOD0(GetMemory, MinidumpMemoryRegion*()); +}; + +// This is crappy, but MinidumpProcessor really does want a +// MinidumpMemoryRegion. +class MockMinidumpMemoryRegion : public MinidumpMemoryRegion { + public: + MockMinidumpMemoryRegion(u_int64_t base, const string& contents) : + MinidumpMemoryRegion(NULL) { + region_.Init(base, contents); + } + + u_int64_t GetBase() const { return region_.GetBase(); } + u_int32_t GetSize() const { return region_.GetSize(); } + + bool GetMemoryAtAddress(u_int64_t address, u_int8_t *value) const { + return region_.GetMemoryAtAddress(address, value); + } + bool GetMemoryAtAddress(u_int64_t address, u_int16_t *value) const { + return region_.GetMemoryAtAddress(address, value); + } + bool GetMemoryAtAddress(u_int64_t address, u_int32_t *value) const { + return region_.GetMemoryAtAddress(address, value); + } + bool GetMemoryAtAddress(u_int64_t address, u_int64_t *value) const { + return region_.GetMemoryAtAddress(address, value); + } + + MockMemoryRegion region_; +}; + +} // namespace google_breakpad namespace { using google_breakpad::BasicSourceLineResolver; using google_breakpad::CallStack; using google_breakpad::CodeModule; +using google_breakpad::MinidumpContext; +using google_breakpad::MinidumpMemoryRegion; using google_breakpad::MinidumpProcessor; +using google_breakpad::MinidumpSystemInfo; using google_breakpad::MinidumpThreadList; using google_breakpad::MinidumpThread; using google_breakpad::MockMinidump; +using google_breakpad::MockMinidumpMemoryRegion; +using google_breakpad::MockMinidumpThread; +using google_breakpad::MockMinidumpThreadList; using google_breakpad::ProcessState; using google_breakpad::scoped_ptr; using google_breakpad::SymbolSupplier; using google_breakpad::SystemInfo; using ::testing::_; +using ::testing::DoAll; using ::testing::Mock; using ::testing::Ne; using ::testing::Property; using ::testing::Return; +using ::testing::SetArgumentPointee; static const char *kSystemInfoOS = "Windows NT"; static const char *kSystemInfoOSShort = "windows"; @@ -206,23 +266,27 @@ void TestSymbolSupplier::FreeSymbolData(const CodeModule *module) { } } -// A mock symbol supplier that always returns NOT_FOUND; one current -// use for testing the processor's caching of symbol lookups. -class MockSymbolSupplier : public SymbolSupplier { +// A test system info stream, just returns values from the +// MDRawSystemInfo fed to it. +class TestMinidumpSystemInfo : public MinidumpSystemInfo { public: - MockSymbolSupplier() { } - MOCK_METHOD3(GetSymbolFile, SymbolResult(const CodeModule*, - const SystemInfo*, - string*)); - MOCK_METHOD4(GetSymbolFile, SymbolResult(const CodeModule*, - const SystemInfo*, - string*, - string*)); - MOCK_METHOD4(GetCStringSymbolData, SymbolResult(const CodeModule*, - const SystemInfo*, - string*, - char**)); - MOCK_METHOD1(FreeSymbolData, void(const CodeModule*)); + TestMinidumpSystemInfo(MDRawSystemInfo info) : + MinidumpSystemInfo(NULL) { + valid_ = true; + system_info_ = info; + csd_version_ = new string(""); + } +}; + +// A test minidump context, just returns the MDRawContextX86 +// fed to it. +class TestMinidumpContext : public MinidumpContext { +public: + TestMinidumpContext(const MDRawContextX86& context) : MinidumpContext(NULL) { + valid_ = true; + context_.x86 = new MDRawContextX86(context); + context_flags_ = MD_CONTEXT_X86; + } }; class MinidumpProcessorTest : public ::testing::Test { @@ -245,11 +309,15 @@ TEST_F(MinidumpProcessorTest, TestCorruptMinidumps) { fakeHeader.time_date_stamp = 0; EXPECT_CALL(dump, header()).WillOnce(Return((MDRawHeader*)NULL)). WillRepeatedly(Return(&fakeHeader)); + EXPECT_EQ(processor.Process(&dump, &state), google_breakpad::PROCESS_ERROR_NO_MINIDUMP_HEADER); EXPECT_CALL(dump, GetThreadList()). WillOnce(Return((MinidumpThreadList*)NULL)); + EXPECT_CALL(dump, GetSystemInfo()). + WillRepeatedly(Return((MinidumpSystemInfo*)NULL)); + EXPECT_EQ(processor.Process(&dump, &state), google_breakpad::PROCESS_ERROR_NO_THREAD_LIST); } @@ -372,6 +440,113 @@ TEST_F(MinidumpProcessorTest, TestBasicProcessing) { ASSERT_EQ(processor.Process(minidump_file, &state), google_breakpad::PROCESS_SYMBOL_SUPPLIER_INTERRUPTED); } + +TEST_F(MinidumpProcessorTest, TestThreadMissingMemory) { + MockMinidump dump; + EXPECT_CALL(dump, path()).WillRepeatedly(Return("mock minidump")); + EXPECT_CALL(dump, Read()).WillRepeatedly(Return(true)); + + MDRawHeader fake_header; + fake_header.time_date_stamp = 0; + EXPECT_CALL(dump, header()).WillRepeatedly(Return(&fake_header)); + + MDRawSystemInfo raw_system_info; + memset(&raw_system_info, 0, sizeof(raw_system_info)); + raw_system_info.processor_architecture = MD_CPU_ARCHITECTURE_X86; + raw_system_info.platform_id = MD_OS_WIN32_NT; + TestMinidumpSystemInfo dump_system_info(raw_system_info); + + EXPECT_CALL(dump, GetSystemInfo()). + WillRepeatedly(Return(&dump_system_info)); + + MockMinidumpThreadList thread_list; + EXPECT_CALL(dump, GetThreadList()). + WillOnce(Return(&thread_list)); + + // Return a thread missing stack memory. + MockMinidumpThread no_memory_thread; + EXPECT_CALL(no_memory_thread, GetThreadID(_)). + WillRepeatedly(DoAll(SetArgumentPointee<0>(1), + Return(true))); + EXPECT_CALL(no_memory_thread, GetMemory()). + WillRepeatedly(Return((MinidumpMemoryRegion*)NULL)); + + MDRawContextX86 no_memory_thread_raw_context; + memset(&no_memory_thread_raw_context, 0, + sizeof(no_memory_thread_raw_context)); + no_memory_thread_raw_context.context_flags = MD_CONTEXT_X86_FULL; + const u_int32_t kExpectedEIP = 0xabcd1234; + no_memory_thread_raw_context.eip = kExpectedEIP; + TestMinidumpContext no_memory_thread_context(no_memory_thread_raw_context); + EXPECT_CALL(no_memory_thread, GetContext()). + WillRepeatedly(Return(&no_memory_thread_context)); + + EXPECT_CALL(thread_list, thread_count()). + WillRepeatedly(Return(1)); + EXPECT_CALL(thread_list, GetThreadAtIndex(0)). + WillOnce(Return(&no_memory_thread)); + + MinidumpProcessor processor((SymbolSupplier*)NULL, NULL); + ProcessState state; + EXPECT_EQ(processor.Process(&dump, &state), + google_breakpad::PROCESS_OK); + + // Should have a single thread with a single frame in it. + ASSERT_EQ(1, state.threads()->size()); + ASSERT_EQ(1, state.threads()->at(0)->frames()->size()); + ASSERT_EQ(kExpectedEIP, state.threads()->at(0)->frames()->at(0)->instruction); +} + +TEST_F(MinidumpProcessorTest, TestThreadMissingContext) { + MockMinidump dump; + EXPECT_CALL(dump, path()).WillRepeatedly(Return("mock minidump")); + EXPECT_CALL(dump, Read()).WillRepeatedly(Return(true)); + + MDRawHeader fake_header; + fake_header.time_date_stamp = 0; + EXPECT_CALL(dump, header()).WillRepeatedly(Return(&fake_header)); + + MDRawSystemInfo raw_system_info; + memset(&raw_system_info, 0, sizeof(raw_system_info)); + raw_system_info.processor_architecture = MD_CPU_ARCHITECTURE_X86; + raw_system_info.platform_id = MD_OS_WIN32_NT; + TestMinidumpSystemInfo dump_system_info(raw_system_info); + + EXPECT_CALL(dump, GetSystemInfo()). + WillRepeatedly(Return(&dump_system_info)); + + MockMinidumpThreadList thread_list; + EXPECT_CALL(dump, GetThreadList()). + WillOnce(Return(&thread_list)); + + // Return a thread missing a thread context. + MockMinidumpThread no_context_thread; + EXPECT_CALL(no_context_thread, GetThreadID(_)). + WillRepeatedly(DoAll(SetArgumentPointee<0>(1), + Return(true))); + EXPECT_CALL(no_context_thread, GetContext()). + WillRepeatedly(Return((MinidumpContext*)NULL)); + + // The memory contents don't really matter here, since it won't be used. + MockMinidumpMemoryRegion no_context_thread_memory(0x1234, "xxx"); + EXPECT_CALL(no_context_thread, GetMemory()). + WillRepeatedly(Return(&no_context_thread_memory)); + + EXPECT_CALL(thread_list, thread_count()). + WillRepeatedly(Return(1)); + EXPECT_CALL(thread_list, GetThreadAtIndex(0)). + WillOnce(Return(&no_context_thread)); + + MinidumpProcessor processor((SymbolSupplier*)NULL, NULL); + ProcessState state; + EXPECT_EQ(processor.Process(&dump, &state), + google_breakpad::PROCESS_OK); + + // Should have a single thread with zero frames. + ASSERT_EQ(1, state.threads()->size()); + ASSERT_EQ(0, state.threads()->at(0)->frames()->size()); +} + } // namespace int main(int argc, char *argv[]) { diff --git a/src/processor/minidump_stackwalk.cc b/src/processor/minidump_stackwalk.cc index b37148f4..2ff7a57c 100644 --- a/src/processor/minidump_stackwalk.cc +++ b/src/processor/minidump_stackwalk.cc @@ -137,6 +137,9 @@ static string StripSeparator(const string &original) { // frame printed is also output, if available. static void PrintStack(const CallStack *stack, const string &cpu) { int frame_count = stack->frames()->size(); + if (frame_count == 0) { + printf(" \n"); + } for (int frame_index = 0; frame_index < frame_count; ++frame_index) { const StackFrame *frame = stack->frames()->at(frame_index); printf("%2d ", frame_index); diff --git a/src/processor/minidump_unittest.cc b/src/processor/minidump_unittest.cc index 60cb37a6..eb203ec6 100644 --- a/src/processor/minidump_unittest.cc +++ b/src/processor/minidump_unittest.cc @@ -298,6 +298,93 @@ TEST(Dump, OneThread) { EXPECT_EQ(0x2e951ef7U, raw_context.ss); } +TEST(Dump, ThreadMissingMemory) { + Dump dump(0, kLittleEndian); + Memory stack(dump, 0x2326a0fa); + // Stack has no contents. + + MDRawContextX86 raw_context; + memset(&raw_context, 0, sizeof(raw_context)); + raw_context.context_flags = MD_CONTEXT_X86_INTEGER | MD_CONTEXT_X86_CONTROL; + Context context(dump, raw_context); + + Thread thread(dump, 0xa898f11b, stack, context, + 0x9e39439f, 0x4abfc15f, 0xe499898a, 0x0d43e939dcfd0372ULL); + + dump.Add(&stack); + dump.Add(&context); + dump.Add(&thread); + dump.Finish(); + + string contents; + ASSERT_TRUE(dump.GetContents(&contents)); + + istringstream minidump_stream(contents); + Minidump minidump(minidump_stream); + ASSERT_TRUE(minidump.Read()); + ASSERT_EQ(2U, minidump.GetDirectoryEntryCount()); + + // This should succeed even though the thread has no stack memory. + MinidumpThreadList* thread_list = minidump.GetThreadList(); + ASSERT_TRUE(thread_list != NULL); + ASSERT_EQ(1U, thread_list->thread_count()); + + MinidumpThread* md_thread = thread_list->GetThreadAtIndex(0); + ASSERT_TRUE(md_thread != NULL); + + u_int32_t thread_id; + ASSERT_TRUE(md_thread->GetThreadID(&thread_id)); + ASSERT_EQ(0xa898f11bU, thread_id); + + MinidumpContext* md_context = md_thread->GetContext(); + ASSERT_NE(reinterpret_cast(NULL), md_context); + + MinidumpMemoryRegion* md_stack = md_thread->GetMemory(); + ASSERT_EQ(reinterpret_cast(NULL), md_stack); +} + +TEST(Dump, ThreadMissingContext) { + Dump dump(0, kLittleEndian); + Memory stack(dump, 0x2326a0fa); + stack.Append("stack for thread"); + + // Context is empty. + Context context(dump); + + Thread thread(dump, 0xa898f11b, stack, context, + 0x9e39439f, 0x4abfc15f, 0xe499898a, 0x0d43e939dcfd0372ULL); + + dump.Add(&stack); + dump.Add(&context); + dump.Add(&thread); + dump.Finish(); + + string contents; + ASSERT_TRUE(dump.GetContents(&contents)); + + istringstream minidump_stream(contents); + Minidump minidump(minidump_stream); + ASSERT_TRUE(minidump.Read()); + ASSERT_EQ(2U, minidump.GetDirectoryEntryCount()); + + // This should succeed even though the thread has no stack memory. + MinidumpThreadList* thread_list = minidump.GetThreadList(); + ASSERT_TRUE(thread_list != NULL); + ASSERT_EQ(1U, thread_list->thread_count()); + + MinidumpThread* md_thread = thread_list->GetThreadAtIndex(0); + ASSERT_TRUE(md_thread != NULL); + + u_int32_t thread_id; + ASSERT_TRUE(md_thread->GetThreadID(&thread_id)); + ASSERT_EQ(0xa898f11bU, thread_id); + MinidumpMemoryRegion* md_stack = md_thread->GetMemory(); + ASSERT_NE(reinterpret_cast(NULL), md_stack); + + MinidumpContext* md_context = md_thread->GetContext(); + ASSERT_EQ(reinterpret_cast(NULL), md_context); +} + TEST(Dump, OneModule) { static const MDVSFixedFileInfo fixed_file_info = { 0xb2fba33a, // signature diff --git a/src/processor/stackwalker_amd64.cc b/src/processor/stackwalker_amd64.cc index 12aa6333..b7d3274d 100644 --- a/src/processor/stackwalker_amd64.cc +++ b/src/processor/stackwalker_amd64.cc @@ -102,8 +102,8 @@ StackwalkerAMD64::StackwalkerAMD64(const SystemInfo* system_info, StackFrame* StackwalkerAMD64::GetContextFrame() { - if (!context_ || !memory_) { - BPLOG(ERROR) << "Can't get context frame without context or memory"; + if (!context_) { + BPLOG(ERROR) << "Can't get context frame without context"; return NULL; } diff --git a/src/processor/stackwalker_amd64_unittest.cc b/src/processor/stackwalker_amd64_unittest.cc index 1721b8cb..2d679abb 100644 --- a/src/processor/stackwalker_amd64_unittest.cc +++ b/src/processor/stackwalker_amd64_unittest.cc @@ -172,6 +172,24 @@ TEST_F(GetContextFrame, Simple) { EXPECT_EQ(0, memcmp(&raw_context, &frame->context, sizeof(raw_context))); } +// The stackwalker should be able to produce the context frame even +// without stack memory present. +TEST_F(GetContextFrame, NoStackMemory) { + raw_context.rip = 0x40000000c0000200ULL; + raw_context.rbp = 0x8000000080000000ULL; + + StackFrameSymbolizer frame_symbolizer(&supplier, &resolver); + StackwalkerAMD64 walker(&system_info, &raw_context, NULL, &modules, + &frame_symbolizer); + ASSERT_TRUE(walker.Walk(&call_stack)); + frames = call_stack.frames(); + ASSERT_GE(1U, frames->size()); + StackFrameAMD64 *frame = static_cast(frames->at(0)); + // Check that the values from the original raw context made it + // through to the context in the stack frame. + EXPECT_EQ(0, memcmp(&raw_context, &frame->context, sizeof(raw_context))); +} + class GetCallerFrame: public StackwalkerAMD64Fixture, public Test { }; TEST_F(GetCallerFrame, ScanWithoutSymbols) { diff --git a/src/processor/stackwalker_arm.cc b/src/processor/stackwalker_arm.cc index 3beaa449..2535cd0f 100644 --- a/src/processor/stackwalker_arm.cc +++ b/src/processor/stackwalker_arm.cc @@ -59,8 +59,8 @@ StackwalkerARM::StackwalkerARM(const SystemInfo* system_info, StackFrame* StackwalkerARM::GetContextFrame() { - if (!context_ || !memory_) { - BPLOG(ERROR) << "Can't get context frame without context or memory"; + if (!context_) { + BPLOG(ERROR) << "Can't get context frame without context"; return NULL; } diff --git a/src/processor/stackwalker_arm_unittest.cc b/src/processor/stackwalker_arm_unittest.cc index 88517b77..e5988f41 100644 --- a/src/processor/stackwalker_arm_unittest.cc +++ b/src/processor/stackwalker_arm_unittest.cc @@ -166,6 +166,21 @@ TEST_F(GetContextFrame, Simple) { EXPECT_EQ(0, memcmp(&raw_context, &frame->context, sizeof(raw_context))); } +// The stackwalker should be able to produce the context frame even +// without stack memory present. +TEST_F(GetContextFrame, NoStackMemory) { + StackFrameSymbolizer frame_symbolizer(&supplier, &resolver); + StackwalkerARM walker(&system_info, &raw_context, -1, NULL, &modules, + &frame_symbolizer); + ASSERT_TRUE(walker.Walk(&call_stack)); + frames = call_stack.frames(); + ASSERT_EQ(1U, frames->size()); + StackFrameARM *frame = static_cast(frames->at(0)); + // Check that the values from the original raw context made it + // through to the context in the stack frame. + EXPECT_EQ(0, memcmp(&raw_context, &frame->context, sizeof(raw_context))); +} + class GetCallerFrame: public StackwalkerARMFixture, public Test { }; TEST_F(GetCallerFrame, ScanWithoutSymbols) { diff --git a/src/processor/stackwalker_ppc.cc b/src/processor/stackwalker_ppc.cc index 6359e737..40bec8de 100644 --- a/src/processor/stackwalker_ppc.cc +++ b/src/processor/stackwalker_ppc.cc @@ -50,7 +50,7 @@ StackwalkerPPC::StackwalkerPPC(const SystemInfo* system_info, StackFrameSymbolizer* resolver_helper) : Stackwalker(system_info, memory, modules, resolver_helper), context_(context) { - if (memory_->GetBase() + memory_->GetSize() - 1 > 0xffffffff) { + if (memory_ && memory_->GetBase() + memory_->GetSize() - 1 > 0xffffffff) { // This implementation only covers 32-bit ppc CPUs. The limits of the // supplied stack are invalid. Mark memory_ = NULL, which will cause // stackwalking to fail. @@ -63,8 +63,8 @@ StackwalkerPPC::StackwalkerPPC(const SystemInfo* system_info, StackFrame* StackwalkerPPC::GetContextFrame() { - if (!context_ || !memory_) { - BPLOG(ERROR) << "Can't get context frame without context or memory"; + if (!context_) { + BPLOG(ERROR) << "Can't get context frame without context"; return NULL; } diff --git a/src/processor/stackwalker_sparc.cc b/src/processor/stackwalker_sparc.cc index ac356baa..d1d5d236 100644 --- a/src/processor/stackwalker_sparc.cc +++ b/src/processor/stackwalker_sparc.cc @@ -54,8 +54,8 @@ StackwalkerSPARC::StackwalkerSPARC(const SystemInfo* system_info, StackFrame* StackwalkerSPARC::GetContextFrame() { - if (!context_ || !memory_) { - BPLOG(ERROR) << "Can't get context frame without context or memory"; + if (!context_) { + BPLOG(ERROR) << "Can't get context frame without context"; return NULL; } diff --git a/src/processor/stackwalker_x86.cc b/src/processor/stackwalker_x86.cc index 6a4569a2..61d75ded 100644 --- a/src/processor/stackwalker_x86.cc +++ b/src/processor/stackwalker_x86.cc @@ -86,7 +86,7 @@ StackwalkerX86::StackwalkerX86(const SystemInfo* system_info, context_(context), cfi_walker_(cfi_register_map_, (sizeof(cfi_register_map_) / sizeof(cfi_register_map_[0]))) { - if (memory_->GetBase() + memory_->GetSize() - 1 > 0xffffffff) { + if (memory_ && memory_->GetBase() + memory_->GetSize() - 1 > 0xffffffff) { // The x86 is a 32-bit CPU, the limits of the supplied stack are invalid. // Mark memory_ = NULL, which will cause stackwalking to fail. BPLOG(ERROR) << "Memory out of range for stackwalking: " << @@ -106,8 +106,8 @@ StackFrameX86::~StackFrameX86() { } StackFrame* StackwalkerX86::GetContextFrame() { - if (!context_ || !memory_) { - BPLOG(ERROR) << "Can't get context frame without context or memory"; + if (!context_) { + BPLOG(ERROR) << "Can't get context frame without context"; return NULL; } diff --git a/src/processor/stackwalker_x86_unittest.cc b/src/processor/stackwalker_x86_unittest.cc index bf76e881..25801a2a 100644 --- a/src/processor/stackwalker_x86_unittest.cc +++ b/src/processor/stackwalker_x86_unittest.cc @@ -181,6 +181,23 @@ TEST_F(GetContextFrame, Simple) { EXPECT_EQ(0, memcmp(&raw_context, &frame->context, sizeof(raw_context))); } +// The stackwalker should be able to produce the context frame even +// without stack memory present. +TEST_F(GetContextFrame, NoStackMemory) { + raw_context.eip = 0x40000200; + raw_context.ebp = 0x80000000; + + StackFrameSymbolizer frame_symbolizer(&supplier, &resolver); + StackwalkerX86 walker(&system_info, &raw_context, NULL, &modules, + &frame_symbolizer); + ASSERT_TRUE(walker.Walk(&call_stack)); + frames = call_stack.frames(); + StackFrameX86 *frame = static_cast(frames->at(0)); + // Check that the values from the original raw context made it + // through to the context in the stack frame. + EXPECT_EQ(0, memcmp(&raw_context, &frame->context, sizeof(raw_context))); +} + class GetCallerFrame: public StackwalkerX86Fixture, public Test { }; // Walk a traditional frame. A traditional frame saves the caller's diff --git a/src/processor/synth_minidump.h b/src/processor/synth_minidump.h index 531603ad..17dbc480 100644 --- a/src/processor/synth_minidump.h +++ b/src/processor/synth_minidump.h @@ -228,6 +228,8 @@ class Context: public Section { // Create a context belonging to DUMP whose contents are a copy of CONTEXT. Context(const Dump &dump, const MDRawContextX86 &context); Context(const Dump &dump, const MDRawContextARM &context); + // Add an empty context to the dump. + Context(const Dump &dump) : Section(dump) {} // Add constructors for other architectures here. Remember to byteswap. };