Make SourceLineResolver fill a StackFrame rather than using its own struct (#16), r=mmentovai.

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

src/google/stack_frame.h

@@ -59,7 +59,8 @@ struct StackFrame {
   // The source file name, may be omitted if debug symbols are not available
   string source_file_name;
 
-  // The source line number, may be omitted if debug symbols are not available
+  // The (1-based) source line number,
+  // may be omitted if debug symbols are not available
   int source_line;
 
   // TODO(bryner): saved registers

src/processor/source_line_resolver.cc

@@ -18,6 +18,7 @@
 #include <vector>
 #include <utility>
 #include "processor/source_line_resolver.h"
+#include "google/stack_frame.h"
 
 using std::map;
 using std::vector;
@@ -26,12 +27,6 @@ using __gnu_cxx::hash;
 
 namespace google_airbag {
 
-void SourceLineResolver::SourceLineInfo::Reset() {
-  function_name.clear();
-  source_file.clear();
-  source_line = 0;
-}
-
 // MemAddrMap is a map subclass which has the following properties:
 //  - stores pointers to an "entry" type, which are deleted on destruction
 //  - suitable for address lookup via FindContainingEntry
@@ -97,9 +92,9 @@ class SourceLineResolver::Module {
   // Loads the given map file, returning true on success.
   bool LoadMap(const string &map_file);
 
-  // Looks up the given relative address, and fills the SourceLineInfo struct
+  // Looks up the given relative address, and fills the StackFrame struct
   // with the result.
-  void LookupAddress(MemAddr address, SourceLineInfo *info) const;
+  void LookupAddress(MemAddr address, StackFrame *frame) const;
 
  private:
   friend class SourceLineResolver;
@@ -147,13 +142,10 @@ bool SourceLineResolver::LoadModule(const string &module_name,
   return true;
 }
 
-void SourceLineResolver::LookupAddress(MemAddr address,
-                                       const string &module_name,
-                                       SourceLineInfo *info) const {
-  info->Reset();
-  ModuleMap::const_iterator it = modules_->find(module_name);
+void SourceLineResolver::FillSourceLineInfo(StackFrame *frame) const {
+  ModuleMap::const_iterator it = modules_->find(frame->module_name);
   if (it != modules_->end()) {
-    it->second->LookupAddress(address, info);
+    it->second->LookupAddress(frame->instruction, frame);
   }
 }
 
@@ -192,13 +184,13 @@ bool SourceLineResolver::Module::LoadMap(const string &map_file) {
 }
 
 void SourceLineResolver::Module::LookupAddress(MemAddr address,
-                                               SourceLineInfo *info) const {
+                                               StackFrame *frame) const {
   Function *func = functions_.FindContainingEntry(address);
   if (!func) {
     return;
   }
 
-  info->function_name = func->name;
+  frame->function_name = func->name;
   Line *line = func->lines.FindContainingEntry(address);
   if (!line) {
     return;
@@ -206,9 +198,9 @@ void SourceLineResolver::Module::LookupAddress(MemAddr address,
 
   FileMap::const_iterator it = files_.find(line->source_file_id);
   if (it != files_.end()) {
-    info->source_file = files_.find(line->source_file_id)->second;
+    frame->source_file_name = files_.find(line->source_file_id)->second;
   }
-  info->source_line = line->line;
+  frame->source_line = line->line;
 }
 
 void SourceLineResolver::Module::ParseFile(char *file_line) {

src/processor/source_line_resolver.h

@@ -19,7 +19,6 @@
 #ifndef PROCESSOR_SOURCE_LINE_RESOLVER_H__
 #define PROCESSOR_SOURCE_LINE_RESOLVER_H__
 
-#include "config.h"
 #include <string>
 #include <ext/hash_map>
 
@@ -28,25 +27,12 @@ namespace google_airbag {
 using std::string;
 using __gnu_cxx::hash_map;
 
+class StackFrame;
+
 class SourceLineResolver {
  public:
   typedef unsigned long long MemAddr;
 
-  // A struct that gives source file information for a memory address.
-  struct SourceLineInfo {
-    // Resets all fields to their default empty values
-    void Reset();
-
-    // The function name, for example Foo::Foo()
-    string function_name;
-
-    // The source file, for example C:\foo\bar.cc
-    string source_file;
-
-    // The line number within the source file (1-based)
-    int source_line;
-  };
-
   SourceLineResolver();
   ~SourceLineResolver();
 
@@ -58,11 +44,10 @@ class SourceLineResolver {
   // map_file should contain line/address mappings for this module.
   bool LoadModule(const string &module_name, const string &map_file);
 
-  // Determines the source line for the given address, and fills info
-  // with the result.  module_name must match a module name that was
-  // passed to LoadModule().  The address should be module-relative.
-  void LookupAddress(MemAddr address, const string &module_name,
-                     SourceLineInfo *info) const;
+  // Fills in the function_base, function_name, source_file_name,
+  // and source_line fields of the StackFrame.  The instruction and
+  // module_name fields must already be filled in.
+  void FillSourceLineInfo(StackFrame *frame) const;
 
  private:
   template<class T> class MemAddrMap;

src/processor/source_line_resolver_unittest.cc

@@ -15,9 +15,11 @@
 #include <stdio.h>
 #include <string>
 #include "processor/source_line_resolver.h"
+#include "google/stack_frame.h"
 
 using std::string;
 using google_airbag::SourceLineResolver;
+using google_airbag::StackFrame;
 
 #define ASSERT_TRUE(cond) \
   if (!(cond)) {                                                        \
@@ -29,13 +31,19 @@ using google_airbag::SourceLineResolver;
 
 #define ASSERT_EQ(e1, e2) ASSERT_TRUE((e1) == (e2))
 
-static bool VerifyEmpty(const SourceLineResolver::SourceLineInfo &info) {
-  ASSERT_TRUE(info.function_name.empty());
-  ASSERT_TRUE(info.source_file.empty());
-  ASSERT_EQ(info.source_line, 0);
+static bool VerifyEmpty(const StackFrame &frame) {
+  ASSERT_TRUE(frame.function_name.empty());
+  ASSERT_TRUE(frame.source_file_name.empty());
+  ASSERT_EQ(frame.source_line, 0);
   return true;
 }
 
+static void ClearSourceLineInfo(StackFrame *frame) {
+  frame->function_name.clear();
+  frame->source_file_name.clear();
+  frame->source_line = 0;
+}
+
 static bool RunTests() {
   string testdata_dir = string(getenv("srcdir") ? getenv("srcdir") : ".") +
                         "/src/processor/testdata";
@@ -44,32 +52,37 @@ static bool RunTests() {
   ASSERT_TRUE(resolver.LoadModule("module1", testdata_dir + "/module1.out"));
   ASSERT_TRUE(resolver.LoadModule("module2", testdata_dir + "/module2.out"));
 
-  SourceLineResolver::SourceLineInfo info;
-  resolver.LookupAddress(0x1000, "module1", &info);
-  ASSERT_EQ(info.function_name, "Function1_1");
-  ASSERT_EQ(info.source_file, "file1_1.cc");
-  ASSERT_EQ(info.source_line, 44);
+  StackFrame frame;
+  frame.instruction = 0x1000;
+  frame.module_name = "module1";
+  resolver.FillSourceLineInfo(&frame);
+  ASSERT_EQ(frame.function_name, "Function1_1");
+  ASSERT_EQ(frame.source_file_name, "file1_1.cc");
+  ASSERT_EQ(frame.source_line, 44);
 
-  info.Reset();
-  ASSERT_TRUE(VerifyEmpty(info));
+  ClearSourceLineInfo(&frame);
+  frame.instruction = 0x800;
+  resolver.FillSourceLineInfo(&frame);
+  ASSERT_TRUE(VerifyEmpty(frame));
 
-  resolver.LookupAddress(0x800, "module1", &info);
-  ASSERT_TRUE(VerifyEmpty(info));
+  frame.instruction = 0x1280;
+  resolver.FillSourceLineInfo(&frame);
+  ASSERT_EQ(frame.function_name, "Function1_3");
+  ASSERT_TRUE(frame.source_file_name.empty());
+  ASSERT_EQ(frame.source_line, 0);
 
-  resolver.LookupAddress(0x1280, "module1", &info);
-  ASSERT_EQ(info.function_name, "Function1_3");
-  ASSERT_TRUE(info.source_file.empty());
-  ASSERT_EQ(info.source_line, 0);
+  frame.instruction = 0x1380;
+  resolver.FillSourceLineInfo(&frame);
+  ASSERT_EQ(frame.function_name, "Function1_4");
+  ASSERT_TRUE(frame.source_file_name.empty());
+  ASSERT_EQ(frame.source_line, 0);
 
-  resolver.LookupAddress(0x1380, "module1", &info);
-  ASSERT_EQ(info.function_name, "Function1_4");
-  ASSERT_TRUE(info.source_file.empty());
-  ASSERT_EQ(info.source_line, 0);
-
-  resolver.LookupAddress(0x2180, "module2", &info);
-  ASSERT_EQ(info.function_name, "Function2_2");
-  ASSERT_EQ(info.source_file, "file2_2.cc");
-  ASSERT_EQ(info.source_line, 21);
+  frame.instruction = 0x2180;
+  frame.module_name = "module2";
+  resolver.FillSourceLineInfo(&frame);
+  ASSERT_EQ(frame.function_name, "Function2_2");
+  ASSERT_EQ(frame.source_file_name, "file2_2.cc");
+  ASSERT_EQ(frame.source_line, 21);
 
   ASSERT_FALSE(resolver.LoadModule("module3",
                                    testdata_dir + "/module3_bad.out"));