Add new "target module replace" command

Summary:
For dump debugging (both minidump and coredump), the dump most likely only contains the process memory without real modules so placeholder object files are created for the modules.

LLDB lacks a feature to hydrate/upgrade these dummy modules into real modules. This diff bridges the gap with a new `target module replace` command.

The new command would replace the place holder object file in target module with input new/real object file and refresh the symbol table/stack. The target module is located with file name matching, users can also specify the target module via "-s" option if the module names differ.

Another change is placeholder modules will have `(*)` added at the end as hint.

The workflow would be:
* lldb -c <path_to_dump>
* `image list` to see place holder modules
* download fbpkg and unzip it
* `target module replace <downloaded_fbpkg_module>`

Test Plan:
A new unit test is added.
c4crasher end-to-end debugging is tested.
c4crasher testing workflow is documented here: https://www.internalfb.com/intern/wiki/Coredumper/Developer_Guide/Development_Process/

Reviewers: wanyi, #lldb_team

Reviewed By: wanyi

Subscribers: gclayton, #lldb_team

Differential Revision: https://phabricator.intern.facebook.com/D43756670

Author: jeffreytan81

lldb/include/lldb/Core/Module.h

@@ -547,6 +547,22 @@ class Module : public std::enable_shared_from_this<Module>,
   ///     remains valid as long as the object is around.
   virtual ObjectFile *GetObjectFile();
 
+  /// Replace existing backing object file with new \param object_file.
+  ///
+  /// The old object file being replaced will be kept alive to prevent from
+  /// dangling symbol references.
+  /// UUID and underlying symbol file will be reparsed during further access.
+  /// A common use case is to replace an Placeholder object file with a real
+  /// one during dump debugging.
+  ///
+  /// \param[in] target
+  ///    The target to update object file load address.
+  ///
+  /// \param[in] object_file
+  ///     The new object file spec to replace the existing one.
+  virtual void ReplaceObjectFile(Target &target, FileSpec object_file,
+                                 uint64_t object_offset);
+
   /// Get the unified section list for the module. This is the section list
   /// created by the module's object file and any debug info and symbol files
   /// created by the symbol vendor.
@@ -1032,6 +1048,9 @@ class Module : public std::enable_shared_from_this<Module>,
   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::ObjectFileSP m_old_objfile_sp; /// Strong reference to keep the old
+                                       /// object file being replaced alive.
+
   UnwindTable m_unwind_table;      ///< Table of FuncUnwinders
                                    /// objects created for this
                                    /// Module's functions
@@ -1093,6 +1112,8 @@ class Module : public std::enable_shared_from_this<Module>,
 private:
   Module(); // Only used internally by CreateJITModule ()
 
+  void LoadObjectFile();
+
   Module(const Module &) = delete;
   const Module &operator=(const Module &) = delete;
 

lldb/source/Commands/CommandObjectTarget.cpp

@@ -2866,6 +2866,150 @@ class CommandObjectTargetModulesAdd : public CommandObjectParsed {
   }
 };
 
+class CommandObjectTargetModulesReplace : public CommandObjectParsed {
+public:
+  CommandObjectTargetModulesReplace(CommandInterpreter &interpreter)
+      : CommandObjectParsed(
+            interpreter, "target modules replace",
+            "Replace module's existing object file with a new object file.",
+            "target modules replace [<module>]", eCommandRequiresTarget),
+        m_file_to_replace(LLDB_OPT_SET_1, false, "shlib", 's',
+                          lldb::eModuleCompletion, eArgTypeShlibName,
+                          "File name of the shared library to replace.") {
+    m_option_group.Append(&m_uuid_option_group, LLDB_OPT_SET_ALL,
+                          LLDB_OPT_SET_1);
+    m_option_group.Append(&m_file_to_replace, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
+    m_option_group.Finalize();
+    CommandArgumentData module_arg{eArgTypePath, eArgRepeatStar};
+    m_arguments.push_back({module_arg});
+  }
+
+  ~CommandObjectTargetModulesReplace() override = default;
+
+  Options *GetOptions() override { return &m_option_group; }
+
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+                           OptionElementVector &opt_element_vector) override {
+    CommandCompletions::InvokeCommonCompletionCallbacks(
+        GetCommandInterpreter(), lldb::eDiskFileCompletion, request, nullptr);
+  }
+
+protected:
+  OptionGroupOptions m_option_group;
+  OptionGroupUUID m_uuid_option_group;
+  OptionGroupFile m_file_to_replace;
+
+  void DoExecute(Args &args, CommandReturnObject &result) override {
+    if (args.GetArgumentCount() == 0) {
+      result.AppendError(
+          "one or more executable image paths must be specified");
+      return;
+    }
+
+    Target &target = GetTarget();
+    bool flush = false;
+    // TODO: investigate if we should only allow one module. Similar for
+    // CommandObjectTargetModulesAdd and CommandObjectTargetSymbolsAdd.
+    for (auto &entry : args.entries()) {
+      if (entry.ref().empty())
+        continue;
+
+      FileSpec file_spec(entry.ref());
+      if (FileSystem::Instance().Exists(file_spec)) {
+        ModuleSpec module_spec(file_spec);
+        if (m_uuid_option_group.GetOptionValue().OptionWasSet())
+          module_spec.GetUUID() =
+              m_uuid_option_group.GetOptionValue().GetCurrentValue();
+        if (!module_spec.GetArchitecture().IsValid())
+          module_spec.GetArchitecture() = target.GetArchitecture();
+        if (m_file_to_replace.GetOptionValue().OptionWasSet())
+          module_spec.GetFileSpec().SetFilename(
+              m_file_to_replace.GetOptionValue()
+                  .GetCurrentValue()
+                  .GetFilename());
+
+        ModuleList matching_modules = findMatchingModules(module_spec);
+        if (matching_modules.IsEmpty()) {
+          result.AppendErrorWithFormat("can't find matching modules for '%s'",
+                                       entry.ref().str().c_str());
+          return;
+        }
+
+        if (matching_modules.GetSize() > 1) {
+          result.AppendErrorWithFormat(
+              "multiple modules match symbol file '%s', "
+              "use the --uuid option to resolve the "
+              "ambiguity.\n",
+              entry.ref().str().c_str());
+          return;
+        }
+
+        assert(matching_modules.GetSize() == 1);
+        auto module_sp = matching_modules.GetModuleAtIndex(0);
+        module_sp->ReplaceObjectFile(target, file_spec, /*object_offset=*/0);
+
+        if (target.GetPreloadSymbols())
+          module_sp->PreloadSymbols();
+
+        flush = true;
+        result.SetStatus(eReturnStatusSuccessFinishResult);
+      } else {
+        std::string resolved_path = file_spec.GetPath();
+        if (resolved_path != entry.ref()) {
+          result.AppendErrorWithFormat(
+              "invalid module path '%s' with resolved path '%s'\n",
+              entry.ref().str().c_str(), resolved_path.c_str());
+          break;
+        }
+        result.AppendErrorWithFormat("invalid module path '%s'\n",
+                                     entry.c_str());
+        break;
+      }
+    }
+
+    if (flush) {
+      ProcessSP process = target.GetProcessSP();
+      if (process)
+        process->Flush();
+    }
+    return;
+  }
+
+  ModuleList findMatchingModules(const ModuleSpec &module_spec) {
+    Target &target = GetTarget();
+    ModuleList matching_modules;
+    lldb_private::ModuleSpecList module_specs;
+    if (ObjectFile::GetModuleSpecifications(module_spec.GetFileSpec(), 0, 0,
+                                            module_specs)) {
+      // Now extract the module spec that matches the target architecture
+      ModuleSpec target_arch_module_spec;
+      ModuleSpec arch_matched_module_spec;
+      target_arch_module_spec.GetArchitecture() = target.GetArchitecture();
+      if (module_specs.FindMatchingModuleSpec(target_arch_module_spec,
+                                              arch_matched_module_spec)) {
+        if (arch_matched_module_spec.GetUUID().IsValid()) {
+          // It has a UUID, look for this UUID in the target modules
+          ModuleSpec uuid_module_spec;
+          uuid_module_spec.GetUUID() = arch_matched_module_spec.GetUUID();
+          target.GetImages().FindModules(uuid_module_spec, matching_modules);
+        }
+      }
+    }
+
+    // Just try to match up the file by basename if we have no matches at
+    // this point.
+    if (matching_modules.IsEmpty()) {
+      ModuleSpec filename_only_spec;
+      filename_only_spec.GetFileSpec().SetFilename(
+          module_spec.GetFileSpec().GetFilename());
+      target.GetImages().FindModules(filename_only_spec, matching_modules);
+    }
+
+    return matching_modules;
+  }
+};
+
 class CommandObjectTargetModulesLoad
     : public CommandObjectTargetModulesModuleAutoComplete {
 public:
@@ -3333,10 +3477,14 @@ class CommandObjectTargetModulesList : public CommandObjectParsed {
         DumpModuleArchitecture(strm, module, true, width);
         break;
 
-      case 'f':
+      case 'f': {
         DumpFullpath(strm, &module->GetFileSpec(), width);
         dump_object_name = true;
-        break;
+
+        ObjectFile *objfile = module->GetObjectFile();
+        if (objfile && objfile->GetPluginName() == "placeholder")
+          strm.Printf("(*)");
+      } break;
 
       case 'd':
         DumpDirectory(strm, &module->GetFileSpec(), width);
@@ -4205,6 +4353,9 @@ class CommandObjectTargetModules : public CommandObjectMultiword {
                                "target modules <sub-command> ...") {
     LoadSubCommand(
         "add", CommandObjectSP(new CommandObjectTargetModulesAdd(interpreter)));
+    LoadSubCommand(
+        "replace",
+        CommandObjectSP(new CommandObjectTargetModulesReplace(interpreter)));
     LoadSubCommand("load", CommandObjectSP(new CommandObjectTargetModulesLoad(
                                interpreter)));
     LoadSubCommand("dump", CommandObjectSP(new CommandObjectTargetModulesDump(

lldb/source/Core/Module.cpp

@@ -1177,42 +1177,69 @@ ObjectFile *Module::GetObjectFile() {
     if (!m_did_load_objfile.load()) {
       LLDB_SCOPED_TIMERF("Module::GetObjectFile () module = %s",
                          GetFileSpec().GetFilename().AsCString(""));
-      lldb::offset_t data_offset = 0;
-      lldb::offset_t file_size = 0;
-
-      if (m_data_sp)
-        file_size = m_data_sp->GetByteSize();
-      else if (m_file)
-        file_size = FileSystem::Instance().GetByteSize(m_file);
-
-      if (file_size > m_object_offset) {
-        m_did_load_objfile = true;
-        // FindPlugin will modify its data_sp argument. Do not let it
-        // modify our m_data_sp member.
-        auto data_sp = m_data_sp;
-        m_objfile_sp = ObjectFile::FindPlugin(
-            shared_from_this(), &m_file, m_object_offset,
-            file_size - m_object_offset, data_sp, data_offset);
-        if (m_objfile_sp) {
-          // Once we get the object file, update our module with the object
-          // file's architecture since it might differ in vendor/os if some
-          // parts were unknown.  But since the matching arch might already be
-          // more specific than the generic COFF architecture, only merge in
-          // those values that overwrite unspecified unknown values.
-          m_arch.MergeFrom(m_objfile_sp->GetArchitecture());
-
-          m_unwind_table.ModuleWasUpdated();
-        } else {
-          ReportError("failed to load objfile for {0}\nDebugging will be "
-                      "degraded for this module.",
-                      GetFileSpec().GetPath().c_str());
-        }
-      }
+      LoadObjectFile();
     }
   }
   return m_objfile_sp.get();
 }
 
+void Module::LoadObjectFile() {
+  lldb::offset_t data_offset = 0;
+  lldb::offset_t file_size = 0;
+
+  if (m_data_sp)
+    file_size = m_data_sp->GetByteSize();
+  else if (m_file)
+    file_size = FileSystem::Instance().GetByteSize(m_file);
+
+  if (file_size <= m_object_offset)
+    return;
+
+  m_did_load_objfile = true;
+  // FindPlugin will modify its data_sp argument. Do not let it
+  // modify our m_data_sp member.
+  auto data_sp = m_data_sp;
+  m_objfile_sp =
+      ObjectFile::FindPlugin(shared_from_this(), &m_file, m_object_offset,
+                             file_size - m_object_offset, data_sp, data_offset);
+  if (m_objfile_sp) {
+    // Once we get the object file, update our module with the object
+    // file's architecture since it might differ in vendor/os if some
+    // parts were unknown.  But since the matching arch might already be
+    // more specific than the generic COFF architecture, only merge in
+    // those values that overwrite unspecified unknown values.
+    m_arch.MergeFrom(m_objfile_sp->GetArchitecture());
+    m_unwind_table.ModuleWasUpdated();
+  } else {
+    ReportError("failed to load objfile for {0}",
+                GetFileSpec().GetPath().c_str());
+  }
+}
+
+void Module::ReplaceObjectFile(Target &target, FileSpec object_file,
+                               uint64_t object_offset) {
+  m_old_objfile_sp = m_objfile_sp;
+  m_file = object_file;
+  m_object_offset = object_offset;
+  lldb::addr_t load_address =
+      GetObjectFile()->GetBaseAddress().GetLoadAddress(&target);
+
+  // Scope locking.
+  {
+    std::lock_guard<std::recursive_mutex> guard(m_mutex);
+    LLDB_SCOPED_TIMERF("Module::ReplaceObjectFile () module = %s",
+                       GetFileSpec().GetFilename().AsCString(""));
+    LoadObjectFile();
+  }
+
+  bool changed = false;
+  SetLoadAddress(target, load_address, false, changed);
+
+  // Force reparsing UUID and symbol files.
+  m_did_set_uuid = false;
+  m_did_load_symfile = false;
+}
+
 SectionList *Module::GetSectionList() {
   // Populate m_sections_up with sections from objfile.
   if (!m_sections_up) {

lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

@@ -1005,6 +1005,41 @@ def test_read_only_cstring(self):
         cstr = var.GetSummary()
         self.assertEqual(cstr, '"_start"')
 
+    @skipIfLLVMTargetMissing("X86")
+    @skipIfWindows
+    def test_module_list_dyld(self):
+        """
+        Test module list based dyld can successfully get images
+        list from NT_FILE without main executable
+        """
+        self.runCmd("settings set use-module-list-dyld true")
+        target = self.dbg.CreateTarget(None)
+        process = target.LoadCore("linux-x86_64.core")
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        self.assertEqual(process.GetTarget().GetNumModules(), 1)
+        exe_module = process.GetTarget().GetModuleAtIndex(0)
+        # Module load address is got from coredump NT_FILE.
+        self.assertEqual(
+            exe_module.GetObjectFileHeaderAddress().GetLoadAddress(target), 0x400000
+        )
+        self.dbg.DeleteTarget(target)
+
+    def test_replace_placeholder_module(self):
+        """
+        Test module list based dyld can successfully get images list from
+        NT_FILE without main executable. And `target module replace`
+        command can replace the Placeholder object file with real one.
+        """
+        self.runCmd("settings set use-module-list-dyld true")
+        target = self.dbg.CreateTarget(None)
+        process = target.LoadCore("linux-x86_64.core")
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        self.runCmd("target module replace linux-x86_64.out -s a.out")
+        self.check_all(process, self._x86_64_pid, self._x86_64_regions, "a.out")
+        self.dbg.DeleteTarget(target)
+
     def check_memory_regions(self, process, region_count):
         region_list = process.GetMemoryRegions()
         self.assertEqual(region_list.GetSize(), region_count)