From fd18beeb5c817aa3ecdb21caceee8e6ce08c6ab3 Mon Sep 17 00:00:00 2001 From: jimblandy Date: Fri, 11 Mar 2011 22:16:12 +0000 Subject: [PATCH] Google Breakpad Issue 417: Handle DWARF that omits function names. This patch makes sure dump_syms behaves properly when presented with malformed DWARF data that provides no name for a function. We print a warning message to stderr, and subsitute "" for the empty string, so that the "FUNC" record written to the symbol file for the function is still well-formed. (We may have line number data covering the function, so it would be a shame to omit the function altogether.) Unit tests included. a=jimblandy, r=ted.mielczarek git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@779 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/common/dwarf_cu_to_module.cc | 15 +++++++++++++- src/common/dwarf_cu_to_module.h | 5 +++++ src/common/dwarf_cu_to_module_unittest.cc | 25 ++++++++++++++++++++++- src/common/module.cc | 4 ++++ src/common/module.h | 2 +- 5 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/common/dwarf_cu_to_module.cc b/src/common/dwarf_cu_to_module.cc index 425fd332..1922066a 100644 --- a/src/common/dwarf_cu_to_module.cc +++ b/src/common/dwarf_cu_to_module.cc @@ -428,7 +428,14 @@ void DwarfCUToModule::FuncHandler::Finish() { // Create a Module::Function based on the data we've gathered, and // add it to the functions_ list. Module::Function *func = new Module::Function; - func->name = name_; + // Malformed DWARF may omit the name, but all Module::Functions must + // have names. + if (!name_.empty()) { + func->name = name_; + } else { + cu_context_->reporter->UnnamedFunction(offset_); + func->name = ""; + } func->address = low_pc_; func->size = high_pc_ - low_pc_; func->parameter_size = 0; @@ -543,6 +550,12 @@ void DwarfCUToModule::WarningReporter::UncoveredLine(const Module::Line &line) { line.file->name.c_str(), line.number, line.address); } +void DwarfCUToModule::WarningReporter::UnnamedFunction(uint64 offset) { + CUHeading(); + fprintf(stderr, "%s: warning: function at offset 0x%" PRIx64 " has no name\n", + filename_.c_str(), offset); +} + DwarfCUToModule::DwarfCUToModule(FileContext *file_context, LineToModuleFunctor *line_reader, WarningReporter *reporter) diff --git a/src/common/dwarf_cu_to_module.h b/src/common/dwarf_cu_to_module.h index 2b59350f..a262f3b5 100644 --- a/src/common/dwarf_cu_to_module.h +++ b/src/common/dwarf_cu_to_module.h @@ -158,6 +158,11 @@ class DwarfCUToModule: public dwarf2reader::RootDIEHandler { // covered by no function. virtual void UncoveredLine(const Module::Line &line); + // The DW_TAG_subprogram DIE at OFFSET has no name specified directly + // in the DIE, nor via a DW_AT_specification or DW_AT_abstract_origin + // link. + virtual void UnnamedFunction(uint64 offset); + protected: string filename_; uint64 cu_offset_; diff --git a/src/common/dwarf_cu_to_module_unittest.cc b/src/common/dwarf_cu_to_module_unittest.cc index f0a478dd..2930ff5a 100644 --- a/src/common/dwarf_cu_to_module_unittest.cc +++ b/src/common/dwarf_cu_to_module_unittest.cc @@ -80,6 +80,7 @@ class MockWarningReporter: public DwarfCUToModule::WarningReporter { MOCK_METHOD1(BadLineInfoOffset, void(uint64 offset)); MOCK_METHOD1(UncoveredFunction, void(const Module::Function &function)); MOCK_METHOD1(UncoveredLine, void(const Module::Line &line)); + MOCK_METHOD1(UnnamedFunction, void(uint64 offset)); }; // A fixture class including all the objects needed to handle a @@ -135,6 +136,7 @@ class CUFixtureBase { EXPECT_CALL(reporter_, BadLineInfoOffset(_)).Times(0); EXPECT_CALL(reporter_, UncoveredFunction(_)).Times(0); EXPECT_CALL(reporter_, UncoveredLine(_)).Times(0); + EXPECT_CALL(reporter_, UnnamedFunction(_)).Times(0); // By default, expect the line program reader not to be invoked. We // may override this in StartCU. @@ -776,6 +778,8 @@ TEST_F(SimpleCU, AbstractOriginNotInlined) { TEST_F(SimpleCU, UnknownAbstractOrigin) { EXPECT_CALL(reporter_, UnknownAbstractOrigin(_, 1ULL)).WillOnce(Return()); + EXPECT_CALL(reporter_, UnnamedFunction(0x11c70f94c6e87ccdLL)) + .WillOnce(Return()); PushLine(0x1758a0f941b71efbULL, 0x1cf154f1f545e146ULL, "line-file", 75173118); StartCU(); @@ -786,10 +790,25 @@ TEST_F(SimpleCU, UnknownAbstractOrigin) { root_handler_.Finish(); TestFunctionCount(1); - TestFunction(0, "", + TestFunction(0, "", 0x1758a0f941b71efbULL, 0x1cf154f1f545e146ULL); } +TEST_F(SimpleCU, UnnamedFunction) { + EXPECT_CALL(reporter_, UnnamedFunction(0xe34797c7e68590a8LL)) + .WillOnce(Return()); + PushLine(0x72b80e41a0ac1d40ULL, 0x537174f231ee181cULL, "line-file", 14044850); + + StartCU(); + DefineFunction(&root_handler_, "", + 0x72b80e41a0ac1d40ULL, 0x537174f231ee181cULL); + root_handler_.Finish(); + + TestFunctionCount(1); + TestFunction(0, "", + 0x72b80e41a0ac1d40ULL, 0x537174f231ee181cULL); +} + // An address range. struct Range { Module::Address start, end; @@ -1697,5 +1716,9 @@ TEST_F(Reporter, UncoveredLineEnabled) { EXPECT_TRUE(reporter.uncovered_warnings_enabled()); } +TEST_F(Reporter, UnnamedFunction) { + reporter.UnnamedFunction(0x90c0baff9dedb2d9ULL); +} + // Would be nice to also test: // - overlapping lines, functions diff --git a/src/common/module.cc b/src/common/module.cc index 0c36516a..ffaeb87a 100644 --- a/src/common/module.cc +++ b/src/common/module.cc @@ -33,6 +33,7 @@ #include "common/module.h" +#include #include #include @@ -65,6 +66,9 @@ void Module::SetLoadAddress(Address address) { } void Module::AddFunction(Function *function) { + // FUNC lines must not hold an empty name, so catch the problem early if + // callers try to add one. + assert(!function->name.empty()); std::pair ret = functions_.insert(function); if (!ret.second) { // Free the duplicate that was not inserted because this Module diff --git a/src/common/module.h b/src/common/module.h index 83e8403c..8e6a05e2 100644 --- a/src/common/module.h +++ b/src/common/module.h @@ -189,7 +189,7 @@ class Module { // Write is used. void SetLoadAddress(Address load_address); - // Add FUNCTION to the module. + // Add FUNCTION to the module. FUNCTION's name must not be empty. // This module owns all Function objects added with this function: // destroying the module destroys them as well. void AddFunction(Function *function);