labath created this revision.
labath added reviewers: JDevlieghere, clayborg, jingham.

This is the next step in avoiding funneling all SymbolFile calls through
the SymbolVendor. Right now, it is just a convenience function, but it
allows us to update all calls to SymbolVendor functions to access the
SymbolFile directly. Once all call sites have been updated, we can
remove the GetSymbolVendor member function.

This patch just updates the calls to GetSymbolVendor, which were calling
it just so they could fetch the underlying symbol file. Other calls will
be done in follow-ups.


https://reviews.llvm.org/D65435

Files:
  include/lldb/Core/Module.h
  source/Commands/CommandObjectTarget.cpp
  source/Core/Module.cpp
  source/Expression/IRExecutionUnit.cpp
  source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Symbol/Block.cpp
  source/Symbol/Function.cpp
  source/Symbol/UnwindTable.cpp

Index: source/Symbol/UnwindTable.cpp
===================================================================
--- source/Symbol/UnwindTable.cpp
+++ source/Symbol/UnwindTable.cpp
@@ -182,11 +182,7 @@
   return m_arm_unwind_up.get();
 }
 
-SymbolFile *UnwindTable::GetSymbolFile() {
-  if (SymbolVendor *vendor = m_module.GetSymbolVendor())
-    return vendor->GetSymbolFile();
-  return nullptr;
-}
+SymbolFile *UnwindTable::GetSymbolFile() { return m_module.GetSymbolFile(); }
 
 ArchSpec UnwindTable::GetArchitecture() { return m_module.GetArchitecture(); }
 
Index: source/Symbol/Function.cpp
===================================================================
--- source/Symbol/Function.cpp
+++ source/Symbol/Function.cpp
@@ -428,14 +428,8 @@
   ModuleSP module_sp = CalculateSymbolContextModule();
 
   if (module_sp) {
-    SymbolVendor *sym_vendor = module_sp->GetSymbolVendor();
-
-    if (sym_vendor) {
-      SymbolFile *sym_file = sym_vendor->GetSymbolFile();
-
-      if (sym_file)
-        return sym_file->GetDeclContextForUID(GetID());
-    }
+    if (SymbolFile *sym_file = module_sp->GetSymbolFile())
+      return sym_file->GetDeclContextForUID(GetID());
   }
   return CompilerDeclContext();
 }
@@ -449,12 +443,7 @@
     if (!sc.module_sp)
       return nullptr;
 
-    SymbolVendor *sym_vendor = sc.module_sp->GetSymbolVendor();
-
-    if (sym_vendor == nullptr)
-      return nullptr;
-
-    SymbolFile *sym_file = sym_vendor->GetSymbolFile();
+    SymbolFile *sym_file = sc.module_sp->GetSymbolFile();
 
     if (sym_file == nullptr)
       return nullptr;
Index: source/Symbol/Block.cpp
===================================================================
--- source/Symbol/Block.cpp
+++ source/Symbol/Block.cpp
@@ -464,8 +464,7 @@
 
 SymbolFile *Block::GetSymbolFile() {
   if (ModuleSP module_sp = CalculateSymbolContextModule())
-    if (SymbolVendor *sym_vendor = module_sp->GetSymbolVendor())
-      return sym_vendor->GetSymbolFile();
+    return module_sp->GetSymbolFile();
   return nullptr;
 }
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3723,10 +3723,8 @@
   if (m_debug_map_symfile == nullptr && !m_debug_map_module_wp.expired()) {
     lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
     if (module_sp) {
-      SymbolVendor *sym_vendor = module_sp->GetSymbolVendor();
-      if (sym_vendor)
-        m_debug_map_symfile =
-            (SymbolFileDWARFDebugMap *)sym_vendor->GetSymbolFile();
+      m_debug_map_symfile =
+          (SymbolFileDWARFDebugMap *)module_sp->GetSymbolFile();
     }
   }
   return m_debug_map_symfile;
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===================================================================
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -72,112 +72,106 @@
     FileSpec module_spec = module.GetFileSpec();
 
     if (module_spec) {
-      SymbolVendor *symbols = module.GetSymbolVendor();
-      if (symbols) {
-        SymbolFile *symfile = symbols->GetSymbolFile();
-        if (symfile) {
-          ObjectFile *objfile = symfile->GetObjectFile();
-          if (objfile) {
-            FileSpec symfile_spec(objfile->GetFileSpec());
-            if (symfile_spec && 
-                strcasestr (symfile_spec.GetPath().c_str(), 
-                        ".dSYM/Contents/Resources/DWARF") != nullptr &&
-                FileSystem::Instance().Exists(symfile_spec)) {
-              while (module_spec.GetFilename()) {
-                std::string module_basename(
-                    module_spec.GetFilename().GetCString());
-                std::string original_module_basename(module_basename);
-
-                bool was_keyword = false;
-
-                // FIXME: for Python, we cannot allow certain characters in
-                // module
-                // filenames we import. Theoretically, different scripting
-                // languages may have different sets of forbidden tokens in
-                // filenames, and that should be dealt with by each
-                // ScriptInterpreter. For now, we just replace dots with
-                // underscores, but if we ever support anything other than
-                // Python we will need to rework this
-                std::replace(module_basename.begin(), module_basename.end(),
-                             '.', '_');
-                std::replace(module_basename.begin(), module_basename.end(),
-                             ' ', '_');
-                std::replace(module_basename.begin(), module_basename.end(),
-                             '-', '_');
-                ScriptInterpreter *script_interpreter =
-                    target->GetDebugger().GetScriptInterpreter();
-                if (script_interpreter &&
-                    script_interpreter->IsReservedWord(
-                        module_basename.c_str())) {
-                  module_basename.insert(module_basename.begin(), '_');
-                  was_keyword = true;
-                }
+      if (SymbolFile *symfile = module.GetSymbolFile()) {
+        ObjectFile *objfile = symfile->GetObjectFile();
+        if (objfile) {
+          FileSpec symfile_spec(objfile->GetFileSpec());
+          if (symfile_spec &&
+              strcasestr(symfile_spec.GetPath().c_str(),
+                         ".dSYM/Contents/Resources/DWARF") != nullptr &&
+              FileSystem::Instance().Exists(symfile_spec)) {
+            while (module_spec.GetFilename()) {
+              std::string module_basename(
+                  module_spec.GetFilename().GetCString());
+              std::string original_module_basename(module_basename);
+
+              bool was_keyword = false;
+
+              // FIXME: for Python, we cannot allow certain characters in
+              // module
+              // filenames we import. Theoretically, different scripting
+              // languages may have different sets of forbidden tokens in
+              // filenames, and that should be dealt with by each
+              // ScriptInterpreter. For now, we just replace dots with
+              // underscores, but if we ever support anything other than
+              // Python we will need to rework this
+              std::replace(module_basename.begin(), module_basename.end(), '.',
+                           '_');
+              std::replace(module_basename.begin(), module_basename.end(), ' ',
+                           '_');
+              std::replace(module_basename.begin(), module_basename.end(), '-',
+                           '_');
+              ScriptInterpreter *script_interpreter =
+                  target->GetDebugger().GetScriptInterpreter();
+              if (script_interpreter &&
+                  script_interpreter->IsReservedWord(module_basename.c_str())) {
+                module_basename.insert(module_basename.begin(), '_');
+                was_keyword = true;
+              }
 
-                StreamString path_string;
-                StreamString original_path_string;
-                // for OSX we are going to be in
-                // .dSYM/Contents/Resources/DWARF/<basename> let us go to
-                // .dSYM/Contents/Resources/Python/<basename>.py and see if the
-                // file exists
-                path_string.Printf("%s/../Python/%s.py",
-                                   symfile_spec.GetDirectory().GetCString(),
-                                   module_basename.c_str());
-                original_path_string.Printf(
-                    "%s/../Python/%s.py",
-                    symfile_spec.GetDirectory().GetCString(),
-                    original_module_basename.c_str());
-                FileSpec script_fspec(path_string.GetString());
-                FileSystem::Instance().Resolve(script_fspec);
-                FileSpec orig_script_fspec(original_path_string.GetString());
-                FileSystem::Instance().Resolve(orig_script_fspec);
-
-                // if we did some replacements of reserved characters, and a
-                // file with the untampered name exists, then warn the user
-                // that the file as-is shall not be loaded
-                if (feedback_stream) {
-                  if (module_basename != original_module_basename &&
-                      FileSystem::Instance().Exists(orig_script_fspec)) {
-                    const char *reason_for_complaint =
-                        was_keyword ? "conflicts with a keyword"
-                                    : "contains reserved characters";
-                    if (FileSystem::Instance().Exists(script_fspec))
-                      feedback_stream->Printf(
-                          "warning: the symbol file '%s' contains a debug "
-                          "script. However, its name"
-                          " '%s' %s and as such cannot be loaded. LLDB will"
-                          " load '%s' instead. Consider removing the file with "
-                          "the malformed name to"
-                          " eliminate this warning.\n",
-                          symfile_spec.GetPath().c_str(),
-                          original_path_string.GetData(), reason_for_complaint,
-                          path_string.GetData());
-                    else
-                      feedback_stream->Printf(
-                          "warning: the symbol file '%s' contains a debug "
-                          "script. However, its name"
-                          " %s and as such cannot be loaded. If you intend"
-                          " to have this script loaded, please rename '%s' to "
-                          "'%s' and retry.\n",
-                          symfile_spec.GetPath().c_str(), reason_for_complaint,
-                          original_path_string.GetData(),
-                          path_string.GetData());
-                  }
+              StreamString path_string;
+              StreamString original_path_string;
+              // for OSX we are going to be in
+              // .dSYM/Contents/Resources/DWARF/<basename> let us go to
+              // .dSYM/Contents/Resources/Python/<basename>.py and see if the
+              // file exists
+              path_string.Printf("%s/../Python/%s.py",
+                                 symfile_spec.GetDirectory().GetCString(),
+                                 module_basename.c_str());
+              original_path_string.Printf(
+                  "%s/../Python/%s.py",
+                  symfile_spec.GetDirectory().GetCString(),
+                  original_module_basename.c_str());
+              FileSpec script_fspec(path_string.GetString());
+              FileSystem::Instance().Resolve(script_fspec);
+              FileSpec orig_script_fspec(original_path_string.GetString());
+              FileSystem::Instance().Resolve(orig_script_fspec);
+
+              // if we did some replacements of reserved characters, and a
+              // file with the untampered name exists, then warn the user
+              // that the file as-is shall not be loaded
+              if (feedback_stream) {
+                if (module_basename != original_module_basename &&
+                    FileSystem::Instance().Exists(orig_script_fspec)) {
+                  const char *reason_for_complaint =
+                      was_keyword ? "conflicts with a keyword"
+                                  : "contains reserved characters";
+                  if (FileSystem::Instance().Exists(script_fspec))
+                    feedback_stream->Printf(
+                        "warning: the symbol file '%s' contains a debug "
+                        "script. However, its name"
+                        " '%s' %s and as such cannot be loaded. LLDB will"
+                        " load '%s' instead. Consider removing the file with "
+                        "the malformed name to"
+                        " eliminate this warning.\n",
+                        symfile_spec.GetPath().c_str(),
+                        original_path_string.GetData(), reason_for_complaint,
+                        path_string.GetData());
+                  else
+                    feedback_stream->Printf(
+                        "warning: the symbol file '%s' contains a debug "
+                        "script. However, its name"
+                        " %s and as such cannot be loaded. If you intend"
+                        " to have this script loaded, please rename '%s' to "
+                        "'%s' and retry.\n",
+                        symfile_spec.GetPath().c_str(), reason_for_complaint,
+                        original_path_string.GetData(), path_string.GetData());
                 }
+              }
 
-                if (FileSystem::Instance().Exists(script_fspec)) {
-                  file_list.Append(script_fspec);
-                  break;
-                }
+              if (FileSystem::Instance().Exists(script_fspec)) {
+                file_list.Append(script_fspec);
+                break;
+              }
 
-                // If we didn't find the python file, then keep stripping the
-                // extensions and try again
-                ConstString filename_no_extension(
-                    module_spec.GetFileNameStrippingExtension());
-                if (module_spec.GetFilename() == filename_no_extension)
-                  break;
+              // If we didn't find the python file, then keep stripping the
+              // extensions and try again
+              ConstString filename_no_extension(
+                  module_spec.GetFileNameStrippingExtension());
+              if (module_spec.GetFilename() == filename_no_extension)
+                break;
 
-                module_spec.GetFilename() = filename_no_extension;
-              }
+              module_spec.GetFilename() = filename_no_extension;
             }
           }
         }
Index: source/Expression/IRExecutionUnit.cpp
===================================================================
--- source/Expression/IRExecutionUnit.cpp
+++ source/Expression/IRExecutionUnit.cpp
@@ -658,11 +658,7 @@
   if (!sym_ctx.module_sp)
     return ConstString();
 
-  SymbolVendor *sym_vendor = sym_ctx.module_sp->GetSymbolVendor();
-  if (!sym_vendor)
-    return ConstString();
-
-  lldb_private::SymbolFile *sym_file = sym_vendor->GetSymbolFile();
+  lldb_private::SymbolFile *sym_file = sym_ctx.module_sp->GetSymbolFile();
   if (!sym_file)
     return ConstString();
 
Index: source/Core/Module.cpp
===================================================================
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -456,8 +456,8 @@
     sc.module_sp = shared_from_this();
     resolved_flags |= eSymbolContextModule;
 
-    SymbolVendor *sym_vendor = GetSymbolVendor();
-    if (!sym_vendor)
+    SymbolFile *symfile = GetSymbolFile();
+    if (!symfile)
       return resolved_flags;
 
     // Resolve the compile unit, function, block, line table or line entry if
@@ -468,14 +468,14 @@
         resolve_scope & eSymbolContextLineEntry ||
         resolve_scope & eSymbolContextVariable) {
       resolved_flags |=
-          sym_vendor->ResolveSymbolContext(so_addr, resolve_scope, sc);
+          symfile->ResolveSymbolContext(so_addr, resolve_scope, sc);
     }
 
     // Resolve the symbol if requested, but don't re-look it up if we've
     // already found it.
     if (resolve_scope & eSymbolContextSymbol &&
         !(resolved_flags & eSymbolContextSymbol)) {
-      Symtab *symtab = sym_vendor->GetSymtab();
+      Symtab *symtab = symfile->GetSymtab();
       if (symtab && so_addr.IsSectionOffset()) {
         Symbol *matching_symbol = nullptr;
 
@@ -508,18 +508,15 @@
             // files on MacOSX have an unstripped symbol table inside of them.
             ObjectFile *symtab_objfile = symtab->GetObjectFile();
             if (symtab_objfile && symtab_objfile->IsStripped()) {
-              SymbolFile *symfile = sym_vendor->GetSymbolFile();
-              if (symfile) {
-                ObjectFile *symfile_objfile = symfile->GetObjectFile();
-                if (symfile_objfile != symtab_objfile) {
-                  Symtab *symfile_symtab = symfile_objfile->GetSymtab();
-                  if (symfile_symtab) {
-                    Symbol *symbol =
-                        symfile_symtab->FindSymbolContainingFileAddress(
-                            so_addr.GetFileAddress());
-                    if (symbol && !symbol->IsSynthetic()) {
-                      sc.symbol = symbol;
-                    }
+              ObjectFile *symfile_objfile = symfile->GetObjectFile();
+              if (symfile_objfile != symtab_objfile) {
+                Symtab *symfile_symtab = symfile_objfile->GetSymtab();
+                if (symfile_symtab) {
+                  Symbol *symbol =
+                      symfile_symtab->FindSymbolContainingFileAddress(
+                          so_addr.GetFileAddress());
+                  if (symbol && !symbol->IsSynthetic()) {
+                    sc.symbol = symbol;
                   }
                 }
               }
@@ -1063,6 +1060,12 @@
   return m_symfile_up.get();
 }
 
+SymbolFile *Module::GetSymbolFile(bool can_create, Stream *feedback_strm) {
+  if (SymbolVendor *vendor = GetSymbolVendor(can_create, feedback_strm))
+    return vendor->GetSymbolFile();
+  return nullptr;
+}
+
 void Module::SetFileSpecAndObjectName(const FileSpec &file,
                                       ConstString object_name) {
   // Container objects whose paths do not specify a file directly can call this
@@ -1406,18 +1409,16 @@
 
 void Module::PreloadSymbols() {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  SymbolVendor * sym_vendor = GetSymbolVendor();
-  if (!sym_vendor) {
+  SymbolFile *sym_file = GetSymbolFile();
+  if (!sym_file)
     return;
-  }
+
   // Prime the symbol file first, since it adds symbols to the symbol table.
-  if (SymbolFile *symbol_file = sym_vendor->GetSymbolFile()) {
-    symbol_file->PreloadSymbols();
-  }
+  sym_file->PreloadSymbols();
+
   // Now we can prime the symbol table.
-  if (Symtab * symtab = sym_vendor->GetSymtab()) {
+  if (Symtab *symtab = sym_file->GetSymtab())
     symtab->PreloadSymbols();
-  }
 }
 
 void Module::SetSymbolFileFileSpec(const FileSpec &file) {
@@ -1427,7 +1428,7 @@
     // Remove any sections in the unified section list that come from the
     // current symbol vendor.
     SectionList *section_list = GetSectionList();
-    SymbolFile *symbol_file = m_symfile_up->GetSymbolFile();
+    SymbolFile *symbol_file = GetSymbolFile();
     if (section_list && symbol_file) {
       ObjectFile *obj_file = symbol_file->GetObjectFile();
       // Make sure we have an object file and that the symbol vendor's objfile
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -2237,8 +2237,7 @@
         if (m_interpreter.WasInterrupted())
           break;
         Module *m = target->GetImages().GetModulePointerAtIndex(image_idx);
-        SymbolFile *sf = m->GetSymbolVendor()->GetSymbolFile();
-        sf->DumpClangAST(result.GetOutputStream());
+        m->GetSymbolFile()->DumpClangAST(result.GetOutputStream());
       }
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
@@ -2263,8 +2262,7 @@
         if (m_interpreter.WasInterrupted())
           break;
         Module *m = module_list.GetModulePointerAtIndex(i);
-        SymbolFile *sf = m->GetSymbolVendor()->GetSymbolFile();
-        sf->DumpClangAST(result.GetOutputStream());
+        m->GetSymbolFile()->DumpClangAST(result.GetOutputStream());
       }
     }
     result.SetStatus(eReturnStatusSuccessFinishResult);
@@ -3250,9 +3248,9 @@
 
       case 's':
       case 'S': {
-        const SymbolVendor *symbol_vendor = module->GetSymbolVendor();
-        if (symbol_vendor) {
-          const FileSpec symfile_spec = symbol_vendor->GetMainFileSpec();
+        if (const SymbolFile *symbol_file = module->GetSymbolFile()) {
+          const FileSpec symfile_spec =
+              symbol_file->GetObjectFile()->GetFileSpec();
           if (format_char == 'S') {
             // Dump symbol file only if different from module file
             if (!symfile_spec || symfile_spec == module->GetFileSpec()) {
@@ -4185,48 +4183,44 @@
         // decides to create it!
         module_sp->SetSymbolFileFileSpec(symbol_fspec);
 
-        SymbolVendor *symbol_vendor =
-            module_sp->GetSymbolVendor(true, &result.GetErrorStream());
-        if (symbol_vendor) {
-          SymbolFile *symbol_file = symbol_vendor->GetSymbolFile();
-
-          if (symbol_file) {
-            ObjectFile *object_file = symbol_file->GetObjectFile();
-
-            if (object_file && object_file->GetFileSpec() == symbol_fspec) {
-              // Provide feedback that the symfile has been successfully added.
-              const FileSpec &module_fs = module_sp->GetFileSpec();
-              result.AppendMessageWithFormat(
-                  "symbol file '%s' has been added to '%s'\n", symfile_path,
-                  module_fs.GetPath().c_str());
-
-              // Let clients know something changed in the module if it is
-              // currently loaded
-              ModuleList module_list;
-              module_list.Append(module_sp);
-              target->SymbolsDidLoad(module_list);
-
-              // Make sure we load any scripting resources that may be embedded
-              // in the debug info files in case the platform supports that.
-              Status error;
-              StreamString feedback_stream;
-              module_sp->LoadScriptingResourceInTarget(target, error,
-                                                       &feedback_stream);
-              if (error.Fail() && error.AsCString())
-                result.AppendWarningWithFormat(
-                    "unable to load scripting data for module %s - error "
-                    "reported was %s",
-                    module_sp->GetFileSpec()
-                        .GetFileNameStrippingExtension()
-                        .GetCString(),
-                    error.AsCString());
-              else if (feedback_stream.GetSize())
-                result.AppendWarningWithFormat("%s", feedback_stream.GetData());
-
-              flush = true;
-              result.SetStatus(eReturnStatusSuccessFinishResult);
-              return true;
-            }
+        SymbolFile *symbol_file =
+            module_sp->GetSymbolFile(true, &result.GetErrorStream());
+        if (symbol_file) {
+          ObjectFile *object_file = symbol_file->GetObjectFile();
+
+          if (object_file && object_file->GetFileSpec() == symbol_fspec) {
+            // Provide feedback that the symfile has been successfully added.
+            const FileSpec &module_fs = module_sp->GetFileSpec();
+            result.AppendMessageWithFormat(
+                "symbol file '%s' has been added to '%s'\n", symfile_path,
+                module_fs.GetPath().c_str());
+
+            // Let clients know something changed in the module if it is
+            // currently loaded
+            ModuleList module_list;
+            module_list.Append(module_sp);
+            target->SymbolsDidLoad(module_list);
+
+            // Make sure we load any scripting resources that may be embedded
+            // in the debug info files in case the platform supports that.
+            Status error;
+            StreamString feedback_stream;
+            module_sp->LoadScriptingResourceInTarget(target, error,
+                                                     &feedback_stream);
+            if (error.Fail() && error.AsCString())
+              result.AppendWarningWithFormat(
+                  "unable to load scripting data for module %s - error "
+                  "reported was %s",
+                  module_sp->GetFileSpec()
+                      .GetFileNameStrippingExtension()
+                      .GetCString(),
+                  error.AsCString());
+            else if (feedback_stream.GetSize())
+              result.AppendWarningWithFormat("%s", feedback_stream.GetData());
+
+            flush = true;
+            result.SetStatus(eReturnStatusSuccessFinishResult);
+            return true;
           }
         }
         // Clear the symbol file spec if anything went wrong
Index: include/lldb/Core/Module.h
===================================================================
--- include/lldb/Core/Module.h
+++ include/lldb/Core/Module.h
@@ -653,6 +653,9 @@
   GetSymbolVendor(bool can_create = true,
                   lldb_private::Stream *feedback_strm = nullptr);
 
+  SymbolFile *GetSymbolFile(bool can_create = true,
+                            Stream *feedback_strm = nullptr);
+
   /// Get a reference to the UUID value contained in this object.
   ///
   /// If the executable image file doesn't not have a UUID value built into
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to