<rdar://problem/13443931>

Fixed a crasher in the SourceManager where it wasn't checking the m_target member variable for NULL.

In doing this fix, I hardened this class to have weak pointers to the debugger and target in case they do go away. I also changed SBSourceManager to hold onto weak pointers to the debugger and target so they don't keep objects alive by holding a strong reference to them.

llvm-svn: 177365

4 files changed, 106 insertions, 66 deletions

diff --git a/lldb/source/API/SBSourceManager.cpp b/lldb/source/API/SBSourceManager.cpp
index caa1b0c1b52..0b8cbfceda0 100644
--- a/lldb/source/API/SBSourceManager.cpp
+++ b/lldb/source/API/SBSourceManager.cpp
@@ -27,22 +27,24 @@ namespace lldb_private
     class SourceManagerImpl
     {
     public:
-        SourceManagerImpl (const lldb::DebuggerSP &debugger_sp)
+        SourceManagerImpl (const lldb::DebuggerSP &debugger_sp) :
+            m_debugger_wp (debugger_sp),
+            m_target_wp ()
         {
-            m_debugger_sp = debugger_sp;
         }
         
-        SourceManagerImpl (const lldb::TargetSP &target_sp)
+        SourceManagerImpl (const lldb::TargetSP &target_sp) :
+            m_debugger_wp (),
+            m_target_wp (target_sp)
         {
-            m_target_sp = target_sp;
         }
         
         SourceManagerImpl (const SourceManagerImpl &rhs)
         {
             if (&rhs == this)
                 return;
-            m_debugger_sp = rhs.m_debugger_sp;
-            m_target_sp   = rhs.m_target_sp;
+            m_debugger_wp = rhs.m_debugger_wp;
+            m_target_wp   = rhs.m_target_wp;
         }
         
         size_t
@@ -56,27 +58,35 @@ namespace lldb_private
             if (!file)
                 return 0;
             
-            if (m_debugger_sp)
-                return m_debugger_sp->GetSourceManager().DisplaySourceLinesWithLineNumbers (file,
-                                                                                            line,
-                                                                                            context_before,
-                                                                                            context_after,
-                                                                                            current_line_cstr,
-                                                                                            s);
-            else if (m_target_sp)
-                return m_target_sp->GetSourceManager().DisplaySourceLinesWithLineNumbers (file,
-                                                                                          line,
-                                                                                          context_before,
-                                                                                          context_after,
-                                                                                          current_line_cstr,
-                                                                                          s);
+            lldb::TargetSP target_sp (m_target_wp.lock());
+            if (target_sp)
+            {
+                return target_sp->GetSourceManager().DisplaySourceLinesWithLineNumbers (file,
+                                                                                        line,
+                                                                                        context_before,
+                                                                                        context_after,
+                                                                                        current_line_cstr,
+                                                                                        s);
+            }
             else
-                return 0;
+            {
+                lldb::DebuggerSP debugger_sp (m_debugger_wp.lock());
+                if (debugger_sp)
+                {
+                    return debugger_sp->GetSourceManager().DisplaySourceLinesWithLineNumbers (file,
+                                                                                              line,
+                                                                                              context_before,
+                                                                                              context_after,
+                                                                                              current_line_cstr,
+                                                                                              s);
+                }
+            }
+            return 0;
         }
         
     private:
-        lldb::DebuggerSP m_debugger_sp;
-        lldb::TargetSP   m_target_sp;
+        lldb::DebuggerWP m_debugger_wp;
+        lldb::TargetWP   m_target_wp;
         
     };
 }
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 327965c1afd..8bc3ab09fe7 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -554,7 +554,7 @@ Debugger::Debugger (lldb::LogOutputCallback log_callback, void *baton) :
     m_target_list (*this),
     m_platform_list (),
     m_listener ("lldb.Debugger"),
-    m_source_manager(*this),
+    m_source_manager_ap(),
     m_source_file_cache(),
     m_command_interpreter_ap (new CommandInterpreter (*this, eScriptLanguageDefault, false)),
     m_input_reader_stack (),
@@ -2651,3 +2651,12 @@ Debugger::EnableLog (const char *channel, const char **categories, const char *l
     return false;
 }
 
+SourceManager &
+Debugger::GetSourceManager ()
+{
+    if (m_source_manager_ap.get() == NULL)
+        m_source_manager_ap.reset (new SourceManager (shared_from_this()));
+    return *m_source_manager_ap;
+}
+
+
diff --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp
index 4267da3e467..9f289348fd9 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -25,8 +25,10 @@
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/Target.h"
 
+using namespace lldb;
 using namespace lldb_private;
 
+
 static inline bool is_newline_char(char ch)
 {
     return ch == '\n' || ch == '\r';
@@ -36,24 +38,23 @@ static inline bool is_newline_char(char ch)
 //----------------------------------------------------------------------
 // SourceManager constructor
 //----------------------------------------------------------------------
-SourceManager::SourceManager(Target &target) :
+SourceManager::SourceManager(const TargetSP &target_sp) :
     m_last_file_sp (),
     m_last_line (0),
     m_last_count (0),
     m_default_set(false),
-    m_target (&target),
-    m_debugger(NULL)
+    m_target_wp (target_sp),
+    m_debugger_wp(target_sp->GetDebugger().shared_from_this())
 {
-    m_debugger = &(m_target->GetDebugger());
 }
 
-SourceManager::SourceManager(Debugger &debugger) :
+SourceManager::SourceManager(const DebuggerSP &debugger_sp) :
     m_last_file_sp (),
     m_last_line (0),
     m_last_count (0),
     m_default_set(false),
-    m_target (NULL),
-    m_debugger (&debugger)
+    m_target_wp (),
+    m_debugger_wp (debugger_sp)
 {
 }
 
@@ -69,23 +70,27 @@ SourceManager::GetFile (const FileSpec &file_spec)
 {
     bool same_as_previous = m_last_file_sp && m_last_file_sp->FileSpecMatches (file_spec);
 
+    DebuggerSP debugger_sp (m_debugger_wp.lock());
     FileSP file_sp;
     if (same_as_previous)
         file_sp = m_last_file_sp;
-    else
-        file_sp = m_debugger->GetSourceFileCache().FindSourceFile (file_spec);
+    else if (debugger_sp)
+        file_sp = debugger_sp->GetSourceFileCache().FindSourceFile (file_spec);
     
+    TargetSP target_sp (m_target_wp.lock());
+
     // It the target source path map has been updated, get this file again so we
     // can successfully remap the source file
-    if (file_sp && file_sp->GetSourceMapModificationID() != m_target->GetSourcePathMap().GetModificationID())
+    if (target_sp && file_sp && file_sp->GetSourceMapModificationID() != target_sp->GetSourcePathMap().GetModificationID())
         file_sp.reset();
 
     // If file_sp is no good or it points to a non-existent file, reset it.
     if (!file_sp || !file_sp->GetFileSpec().Exists())
     {
-        file_sp.reset (new File (file_spec, m_target));
+        file_sp.reset (new File (file_spec, target_sp.get()));
 
-        m_debugger->GetSourceFileCache().AddSourceFile(file_sp);
+        if (debugger_sp)
+            debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
     }
     return file_sp;
 }
@@ -195,6 +200,7 @@ SourceManager::DisplayMoreWithLineNumbers (Stream *s,
                                            const SymbolContextList *bp_locs)
 {
     // If we get called before anybody has set a default file and line, then try to figure it out here.
+    const bool have_default_file_line = m_last_file_sp && m_last_line > 0;
     if (!m_default_set)
     {
         FileSpec tmp_spec;
@@ -226,7 +232,7 @@ SourceManager::DisplayMoreWithLineNumbers (Stream *s,
                 else
                     m_last_line = 1;
             }
-            else
+            else if (have_default_file_line)
                 m_last_line += m_last_count;
         }
         else
@@ -267,38 +273,43 @@ SourceManager::GetDefaultFileAndLine (FileSpec &file_spec, uint32_t &line)
     }
     else if (!m_default_set)
     {
-        // If nobody has set the default file and line then try here.  If there's no executable, then we
-        // will try again later when there is one.  Otherwise, if we can't find it we won't look again,
-        // somebody will have to set it (for instance when we stop somewhere...)
-        Module *executable_ptr = m_target->GetExecutableModulePointer();
-        if (executable_ptr)
+        TargetSP target_sp (m_target_wp.lock());
+
+        if (target_sp)
         {
-            SymbolContextList sc_list;
-            ConstString main_name("main");
-            bool symbols_okay = false;  // Force it to be a debug symbol.
-            bool inlines_okay = true;
-            bool append = false;
-            size_t num_matches = executable_ptr->FindFunctions (main_name,
-                                                                NULL,
-                                                                lldb::eFunctionNameTypeBase,
-                                                                inlines_okay,
-                                                                symbols_okay,
-                                                                append,
-                                                                sc_list);
-            for (size_t idx = 0; idx < num_matches; idx++)
+            // If nobody has set the default file and line then try here.  If there's no executable, then we
+            // will try again later when there is one.  Otherwise, if we can't find it we won't look again,
+            // somebody will have to set it (for instance when we stop somewhere...)
+            Module *executable_ptr = target_sp->GetExecutableModulePointer();
+            if (executable_ptr)
             {
-                SymbolContext sc;
-                sc_list.GetContextAtIndex(idx, sc);
-                if (sc.function)
+                SymbolContextList sc_list;
+                ConstString main_name("main");
+                bool symbols_okay = false;  // Force it to be a debug symbol.
+                bool inlines_okay = true;
+                bool append = false;
+                size_t num_matches = executable_ptr->FindFunctions (main_name,
+                                                                    NULL,
+                                                                    lldb::eFunctionNameTypeBase,
+                                                                    inlines_okay,
+                                                                    symbols_okay,
+                                                                    append,
+                                                                    sc_list);
+                for (size_t idx = 0; idx < num_matches; idx++)
                 {
-                    lldb_private::LineEntry line_entry;
-                    if (sc.function->GetAddressRange().GetBaseAddress().CalculateSymbolContextLineEntry (line_entry))
+                    SymbolContext sc;
+                    sc_list.GetContextAtIndex(idx, sc);
+                    if (sc.function)
                     {
-                        SetDefaultFileAndLine (line_entry.file, 
-                                               line_entry.line);
-                        file_spec = m_last_file_sp->GetFileSpec();
-                        line = m_last_line;
-                        return true;
+                        lldb_private::LineEntry line_entry;
+                        if (sc.function->GetAddressRange().GetBaseAddress().CalculateSymbolContextLineEntry (line_entry))
+                        {
+                            SetDefaultFileAndLine (line_entry.file, 
+                                                   line_entry.line);
+                            file_spec = m_last_file_sp->GetFileSpec();
+                            line = m_last_line;
+                            return true;
+                        }
                     }
                 }
             }
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 9e9aa563f7d..b8b6c0c3216 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -27,6 +27,7 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/Section.h"
+#include "lldb/Core/SourceManager.h"
 #include "lldb/Core/StreamString.h"
 #include "lldb/Core/Timer.h"
 #include "lldb/Core/ValueObject.h"
@@ -79,7 +80,7 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch, const lldb::Plat
     m_scratch_ast_source_ap (NULL),
     m_ast_importer_ap (NULL),
     m_persistent_variables (),
-    m_source_manager(*this),
+    m_source_manager_ap(),
     m_stop_hooks (),
     m_stop_hook_next_id (0),
     m_suppress_stop_hooks (false),
@@ -1908,6 +1909,15 @@ Target::GetOpcodeLoadAddress (lldb::addr_t load_addr, AddressClass addr_class) c
     return opcode_addr;
 }
 
+SourceManager &
+Target::GetSourceManager ()
+{
+    if (m_source_manager_ap.get() == NULL)
+        m_source_manager_ap.reset (new SourceManager(shared_from_this()));
+    return *m_source_manager_ap;
+}
+
+
 lldb::user_id_t
 Target::AddStopHook (Target::StopHookSP &new_hook_sp)
 {