<rdar://problem/10103468>

I started work on being able to add symbol files after a debug session
had started with a new "target symfile add" command and quickly ran into
problems with stale Address objects in breakpoint locations that had 
lldb_private::Section pointers into modules that had been removed or 
replaced. This also let to grabbing stale modules from those sections. 
So I needed to thread harded the Address, Section and related objects.

To do this I modified the ModuleChild class to now require a ModuleSP
on initialization so that a weak reference can created. I also changed
all places that were handing out "Section *" to have them hand out SectionSP.
All ObjectFile, SymbolFile and SymbolVendors were inheriting from ModuleChild
so all of the find plug-in, static creation function and constructors now
require ModuleSP references instead of Module *. 

Address objects now have weak references to their sections which can
safely go stale when a module gets destructed. 

This checkin doesn't complete the "target symfile add" command, but it
does get us a lot clioser to being able to do such things without a high
risk of crashing or memory corruption.

llvm-svn: 151336

9 files changed, 108 insertions, 197 deletions

diff --git a/lldb/source/API/SBAddress.cpp b/lldb/source/API/SBAddress.cpp
index 7d7d1d7789e..a2e5d06564d 100644
--- a/lldb/source/API/SBAddress.cpp
+++ b/lldb/source/API/SBAddress.cpp
@@ -32,7 +32,7 @@ namespace lldb_private
         }
 
         AddressImpl (const Address &addr) :
-            m_module_sp (addr.GetModuleSP()),
+            m_module_sp (addr.GetModule()),
             m_address (addr)
         {
         }
@@ -105,7 +105,7 @@ SBAddress::SBAddress (const SBAddress &rhs) :
 
 
 SBAddress::SBAddress (lldb::SBSection section, lldb::addr_t offset) :
-    m_opaque_ap(new AddressImpl (Address(section.GetSection(), offset)))
+    m_opaque_ap(new AddressImpl (Address(section.GetSP(), offset)))
 {
 }
 
@@ -151,7 +151,7 @@ void
 SBAddress::SetAddress (lldb::SBSection section, lldb::addr_t offset)
 {
     Address &addr = ref();
-    addr.SetSection (section.GetSection());
+    addr.SetSection (section.GetSP());
     addr.SetOffset (offset);
 }
 
@@ -241,7 +241,7 @@ SBAddress::GetSection ()
 {
     lldb::SBSection sb_section;
     if (m_opaque_ap.get())
-        sb_section.SetSection(m_opaque_ap->GetAddress().GetSection());
+        sb_section.SetSP (m_opaque_ap->GetAddress().GetSection());
     return sb_section;
 }
 
diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp
index 228ebfb755a..3adb031e036 100644
--- a/lldb/source/API/SBBreakpoint.cpp
+++ b/lldb/source/API/SBBreakpoint.cpp
@@ -151,8 +151,7 @@ SBBreakpoint::FindLocationByAddress (addr_t vm_addr)
             Target &target = m_opaque_sp->GetTarget();
             if (target.GetSectionLoadList().ResolveLoadAddress (vm_addr, address) == false)
             {
-                address.SetSection (NULL);
-                address.SetOffset (vm_addr);
+                address.SetRawAddress (vm_addr);
             }
             sb_bp_location.SetLocation (m_opaque_sp->FindLocationByAddress (address));
         }
@@ -172,8 +171,7 @@ SBBreakpoint::FindLocationIDByAddress (addr_t vm_addr)
         Target &target = m_opaque_sp->GetTarget();
         if (target.GetSectionLoadList().ResolveLoadAddress (vm_addr, address) == false)
         {
-            address.SetSection (NULL);
-            address.SetOffset (vm_addr);
+            address.SetRawAddress (vm_addr);
         }
         break_id = m_opaque_sp->FindLocationIDByAddress (address);
     }
diff --git a/lldb/source/API/SBFunction.cpp b/lldb/source/API/SBFunction.cpp
index cf80cbf333d..d906e6e7af7 100644
--- a/lldb/source/API/SBFunction.cpp
+++ b/lldb/source/API/SBFunction.cpp
@@ -134,7 +134,7 @@ SBFunction::GetInstructions (SBTarget target)
             target_sp->CalculateExecutionContext (exe_ctx);
             exe_ctx.SetProcessSP(target_sp->GetProcessSP());
         }
-        ModuleSP module_sp = m_opaque_ptr->GetAddressRange().GetBaseAddress().GetModuleSP();
+        ModuleSP module_sp (m_opaque_ptr->GetAddressRange().GetBaseAddress().GetModule());
         if (module_sp)
         {
             sb_instructions.SetDisassembler (Disassembler::DisassembleRange (module_sp->GetArchitecture(),
diff --git a/lldb/source/API/SBModule.cpp b/lldb/source/API/SBModule.cpp
index 77a7af2c62e..c23a35e711b 100644
--- a/lldb/source/API/SBModule.cpp
+++ b/lldb/source/API/SBModule.cpp
@@ -331,7 +331,7 @@ SBModule::GetSectionAtIndex (size_t idx)
             SectionList *section_list = obj_file->GetSectionList ();
 
             if (section_list)
-                sb_section.SetSection(section_list->GetSectionAtIndex (idx).get());
+                sb_section.SetSP(section_list->GetSectionAtIndex (idx));
         }
     }
     return sb_section;
@@ -467,7 +467,7 @@ SBModule::FindSection (const char *sect_name)
                 SectionSP section_sp (section_list->FindSectionByName(const_sect_name));
                 if (section_sp)
                 {
-                    sb_section.SetSection(section_sp.get());
+                    sb_section.SetSP (section_sp);
                 }
             }
         }
diff --git a/lldb/source/API/SBSection.cpp b/lldb/source/API/SBSection.cpp
index dedcc7a2850..24b94a7a53a 100644
--- a/lldb/source/API/SBSection.cpp
+++ b/lldb/source/API/SBSection.cpp
@@ -16,106 +16,34 @@
 #include "lldb/Core/Section.h"
 #include "lldb/Core/StreamString.h"
 
-namespace lldb_private 
-{
-    // We need a section implementation to hold onto a reference to the module
-    // since if the module goes away and we have anyone still holding onto a 
-    // SBSection object, we could crash.
-    class SectionImpl
-    {
-    public:
-        SectionImpl (const lldb_private::Section *section = NULL) :
-            m_module_sp (),
-            m_section (section)
-        {
-            if (section)
-                m_module_sp = section->GetModule()->shared_from_this();
-        }
-        
-        SectionImpl (const SectionImpl &rhs) :
-            m_module_sp (rhs.m_module_sp),
-            m_section   (rhs.m_section)
-        {
-        }
-
-        bool 
-        IsValid () const
-        {
-            return m_section != NULL;
-        }
-
-        void
-        operator = (const SectionImpl &rhs)
-        {
-            m_module_sp = rhs.m_module_sp;
-            m_section = rhs.m_section;
-        }
-
-        void
-        operator =(const lldb_private::Section *section)
-        {
-            m_section = section;
-            if (section)
-                m_module_sp.reset(section->GetModule());
-            else
-                m_module_sp.reset();
-        }
-
-        const lldb_private::Section *
-        GetSection () const
-        {
-            return m_section;
-        }
-
-        Module *
-        GetModule()
-        {
-            return m_module_sp.get();
-        }
-
-        const lldb::ModuleSP &
-        GetModuleSP() const
-        {
-            return m_module_sp;
-        }
-    protected:
-        lldb::ModuleSP m_module_sp;
-        const lldb_private::Section *m_section;
-    };
-}
 
 using namespace lldb;
 using namespace lldb_private;
 
 
 SBSection::SBSection () :
-    m_opaque_ap ()
+    m_opaque_wp ()
 {
 }
 
 SBSection::SBSection (const SBSection &rhs) :
-    m_opaque_ap ()
+    m_opaque_wp (rhs.m_opaque_wp)
 {
-    if (rhs.IsValid())
-        m_opaque_ap.reset (new SectionImpl (*rhs.m_opaque_ap));
 }
 
 
 
-SBSection::SBSection (const lldb_private::Section *section) :
-    m_opaque_ap ()
+SBSection::SBSection (const lldb::SectionSP &section_sp) :
+    m_opaque_wp () // Don't init with section_sp otherwise this will throw if section_sp doesn't contain a valid Section *
 {
-    if (section)
-        m_opaque_ap.reset (new SectionImpl(section));
+    if (section_sp)
+        m_opaque_wp = section_sp;
 }
 
 const SBSection &
 SBSection::operator = (const SBSection &rhs)
 {
-    if (this != &rhs && rhs.IsValid())
-        m_opaque_ap.reset (new SectionImpl(*rhs.m_opaque_ap));
-    else
-        m_opaque_ap.reset ();
+    m_opaque_wp = rhs.m_opaque_wp;
     return *this;
 }
 
@@ -126,14 +54,16 @@ SBSection::~SBSection ()
 bool
 SBSection::IsValid () const
 {
-    return m_opaque_ap.get() != NULL && m_opaque_ap->IsValid();
+    SectionSP section_sp (GetSP());
+    return section_sp && section_sp->GetModule().get() != NULL;
 }
 
 const char *
 SBSection::GetName ()
 {
-    if (IsValid())
-        return m_opaque_ap->GetSection()->GetName().GetCString();
+    SectionSP section_sp (GetSP());
+    if (section_sp)
+        return section_sp->GetName().GetCString();
     return NULL;
 }
 
@@ -142,10 +72,14 @@ lldb::SBSection
 SBSection::FindSubSection (const char *sect_name)
 {
     lldb::SBSection sb_section;
-    if (sect_name && IsValid())
+    if (sect_name)
     {
-        ConstString const_sect_name(sect_name);
-        sb_section.SetSection(m_opaque_ap->GetSection()->GetChildren ().FindSectionByName(const_sect_name).get());
+        SectionSP section_sp (GetSP());
+        if (section_sp)
+        {
+            ConstString const_sect_name(sect_name);
+            sb_section.SetSP(section_sp->GetChildren ().FindSectionByName(const_sect_name));
+        }
     }
     return sb_section;
 }
@@ -153,8 +87,9 @@ SBSection::FindSubSection (const char *sect_name)
 size_t
 SBSection::GetNumSubSections ()
 {
-    if (IsValid())
-        return m_opaque_ap->GetSection()->GetChildren ().GetSize();
+    SectionSP section_sp (GetSP());
+    if (section_sp)
+        return section_sp->GetChildren ().GetSize();
     return 0;
 }
 
@@ -162,79 +97,66 @@ lldb::SBSection
 SBSection::GetSubSectionAtIndex (size_t idx)
 {
     lldb::SBSection sb_section;
-    if (IsValid())
-        sb_section.SetSection(m_opaque_ap->GetSection()->GetChildren ().GetSectionAtIndex(idx).get());
+    SectionSP section_sp (GetSP());
+    if (section_sp)
+        sb_section.SetSP (section_sp->GetChildren ().GetSectionAtIndex(idx));
     return sb_section;
 }
 
-const lldb_private::Section *
-SBSection::GetSection()
+lldb::SectionSP
+SBSection::GetSP() const
 {
-    if (m_opaque_ap.get())
-        return m_opaque_ap->GetSection();
-    return NULL;
+    return m_opaque_wp.lock();
 }
 
 void
-SBSection::SetSection (const lldb_private::Section *section)
+SBSection::SetSP(const lldb::SectionSP &section_sp)
 {
-    m_opaque_ap.reset (new SectionImpl(section));
+    m_opaque_wp = section_sp;
 }
 
-
-
-
 lldb::addr_t
 SBSection::GetFileAddress ()
 {
     lldb::addr_t file_addr = LLDB_INVALID_ADDRESS;
-    if (IsValid())
-        return m_opaque_ap->GetSection()->GetFileAddress();
+    SectionSP section_sp (GetSP());
+    if (section_sp)
+        return section_sp->GetFileAddress();
     return file_addr;
 }
 
 lldb::addr_t
 SBSection::GetByteSize ()
 {
-    if (IsValid())
-    {
-        const Section *section = m_opaque_ap->GetSection();
-        if (section)
-            return section->GetByteSize();
-    }
+    SectionSP section_sp (GetSP());
+    if (section_sp)
+        return section_sp->GetByteSize();
     return 0;
 }
 
 uint64_t
 SBSection::GetFileOffset ()
 {
-    if (IsValid())
+    SectionSP section_sp (GetSP());
+    if (section_sp)
     {
-        const Section *section = m_opaque_ap->GetSection();
-        if (section)
+        ModuleSP module_sp (section_sp->GetModule());
+        if (module_sp)
         {
-            Module *module = m_opaque_ap->GetModule();
-            if (module)
-            {
-                ObjectFile *objfile = module->GetObjectFile();
-                if (objfile)
-                    return objfile->GetOffset() + section->GetFileOffset();
-            }
-            return section->GetFileOffset();
+            ObjectFile *objfile = module_sp->GetObjectFile();
+            if (objfile)
+                return objfile->GetOffset() + section_sp->GetFileOffset();
         }
     }
-    return 0;
+    return UINT64_MAX;
 }
 
 uint64_t
 SBSection::GetFileByteSize ()
 {
-    if (IsValid())
-    {
-        const Section *section = m_opaque_ap->GetSection();
-        if (section)
-            return section->GetFileSize();
-    }
+    SectionSP section_sp (GetSP());
+    if (section_sp)
+        return section_sp->GetFileSize();
     return 0;
 }
 
@@ -248,40 +170,37 @@ SBData
 SBSection::GetSectionData (uint64_t offset, uint64_t size)
 {
     SBData sb_data;
-    if (IsValid())
+    SectionSP section_sp (GetSP());
+    if (section_sp)
     {
-        const Section *section = m_opaque_ap->GetSection();
-        if (section)
+        const uint64_t sect_file_size = section_sp->GetFileSize();
+        if (sect_file_size > 0)
         {
-            const uint64_t sect_file_size = section->GetFileSize();
-            if (sect_file_size > 0)
+            ModuleSP module_sp (section_sp->GetModule());
+            if (module_sp)
             {
-                Module *module = m_opaque_ap->GetModule();
-                if (module)
+                ObjectFile *objfile = module_sp->GetObjectFile();
+                if (objfile)
                 {
-                    ObjectFile *objfile = module->GetObjectFile();
-                    if (objfile)
+                    const uint64_t sect_file_offset = objfile->GetOffset() + section_sp->GetFileOffset();
+                    const uint64_t file_offset = sect_file_offset + offset;
+                    uint64_t file_size = size;
+                    if (file_size == UINT64_MAX)
                     {
-                        const uint64_t sect_file_offset = objfile->GetOffset() + section->GetFileOffset();
-                        const uint64_t file_offset = sect_file_offset + offset;
-                        uint64_t file_size = size;
-                        if (file_size == UINT64_MAX)
-                        {
-                            file_size = section->GetByteSize();
-                            if (file_size > offset)
-                                file_size -= offset;
-                            else
-                                file_size = 0;
-                        }
-                        DataBufferSP data_buffer_sp (objfile->GetFileSpec().ReadFileContents (file_offset, file_size));
-                        if (data_buffer_sp && data_buffer_sp->GetByteSize() > 0)
-                        {
-                            DataExtractorSP data_extractor_sp (new DataExtractor (data_buffer_sp, 
-                                                                                  objfile->GetByteOrder(), 
-                                                                                  objfile->GetAddressByteSize()));
-                            
-                            sb_data.SetOpaque (data_extractor_sp);
-                        }
+                        file_size = section_sp->GetByteSize();
+                        if (file_size > offset)
+                            file_size -= offset;
+                        else
+                            file_size = 0;
+                    }
+                    DataBufferSP data_buffer_sp (objfile->GetFileSpec().ReadFileContents (file_offset, file_size));
+                    if (data_buffer_sp && data_buffer_sp->GetByteSize() > 0)
+                    {
+                        DataExtractorSP data_extractor_sp (new DataExtractor (data_buffer_sp, 
+                                                                              objfile->GetByteOrder(), 
+                                                                              objfile->GetAddressByteSize()));
+                        
+                        sb_data.SetOpaque (data_extractor_sp);
                     }
                 }
             }
@@ -293,12 +212,9 @@ SBSection::GetSectionData (uint64_t offset, uint64_t size)
 SectionType
 SBSection::GetSectionType ()
 {
-    if (m_opaque_ap.get())
-    {
-        const Section *section = m_opaque_ap->GetSection();
-        if (section)
-            return section->GetType();
-    }
+    SectionSP section_sp (GetSP());
+    if (section_sp.get())
+        return section_sp->GetType();
     return eSectionTypeInvalid;
 }
 
@@ -306,21 +222,19 @@ SBSection::GetSectionType ()
 bool
 SBSection::operator == (const SBSection &rhs)
 {
-    SectionImpl *lhs_ptr = m_opaque_ap.get();
-    SectionImpl *rhs_ptr = rhs.m_opaque_ap.get();
-    if (lhs_ptr && rhs_ptr)
-        return lhs_ptr->GetSection() == rhs_ptr->GetSection();
+    SectionSP lhs_section_sp (GetSP());
+    SectionSP rhs_section_sp (rhs.GetSP());
+    if (lhs_section_sp && rhs_section_sp)
+        return lhs_section_sp == rhs_section_sp;
     return false;
 }
 
 bool
 SBSection::operator != (const SBSection &rhs)
 {
-    SectionImpl *lhs_ptr = m_opaque_ap.get();
-    SectionImpl *rhs_ptr = rhs.m_opaque_ap.get();
-    if (lhs_ptr && rhs_ptr)
-        return lhs_ptr->GetSection() != rhs_ptr->GetSection();
-    return false;
+    SectionSP lhs_section_sp (GetSP());
+    SectionSP rhs_section_sp (rhs.GetSP());
+    return lhs_section_sp != rhs_section_sp;
 }
 
 bool
@@ -328,12 +242,12 @@ SBSection::GetDescription (SBStream &description)
 {
     Stream &strm = description.ref();
 
-    if (IsValid())
+    SectionSP section_sp (GetSP());
+    if (section_sp)
     {
-        const Section *section = m_opaque_ap->GetSection();
-        const addr_t file_addr = section->GetFileAddress();
-        strm.Printf ("[0x%16.16llx-0x%16.16llx) ", file_addr, file_addr + section->GetByteSize());
-        section->DumpName(&strm);
+        const addr_t file_addr = section_sp->GetFileAddress();
+        strm.Printf ("[0x%16.16llx-0x%16.16llx) ", file_addr, file_addr + section_sp->GetByteSize());
+        section_sp->DumpName(&strm);
     }
     else
     {
diff --git a/lldb/source/API/SBSymbol.cpp b/lldb/source/API/SBSymbol.cpp
index 697fff8e101..519f96be225 100644
--- a/lldb/source/API/SBSymbol.cpp
+++ b/lldb/source/API/SBSymbol.cpp
@@ -132,7 +132,7 @@ SBSymbol::GetInstructions (SBTarget target)
         const AddressRange *symbol_range = m_opaque_ptr->GetAddressRangePtr();
         if (symbol_range)
         {
-            ModuleSP module_sp = symbol_range->GetBaseAddress().GetModuleSP();
+            ModuleSP module_sp (symbol_range->GetBaseAddress().GetModule());
             if (module_sp)
             {
                 sb_instructions.SetDisassembler (Disassembler::DisassembleRange (module_sp->GetArchitecture (),
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 7fdfb347429..2c9c44f19b4 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -557,8 +557,7 @@ SBTarget::ResolveLoadAddress (lldb::addr_t vm_addr)
 
     // We have a load address that isn't in a section, just return an address
     // with the offset filled in (the address) and the section set to NULL
-    addr.SetSection(NULL);
-    addr.SetOffset(vm_addr);
+    addr.SetRawAddress(vm_addr);
     return sb_addr;
 }
 
@@ -1411,7 +1410,7 @@ SBTarget::SetSectionLoadAddress (lldb::SBSection section,
         }
         else
         {
-            target_sp->GetSectionLoadList().SetSectionLoadAddress (section.GetSection(), section_base_addr);
+            target_sp->GetSectionLoadList().SetSectionLoadAddress (section.GetSP().get(), section_base_addr);
         }
     }
     else
@@ -1435,7 +1434,7 @@ SBTarget::ClearSectionLoadAddress (lldb::SBSection section)
         }
         else
         {
-            target_sp->GetSectionLoadList().SetSectionUnloaded (section.GetSection());
+            target_sp->GetSectionLoadList().SetSectionUnloaded (section.GetSP().get());
         }
     }
     else
diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index 806f93579ea..44306d4dd42 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -635,7 +635,7 @@ SBThread::RunToAddress (lldb::addr_t addr)
         bool abort_other_plans = true;
         bool stop_other_threads = true;
 
-        Address target_addr (NULL, addr);
+        Address target_addr (addr);
 
         Thread *thread = exe_ctx.GetThreadPtr();
         Process *process = exe_ctx.GetProcessPtr();
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 9818eb35e8b..fa2fa778fe3 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -1221,13 +1221,13 @@ SBValue::GetLoadAddress()
             value = value_sp->GetAddressOf(scalar_is_load_address, &addr_type);
             if (addr_type == eAddressTypeFile)
             {
-                Module* module = value_sp->GetModule();
-                if (!module)
+                ModuleSP module_sp (value_sp->GetModule());
+                if (!module_sp)
                     value = LLDB_INVALID_ADDRESS;
                 else
                 {
                     Address addr;
-                    module->ResolveFileAddress(value, addr);
+                    module_sp->ResolveFileAddress(value, addr);
                     value = addr.GetLoadAddress(target_sp.get());
                 }
             }
@@ -1259,9 +1259,9 @@ SBValue::GetAddress()
             value = value_sp->GetAddressOf(scalar_is_load_address, &addr_type);
             if (addr_type == eAddressTypeFile)
             {
-                Module* module = value_sp->GetModule();
-                if (module)
-                    module->ResolveFileAddress(value, addr);
+                ModuleSP module_sp (value_sp->GetModule());
+                if (module_sp)
+                    module_sp->ResolveFileAddress(value, addr);
             }
             else if (addr_type == eAddressTypeLoad)
             {