Refactor the logic of resolving source line info into helper class.

http://breakpad.appspot.com/459002/


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

3 files changed, 52 insertions, 56 deletions

diff --git a/src/google_breakpad/processor/minidump_processor.h b/src/google_breakpad/processor/minidump_processor.h
index 216e9728..5da11cd3 100644
--- a/src/google_breakpad/processor/minidump_processor.h
+++ b/src/google_breakpad/processor/minidump_processor.h
@@ -40,6 +40,7 @@ namespace google_breakpad {
 
 class Minidump;
 class ProcessState;
+class StackFrameSymbolizer;
 class SourceLineResolverInterface;
 class SymbolSupplier;
 struct SystemInfo;
@@ -92,37 +93,44 @@ class MinidumpProcessor {
  public:
   // Initializes this MinidumpProcessor.  supplier should be an
   // implementation of the SymbolSupplier abstract base class.
-  MinidumpProcessor(SymbolSupplier *supplier,
-                    SourceLineResolverInterface *resolver);
+  MinidumpProcessor(SymbolSupplier* supplier,
+                    SourceLineResolverInterface* resolver);
 
   // Initializes the MinidumpProcessor with the option of
   // enabling the exploitability framework to analyze dumps
   // for probable security relevance.
-  MinidumpProcessor(SymbolSupplier *supplier,
-                    SourceLineResolverInterface *resolver,
+  MinidumpProcessor(SymbolSupplier* supplier,
+                    SourceLineResolverInterface* resolver,
+                    bool enable_exploitability);
+
+  // Initializes the MinidumpProcessor with source line resolver helper, and
+  // the option of enabling the exploitability framework to analyze dumps
+  // for probable security relevance.
+  // Does not take ownership of resolver_helper, which must NOT be NULL.
+  MinidumpProcessor(StackFrameSymbolizer* stack_frame_symbolizer,
                     bool enable_exploitability);
 
   ~MinidumpProcessor();
 
   // Processes the minidump file and fills process_state with the result.
   ProcessResult Process(const string &minidump_file,
-                        ProcessState *process_state);
+                        ProcessState* process_state);
 
   // Processes the minidump structure and fills process_state with the
   // result.
-  ProcessResult Process(Minidump *minidump,
-                        ProcessState *process_state);
+  ProcessResult Process(Minidump* minidump,
+                        ProcessState* process_state);
   // Populates the cpu_* fields of the |info| parameter with textual
   // representations of the CPU type that the minidump in |dump| was
   // produced on.  Returns false if this information is not available in
   // the minidump.
-  static bool GetCPUInfo(Minidump *dump, SystemInfo *info);
+  static bool GetCPUInfo(Minidump* dump, SystemInfo* info);
 
   // Populates the os_* fields of the |info| parameter with textual
   // representations of the operating system that the minidump in |dump|
   // was produced on.  Returns false if this information is not available in
   // the minidump.
-  static bool GetOSInfo(Minidump *dump, SystemInfo *info);
+  static bool GetOSInfo(Minidump* dump, SystemInfo* info);
 
   // Returns a textual representation of the reason that a crash occurred,
   // if the minidump in dump was produced as a result of a crash.  Returns
@@ -132,7 +140,7 @@ class MinidumpProcessor {
   // address when the crash was caused by problems such as illegal
   // instructions or divisions by zero, or a data address when the crash
   // was caused by a memory access violation.
-  static string GetCrashReason(Minidump *dump, u_int64_t *address);
+  static string GetCrashReason(Minidump* dump, u_int64_t* address);
 
   // This function returns true if the passed-in error code is
   // something unrecoverable(i.e. retry should not happen).  For
@@ -152,11 +160,12 @@ class MinidumpProcessor {
   // Returns a textual representation of an assertion included
   // in the minidump.  Returns an empty string if this information
   // does not exist or cannot be determined.
-  static string GetAssertion(Minidump *dump);
+  static string GetAssertion(Minidump* dump);
 
  private:
-  SymbolSupplier *supplier_;
-  SourceLineResolverInterface *resolver_;
+  StackFrameSymbolizer* frame_symbolizer_;
+  // Indicate whether resolver_helper_ is owned by this instance.
+  bool own_frame_symbolizer_;
 
   // This flag enables the exploitability scanner which attempts to
   // guess how likely it is that the crash represents an exploitable
diff --git a/src/google_breakpad/processor/stack_frame_cpu.h b/src/google_breakpad/processor/stack_frame_cpu.h
index cc6b0149..90328760 100644
--- a/src/google_breakpad/processor/stack_frame_cpu.h
+++ b/src/google_breakpad/processor/stack_frame_cpu.h
@@ -70,7 +70,7 @@ struct StackFrameX86 : public StackFrame {
     CONTEXT_VALID_ALL  = -1
   };
 
- StackFrameX86()
+  StackFrameX86()
      : context(),
        context_validity(CONTEXT_VALID_NONE),
        windows_frame_info(NULL),
@@ -220,7 +220,7 @@ struct StackFrameARM : public StackFrame {
   // Return the ContextValidity flag for register rN.
   static ContextValidity RegisterValidFlag(int n) {
     return ContextValidity(1 << n);
-  } 
+  }
 
   // Register state.  This is only fully valid for the topmost frame in a
   // stack.  In other frames, the values of nonvolatile registers may be
diff --git a/src/google_breakpad/processor/stackwalker.h b/src/google_breakpad/processor/stackwalker.h
index 6a0c3e72..2979fef8 100644
--- a/src/google_breakpad/processor/stackwalker.h
+++ b/src/google_breakpad/processor/stackwalker.h
@@ -48,19 +48,16 @@
 #include "google_breakpad/common/breakpad_types.h"
 #include "google_breakpad/processor/code_modules.h"
 #include "google_breakpad/processor/memory_region.h"
+#include "google_breakpad/processor/stack_frame_symbolizer.h"
 
 namespace google_breakpad {
 
 class CallStack;
 class MinidumpContext;
-class SourceLineResolverInterface;
-struct StackFrame;
-class SymbolSupplier;
-struct SystemInfo;
+class StackFrameSymbolizer;
 
 using std::set;
 
-
 class Stackwalker {
  public:
   virtual ~Stackwalker() {}
@@ -69,17 +66,17 @@ class Stackwalker {
   // GetCallerFrame.  The frames are further processed to fill all available
   // data.  Returns true if the stackwalk completed, or false if it was
   // interrupted by SymbolSupplier::GetSymbolFile().
-  bool Walk(CallStack *stack);
+  bool Walk(CallStack* stack);
 
   // Returns a new concrete subclass suitable for the CPU that a stack was
   // generated on, according to the CPU type indicated by the context
   // argument.  If no suitable concrete subclass exists, returns NULL.
-  static Stackwalker* StackwalkerForCPU(const SystemInfo *system_info,
-                                        MinidumpContext *context,
-                                        MemoryRegion *memory,
-                                        const CodeModules *modules,
-                                        SymbolSupplier *supplier,
-                                        SourceLineResolverInterface *resolver);
+  static Stackwalker* StackwalkerForCPU(
+     const SystemInfo* system_info,
+     MinidumpContext* context,
+     MemoryRegion* memory,
+     const CodeModules* modules,
+     StackFrameSymbolizer* resolver_helper);
 
   static void set_max_frames(u_int32_t max_frames) { max_frames_ = max_frames; }
   static u_int32_t max_frames() { return max_frames_; }
@@ -89,16 +86,15 @@ class Stackwalker {
   // memory identifies a MemoryRegion that provides the stack memory
   // for the stack to walk.  modules, if non-NULL, is a CodeModules
   // object that is used to look up which code module each stack frame is
-  // associated with.  supplier is an optional caller-supplied SymbolSupplier
-  // implementation.  If supplier is NULL, source line info will not be
-  // resolved.  resolver is an instance of SourceLineResolverInterface
-  // (see source_line_resolver_interface.h and basic_source_line_resolver.h).
-  // If resolver is NULL, source line info will not be resolved.
-  Stackwalker(const SystemInfo *system_info,
-              MemoryRegion *memory,
-              const CodeModules *modules,
-              SymbolSupplier *supplier,
-              SourceLineResolverInterface *resolver);
+  // associated with.  frame_symbolizer is a StackFrameSymbolizer object that
+  // encapsulates the logic of how source line resolver interacts with symbol
+  // supplier to symbolize stack frame and look up caller frame information
+  // (see stack_frame_symbolizer.h).
+  // frame_symbolizer MUST NOT be NULL (asserted).
+  Stackwalker(const SystemInfo* system_info,
+              MemoryRegion* memory,
+              const CodeModules* modules,
+              StackFrameSymbolizer* frame_symbolizer);
 
   // This can be used to filter out potential return addresses when
   // the stack walker resorts to stack scanning.
@@ -112,8 +108,8 @@ class Stackwalker {
 
   template<typename InstructionType>
   bool ScanForReturnAddress(InstructionType location_start,
-                            InstructionType *location_found,
-                            InstructionType *ip_found) {
+                            InstructionType* location_found,
+                            InstructionType* ip_found) {
     const int kRASearchWords = 30;
     return ScanForReturnAddress(location_start, location_found, ip_found,
                                 kRASearchWords);
@@ -130,8 +126,8 @@ class Stackwalker {
   // location in memory.
   template<typename InstructionType>
   bool ScanForReturnAddress(InstructionType location_start,
-                            InstructionType *location_found,
-                            InstructionType *ip_found,
+                            InstructionType* location_found,
+                            InstructionType* ip_found,
                             int searchwords) {
     for (InstructionType location = location_start;
          location <= location_start + searchwords * sizeof(InstructionType);
@@ -142,7 +138,6 @@ class Stackwalker {
 
       if (modules_ && modules_->GetModuleForAddress(ip) &&
           InstructionAddressSeemsValid(ip)) {
-
         *ip_found = ip;
         *location_found = location;
         return true;
@@ -154,19 +149,19 @@ class Stackwalker {
 
   // Information about the system that produced the minidump.  Subclasses
   // and the SymbolSupplier may find this information useful.
-  const SystemInfo *system_info_;
+  const SystemInfo* system_info_;
 
   // The stack memory to walk.  Subclasses will require this region to
   // get information from the stack.
-  MemoryRegion *memory_;
+  MemoryRegion* memory_;
 
   // A list of modules, for populating each StackFrame's module information.
   // This field is optional and may be NULL.
-  const CodeModules *modules_;
+  const CodeModules* modules_;
 
  protected:
-  // The SourceLineResolver implementation.
-  SourceLineResolverInterface *resolver_;
+  // The StackFrameSymbolizer implementation.
+  StackFrameSymbolizer* frame_symbolizer_;
 
  private:
   // Obtains the context frame, the innermost called procedure in a stack
@@ -183,15 +178,7 @@ class Stackwalker {
   // the end of the stack has been reached).  GetCallerFrame allocates a new
   // StackFrame (or StackFrame subclass), ownership of which is taken by
   // the caller.
-  virtual StackFrame* GetCallerFrame(const CallStack *stack) = 0;
-
-  // The optional SymbolSupplier for resolving source line info.
-  SymbolSupplier *supplier_;
-
-  // A list of modules that we haven't found symbols for.  We track
-  // this in order to avoid repeatedly looking them up again within
-  // one minidump.
-  set<string> no_symbol_modules_;
+  virtual StackFrame* GetCallerFrame(const CallStack* stack) = 0;
 
   // The maximum number of frames Stackwalker will walk through.
   // This defaults to 1024 to prevent infinite loops.