Breakpad STABS reader: Properly compute function end addresses.

An N_FUN stabs with no name is an explicit end-of-function marker, whose
value is the size of the function. This patch changes the stabs reader to
recognize these and use them to compute the function's ending address,
instead of treating them as functions with no names and mysterious
addresses. It also adds appropriate unit tests.

a=jimblandy, r=thestig


git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@585 4c0a9323-5329-0410-9bdc-e9ce6186880e

3 files changed, 72 insertions, 7 deletions

diff --git a/src/common/stabs_reader.cc b/src/common/stabs_reader.cc
index add2a8dd..cdad80e7 100644
--- a/src/common/stabs_reader.cc
+++ b/src/common/stabs_reader.cc
@@ -218,13 +218,31 @@ bool StabsReader::ProcessFunction() {
       ++iterator_;
   }
 
-  // If there is a subsequent N_SO or N_FUN entry, its address is our
-  // end address.
+  // We've reached the end of the function. See if we can figure out its
+  // ending address.
   uint64_t ending_address = 0;
   if (!iterator_->at_end) {
     assert(iterator_->type == N_SO || iterator_->type == N_FUN);
-    ending_address = iterator_->value;
-    // Note: we do not advance iterator_ here, since we haven't consumed it.
+    if (iterator_->type == N_FUN) {
+      const char *name = SymbolString();
+      if (name[0] == '\0') {
+        // An N_FUN entry with no name is a terminator for this function;
+        // its value is the function's size.
+        ending_address = function_address + iterator_->value;
+        ++iterator_;
+      } else {
+        // An N_FUN entry with a name is the next function, and we can take
+        // its value as our ending address. Don't advance the iterator, as
+        // we'll use this symbol to start the next function as well.
+        ending_address = iterator_->value;
+      }
+    } else {
+      // An N_SO entry could be an end-of-compilation-unit marker, or the
+      // start of the next compilation unit, but in either case, its value
+      // is our ending address. We don't advance the iterator;
+      // ProcessCompilationUnit will decide what to do with this symbol.
+      ending_address = iterator_->value;
+    }
   }
 
   if (! handler_->EndFunction(ending_address))
diff --git a/src/common/stabs_reader_unittest.cc b/src/common/stabs_reader_unittest.cc
index 74897c46..df37995c 100644
--- a/src/common/stabs_reader_unittest.cc
+++ b/src/common/stabs_reader_unittest.cc
@@ -455,6 +455,49 @@ TEST(StabsReader, MultipleCUs) {
   ASSERT_TRUE(ApplyHandlerToMockStabsData(&stabs, &strings, &mock_handler));
 }
 
+TEST_F(Stabs, FunctionEnd) {
+  stabs.set_endianness(kLittleEndian);
+  stabs.set_value_size(8);
+  stabs
+      .Stab(N_SO,    102, 62362, 0x52a830d644cd6942ULL, "compilation unit")
+      // This function is terminated by the start of the next function.
+      .Stab(N_FUN,   216, 38405, 0xbb5ab70ecdd23bfeULL, "function 1")
+      // This function is terminated by an explicit end-of-function stab,
+      // whose value is a size in bytes.
+      .Stab(N_FUN,   240, 10973, 0xc954de9b8fb3e5e2ULL, "function 2")
+      .Stab(N_FUN,    14, 36749, 0xc1ab,     "")
+      // This function is terminated by the end of the compilation unit.
+      .Stab(N_FUN,   143, 64514, 0xdff98c9a35386e1fULL, "function 3")
+      .Stab(N_SO,    164, 60142, 0xfdacb856e78bbf57ULL, "");
+
+  {
+    InSequence s;
+    EXPECT_CALL(mock_handler,
+                StartCompilationUnit(StrEq("compilation unit"),
+                                     0x52a830d644cd6942ULL, NULL))
+        .WillOnce(Return(true));
+    EXPECT_CALL(mock_handler,
+                StartFunction(Eq("function 1"), 0xbb5ab70ecdd23bfeULL))
+        .WillOnce(Return(true));
+    EXPECT_CALL(mock_handler, EndFunction(0xc954de9b8fb3e5e2ULL))
+        .WillOnce(Return(true));
+    EXPECT_CALL(mock_handler,
+                StartFunction(Eq("function 2"), 0xc954de9b8fb3e5e2ULL))
+        .WillOnce(Return(true));
+    EXPECT_CALL(mock_handler, EndFunction(0xc954de9b8fb3e5e2ULL + 0xc1ab))
+        .WillOnce(Return(true));
+    EXPECT_CALL(mock_handler,
+                StartFunction(Eq("function 3"), 0xdff98c9a35386e1fULL))
+        .WillOnce(Return(true));
+    EXPECT_CALL(mock_handler, EndFunction(0xfdacb856e78bbf57ULL))
+        .WillOnce(Return(true));
+    EXPECT_CALL(mock_handler, EndCompilationUnit(0xfdacb856e78bbf57ULL))
+        .WillOnce(Return(true));
+  }
+
+  ASSERT_TRUE(ApplyHandlerToMockStabsData());
+}
+
 // name duplication
 
 } // anonymous namespace
diff --git a/src/common/stabs_to_module.cc b/src/common/stabs_to_module.cc
index 858d0dff..fbe4c02f 100644
--- a/src/common/stabs_to_module.cc
+++ b/src/common/stabs_to_module.cc
@@ -107,9 +107,13 @@ bool StabsToModule::EndFunction(uint64_t address) {
   // of duplicated entries for functions in the STABS data; only one
   // entry can meet this requirement.
   //
-  // (I don't really understand the above comment; just bringing it
-  // along from the previous code, and leaving the behaivor unchanged.
-  // If you know the whole story, please patch this comment.  --jimb)
+  // (I don't really understand the above comment; just bringing it along
+  // from the previous code, and leaving the behavior unchanged. GCC marks
+  // the end of each function with an N_FUN entry with no name, whose value
+  // is the size of the function; perhaps this test was concerned with
+  // skipping those. Now StabsReader interprets them properly. If you know
+  // the whole story, please patch this comment. --jimb)
+  //
   if (current_function_->address >= comp_unit_base_address_)
     functions_.push_back(current_function_);
   else