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 "<name omitted>" 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

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 = "<name omitted>";
+    }
     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, "<name omitted>",
                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, "<name omitted>",
+               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 <assert.h>
 #include <errno.h>
 #include <string.h>
 
@@ -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<FunctionSet::iterator,bool> 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);