[Lldb-commits] [lldb] r233757 - Make sure that "add-dsym" can't crash us when using it.

[Greg Clayton]

Author: gclayton
Date: Tue Mar 31 16:01:48 2015
New Revision: 233757

URL: http://llvm.org/viewvc/llvm-project?rev=233757&view=rev
Log:
Make sure that "add-dsym" can't crash us when using it.

I am fixing this by:
1 - make sure we aren't trying to set the symbol file for a module to the same thing it already has and leaving it alone if it is the same
2 - keep all old symbol files around in the module in case there are any outstanding type references

<rdar://problem/18029116>


Modified:
    lldb/trunk/include/lldb/Core/Module.h
    lldb/trunk/source/Core/Module.cpp

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=233757&r1=233756&r2=233757&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Tue Mar 31 16:01:48 2015
@@ -1108,6 +1108,8 @@ protected:
     TimeValue                   m_object_mod_time;
     lldb::ObjectFileSP          m_objfile_sp;   ///< A shared pointer to the object file parser for this module as it may or may not be shared with the SymbolFile
     lldb::SymbolVendorUP        m_symfile_ap;   ///< A pointer to the symbol vendor for this module.
+    std::vector<lldb::SymbolVendorUP> m_old_symfiles; ///< If anyone calls Module::SetSymbolFileFileSpec() and changes the symbol file,
+                                                      ///< we need to keep all old symbol files around in case anyone has type references to them
     lldb::ClangASTContextUP     m_ast;          ///< The AST context for this module.
     PathMappingList             m_source_mappings; ///< Module specific source remappings for when you have debug info for a module that doesn't match where the sources currently are
     lldb::SectionListUP         m_sections_ap; ///< Unified section list for module that is used by the ObjectFile and and ObjectFile instances for the debug info

Modified: lldb/trunk/source/Core/Module.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Module.cpp?rev=233757&r1=233756&r2=233757&view=diff
==============================================================================
--- lldb/trunk/source/Core/Module.cpp (original)
+++ lldb/trunk/source/Core/Module.cpp Tue Mar 31 16:01:48 2015
@@ -1469,9 +1469,11 @@ Module::FindSymbolsMatchingRegExAndType
 void
 Module::SetSymbolFileFileSpec (const FileSpec &file)
 {
-    // Remove any sections in the unified section list that come from the current symbol vendor.
+    if (!file.Exists())
+        return;
     if (m_symfile_ap)
     {
+        // Remove any sections in the unified section list that come from the current symbol vendor.
         SectionList *section_list = GetSectionList();
         SymbolFile *symbol_file = m_symfile_ap->GetSymbolFile();
         if (section_list && symbol_file)
@@ -1479,21 +1481,49 @@ Module::SetSymbolFileFileSpec (const Fil
             ObjectFile *obj_file = symbol_file->GetObjectFile();
             // Make sure we have an object file and that the symbol vendor's objfile isn't
             // the same as the module's objfile before we remove any sections for it...
-            if (obj_file && obj_file != m_objfile_sp.get())
+            if (obj_file)
             {
-                size_t num_sections = section_list->GetNumSections (0);
-                for (size_t idx = num_sections; idx > 0; --idx)
+                // Check to make sure we aren't trying to specify the file we already have
+                if (obj_file->GetFileSpec() == file)
                 {
-                    lldb::SectionSP section_sp (section_list->GetSectionAtIndex (idx - 1));
-                    if (section_sp->GetObjectFile() == obj_file)
+                    // We are being told to add the exact same file that we already have
+                    // we don't have to do anything.
+                    return;
+                }
+
+                // The symbol file might be a directory bundle ("/tmp/a.out.dSYM") instead
+                // of a full path to the symbol file within the bundle
+                // ("/tmp/a.out.dSYM/Contents/Resources/DWARF/a.out"). So we need to check this
+
+                if (file.IsDirectory())
+                {
+                    std::string new_path(file.GetPath());
+                    std::string old_path(obj_file->GetFileSpec().GetPath());
+                    if (old_path.find(new_path) == 0)
+                    {
+                        // We specified the same bundle as the symbol file that we already have
+                        return;
+                    }
+                }
+
+                if (obj_file != m_objfile_sp.get())
+                {
+                    size_t num_sections = section_list->GetNumSections (0);
+                    for (size_t idx = num_sections; idx > 0; --idx)
                     {
-                        section_list->DeleteSection (idx - 1);
+                        lldb::SectionSP section_sp (section_list->GetSectionAtIndex (idx - 1));
+                        if (section_sp->GetObjectFile() == obj_file)
+                        {
+                            section_list->DeleteSection (idx - 1);
+                        }
                     }
                 }
             }
         }
+        // Keep all old symbol files around in case there are any lingering type references in
+        // any SBValue objects that might have been handed out.
+        m_old_symfiles.push_back(std::move(m_symfile_ap));
     }
-
     m_symfile_spec = file;
     m_symfile_ap.reset();
     m_did_load_symbol_vendor = false;